Re: Devices with a front and back webcam represented as a single UVC device
Hi,
On 18-07-18 13:53, Carlos Garnacho wrote:
Hey,
On Wed, Jul 11, 2018 at 9:51 PM, Hans de Goede wrote:
Hi,
On 11-07-18 20:26, Carlos Garnacho wrote:
Hi!,
On Wed, Jul 11, 2018 at 7:41 PM, Hans de Goede
wrote:
Hi,
On 11-07-18 18:07, Carlos Garnacho wrote:
Hi!,
On Wed, Jul 11, 2018 at 2:41 PM, Hans de Goede
wrote:
HI,
On 11-07-18 14:08, Laurent Pinchart wrote:
Hi Carlos,
On Wednesday, 11 July 2018 14:36:48 EEST Carlos Garnacho wrote:
On Wed, Jul 11, 2018 at 1:00 PM, Laurent Pinchart wrote:
On Wednesday, 11 July 2018 11:37:14 EEST Hans de Goede wrote:
Hi Laurent,
At Guadec Carlos (in the Cc) told me that on his Acer 2-in-1 only
the frontcam is working and it seems both are represented by a
single UVC USB device. I've told him to check for some v4l control
to flip between front and back.
Carlos, as I mentioned you can try gtk-v4l
("sudo dnf install gtk-v4l") or qv4l2
("sudo dnf install qv4l2") these will both show
you various controls for the camera. One of those might do the
trick.
But I recently bought a 2nd second hand Cherry Trail based HP
X2 2-in-1 and much to my surprise that is actually using an UVC
cam, rather then the usual ATOMISP crap and it has the same issue.
This device does not seem to have a control to flip between the
2 cams, instead it registers 2 /dev/video? nodes but the second
node does not work
The second node is there to expose metadata to userspace, not image
data.
That's a recent addition to the uvcvideo driver.
and dmesg contains:
[ 26.079868] uvcvideo: Found UVC 1.00 device HP TrueVision HD
(05c8:03a3)
[ 26.095485] uvcvideo 1-4.2:1.0: Entity type for entity Extension
4
was
not initialized!
[ 26.095492] uvcvideo 1-4.2:1.0: Entity type for entity
Processing
2
was
not initialized!
[ 26.095496] uvcvideo 1-4.2:1.0: Entity type for entity Camera 1
was
not
initialized!
You can safely ignore those messages. I need to submit a patch to
get
rid
of them.
Laurent, I've attached lsusb -v output so that you can check the
descriptors.
Thank you.
It's funny how UVC specifies a standard way to describe a device
with
two
camera sensors with dynamic selection of one of them at runtime, and
vendors instead implement vendor-specific crap :-(
The interesting part in the descriptors is
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode
{1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
The extension unit exposes two controls (bmControls is a bitmask).
They
can be accessed from userspace through the UVCIOC_CTRL_QUERY ioctl,
or
mapped to V4L2 controls through the UVCIOC_CTRL_MAP ioctl, in which
case
they will be exposed to standard V4L2 applications.
If you want to experiment with this, I would advise querying both
controls
with UVCIOC_CTRL_QUERY. You can use the UVC_GET_CUR, UVC_GET_MIN,
UVC_GET_MAX, UVC_GET_DEF and UVC_GET_RES requests to get the control
current, minimum, maximum, default and resolution values, and
UVC_GET_LEN
and UVC_GET_INFO to get the control size (in bytes) and flags. Based
on
that you can start experimenting with UVC_SET_CUR to set semi-random
values.
I'm however worried that those two controls would be a register
address
and a register value, for indirect access to all hardware registers
in
the device. In that case, you would likely need information from the
device vendor, or possibly a USB traffic dump from a Windows machine
when
switching between the front and back cameras.
Carlos, it might be good to get Laurent your descriptors too, to do
this do "lsusb", note what is the : for your camera and
then
run:
sudo lsusb -v -d : > lsusb.log
And send Laurent a mail with the generated lsusb
That would be appreciated, but I expect the same issue :-(
Please find it attached. IIUC your last email, it might not be the
exact same issue, but you can definitely judge better.
Your device is similar in the sense that it doesn't use the standard
UVC
support for multiple camera sensors. It instead exposes two extension
units:
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode
{1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x
Re: [PATCH] libv4l: Add support for BAYER10P format conversion
Hi,
On 20-09-18 22:04, Ricardo Ribalda Delgado wrote:
Add support for 10 bit packet Bayer formats:
-V4L2_PIX_FMT_SBGGR10P
-V4L2_PIX_FMT_SGBRG10P
-V4L2_PIX_FMT_SGRBG10P
-V4L2_PIX_FMT_SRGGB10P
These formats pack the 2 LSBs for every 4 pixels in an indeppendent
byte.
Signed-off-by: Ricardo Ribalda Delgado
---
lib/libv4lconvert/bayer.c | 15 +++
lib/libv4lconvert/libv4lconvert-priv.h | 4 +++
lib/libv4lconvert/libv4lconvert.c | 35 ++
3 files changed, 54 insertions(+)
diff --git a/lib/libv4lconvert/bayer.c b/lib/libv4lconvert/bayer.c
index 4b70ddd9..d7d488f9 100644
--- a/lib/libv4lconvert/bayer.c
+++ b/lib/libv4lconvert/bayer.c
@@ -631,3 +631,18 @@ void v4lconvert_bayer_to_yuv420(const unsigned char
*bayer, unsigned char *yuv,
v4lconvert_border_bayer_line_to_y(bayer + stride, bayer, ydst, width,
!start_with_green, !blue_line);
}
+
+void v4lconvert_bayer10p_to_bayer8(unsigned char *bayer10p,
+ unsigned char *bayer8, int width, int height)
+{
+ long i, len = width * height;
+ uint32_t *src, *dst;
+
+ src = (uint32_t *)bayer10p;
+ dst = (uint32_t *)bayer8;
+ for (i = 0; i < len ; i += 4) {
+ *dst = *src;
+ dst++;
+ src = (uint32_t *)(((uint8_t *)src) + 5);
This will lead to unaligned 32 bit integer accesses which will terminate
the program with an illegal instruction on pretty much all architectures
except for x86.
You will need to copy the 4 components 1 by 1 so that you only
use byte accesses.
Also you seem to simply be throwing away the extra 2 bits, although
that will work I wonder if that is the best we can do?
Regards,
Hans
+ }
+}
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index 9a467e10..3020a39e 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -264,6 +264,10 @@ void v4lconvert_bayer_to_bgr24(const unsigned char *bayer,
void v4lconvert_bayer_to_yuv420(const unsigned char *bayer, unsigned char
*yuv,
int width, int height, const unsigned int stride, unsigned int
src_pixfmt, int yvu);
+
+void v4lconvert_bayer10p_to_bayer8(unsigned char *bayer10p,
+ unsigned char *bayer8, int width, int height);
+
void v4lconvert_hm12_to_rgb24(const unsigned char *src,
unsigned char *dst, int width, int height);
diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
index d666bd97..b3dbf5a0 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -133,6 +133,10 @@ static const struct v4lconvert_pixfmt
supported_src_pixfmts[] = {
{ V4L2_PIX_FMT_SRGGB8, 8, 8, 8, 0 },
{ V4L2_PIX_FMT_STV0680, 8, 8, 8, 1 },
{ V4L2_PIX_FMT_SGRBG10, 16, 8, 8, 1 },
+ { V4L2_PIX_FMT_SBGGR10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SGBRG10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SGRBG10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SRGGB10P,10, 8, 8, 1 },
/* compressed bayer */
{ V4L2_PIX_FMT_SPCA561, 0, 9, 9, 1 },
{ V4L2_PIX_FMT_SN9C10X, 0, 9, 9, 1 },
@@ -687,6 +691,10 @@ static int v4lconvert_processing_needs_double_conversion(
case V4L2_PIX_FMT_SGBRG8:
case V4L2_PIX_FMT_SGRBG8:
case V4L2_PIX_FMT_SRGGB8:
+ case V4L2_PIX_FMT_SBGGR10P:
+ case V4L2_PIX_FMT_SGBRG10P:
+ case V4L2_PIX_FMT_SGRBG10P:
+ case V4L2_PIX_FMT_SRGGB10P:
case V4L2_PIX_FMT_STV0680:
return 0;
}
@@ -979,6 +987,33 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
}
/* Raw bayer formats */
+ case V4L2_PIX_FMT_SBGGR10P:
+ case V4L2_PIX_FMT_SGBRG10P:
+ case V4L2_PIX_FMT_SGRBG10P:
+ case V4L2_PIX_FMT_SRGGB10P:
+ if (src_size < ((width * height * 10)/8)) {
+ V4LCONVERT_ERR("short raw bayer10 data frame\n");
+ errno = EPIPE;
+ result = -1;
+ }
+ switch (src_pix_fmt) {
+ case V4L2_PIX_FMT_SBGGR10P:
+ src_pix_fmt = V4L2_PIX_FMT_SBGGR8;
+ break;
+ case V4L2_PIX_FMT_SGBRG10P:
+ src_pix_fmt = V4L2_PIX_FMT_SGBRG8;
+ break;
+ case V4L2_PIX_FMT_SGRBG10P:
+ src_pix_fmt = V4L2_PIX_FMT_SGRBG8;
+ break;
+ case V4L2_PIX_FMT_SRGGB10P:
+ src_pix_fmt = V4L2_PIX_FMT_SRGGB8;
+ break;
+ }
+ v4lconvert_bayer10p_to_bayer8(src, src, width, height);
+
Re: [PATCH] libv4l: Add support for BAYER10P format conversion
Hi,
On 21-09-18 09:40, Ricardo Ribalda Delgado wrote:
Hi Hans
On Fri, Sep 21, 2018 at 9:38 AM Hans de Goede wrote:
Hi,
On 20-09-18 22:04, Ricardo Ribalda Delgado wrote:
Add support for 10 bit packet Bayer formats:
-V4L2_PIX_FMT_SBGGR10P
-V4L2_PIX_FMT_SGBRG10P
-V4L2_PIX_FMT_SGRBG10P
-V4L2_PIX_FMT_SRGGB10P
These formats pack the 2 LSBs for every 4 pixels in an indeppendent
byte.
Signed-off-by: Ricardo Ribalda Delgado
---
lib/libv4lconvert/bayer.c | 15 +++
lib/libv4lconvert/libv4lconvert-priv.h | 4 +++
lib/libv4lconvert/libv4lconvert.c | 35 ++
3 files changed, 54 insertions(+)
diff --git a/lib/libv4lconvert/bayer.c b/lib/libv4lconvert/bayer.c
index 4b70ddd9..d7d488f9 100644
--- a/lib/libv4lconvert/bayer.c
+++ b/lib/libv4lconvert/bayer.c
@@ -631,3 +631,18 @@ void v4lconvert_bayer_to_yuv420(const unsigned char
*bayer, unsigned char *yuv,
v4lconvert_border_bayer_line_to_y(bayer + stride, bayer, ydst, width,
!start_with_green, !blue_line);
}
+
+void v4lconvert_bayer10p_to_bayer8(unsigned char *bayer10p,
+ unsigned char *bayer8, int width, int height)
+{
+ long i, len = width * height;
+ uint32_t *src, *dst;
+
+ src = (uint32_t *)bayer10p;
+ dst = (uint32_t *)bayer8;
+ for (i = 0; i < len ; i += 4) {
+ *dst = *src;
+ dst++;
+ src = (uint32_t *)(((uint8_t *)src) + 5);
This will lead to unaligned 32 bit integer accesses which will terminate
the program with an illegal instruction on pretty much all architectures
except for x86.
I see your point, but I am actually using this code on ARM64 with no issues.
That is weird, this is definitely illegal on armv7 perhaps the compiler
recognizes the problem and fixes it in the generated code?
I will change it.
Thanks.
You will need to copy the 4 components 1 by 1 so that you only
use byte accesses.
Also you seem to simply be throwing away the extra 2 bits, although
that will work I wonder if that is the best we can do?
Those are the LSB. If the user want the extra resolution has to use
the bayer mode.
Ok.
Regards,
Hans
Regards,
Hans
+ }
+}
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index 9a467e10..3020a39e 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -264,6 +264,10 @@ void v4lconvert_bayer_to_bgr24(const unsigned char *bayer,
void v4lconvert_bayer_to_yuv420(const unsigned char *bayer, unsigned char
*yuv,
int width, int height, const unsigned int stride, unsigned int
src_pixfmt, int yvu);
+
+void v4lconvert_bayer10p_to_bayer8(unsigned char *bayer10p,
+ unsigned char *bayer8, int width, int height);
+
void v4lconvert_hm12_to_rgb24(const unsigned char *src,
unsigned char *dst, int width, int height);
diff --git a/lib/libv4lconvert/libv4lconvert.c
b/lib/libv4lconvert/libv4lconvert.c
index d666bd97..b3dbf5a0 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -133,6 +133,10 @@ static const struct v4lconvert_pixfmt
supported_src_pixfmts[] = {
{ V4L2_PIX_FMT_SRGGB8, 8, 8, 8, 0 },
{ V4L2_PIX_FMT_STV0680, 8, 8, 8, 1 },
{ V4L2_PIX_FMT_SGRBG10, 16, 8, 8, 1 },
+ { V4L2_PIX_FMT_SBGGR10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SGBRG10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SGRBG10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SRGGB10P,10, 8, 8, 1 },
/* compressed bayer */
{ V4L2_PIX_FMT_SPCA561, 0, 9, 9, 1 },
{ V4L2_PIX_FMT_SN9C10X, 0, 9, 9, 1 },
@@ -687,6 +691,10 @@ static int v4lconvert_processing_needs_double_conversion(
case V4L2_PIX_FMT_SGBRG8:
case V4L2_PIX_FMT_SGRBG8:
case V4L2_PIX_FMT_SRGGB8:
+ case V4L2_PIX_FMT_SBGGR10P:
+ case V4L2_PIX_FMT_SGBRG10P:
+ case V4L2_PIX_FMT_SGRBG10P:
+ case V4L2_PIX_FMT_SRGGB10P:
case V4L2_PIX_FMT_STV0680:
return 0;
}
@@ -979,6 +987,33 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
}
/* Raw bayer formats */
+ case V4L2_PIX_FMT_SBGGR10P:
+ case V4L2_PIX_FMT_SGBRG10P:
+ case V4L2_PIX_FMT_SGRBG10P:
+ case V4L2_PIX_FMT_SRGGB10P:
+ if (src_size < ((width * height * 10)/8)) {
+ V4LCONVERT_ERR("short raw bayer10 data frame\n");
+ errno = EPIPE;
+ result = -1;
+ }
+ switch (src_pix_fmt) {
+ case V4L2_PIX_FMT_SBGGR10P:
+ src_pix_fmt = V4L2_PIX_FMT_SBGGR8;
+ break;
+ case V4L2_PIX_FMT_SGBRG10P:
+
Re: [PATCH v2] libv4l: Add support for BAYER10P format conversion
Hi,
On 21-09-18 11:04, Ricardo Ribalda Delgado wrote:
Add support for 10 bit packet Bayer formats:
-V4L2_PIX_FMT_SBGGR10P
-V4L2_PIX_FMT_SGBRG10P
-V4L2_PIX_FMT_SGRBG10P
-V4L2_PIX_FMT_SRGGB10P
These formats pack the 2 LSBs for every 4 pixels in an indeppendent
byte.
Signed-off-by: Ricardo Ribalda Delgado
Patch looks good to me now:
Acked-by: Hans de Goede
Regards,
Hans
---
lib/libv4lconvert/bayer.c | 21
lib/libv4lconvert/libv4lconvert-priv.h | 4 +++
lib/libv4lconvert/libv4lconvert.c | 35 ++
3 files changed, 60 insertions(+)
diff --git a/lib/libv4lconvert/bayer.c b/lib/libv4lconvert/bayer.c
index 4b70ddd9..11af6543 100644
--- a/lib/libv4lconvert/bayer.c
+++ b/lib/libv4lconvert/bayer.c
@@ -631,3 +631,24 @@ void v4lconvert_bayer_to_yuv420(const unsigned char
*bayer, unsigned char *yuv,
v4lconvert_border_bayer_line_to_y(bayer + stride, bayer, ydst, width,
!start_with_green, !blue_line);
}
+
+void v4lconvert_bayer10p_to_bayer8(unsigned char *bayer10p,
+ unsigned char *bayer8, int width, int height)
+{
+ unsigned long i;
+ unsigned long len = width * height;
+
+ for (i = 0; i < len ; i += 4) {
+ /*
+* Do not use a second loop, hoping that
+* a clever compiler with understand the
+* pattern and will optimize it.
+*/
+ bayer8[0] = bayer10p[0];
+ bayer8[1] = bayer10p[1];
+ bayer8[2] = bayer10p[2];
+ bayer8[3] = bayer10p[3];
+ bayer10p += 5;
+ bayer8 += 4;
+ }
+}
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index 9a467e10..3020a39e 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -264,6 +264,10 @@ void v4lconvert_bayer_to_bgr24(const unsigned char *bayer,
void v4lconvert_bayer_to_yuv420(const unsigned char *bayer, unsigned char
*yuv,
int width, int height, const unsigned int stride, unsigned int
src_pixfmt, int yvu);
+
+void v4lconvert_bayer10p_to_bayer8(unsigned char *bayer10p,
+ unsigned char *bayer8, int width, int height);
+
void v4lconvert_hm12_to_rgb24(const unsigned char *src,
unsigned char *dst, int width, int height);
diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
index d666bd97..b3dbf5a0 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -133,6 +133,10 @@ static const struct v4lconvert_pixfmt
supported_src_pixfmts[] = {
{ V4L2_PIX_FMT_SRGGB8, 8, 8, 8, 0 },
{ V4L2_PIX_FMT_STV0680, 8, 8, 8, 1 },
{ V4L2_PIX_FMT_SGRBG10, 16, 8, 8, 1 },
+ { V4L2_PIX_FMT_SBGGR10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SGBRG10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SGRBG10P,10, 8, 8, 1 },
+ { V4L2_PIX_FMT_SRGGB10P,10, 8, 8, 1 },
/* compressed bayer */
{ V4L2_PIX_FMT_SPCA561, 0, 9, 9, 1 },
{ V4L2_PIX_FMT_SN9C10X, 0, 9, 9, 1 },
@@ -687,6 +691,10 @@ static int v4lconvert_processing_needs_double_conversion(
case V4L2_PIX_FMT_SGBRG8:
case V4L2_PIX_FMT_SGRBG8:
case V4L2_PIX_FMT_SRGGB8:
+ case V4L2_PIX_FMT_SBGGR10P:
+ case V4L2_PIX_FMT_SGBRG10P:
+ case V4L2_PIX_FMT_SGRBG10P:
+ case V4L2_PIX_FMT_SRGGB10P:
case V4L2_PIX_FMT_STV0680:
return 0;
}
@@ -979,6 +987,33 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
}
/* Raw bayer formats */
+ case V4L2_PIX_FMT_SBGGR10P:
+ case V4L2_PIX_FMT_SGBRG10P:
+ case V4L2_PIX_FMT_SGRBG10P:
+ case V4L2_PIX_FMT_SRGGB10P:
+ if (src_size < ((width * height * 10)/8)) {
+ V4LCONVERT_ERR("short raw bayer10 data frame\n");
+ errno = EPIPE;
+ result = -1;
+ }
+ switch (src_pix_fmt) {
+ case V4L2_PIX_FMT_SBGGR10P:
+ src_pix_fmt = V4L2_PIX_FMT_SBGGR8;
+ break;
+ case V4L2_PIX_FMT_SGBRG10P:
+ src_pix_fmt = V4L2_PIX_FMT_SGBRG8;
+ break;
+ case V4L2_PIX_FMT_SGRBG10P:
+ src_pix_fmt = V4L2_PIX_FMT_SGRBG8;
+ break;
+ case V4L2_PIX_FMT_SRGGB10P:
+ src_pix_fmt = V4L2_PIX_FMT_SRGGB8;
+ break;
+ }
+ v4lconvert_bayer10p_to_bayer8(src, src, width, height);
+ bytesperline = width;
+
+
Re: [RFC] V4L2_PIX_FMT_MJPEG vs V4L2_PIX_FMT_JPEG
Hi, On 05-10-18 13:55, Mauro Carvalho Chehab wrote: Em Mon, 1 Oct 2018 18:19:21 +0100 Dave Stevenson escreveu: Hi All, On Mon, 1 Oct 2018 at 17:32, Ezequiel Garcia wrote: Hi Hans, Thanks for looking into. I remember MJPEG vs. JPEG being a source of confusion for me a few years ago, so clarification is greatly welcome :-) On Mon, 2018-10-01 at 15:03 +0300, Laurent Pinchart wrote: Hi Hans, On Monday, 1 October 2018 14:54:29 EEST Hans Verkuil wrote: On 10/01/2018 01:48 PM, Laurent Pinchart wrote: On Monday, 1 October 2018 11:43:04 EEST Hans Verkuil wrote: It turns out that we have both JPEG and Motion-JPEG pixel formats defined. Furthermore, some drivers support one, some the other and some both. These pixelformats both mean the same. Do they ? I thought MJPEG was JPEG using fixed Huffman tables that were not included in the JPEG headers. I'm not aware of any difference. If there is one, then it is certainly not documented. What I can tell for sure is that many UVC devices don't include Huffman tables in their JPEG headers. Ezequiel, since you've been working with this recently, do you know anything about this? JPEG frames must include huffman and quantization tables, as per the standard. AFAIK, there's no MJPEG specification per-se and vendors specify its own way of conveying a Motion JPEG stream. There is the specfication for MJPEG in Quicktime containers, which defines the MJPEG-A and MJPEG-B variants [1]. MJPEG-B is not a concatenation of JPEG frames as the framing is different, so can't really be combined into V4L2_PIX_FMT_JPEG. Have people encountered devices that produce MJPEG-A or MJPEG-B via V4L2? I haven't, but I have been forced to support both variants on decode. Checking it is not an easy task. I *suspect* that those cameras are all MJPEG-A, as the libv4l decoder uses tinyjpeg library to handle both JPEG and MJPEG. Maybe Hans de Goede knows more about that, and may have actually tested it with different camera models. I've tested the JPG path in libv4l with quite a lot of cameras and sofar it has worked for all of them. There are some non UVC cameras where the hardware produces raw JPG data, but in that case the kernel driver prefixes a JPG header to each frame so that it looks like a regular JPG. Regards, Hans On that thought, whilst capture devices generally don't care, is there a need to differentiate for M2M codec devices which can support encoding the variants? Or likewise on M2M decoders that support only JPEG, how do they tell userspace that they don't support MJPEG-A or MJPEG-B? Dave [1] https://developer.apple.com/standards/qtff-2001.pdf For instance, omiting the huffman table seems to be a vendor thing. Microsoft explicitly omits the huffman tables from each frame: https://www.fileformat.info/format/bmp/spec/b7c72ebab8064da48ae5ed0c053c67a4/view.htm Others could be following the same things. Like I mentioned before, Gstreamer always check for missing huffman table and adds one if missing. Gstreamer has other quirks for missing markers, e.g. deal with a missing EOI: https://github.com/GStreamer/gst-plugins-good/commit/10ff3c8e14e8fba9e0a5d696dce0bea27de644d7 I think Hans suggestion of settling on JPEG makes sense and it would be consistent with Gstreamer. Otherwise, we should specify exactly what we understand by MJPEG, but I don't think it's worth it. Thanks, Ezequiel Thanks, Mauro
Re: media: uvcvideo: Support realtek's UVC 1.5 device
Hi, On 09-05-18 04:13, ming_q...@realsil.com.cn wrote: From: ming_qian The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1. Change it to 48 for UVC 1.5 device, and the UVC 1.5 device can be recognized. More changes to the driver are needed for full UVC 1.5 compatibility. However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have been reported to work well. Signed-off-by: ming_qian Tested-by: Kai-Heng Feng Looks good to me: Reviewed-by: Hans de Goede Regards, Hans --- drivers/media/usb/uvc/uvc_video.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index aa0082f..32dfb32 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream, int ret; size = stream->dev->uvc_version >= 0x0110 ? 34 : 26; + if (stream->dev->uvc_version >= 0x0150) + size = 48; if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) && query == UVC_GET_DEF) return -EIO; @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream, int ret; size = stream->dev->uvc_version >= 0x0110 ? 34 : 26; + if (stream->dev->uvc_version >= 0x0150) + size = 48; data = kzalloc(size, GFP_KERNEL); if (data == NULL) return -ENOMEM;
Re: [PATCH 1/4] gspca: convert to vb2
return gspca_dev->fr_o != atomic_read(&gspca_dev->fr_i);
+ if (*nplanes)
+ return sizes[0] < gspca_dev->pixfmt.sizeimage ? -EINVAL : 0;
+ *nplanes = 1;
+ sizes[0] = gspca_dev->pixfmt.sizeimage;
+ return 0;
}
-static int frame_ready(struct gspca_dev *gspca_dev, struct file *file,
- enum v4l2_memory memory)
+static int gspca_buffer_prepare(struct vb2_buffer *vb)
{
- int ret;
+ struct gspca_dev *gspca_dev = vb2_get_drv_priv(vb->vb2_queue);
+ unsigned long size = gspca_dev->pixfmt.sizeimage;
- if (mutex_lock_interruptible(&gspca_dev->queue_lock))
- return -ERESTARTSYS;
- ret = frame_ready_nolock(gspca_dev, file, memory);
- mutex_unlock(&gspca_dev->queue_lock);
- return ret;
+ if (vb2_plane_size(vb, 0) < size) {
+ gspca_err(gspca_dev, "buffer too small (%lu < %lu)\n",
+vb2_plane_size(vb, 0), size);
+ return -EINVAL;
+ }
+ return 0;
}
-/*
- * dequeue a video buffer
- *
- * If nonblock_ing is false, block until a buffer is available.
- */
-static int vidioc_dqbuf(struct file *file, void *priv,
- struct v4l2_buffer *v4l2_buf)
+static void gspca_buffer_finish(struct vb2_buffer *vb)
{
- struct gspca_dev *gspca_dev = video_drvdata(file);
- struct gspca_frame *frame;
- int i, j, ret;
-
- gspca_dbg(gspca_dev, D_FRAM, "dqbuf\n");
-
- if (mutex_lock_interruptible(&gspca_dev->queue_lock))
- return -ERESTARTSYS;
-
- for (;;) {
- ret = frame_ready_nolock(gspca_dev, file, v4l2_buf->memory);
- if (ret < 0)
- goto out;
- if (ret > 0)
- break;
-
- mutex_unlock(&gspca_dev->queue_lock);
-
- if (file->f_flags & O_NONBLOCK)
- return -EAGAIN;
-
- /* wait till a frame is ready */
- ret = wait_event_interruptible_timeout(gspca_dev->wq,
- frame_ready(gspca_dev, file, v4l2_buf->memory),
- msecs_to_jiffies(3000));
- if (ret < 0)
- return ret;
- if (ret == 0)
- return -EIO;
-
- if (mutex_lock_interruptible(&gspca_dev->queue_lock))
- return -ERESTARTSYS;
- }
+ struct gspca_dev *gspca_dev = vb2_get_drv_priv(vb->vb2_queue);
- i = gspca_dev->fr_o;
- j = gspca_dev->fr_queue[i];
- frame = &gspca_dev->frame[j];
-
- gspca_dev->fr_o = (i + 1) % GSPCA_MAX_FRAMES;
-
- frame->v4l2_buf.flags &= ~V4L2_BUF_FLAG_DONE;
- memcpy(v4l2_buf, &frame->v4l2_buf, sizeof *v4l2_buf);
- gspca_dbg(gspca_dev, D_FRAM, "dqbuf %d\n", j);
- ret = 0;
-
- if (gspca_dev->memory == V4L2_MEMORY_USERPTR) {
- if (copy_to_user((__u8 __user *) frame->v4l2_buf.m.userptr,
-frame->data,
-frame->v4l2_buf.bytesused)) {
- gspca_err(gspca_dev, "dqbuf cp to user failed\n");
- ret = -EFAULT;
- }
- }
-out:
- mutex_unlock(&gspca_dev->queue_lock);
-
- if (ret == 0 && gspca_dev->sd_desc->dq_callback) {
- mutex_lock(&gspca_dev->usb_lock);
- gspca_dev->usb_err = 0;
- if (gspca_dev->present)
- gspca_dev->sd_desc->dq_callback(gspca_dev);
- mutex_unlock(&gspca_dev->usb_lock);
- }
+ if (!gspca_dev->sd_desc->dq_callback)
+ return;
- return ret;
+ gspca_dev->usb_err = 0;
+ gspca_dev->sd_desc->dq_callback(gspca_dev);
}
You are loosing the "if (gspca_dev->present)" check around
the dq_callback here, this may causes issues if the
buffer_finish method gets called after the device has
been unplugged.
If the vb2 code takes care that the buffer_finish method
doesn't get called then you may add my:
Reviewed-by: Hans de Goede
To this patch.
Patch 2-4 look good to and you may add my:
Reviewed-by: Hans de Goede
To those too.
Regards,
Hans
p.s.
If the v4l2-ctl + vb2 frameworks take care of not having
any driver callbacks called after disconnect, perhaps
the present flag can be removed?
-/*
- * queue a video buffer
- *
- * Attempting to queue a buffer that has already been
- * queued will return -EINVAL.
- */
-static int vidioc_qbuf(struct file *file, void *priv,
- struct v4l2_buffer *v4l2_buf)
+static void gspca_buffer_queue(struct vb2_buffer *vb)
{
- struct gspca_dev *gspca_dev = video_drvdata(fil
Re: [PATCH 1/4] gspca: convert to vb2
Hi,
On 05/13/2018 11:32 AM, Hans Verkuil wrote:
On 05/12/2018 08:00 PM, Hans de Goede wrote:
Hi Hans,
Overall looks good, 1 comment inline.
- if (ret == 0 && gspca_dev->sd_desc->dq_callback) {
- mutex_lock(&gspca_dev->usb_lock);
- gspca_dev->usb_err = 0;
- if (gspca_dev->present)
- gspca_dev->sd_desc->dq_callback(gspca_dev);
- mutex_unlock(&gspca_dev->usb_lock);
- }
+ if (!gspca_dev->sd_desc->dq_callback)
+ return;
- return ret;
+ gspca_dev->usb_err = 0;
+ gspca_dev->sd_desc->dq_callback(gspca_dev);
}
You are loosing the "if (gspca_dev->present)" check around
the dq_callback here, this may causes issues if the
buffer_finish method gets called after the device has
been unplugged.
Good catch, I've added the 'if' here.
Ok, with that change you may add my rev-by to the entire
series.
Regards,
Hans
If the vb2 code takes care that the buffer_finish method
doesn't get called then you may add my:
Reviewed-by: Hans de Goede
To this patch.
Patch 2-4 look good to and you may add my:
Reviewed-by: Hans de Goede
To those too.
Thanks!
Regards,
Hans
p.s.
If the v4l2-ctl + vb2 frameworks take care of not having
any driver callbacks called after disconnect, perhaps
the present flag can be removed?
I actually tried that (using the video_is_registered() function
instead), but it is used all over in gspca subdrivers, and I didn't
want to change them all. It's easier to just keep the field.
The same is true for the streaming field, for that matter. It could
be replaced by a vb2 function, but it would require lots of changes
in gspca subdrivers as well.
Regards,
Hans
-/*
- * queue a video buffer
- *
- * Attempting to queue a buffer that has already been
- * queued will return -EINVAL.
- */
-static int vidioc_qbuf(struct file *file, void *priv,
- struct v4l2_buffer *v4l2_buf)
+static void gspca_buffer_queue(struct vb2_buffer *vb)
{
- struct gspca_dev *gspca_dev = video_drvdata(file);
- struct gspca_frame *frame;
- int i, index, ret;
-
- gspca_dbg(gspca_dev, D_FRAM, "qbuf %d\n", v4l2_buf->index);
-
- if (mutex_lock_interruptible(&gspca_dev->queue_lock))
- return -ERESTARTSYS;
-
- index = v4l2_buf->index;
- if ((unsigned) index >= gspca_dev->nframes) {
- gspca_dbg(gspca_dev, D_FRAM,
- "qbuf idx %d >= %d\n", index, gspca_dev->nframes);
- ret = -EINVAL;
- goto out;
- }
- if (v4l2_buf->memory != gspca_dev->memory) {
- gspca_dbg(gspca_dev, D_FRAM, "qbuf bad memory type\n");
- ret = -EINVAL;
- goto out;
- }
-
- frame = &gspca_dev->frame[index];
- if (frame->v4l2_buf.flags & BUF_ALL_FLAGS) {
- gspca_dbg(gspca_dev, D_FRAM, "qbuf bad state\n");
- ret = -EINVAL;
- goto out;
- }
-
- frame->v4l2_buf.flags |= V4L2_BUF_FLAG_QUEUED;
-
- if (frame->v4l2_buf.memory == V4L2_MEMORY_USERPTR) {
- frame->v4l2_buf.m.userptr = v4l2_buf->m.userptr;
- frame->v4l2_buf.length = v4l2_buf->length;
- }
-
- /* put the buffer in the 'queued' queue */
- i = atomic_read(&gspca_dev->fr_q);
- gspca_dev->fr_queue[i] = index;
- atomic_set(&gspca_dev->fr_q, (i + 1) % GSPCA_MAX_FRAMES);
+ struct gspca_dev *gspca_dev = vb2_get_drv_priv(vb->vb2_queue);
+ struct gspca_buffer *buf = to_gspca_buffer(vb);
+ unsigned long flags;
- v4l2_buf->flags |= V4L2_BUF_FLAG_QUEUED;
- v4l2_buf->flags &= ~V4L2_BUF_FLAG_DONE;
- ret = 0;
-out:
- mutex_unlock(&gspca_dev->queue_lock);
- return ret;
+ spin_lock_irqsave(&gspca_dev->qlock, flags);
+ list_add_tail(&buf->list, &gspca_dev->buf_list);
+ spin_unlock_irqrestore(&gspca_dev->qlock, flags);
}
-/*
- * allocate the resources for read()
- */
-static int read_alloc(struct gspca_dev *gspca_dev,
- struct file *file)
+static void gspca_return_all_buffers(struct gspca_dev *gspca_dev,
+enum vb2_buffer_state state)
{
- struct v4l2_buffer v4l2_buf;
- int i, ret;
-
- gspca_dbg(gspca_dev, D_STREAM, "read alloc\n");
+ struct gspca_buffer *buf, *node;
+ unsigned long flags;
- if (mutex_lock_interruptible(&gspca_dev->usb_lock))
- return -ERESTARTSYS;
-
- if (gspca_dev->nframes == 0) {
- struct v4l2_requestbuffers rb;
-
- memset(&rb,
Re: [PATCH] [media] gspca: Stop using GFP_DMA for buffers for USB bulk transfers
Hi, On 05/13/2018 07:54 PM, Adam Baker wrote: On 05/05/18 09:22, Hans de Goede wrote: The recent "x86 ZONE_DMA love" discussion at LSF/MM pointed out that some gspca sub-drivvers are using GFP_DMA to allocate buffers which are used for USB bulk transfers, there is absolutely no need for this, drop it. The documentation for kmalloc() says GFP_DMA - Allocation suitable for DMA. end at least in sq905.c the allocation is passed to the USB stack that then uses it for DMA. Looking a bit closer the "suitable for DMA" label that GFP_DMA promises is not really a sensible thing for kmalloc() to determine as it is dependent on the DMA controller in question. The USB stack now ensures that everything works correctly as long as the memory is allocated with kmalloc() so acked by me for sq905.c but, is anyone taking care of fixing the kmalloc() documentation? The whole GFP_DMA flag use in the kernel is a mess and fixing the doucmentation is not easy and likely also not the solution, see: https://lwn.net/Articles/753273/ Note this article is currently only available to LWN subscribers (it will become freely available in a week). I'll send you a private mail with a link which will allow you to read it. Regards, Hans
Devices with a front and back webcam represented as a single UVC device
Hi Laurent,
At Guadec Carlos (in the Cc) told me that on his Acer 2-in-1 only
the frontcam is working and it seems both are represented by a
single UVC USB device. I've told him to check for some v4l control
to flip between front and back.
Carlos, as I mentioned you can try gtk-v4l
("sudo dnf install gtk-v4l") or qv4l2
("sudo dnf install qv4l2") these will both show
you various controls for the camera. One of those might do the trick.
But I recently bought a 2nd second hand Cherry Trail based HP
X2 2-in-1 and much to my surprise that is actually using an UVC
cam, rather then the usual ATOMISP crap and it has the same issue.
This device does not seem to have a control to flip between the
2 cams, instead it registers 2 /dev/video? nodes but the second
node does not work and dmesg contains:
[ 26.079868] uvcvideo: Found UVC 1.00 device HP TrueVision HD (05c8:03a3)
[ 26.095485] uvcvideo 1-4.2:1.0: Entity type for entity Extension 4 was not
initialized!
[ 26.095492] uvcvideo 1-4.2:1.0: Entity type for entity Processing 2 was not
initialized!
[ 26.095496] uvcvideo 1-4.2:1.0: Entity type for entity Camera 1 was not
initialized!
Laurent, I've attached lsusb -v output so that you can check the
descriptors.
Carlos, it might be good to get Laurent your descriptors too, to do
this do "lsusb", note what is the : for your camera and then
run:
sudo lsusb -v -d : > lsusb.log
And send Laurent a mail with the generated lsusb
Regards,
Hans
Bus 001 Device 005: ID 05c8:03a3 Cheng Uei Precision Industry Co., Ltd (Foxlink)
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize064
idVendor 0x05c8 Cheng Uei Precision Industry Co., Ltd (Foxlink)
idProduct 0x03a3
bcdDevice1.01
iManufacturer 3 Generic
iProduct1 HP TrueVision HD
iSerial 2 200901010001
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 722
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 4 USB Camera
bmAttributes 0x80
(Bus Powered)
MaxPower 500mA
Interface Association:
bLength 8
bDescriptorType11
bFirstInterface 0
bInterfaceCount 2
bFunctionClass 14 Video
bFunctionSubClass 3 Video Interface Collection
bFunctionProtocol 0
iFunction 5 HP TrueVision HD
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass14 Video
bInterfaceSubClass 1 Video Control
bInterfaceProtocol 0
iInterface 5 HP TrueVision HD
VideoControl Interface Descriptor:
bLength13
bDescriptorType36
bDescriptorSubtype 1 (HEADER)
bcdUVC 1.00
wTotalLength 78
dwClockFrequency 15.00MHz
bInCollection 1
baInterfaceNr( 0) 1
VideoControl Interface Descriptor:
bLength18
bDescriptorType36
bDescriptorSubtype 2 (INPUT_TERMINAL)
bTerminalID 1
wTerminalType 0x0201 Camera Sensor
bAssocTerminal 0
iTerminal 0
wObjectiveFocalLengthMin 0
wObjectiveFocalLengthMax 0
wOcularFocalLength0
bControlSize 3
bmControls 0x000e
Auto-Exposure Mode
Auto-Exposure Priority
Exposure Time (Absolute)
VideoControl Interface Descriptor:
bLength11
bDescriptorType36
bDescriptorSubtype 5 (PROCESSING_UNIT)
Warning: Descriptor too short
bUnitID 2
bSourceID 1
wMaxMultiplier 0
bControlSize2
bmControls 0x177f
Brightness
Contrast
Hue
Saturation
Sharpness
Gamma
White Balance Temperature
Backlight Compensation
Gain
Power Line Frequency
White Balance Temperature, Auto
iProcessing 0
bmVideoStandards 0x09
None
SECAM - 625/50
VideoControl Interface Descriptor:
bLength 9
bDescriptorType36
bDescriptorSubtype 3 (OUTPUT_TERMINAL)
bTerminalID 3
wTerminalType 0x0101 USB St
Re: Devices with a front and back webcam represented as a single UVC device
HI,
On 11-07-18 14:08, Laurent Pinchart wrote:
Hi Carlos,
On Wednesday, 11 July 2018 14:36:48 EEST Carlos Garnacho wrote:
On Wed, Jul 11, 2018 at 1:00 PM, Laurent Pinchart wrote:
On Wednesday, 11 July 2018 11:37:14 EEST Hans de Goede wrote:
Hi Laurent,
At Guadec Carlos (in the Cc) told me that on his Acer 2-in-1 only
the frontcam is working and it seems both are represented by a
single UVC USB device. I've told him to check for some v4l control
to flip between front and back.
Carlos, as I mentioned you can try gtk-v4l
("sudo dnf install gtk-v4l") or qv4l2
("sudo dnf install qv4l2") these will both show
you various controls for the camera. One of those might do the trick.
But I recently bought a 2nd second hand Cherry Trail based HP
X2 2-in-1 and much to my surprise that is actually using an UVC
cam, rather then the usual ATOMISP crap and it has the same issue.
This device does not seem to have a control to flip between the
2 cams, instead it registers 2 /dev/video? nodes but the second
node does not work
The second node is there to expose metadata to userspace, not image data.
That's a recent addition to the uvcvideo driver.
and dmesg contains:
[ 26.079868] uvcvideo: Found UVC 1.00 device HP TrueVision HD
(05c8:03a3)
[ 26.095485] uvcvideo 1-4.2:1.0: Entity type for entity Extension 4 was
not initialized!
[ 26.095492] uvcvideo 1-4.2:1.0: Entity type for entity Processing 2
was
not initialized!
[ 26.095496] uvcvideo 1-4.2:1.0: Entity type for entity Camera 1 was
not
initialized!
You can safely ignore those messages. I need to submit a patch to get rid
of them.
Laurent, I've attached lsusb -v output so that you can check the
descriptors.
Thank you.
It's funny how UVC specifies a standard way to describe a device with two
camera sensors with dynamic selection of one of them at runtime, and
vendors instead implement vendor-specific crap :-(
The interesting part in the descriptors is
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode {1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
The extension unit exposes two controls (bmControls is a bitmask). They
can be accessed from userspace through the UVCIOC_CTRL_QUERY ioctl, or
mapped to V4L2 controls through the UVCIOC_CTRL_MAP ioctl, in which case
they will be exposed to standard V4L2 applications.
If you want to experiment with this, I would advise querying both controls
with UVCIOC_CTRL_QUERY. You can use the UVC_GET_CUR, UVC_GET_MIN,
UVC_GET_MAX, UVC_GET_DEF and UVC_GET_RES requests to get the control
current, minimum, maximum, default and resolution values, and UVC_GET_LEN
and UVC_GET_INFO to get the control size (in bytes) and flags. Based on
that you can start experimenting with UVC_SET_CUR to set semi-random
values.
I'm however worried that those two controls would be a register address
and a register value, for indirect access to all hardware registers in
the device. In that case, you would likely need information from the
device vendor, or possibly a USB traffic dump from a Windows machine when
switching between the front and back cameras.
Carlos, it might be good to get Laurent your descriptors too, to do
this do "lsusb", note what is the : for your camera and then
run:
sudo lsusb -v -d : > lsusb.log
And send Laurent a mail with the generated lsusb
That would be appreciated, but I expect the same issue :-(
Please find it attached. IIUC your last email, it might not be the
exact same issue, but you can definitely judge better.
Your device is similar in the sense that it doesn't use the standard UVC
support for multiple camera sensors. It instead exposes two extension units:
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode {1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
VideoControl Interface Descriptor:
bLength29
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 6
guidExtensionCode {26b8105a-0713-4870-979d-da79444bb68e}
bNumControl 9
bNrPins 1
Re: Devices with a front and back webcam represented as a single UVC device
Hi,
On 11-07-18 18:07, Carlos Garnacho wrote:
Hi!,
On Wed, Jul 11, 2018 at 2:41 PM, Hans de Goede wrote:
HI,
On 11-07-18 14:08, Laurent Pinchart wrote:
Hi Carlos,
On Wednesday, 11 July 2018 14:36:48 EEST Carlos Garnacho wrote:
On Wed, Jul 11, 2018 at 1:00 PM, Laurent Pinchart wrote:
On Wednesday, 11 July 2018 11:37:14 EEST Hans de Goede wrote:
Hi Laurent,
At Guadec Carlos (in the Cc) told me that on his Acer 2-in-1 only
the frontcam is working and it seems both are represented by a
single UVC USB device. I've told him to check for some v4l control
to flip between front and back.
Carlos, as I mentioned you can try gtk-v4l
("sudo dnf install gtk-v4l") or qv4l2
("sudo dnf install qv4l2") these will both show
you various controls for the camera. One of those might do the trick.
But I recently bought a 2nd second hand Cherry Trail based HP
X2 2-in-1 and much to my surprise that is actually using an UVC
cam, rather then the usual ATOMISP crap and it has the same issue.
This device does not seem to have a control to flip between the
2 cams, instead it registers 2 /dev/video? nodes but the second
node does not work
The second node is there to expose metadata to userspace, not image
data.
That's a recent addition to the uvcvideo driver.
and dmesg contains:
[ 26.079868] uvcvideo: Found UVC 1.00 device HP TrueVision HD
(05c8:03a3)
[ 26.095485] uvcvideo 1-4.2:1.0: Entity type for entity Extension 4
was
not initialized!
[ 26.095492] uvcvideo 1-4.2:1.0: Entity type for entity Processing 2
was
not initialized!
[ 26.095496] uvcvideo 1-4.2:1.0: Entity type for entity Camera 1 was
not
initialized!
You can safely ignore those messages. I need to submit a patch to get
rid
of them.
Laurent, I've attached lsusb -v output so that you can check the
descriptors.
Thank you.
It's funny how UVC specifies a standard way to describe a device with
two
camera sensors with dynamic selection of one of them at runtime, and
vendors instead implement vendor-specific crap :-(
The interesting part in the descriptors is
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode
{1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
The extension unit exposes two controls (bmControls is a bitmask). They
can be accessed from userspace through the UVCIOC_CTRL_QUERY ioctl, or
mapped to V4L2 controls through the UVCIOC_CTRL_MAP ioctl, in which case
they will be exposed to standard V4L2 applications.
If you want to experiment with this, I would advise querying both
controls
with UVCIOC_CTRL_QUERY. You can use the UVC_GET_CUR, UVC_GET_MIN,
UVC_GET_MAX, UVC_GET_DEF and UVC_GET_RES requests to get the control
current, minimum, maximum, default and resolution values, and
UVC_GET_LEN
and UVC_GET_INFO to get the control size (in bytes) and flags. Based on
that you can start experimenting with UVC_SET_CUR to set semi-random
values.
I'm however worried that those two controls would be a register address
and a register value, for indirect access to all hardware registers in
the device. In that case, you would likely need information from the
device vendor, or possibly a USB traffic dump from a Windows machine
when
switching between the front and back cameras.
Carlos, it might be good to get Laurent your descriptors too, to do
this do "lsusb", note what is the : for your camera and then
run:
sudo lsusb -v -d : > lsusb.log
And send Laurent a mail with the generated lsusb
That would be appreciated, but I expect the same issue :-(
Please find it attached. IIUC your last email, it might not be the
exact same issue, but you can definitely judge better.
Your device is similar in the sense that it doesn't use the standard UVC
support for multiple camera sensors. It instead exposes two extension
units:
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode {1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
VideoControl Interface Descriptor:
bLength29
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 6
Re: Devices with a front and back webcam represented as a single UVC device
Hi,
On 11-07-18 20:26, Carlos Garnacho wrote:
Hi!,
On Wed, Jul 11, 2018 at 7:41 PM, Hans de Goede wrote:
Hi,
On 11-07-18 18:07, Carlos Garnacho wrote:
Hi!,
On Wed, Jul 11, 2018 at 2:41 PM, Hans de Goede
wrote:
HI,
On 11-07-18 14:08, Laurent Pinchart wrote:
Hi Carlos,
On Wednesday, 11 July 2018 14:36:48 EEST Carlos Garnacho wrote:
On Wed, Jul 11, 2018 at 1:00 PM, Laurent Pinchart wrote:
On Wednesday, 11 July 2018 11:37:14 EEST Hans de Goede wrote:
Hi Laurent,
At Guadec Carlos (in the Cc) told me that on his Acer 2-in-1 only
the frontcam is working and it seems both are represented by a
single UVC USB device. I've told him to check for some v4l control
to flip between front and back.
Carlos, as I mentioned you can try gtk-v4l
("sudo dnf install gtk-v4l") or qv4l2
("sudo dnf install qv4l2") these will both show
you various controls for the camera. One of those might do the trick.
But I recently bought a 2nd second hand Cherry Trail based HP
X2 2-in-1 and much to my surprise that is actually using an UVC
cam, rather then the usual ATOMISP crap and it has the same issue.
This device does not seem to have a control to flip between the
2 cams, instead it registers 2 /dev/video? nodes but the second
node does not work
The second node is there to expose metadata to userspace, not image
data.
That's a recent addition to the uvcvideo driver.
and dmesg contains:
[ 26.079868] uvcvideo: Found UVC 1.00 device HP TrueVision HD
(05c8:03a3)
[ 26.095485] uvcvideo 1-4.2:1.0: Entity type for entity Extension 4
was
not initialized!
[ 26.095492] uvcvideo 1-4.2:1.0: Entity type for entity Processing
2
was
not initialized!
[ 26.095496] uvcvideo 1-4.2:1.0: Entity type for entity Camera 1
was
not
initialized!
You can safely ignore those messages. I need to submit a patch to get
rid
of them.
Laurent, I've attached lsusb -v output so that you can check the
descriptors.
Thank you.
It's funny how UVC specifies a standard way to describe a device with
two
camera sensors with dynamic selection of one of them at runtime, and
vendors instead implement vendor-specific crap :-(
The interesting part in the descriptors is
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode
{1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
The extension unit exposes two controls (bmControls is a bitmask).
They
can be accessed from userspace through the UVCIOC_CTRL_QUERY ioctl, or
mapped to V4L2 controls through the UVCIOC_CTRL_MAP ioctl, in which
case
they will be exposed to standard V4L2 applications.
If you want to experiment with this, I would advise querying both
controls
with UVCIOC_CTRL_QUERY. You can use the UVC_GET_CUR, UVC_GET_MIN,
UVC_GET_MAX, UVC_GET_DEF and UVC_GET_RES requests to get the control
current, minimum, maximum, default and resolution values, and
UVC_GET_LEN
and UVC_GET_INFO to get the control size (in bytes) and flags. Based
on
that you can start experimenting with UVC_SET_CUR to set semi-random
values.
I'm however worried that those two controls would be a register
address
and a register value, for indirect access to all hardware registers in
the device. In that case, you would likely need information from the
device vendor, or possibly a USB traffic dump from a Windows machine
when
switching between the front and back cameras.
Carlos, it might be good to get Laurent your descriptors too, to do
this do "lsusb", note what is the : for your camera and
then
run:
sudo lsusb -v -d : > lsusb.log
And send Laurent a mail with the generated lsusb
That would be appreciated, but I expect the same issue :-(
Please find it attached. IIUC your last email, it might not be the
exact same issue, but you can definitely judge better.
Your device is similar in the sense that it doesn't use the standard UVC
support for multiple camera sensors. It instead exposes two extension
units:
VideoControl Interface Descriptor:
bLength27
bDescriptorType36
bDescriptorSubtype 6 (EXTENSION_UNIT)
bUnitID 4
guidExtensionCode
{1229a78c-47b4-4094-b0ce-db07386fb938}
bNumControl 2
bNrPins 1
baSourceID( 0) 2
bControlSize2
bmControls( 0) 0x00
bmControls( 1) 0x06
iExtension 0
VideoControl Interface Descriptor:
Re: [PATCH] libv4lconvert: Add support for V4L2_PIX_FMT_NV12
Hi Ricardo,
On 16-04-19 14:02, Ricardo Ribalda Delgado wrote:
NV12 is a two-plane version YUV 4:2:0, where the U and V components
are subsampled 2x2.
Signed-off-by: Ricardo Ribalda Delgado
Your patch looks good to me, but it pushed the array-size of
the supported_src_pixfmts array over 64 entries (right now it is
exactly 64 entries big).
Which breaks supported_src_formats as that is only 64 bits big:
struct v4lconvert_data {
int fd;
...
int64_t supported_src_formats; /* bitfield */
See e.g. :
for (j = 0; j < ARRAY_SIZE(supported_src_pixfmts); j++)
if (fmt.pixelformat == supported_src_pixfmts[j].fmt)
break;
if (j < ARRAY_SIZE(supported_src_pixfmts)) {
data->supported_src_formats |= 1ULL << j;
I believe this is best fixed by a preparation patch which introduces the
bit-ops from the kernel for this:
include/asm-generic/bitops/non-atomic.h:
static inline void __set_bit(int nr, volatile unsigned long *addr)
{
unsigned long mask = BIT_MASK(nr);
unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
*p |= mask;
}
static inline void __clear_bit(int nr, volatile unsigned long *addr)
{
unsigned long mask = BIT_MASK(nr);
unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
*p &= ~mask;
}
static inline int test_bit(int nr, const volatile unsigned long *addr)
{
return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
}
So you will have to change your patch into a 2 patch patch-set with
the first patch introducing set_bit / clear_bit / test_bit helpers
modelled after the kernel and changing the declaration of
supported_src_formats to:
unsigned long supported_src_formats[128 / BITS_PER_LONG];
Thanks & Regards,
Hans
---
The code has ben tested with qv4l2 and vivid.
v4lconvert_nv12_to_yuv420 has not been tested!!, any suggestions for
how to do it?
Thanks!
lib/libv4lconvert/libv4lconvert-priv.h | 6 +++
lib/libv4lconvert/libv4lconvert.c | 19 +
lib/libv4lconvert/rgbyuv.c | 56 ++
3 files changed, 81 insertions(+)
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index a8046ce2..c45b086e 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -285,6 +285,12 @@ void v4lconvert_hm12_to_yuv420(const unsigned char *src,
void v4lconvert_hsv_to_rgb24(const unsigned char *src, unsigned char *dest,
int width, int height, int bgr, int Xin, unsigned char hsv_enc);
+void v4lconvert_nv12_to_rgb24(const unsigned char *src, unsigned char *dest,
+ int width, int height, int bgr);
+
+void v4lconvert_nv12_to_yuv420(const unsigned char *src, unsigned char *dest,
+ int width, int height, int yvu);
+
void v4lconvert_rotate90(unsigned char *src, unsigned char *dest,
struct v4l2_format *fmt);
diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
index 78fb3432..2111d19f 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -116,6 +116,7 @@ static const struct v4lconvert_pixfmt
supported_src_pixfmts[] = {
{ V4L2_PIX_FMT_SN9C20X_I420,12, 6, 3, 1 },
{ V4L2_PIX_FMT_M420,12, 6, 3, 1 },
{ V4L2_PIX_FMT_HM12,12, 6, 3, 1 },
+ { V4L2_PIX_FMT_NV12,12, 6, 3, 1 },
{ V4L2_PIX_FMT_CPIA1,0, 6, 3, 1 },
/* JPEG and variants */
{ V4L2_PIX_FMT_MJPEG,0, 7, 7, 0 },
@@ -902,6 +903,24 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
}
break;
+ /* NV12 formats */
+ case V4L2_PIX_FMT_NV12:
+ switch (dest_pix_fmt) {
+ case V4L2_PIX_FMT_RGB24:
+ v4lconvert_nv12_to_rgb24(src, dest, width, height, 0);
+ break;
+ case V4L2_PIX_FMT_BGR24:
+ v4lconvert_nv12_to_rgb24(src, dest, width, height, 1);
+ break;
+ case V4L2_PIX_FMT_YUV420:
+ v4lconvert_nv12_to_yuv420(src, dest, width, height, 0);
+ break;
+ case V4L2_PIX_FMT_YVU420:
+ v4lconvert_nv12_to_yuv420(src, dest, width, height, 1);
+ break;
+ }
+ break;
+
/* compressed bayer formats */
case V4L2_PIX_FMT_SPCA561:
case V4L2_PIX_FMT_SN9C10X:
diff --git a/lib/libv4lconvert/rgbyuv.c b/lib/libv4lconvert/rgbyuv.c
index 02c8cb5b..bfe3b15f 100644
--- a/lib/libv4lconvert/rgbyuv.c
+++ b/lib/libv4lconvert/rgbyuv.c
@@ -845,3 +845,59 @@ void v4lconvert_hsv
[PATCH] configure.ac: Add AM_GNU_GETTEXT_VERSION([0.19.8])
Add AM_GNU_GETTEXT_VERSION([0.19.8]) so that autoreconf will properly run autopoint instead of failing because build-aux/config.rpath is not copied. Signed-off-by: Hans de Goede --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 4970eece..a8d215f3 100644 --- a/configure.ac +++ b/configure.ac @@ -96,6 +96,7 @@ PKG_PROG_PKG_CONFIG DX_DOT_FEATURE(ON) DX_INIT_DOXYGEN($PACKAGE_NAME, doxygen_libdvbv5.cfg) ALL_LINGUAS="" +AM_GNU_GETTEXT_VERSION([0.19.8]) AM_GNU_GETTEXT([external]) LIBDVBV5_DOMAIN="libdvbv5" -- 2.21.0
Re: [PATCH v2 1/2] libv4lconvert: Port supported_src_formats to bit-ops
Hi,
On 17-04-19 13:41, Ricardo Ribalda Delgado wrote:
We have passed the barrier of 64 supported formats, therefore a int64_t
is not enough for holding the bitfield.
Instead use bit-ops ala kernel.
Signed-off-by: Ricardo Ribalda Delgado
Suggested-by: Hans de Goede
Thank you.
I've dropped the unnecessary __prefix from the bitop functions and
I've pushed both patches to the upstream master branch now.
Regards,
Hans
---
lib/libv4lconvert/libv4lconvert-priv.h | 3 +-
lib/libv4lconvert/libv4lconvert.c | 40 ++
2 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index a8046ce2..5286a9b1 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -38,6 +38,7 @@
#include "tinyjpeg.h"
#define ARRAY_SIZE(x) ((int)sizeof(x)/(int)sizeof((x)[0]))
+#define BITS_PER_LONG (8 * sizeof(long))
#define V4LCONVERT_ERROR_MSG_SIZE 256
#define V4LCONVERT_MAX_FRAMESIZES 256
@@ -55,7 +56,7 @@ struct v4lconvert_data {
int flags; /* bitfield */
int control_flags; /* bitfield */
unsigned int no_formats;
- int64_t supported_src_formats; /* bitfield */
+ unsigned long supported_src_formats[128 / BITS_PER_LONG];
char error_msg[V4LCONVERT_ERROR_MSG_SIZE];
struct jdec_private *tinyjpeg;
#ifdef HAVE_JPEG
diff --git a/lib/libv4lconvert/libv4lconvert.c
b/lib/libv4lconvert/libv4lconvert.c
index 78fb3432..0607cc8b 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -32,6 +32,29 @@
#include "libv4lsyscall-priv.h"
#define MIN(a, b) (((a) < (b)) ? (a) : (b))
+#define BIT_MASK(nr) (1UL << ((nr) % BITS_PER_LONG))
+#define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
+
+static inline void __set_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+
+ *p |= mask;
+}
+
+static inline void __clear_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+
+ *p &= ~mask;
+}
+
+static inline int __test_bit(int nr, const volatile unsigned long *addr)
+{
+ return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
+}
static void *dev_init(int fd)
{
@@ -231,7 +254,7 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int
fd, void *dev_ops_pri
break;
if (j < ARRAY_SIZE(supported_src_pixfmts)) {
- data->supported_src_formats |= 1ULL << j;
+ __set_bit(j, data->supported_src_formats);
v4lconvert_get_framesizes(data, fmt.pixelformat, j);
if (!supported_src_pixfmts[j].needs_conversion)
always_needs_conversion = 0;
@@ -316,8 +339,15 @@ int v4lconvert_supported_dst_format(unsigned int
pixelformat)
int v4lconvert_supported_dst_fmt_only(struct v4lconvert_data *data)
{
- return v4lcontrol_needs_conversion(data->control) &&
- data->supported_src_formats;
+ int i;
+
+ for (i = 0 ; i < ARRAY_SIZE(data->supported_src_formats); i++)
+ if (data->supported_src_formats[i])
+ break;
+ if (i == ARRAY_SIZE(data->supported_src_formats))
+ return 0;
+
+ return v4lcontrol_needs_conversion(data->control);
}
/* See libv4lconvert.h for description of in / out parameters */
@@ -334,7 +364,7 @@ int v4lconvert_enum_fmt(struct v4lconvert_data *data,
struct v4l2_fmtdesc *fmt)
for (i = 0; i < ARRAY_SIZE(supported_dst_pixfmts); i++)
if (v4lconvert_supported_dst_fmt_only(data) ||
- !(data->supported_src_formats & (1ULL << i))) {
+ !__test_bit(i, data->supported_src_formats)) {
faked_fmts[no_faked_fmts] =
supported_dst_pixfmts[i].fmt;
no_faked_fmts++;
}
@@ -490,7 +520,7 @@ static int v4lconvert_do_try_format(struct v4lconvert_data
*data,
for (i = 0; i < ARRAY_SIZE(supported_src_pixfmts); i++) {
/* is this format supported? */
- if (!(data->supported_src_formats & (1ULL << i)))
+ if (!__test_bit(i, data->supported_src_formats))
continue;
try_fmt = *dest_fmt;
Wrong use of GFP_DMA32 in drivers/media/platform/vivid/vivid-osd.c
Hi Hans, et. al., While debugging another GFP_DMA32 problem I did a quick grep for GFP_DMA32 on the kernel, this result stood out: drivers/media/platform/vivid/vivid-osd.c 373:dev->video_vbase = kzalloc(dev->video_buffer_size, GFP_KERNEL | GFP_DMA32); Because it is making the same mistake as I was, you cannot use GDP_DMA32 with kmalloc and friends, it will end up being ignored. If you need memory below 4G you must call alloc_pages for get_free_pages with GFP_DMA32 to get it. Regards, Hans
[PATCH] [media] gspca: Stop using GFP_DMA for buffers for USB bulk transfers
The recent "x86 ZONE_DMA love" discussion at LSF/MM pointed out that some
gspca sub-drivvers are using GFP_DMA to allocate buffers which are used
for USB bulk transfers, there is absolutely no need for this, drop it.
Cc: "Luis R. Rodriguez"
Signed-off-by: Hans de Goede
---
drivers/media/usb/gspca/jl2005bcd.c | 2 +-
drivers/media/usb/gspca/sq905.c | 2 +-
drivers/media/usb/gspca/sq905c.c| 2 +-
drivers/media/usb/gspca/vicam.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/gspca/jl2005bcd.c
b/drivers/media/usb/gspca/jl2005bcd.c
index d668589598d6..c40245950553 100644
--- a/drivers/media/usb/gspca/jl2005bcd.c
+++ b/drivers/media/usb/gspca/jl2005bcd.c
@@ -321,7 +321,7 @@ static void jl2005c_dostream(struct work_struct *work)
int ret;
u8 *buffer;
- buffer = kmalloc(JL2005C_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
+ buffer = kmalloc(JL2005C_MAX_TRANSFER, GFP_KERNEL);
if (!buffer) {
pr_err("Couldn't allocate USB buffer\n");
goto quit_stream;
diff --git a/drivers/media/usb/gspca/sq905.c b/drivers/media/usb/gspca/sq905.c
index cc8ff41b8ab3..ffea9c35b0a0 100644
--- a/drivers/media/usb/gspca/sq905.c
+++ b/drivers/media/usb/gspca/sq905.c
@@ -217,7 +217,7 @@ static void sq905_dostream(struct work_struct *work)
u8 *data;
u8 *buffer;
- buffer = kmalloc(SQ905_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
+ buffer = kmalloc(SQ905_MAX_TRANSFER, GFP_KERNEL);
if (!buffer) {
pr_err("Couldn't allocate USB buffer\n");
goto quit_stream;
diff --git a/drivers/media/usb/gspca/sq905c.c b/drivers/media/usb/gspca/sq905c.c
index 5e1269eb7c50..274921c0bb46 100644
--- a/drivers/media/usb/gspca/sq905c.c
+++ b/drivers/media/usb/gspca/sq905c.c
@@ -138,7 +138,7 @@ static void sq905c_dostream(struct work_struct *work)
int ret;
u8 *buffer;
- buffer = kmalloc(SQ905C_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
+ buffer = kmalloc(SQ905C_MAX_TRANSFER, GFP_KERNEL);
if (!buffer) {
pr_err("Couldn't allocate USB buffer\n");
goto quit_stream;
diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index 554b90ef2200..8562bda0ef88 100644
--- a/drivers/media/usb/gspca/vicam.c
+++ b/drivers/media/usb/gspca/vicam.c
@@ -182,7 +182,7 @@ static void vicam_dostream(struct work_struct *work)
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage +
HEADER_SIZE;
- buffer = kmalloc(frame_sz, GFP_KERNEL | GFP_DMA);
+ buffer = kmalloc(frame_sz, GFP_KERNEL);
if (!buffer) {
pr_err("Couldn't allocate USB buffer\n");
goto exit;
--
2.17.0
[PATCH] [media] sn9c20x: Add MSI MS-1039 laptop to flip_dmi_table
Like a bunch of other MSI laptops the MS-1039 uses a 0c45:627b
SN9C201 + OV7660 webcam which is mounted upside down.
Add it to the sn9c20x flip_dmi_table to deal with this.
Cc: sta...@vger.kernel.org
Reported-by: Rui Salvaterra
Signed-off-by: Hans de Goede
---
drivers/media/usb/gspca/sn9c20x.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/usb/gspca/sn9c20x.c
b/drivers/media/usb/gspca/sn9c20x.c
index b43f89fee6c1..700575757c86 100644
--- a/drivers/media/usb/gspca/sn9c20x.c
+++ b/drivers/media/usb/gspca/sn9c20x.c
@@ -123,6 +123,13 @@ static const struct dmi_system_id flip_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_VERSION, "0341")
}
},
+ {
+ .ident = "MSI MS-1039",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "MICRO-STAR INT'L CO.,LTD."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MS-1039"),
+ }
+ },
{
.ident = "MSI MS-1632",
.matches = {
--
2.23.0.rc2
Re: [PATCH] [media] rc: sunxi-cir: Initialize the spinlock properly
Hi,
On 22-12-15 05:27, Chen-Yu Tsai wrote:
The driver allocates the spinlock but fails to initialize it correctly.
The kernel reports a BUG indicating bad spinlock magic when spinlock
debugging is enabled.
Call spin_lock_init() on it to initialize it correctly.
Fixes: b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
Signed-off-by: Chen-Yu Tsai
Good catch:
Acked-by: Hans de Goede
Regards,
Hans
---
drivers/media/rc/sunxi-cir.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 7830aef3db45..40f77685cc4a 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -153,6 +153,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
if (!ir)
return -ENOMEM;
+ spin_lock_init(&ir->ir_lock);
+
if (of_device_is_compatible(dn, "allwinner,sun5i-a13-ir"))
ir->fifo_size = 64;
else
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sn9c20x: incorrect support for 0c45:6270 MT9V011/MT9V111 ?
Hi TJ, On 12/18/2015 12:48 PM, TJ wrote: I've been trying to get the 0c45:6270 Vehoh VMS-001 'Discovery' Microscope to work correctly and discovered what seem to be differences in the bridge_init and other control commands. The most obvious difference currently is the LEDs do not turn on, but there seem to be other problems with empty frames, bad/unrecognised formats, and resolutions, although vlc is able to render a usable JPEG stream. I've installed the Windows XP Sonix driver package in a Qemu virtual machine guest and used wireshark on the host (Kubuntu 15.10, kernel v4.2) to capture and analyse the control commands. https://iam.tj/projects/misc/usbmon-0c45-6270.pcapng That seems to show the bridge_init, and possibly some of the i2c_init, byte sequences are different. It being the first time I've sniffed a USB driver though, I'm not yet 100% confident I'm identifying the correct starting point of the control command flow or the relationships between code and what is on the wire. The Windows .inf seems to indicate the chipset is MT9V111: %USBPCamDesc% = SN.USBPCam,USB\VID_0c45&PID_6270 ; SN9C201 + MI0360\MT9V111 but the sn9c20x is matching as the MT9V011 (I've copied the module to a DKMS build location and named the clone sn9c20x_vehoh, matching only on 0c45_6270, to make testing easier): Right, so it likely actually is an MT9V011, at least we are successfully reading the sensor-id, and it has the id of an MT9V011, reading the id is more reliable then relying on the windows inf file :) gspca_main: v2.14.0 registered gspca_main: sn9c20x_vehoh-2.14.0 probing 0c45:6270 sn9c20x_vehoh: MT9V011 sensor detected sn9c20x_vehoh: MT9VPRB sensor detected input: sn9c20x_vehoh as /devices/pci:00/:00:1d.7/usb2/2-3/input/input34 sn9c20x_vehoh 2-3:1.0: video1 created I'd like to know the best way to add the correct command support in this situation where the existing Linux driver's control data is different to that in use by the Windows driver? The best way I can think of to do this is to add a "#define SENSOR_MT9V011_ALT" to the list of sensor defines which uses the correct init sequences for your cam, and add a module option to override the sd->sensor value from the cmdline, so you would get something like this in sd_config(): if (sensor_override != -1) sd->sensor = sensor_override; else sd->sensor = id->driver_info >> 8; And then you can set the sensor_override module option to SENSOR_MT9V011_ALT when loading the module to work with your cam. I realize that this is not ideal, but I'm afraid it is the best I can come up with, this at least will allow you to develop support for your cam, once we have that we can see if we can find some way to autodetect it. Regards, Hans Do I somehow create another profile in the driver, or directly modify the existing data and command sequences (this latter would seem to risk regressions for other users) ? If creating another profile, how would they differentiate seeing as the device IDs are identical (I've not seen any sign of obvious version responses so far) ? My first attempt to add the correct command values for controlling the LEDs failed, and seems to indicate that more than 1 command is sent to control the LEDs, unlike the sn9c20x driver. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/1] v4lconvert: Add "PEGATRON CORPORATION" to asus_board_vendor
Hans de Goede redhat.com> writes: > > Hi Gregor, > > Here is a patch to add "PEGATRON CORPORATION" to asus_board_vendor, > to fix an upside down bug reported to Fedora: > > https://bugzilla.redhat.com/show_bug.cgi?id=1311545 > > I'm not 100% sure this is a good idea, it might cause a bunch of false > positives, but looking at the existing static PEGATRON entries in the > v4l_control_flags list, it seems that it really is just another vendor > string for Asus and that adding it to asus_board_vendor is best, so > I'm say 95% sure :) > > Anyways your input on this is much appreciated. In the mean time I'll > kick of a scratch-build of the Fedora v4l-utils pkg with this patch > applied for the reporter to test. After sleeping a few days on this, I've decided that this is indeed the best way to deal with this, and given that I've not had any comments on the patch I've just pushed it to v4l-utils master Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] [media] gspca_kinect: enable both video and depth streams
Hi Ulrik, On 06-03-16 14:50, Ulrik de Muelenaere wrote: Hello, The Kinect produces both a video and depth stream, but the current implementation of the gspca_kinect subdriver requires a depth_mode parameter at probe time to select one of the streams which will be exposed as a v4l device. This patchset allows both streams to be accessed as separate video nodes. A possible solution which has been discussed in the past is to call gspca_dev_probe() multiple times in order to create multiple video nodes. See the following mails: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/26194/focus=26213 http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/76715/focus=78344 In the second mail linked above, it was mentioned that gspca_dev_probe() cannot be called multiple times for the same USB interface, since gspca_dev_probe2() stores a pointer to the new gspca_dev as the interface's private data. This is required for gspca_disconnect(), gspca_suspend() and gspca_resume(). As far as I can tell, this is the only reason gspca_dev_probe() cannot be called multiple times. The first patch fixes this by storing other devices linked to the same interface as a linked list. The second patch then calls gspca_dev_probe() twice in the gspca_kinect subdriver, to create a video node for each stream. Some notes on error handling, which I think should be reviewed: * I could not test the gspca_suspend() and gspca_resume() functions. From my evaluation of the code, it seems that the only effect of returning an error code from gspca_resume() is that a message is logged. Therefore I decided to attempt to resume each gspca_dev when the interface is resumed, even if one fails. Bitwise OR seems like the best way to combine potentially multiple error codes. * In the gspca_kinect subdriver, if the second call to gspca_dev_probe() fails, the first video node will still be available. I handle this case by calling gspca_disconnect(), which worked when I was testing, but I am not sure if this is the correct way to handle it. Thanks for the patch-set overall I like it, one thing which worries is me is that sd_start_video and sd_start_depth may race, which is esp. problematic taking into account that both start/stop the depth stream (see the comment about this in sd_start_video()) this will require some coordination, so you like need to do something a bit more advanced and create a shared data struct somewhere for coordination (and with a lock in it). Likewise the v4l2 core is designed to have only one struct v4l2_device per struct device, so you need to modify probe2 to not call v4l2_device_register when it is called a second time for the same intf, and to make gspca_dev->vdev.v4l2_dev point to the v4l2_dev of the first gspca_dev registered. You will also need some changes for this in gspca_disconnect, as you will need to do all the calls on &gspca_dev->v4l2_dev only for the first registered device there, and you will need to do all the calls in gspca_release except for the v4l2_device_unregister() in a loop like you're using in disconnect. Note since you need a shared data struct anyways it might be easier to just use that (add some shared pointer to struct gspca_dev) for the array of gspca_devs rather then using the linked list, as for disconnect / release you will want to use the first registered gspca_dev to get the v4l2_dev address, and your linked approach gives you the last. Regards, Hans Regards, Ulrik Ulrik de Muelenaere (2): [media] gspca: allow multiple probes per USB interface [media] gspca_kinect: enable both video and depth streams drivers/media/usb/gspca/gspca.c | 129 +++ drivers/media/usb/gspca/gspca.h | 1 + drivers/media/usb/gspca/kinect.c | 28 + 3 files changed, 91 insertions(+), 67 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS
Hi,
On 09-03-16 17:03, Antonio Ospite wrote:
When calling VIDIOC_REQBUFS v4l2-compliance fails with this message:
fail: v4l2-test-buffers.cpp(476): q.reqbufs(node, 1)
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
By looking at the v4l2-compliance code the failure happens when trying
to request V4L2_MEMORY_USERPTR buffers without freeing explicitly the
previously allocated V4L2_MEMORY_MMAP buffers.
This would suggest that when changing the memory field in struct
v4l2_requestbuffers the driver is supposed to free automatically any
previous allocated buffers, and looking for inspiration at the code in
drivers/media/v4l2-core/videobuf2-core.c::vb2_core_reqbufs() seems to
confirm this interpretation; however gspca is just returning -EBUSY in
this case.
Removing the special handling for the case of a different memory value
fixes the compliance failure.
Signed-off-by: Antonio Ospite
---
This should be safe, but I'd really like a comment from someone with a more
global knowledge of v4l2.
If my interpretation about how drivers should behave when the value of the
memory field changes is correct, I could send also a documentation update for
Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
Just let me know.
Thanks,
Antonio
drivers/media/usb/gspca/gspca.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 84b0d6a..915b6c7 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1402,13 +1402,6 @@ static int vidioc_reqbufs(struct file *file, void *priv,
if (mutex_lock_interruptible(&gspca_dev->queue_lock))
return -ERESTARTSYS;
- if (gspca_dev->memory != GSPCA_MEMORY_NO
- && gspca_dev->memory != GSPCA_MEMORY_READ
- && gspca_dev->memory != rb->memory) {
- ret = -EBUSY;
- goto out;
- }
-
reqbufs is used internally and this change will allow changing
gspca_dev->memory from USERPTR / MMAP to GSPCA_MEMORY_READ
please replace this check with a check to only allow
rb->memory to be GSPCA_MEMORY_READ when coming from GSPCA_MEMORY_NO
or GSPCA_MEMORY_READ
Regards,
Hans
/* only one file may do the capture */
if (gspca_dev->capt_file != NULL
&& gspca_dev->capt_file != file) {
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] [media] gspca: fix a v4l2-compliance failure during read()
Hi,
On 09-03-16 17:03, Antonio Ospite wrote:
v4l2-compliance fails with this message:
fail: v4l2-test-buffers.cpp(512): Expected EBUSY, got 22
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
Looking at the v4l2-compliance code reveals that this failure is about
the read() callback.
In gspca, dev_read() is calling vidioc_dqbuf() which calls
frame_ready_nolock() but the latter returns -EINVAL in a case when
v4l2-compliance expects -EBUSY.
Fix the failure by changing the return value in frame_ready_nolock().
Signed-off-by: Antonio Ospite
---
drivers/media/usb/gspca/gspca.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 915b6c7..de7e300 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1664,7 +1664,7 @@ static int frame_ready_nolock(struct gspca_dev
*gspca_dev, struct file *file,
return -ENODEV;
if (gspca_dev->capt_file != file || gspca_dev->memory != memory ||
!gspca_dev->streaming)
- return -EINVAL;
+ return -EBUSY;
/* check if a frame is ready */
return gspca_dev->fr_o != atomic_read(&gspca_dev->fr_i);
I'm not sure that this is the right fix:
1) !gspca_dev->streaming should result in -EINVAL, this is the same as what
videobuf2 is doing
2) gspca_dev->memory != memory should result in -EINVAL
3) gspca_dev->capt_file != file means calling dqbuf without having done reqbufs
(through the same fd)
which certainly seemes like -EINVAL to me.
The actual problem is that dev_read() is not catching that mmap is being in use:
static ssize_t dev_read(struct file *file, char __user *data,
size_t count, loff_t *ppos)
{
...
if (gspca_dev->memory == GSPCA_MEMORY_NO) { /* first time ? */
ret = read_alloc(gspca_dev, file);
if (ret != 0)
return ret;
}
It will skip the read_alloc since gspca_dev->memory is USERPTR or MMAP
and then do a dqbuf with memory == GSPCA_MEMORY_READ, triggering the
gspca_dev->memory != memory check.
There are a couple of issues here:
1) gspca_dev->memory check without holding usb_lock, the taking and
releasing of usb_lock should be moved from read_alloc() into dev_read()
itself.
2) dev_read() should not assume that reading is ok if
gspca_dev->memory == GSPCA_MEMORY_NO, it needs a:
if (gspca_dev->memory != GSPCA_MEMORY_NO &&
gspca_dev->memory != GSPCA_MEMORY_READ)
return -EBUSY;
(while holding the usb_lock so the above is wrong)
3) If gspca_dev->memory == GSPCA_MEMORY_READ already the
stream could have been stopped. so we need to check
gspca_dev->streaming (while holding the usb_lock)
and do a streamon if it is not set (and then we can
remove the streamon from read_alloc())
So TL;DR: dev_read needs some love.
Regards,
Hans
p.s.
If you've time to work on v4l2 stuff what gspca really needs
is to just have its buffer handling ripped out and be rewritten
to use videobuf2. I would certainly love to see a patch for that.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v4l-utils PATCHv2] libv4lconvert: Add support for V4L2_PIX_FMT_{NV16,NV61}
Hi,
On 12-03-16 01:45, Niklas Söderlund wrote:
NV16 and NV61 are two-plane versions of the YUV 4:2:2 formats YUYV and
YVYU. Support both formats by merging the two planes into a one and
falling through to the V4L2_PIX_FMT_{YUYV,YVYU} code path.
Signed-off-by: Niklas Söderlund
---
I'm sorry this is a bit of a hack. The support for NV16 are scarce and
this allowed me use it in qv4l2 so I thought it might help someone else.
I'm not to sure about the entry in supported_src_pixfmts[] is it correct
to set 'needs conversion' for my use case?
needs_conversion is set on formats which apps are unlikely to support
natively, so yes it is correct to set it.
Patch looks good to me:
Acked-by: Hans de Goede
Hans Verkuil, if you're happy with this version feel free to push it.
Regards,
Hans
Changes since v1
- Add NV61 support
- Fixed s/YUVU/YUYV/g in commit message
lib/libv4lconvert/libv4lconvert-priv.h | 3 +++
lib/libv4lconvert/libv4lconvert.c | 31 +++
lib/libv4lconvert/rgbyuv.c | 15 +++
3 files changed, 49 insertions(+)
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index b77e3d3..1740efc 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -129,6 +129,9 @@ void v4lconvert_yuyv_to_bgr24(const unsigned char *src,
unsigned char *dst,
void v4lconvert_yuyv_to_yuv420(const unsigned char *src, unsigned char *dst,
int width, int height, int stride, int yvu);
+void v4lconvert_nv16_to_yuyv(const unsigned char *src, unsigned char *dest,
+ int width, int height);
+
void v4lconvert_yvyu_to_rgb24(const unsigned char *src, unsigned char *dst,
int width, int height, int stride);
diff --git a/lib/libv4lconvert/libv4lconvert.c
b/lib/libv4lconvert/libv4lconvert.c
index f62aea1..d3d8936 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -98,6 +98,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[]
= {
{ V4L2_PIX_FMT_YUYV,16, 5, 4, 0 },
{ V4L2_PIX_FMT_YVYU,16, 5, 4, 0 },
{ V4L2_PIX_FMT_UYVY,16, 5, 4, 0 },
+ { V4L2_PIX_FMT_NV16,16, 5, 4, 1 },
+ { V4L2_PIX_FMT_NV61,16, 5, 4, 1 },
/* yuv 4:2:0 formats */
{ V4L2_PIX_FMT_SPCA501, 12, 6, 3, 1 },
{ V4L2_PIX_FMT_SPCA505, 12, 6, 3, 1 },
@@ -1229,6 +1231,20 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
}
break;
+ case V4L2_PIX_FMT_NV16: {
+ unsigned char *tmpbuf;
+
+ tmpbuf = v4lconvert_alloc_buffer(width * height * 2,
+ &data->convert_pixfmt_buf,
&data->convert_pixfmt_buf_size);
+ if (!tmpbuf)
+ return v4lconvert_oom_error(data);
+
+ v4lconvert_nv16_to_yuyv(src, tmpbuf, width, height);
+ src_pix_fmt = V4L2_PIX_FMT_YUYV;
+ src = tmpbuf;
+ bytesperline = bytesperline * 2;
+ /* fall through */
+ }
case V4L2_PIX_FMT_YUYV:
if (src_size < (width * height * 2)) {
V4LCONVERT_ERR("short yuyv data frame\n");
@@ -1251,6 +1267,21 @@ static int v4lconvert_convert_pixfmt(struct
v4lconvert_data *data,
}
break;
+ case V4L2_PIX_FMT_NV61: {
+ unsigned char *tmpbuf;
+
+ tmpbuf = v4lconvert_alloc_buffer(width * height * 2,
+ &data->convert_pixfmt_buf,
&data->convert_pixfmt_buf_size);
+ if (!tmpbuf)
+ return v4lconvert_oom_error(data);
+
+ /* Note NV61 is NV16 with U and V swapped so this becomes yvyu.
*/
+ v4lconvert_nv16_to_yuyv(src, tmpbuf, width, height);
+ src_pix_fmt = V4L2_PIX_FMT_YVYU;
+ src = tmpbuf;
+ bytesperline = bytesperline * 2;
+ /* fall through */
+ }
case V4L2_PIX_FMT_YVYU:
if (src_size < (width * height * 2)) {
V4LCONVERT_ERR("short yvyu data frame\n");
diff --git a/lib/libv4lconvert/rgbyuv.c b/lib/libv4lconvert/rgbyuv.c
index 695255a..a0f8256 100644
--- a/lib/libv4lconvert/rgbyuv.c
+++ b/lib/libv4lconvert/rgbyuv.c
@@ -295,6 +295,21 @@ void v4lconvert_yuyv_to_yuv420(const unsigned char *src,
unsigned char *dest,
}
}
+void v4lconvert_nv16_to_yuyv(const unsigned char *src, unsigned char *dest,
+ int width, int height)
+{
+ const unsigned char *y, *cbcr;
+ int count = 0;
+
+ y = src;
+ cbcr = src + width*
Re: [PATCH 6/7] [media] gspca: fix a v4l2-compliance failure during VIDIOC_REQBUFS
Hi,
On 14-03-16 16:02, Antonio Ospite wrote:
On Thu, 10 Mar 2016 15:54:37 +0100
Hans de Goede wrote:
Hi,
On 09-03-16 17:03, Antonio Ospite wrote:
When calling VIDIOC_REQBUFS v4l2-compliance fails with this message:
fail: v4l2-test-buffers.cpp(476): q.reqbufs(node, 1)
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
By looking at the v4l2-compliance code the failure happens when trying
to request V4L2_MEMORY_USERPTR buffers without freeing explicitly the
previously allocated V4L2_MEMORY_MMAP buffers.
This would suggest that when changing the memory field in struct
v4l2_requestbuffers the driver is supposed to free automatically any
previous allocated buffers, and looking for inspiration at the code in
drivers/media/v4l2-core/videobuf2-core.c::vb2_core_reqbufs() seems to
confirm this interpretation; however gspca is just returning -EBUSY in
this case.
Removing the special handling for the case of a different memory value
fixes the compliance failure.
Signed-off-by: Antonio Ospite
---
This should be safe, but I'd really like a comment from someone with a more
global knowledge of v4l2.
If my interpretation about how drivers should behave when the value of the
memory field changes is correct, I could send also a documentation update for
Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
Just let me know.
Thanks,
Antonio
drivers/media/usb/gspca/gspca.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 84b0d6a..915b6c7 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1402,13 +1402,6 @@ static int vidioc_reqbufs(struct file *file, void *priv,
if (mutex_lock_interruptible(&gspca_dev->queue_lock))
return -ERESTARTSYS;
- if (gspca_dev->memory != GSPCA_MEMORY_NO
- && gspca_dev->memory != GSPCA_MEMORY_READ
- && gspca_dev->memory != rb->memory) {
- ret = -EBUSY;
- goto out;
- }
-
reqbufs is used internally and this change will allow changing
gspca_dev->memory from USERPTR / MMAP to GSPCA_MEMORY_READ
please replace this check with a check to only allow
rb->memory to be GSPCA_MEMORY_READ when coming from GSPCA_MEMORY_NO
or GSPCA_MEMORY_READ
OK, thanks, I'll take a look again later this week.
In the meantime, if patches from 1 to 5 are OK, can we have them merged
so I will just resubmit the last two in the set?
Not sure when I'll have time to do this, I would prefer to also take
v2 of patch 6 and 7 while at it. But I agree that there is no need
to pick op patches 1 - 5. I'll pick them up from patchwork when I've
time.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB xHCI regression after upgrading from kernel 3.19.0-51 to 4.2.0-34.
Hi, It is probably best to resend this mail to linux-usb since this is more of a usb problem then a v4l2 problem, and all the usb experts are subscribed to that list. Regards, Hans On 07-04-16 17:36, Matthew Giassa wrote: Good day, I maintain an SDK for USB2.0 and USB3.0 U3V machine vision cameras, and several of my customers have reported severe issues since upgrading from kernel 3.19.0-51 (Ubuntu 14.04.3 LTS) to kernel 4.2.0-34 (Ubuntu 14.04.4 LTS). I've received helpful advice from members of the libusb and linux-usb mailing list and development groups on how to generate useful logs to help diagnose the issue, and have filed a bug to track this issue at: https://bugzilla.kernel.org/show_bug.cgi?id=115961 It seems that with kernels newer than the 3.19 series (I've tested on 4.2.0-34, and just repeated the tests on the latest 4.5.0 release), the cameras lock up, and cannot stream image data to the user application. I am able to resolve the issue on 4.2.0-34 by disabling USB power management by adding "usbcore.autosuspend=-1". On the 4.5 kernel, this "trick" doesn't work at all, and I have no way to get the cameras to stream data. I can do simple USB control requests to query things like register values and serial numbers, but that's it. Asynchronous bulk transfers never succeed. Special Cases: * The issue does not occur when using USB2.0 cameras on a USB2.0 port, regardless of the kernel in use. * The issues occur only on Intel 8 Series and Intel 9 Series USB3.0 host controllers with 4.x kernels. * Intel 10 Series host controllers have not yet been tested. * The issues never occur on Fresco or Renesas host controllers, regardless of the kernel in use. * From visual inspection of lsusb output, the issue only appears to happen when the U1 and U2 options are available to the device. I would like to request assistance with diagnosing and resolving the issue, as it requires our customers to either not use Intel host controllers, or sticking to older kernel releases. Thank you for your time and assistance. Matthew Giassa, MASc, BASc, EIT Security and Embedded Systems Specialist linkedin: https://ca.linkedin.com/in/giassa e-mail: matt...@giassa.net website: www.giassa.net -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] stagin: atomisp: Fix oops by unbalanced clk enable/disable call
The common-clk core expects clk consumers to always call enable/disable
in a balanced manner. The atomisp driver does not call gmin_flisclk_ctrl()
in a balanced manner, so add a clock_on bool and skip redundant calls.
This fixes kernel oops like this one:
[ 19.811613] gc0310_s_config S
[ 19.811655] [ cut here ]
[ 19.811664] WARNING: CPU: 1 PID: 720 at drivers/clk/clk.c:594 clk_core_disabl
[ 19.811666] Modules linked in: tpm_crb(+) snd_soc_sst_atom_hifi2_platform tpm
[ 19.811744] CPU: 1 PID: 720 Comm: systemd-udevd Tainted: G C OE 4.1
[ 19.811746] Hardware name: Insyde T701/T701, BIOS BYT70A.YNCHENG.WIN.007 08/2
[ 19.811749] task: 988df7ab2500 task.stack: ac1400474000
[ 19.811752] RIP: 0010:clk_core_disable+0xc0/0x130
...
[ 19.811775] Call Trace:
[ 19.811783] clk_core_disable_lock+0x1f/0x30
[ 19.811788] clk_disable+0x1f/0x30
[ 19.811794] gmin_flisclk_ctrl+0x87/0xf0
[ 19.811801] 0xc0528512
[ 19.811805] 0xc05295e2
[ 19.811811] ? acpi_device_wakeup_disable+0x50/0x60
[ 19.811815] ? acpi_dev_pm_attach+0x8e/0xd0
[ 19.811818] ? 0xc05294d0
[ 19.811823] i2c_device_probe+0x1cd/0x280
[ 19.811828] driver_probe_device+0x2ff/0x450
Fixes: "staging: atomisp: use clock framework for camera clocks"
Signed-off-by: Hans de Goede
---
.../media/atomisp/platform/intel-mid/atomisp_gmin_platform.c | 7 +++
1 file changed, 7 insertions(+)
diff --git
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 828fe5abd832..6671ebe4ecc9 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -29,6 +29,7 @@ struct gmin_subdev {
struct v4l2_subdev *subdev;
int clock_num;
int clock_src;
+ bool clock_on;
struct clk *pmc_clk;
struct gpio_desc *gpio0;
struct gpio_desc *gpio1;
@@ -583,6 +584,9 @@ static int gmin_flisclk_ctrl(struct v4l2_subdev *subdev,
int on)
struct gmin_subdev *gs = find_gmin_subdev(subdev);
struct i2c_client *client = v4l2_get_subdevdata(subdev);
+ if (gs->clock_on == !!on)
+ return 0;
+
if (on) {
ret = clk_set_rate(gs->pmc_clk, gs->clock_src);
@@ -591,8 +595,11 @@ static int gmin_flisclk_ctrl(struct v4l2_subdev *subdev,
int on)
gs->clock_src);
ret = clk_prepare_enable(gs->pmc_clk);
+ if (ret == 0)
+ gs->clock_on = true;
} else {
clk_disable_unprepare(gs->pmc_clk);
+ gs->clock_on = false;
}
return ret;
--
2.14.2
[PATCH] staging: atomisp: Fix -Werror=int-in-bool-context compile errors
With gcc-7.1.1 I was getting the following compile error: error: ‘*’ in boolean context, suggest ‘&&’ instead The problem is the definition of CEIL_DIV: #define CEIL_DIV(a, b) ((b) ? ((a) + (b) - 1) / (b) : 0) Which when called as: CEIL_DIV(x, y * z) triggers this error, note we cannot do as the error suggests since b is evaluated multiple times. This commit fixes these compile errors. Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c | 1 - .../pci/atomisp2/css2400/hive_isp_css_include/math_support.h| 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c index b830b241e2e6..ad2c610d2ce3 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c @@ -2506,7 +2506,6 @@ static void __configure_capture_pp_input(struct atomisp_sub_device *asd, struct ia_css_pipe_extra_config *pipe_extra_configs = &stream_env->pipe_extra_configs[pipe_id]; unsigned int hor_ds_factor = 0, ver_ds_factor = 0; -#define CEIL_DIV(a, b) ((b) ? ((a) + (b) - 1) / (b) : 0) if (width == 0 && height == 0) return; diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/math_support.h b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/math_support.h index 48d84bc0ad9e..f74b405b0f39 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/math_support.h +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/math_support.h @@ -62,15 +62,15 @@ #define MAX(a, b)(((a) > (b)) ? (a) : (b)) #define MIN(a, b)(((a) < (b)) ? (a) : (b)) #ifdef ISP2401 -#define ROUND_DIV(a, b) ((b) ? ((a) + ((b) >> 1)) / (b) : 0) +#define ROUND_DIV(a, b) (((b) != 0) ? ((a) + ((b) >> 1)) / (b) : 0) #endif -#define CEIL_DIV(a, b) ((b) ? ((a) + (b) - 1) / (b) : 0) +#define CEIL_DIV(a, b) (((b) != 0) ? ((a) + (b) - 1) / (b) : 0) #define CEIL_MUL(a, b) (CEIL_DIV(a, b) * (b)) #define CEIL_MUL2(a, b) (((a) + (b) - 1) & ~((b) - 1)) #define CEIL_SHIFT(a, b) (((a) + (1 << (b)) - 1)>>(b)) #define CEIL_SHIFT_MUL(a, b) (CEIL_SHIFT(a, b) << (b)) #ifdef ISP2401 -#define ROUND_HALF_DOWN_DIV(a, b) ((b) ? ((a) + (b / 2) - 1) / (b) : 0) +#define ROUND_HALF_DOWN_DIV(a, b) (((b) != 0) ? ((a) + (b / 2) - 1) / (b) : 0) #define ROUND_HALF_DOWN_MUL(a, b) (ROUND_HALF_DOWN_DIV(a, b) * (b)) #endif -- 2.12.2
Firmware for staging atomisp driver
Hi All, I've been trying to get the atomisp driver from staging to work on a couple of devices I have. I started with an Asus T100TA after fixing 2 oopses in the sensor driver there I found out that the BIOS does not allow to put the ISP in PCI mode and that there is no code to drive it in ACPI enumerated mode. So I moved to a generic Insyde T701 tablet which does allow this. After fixing some more sensor driver issues there I was ready to actually load the atomisp driver, but I could not find the exact firmware required, I did find a version which is close: "irci_stable_candrpv_0415_20150423_1753" and tried that but that causes the atomisp driver to explode in a backtrace which contains atomisp_load_firmware() so that one seems no good. Can someone help me to get the right firmware ? The TODO says: "can also be extracted from the upgrade kit" about the firmware files, but it is not clear to me what / where the "upgrade kit" is. More in general it would be a good idea if someone inside Intel would try to get permission to add the firmware to linux-firmware. Anyways I will send out the patches I've currently, once I've the right firmware I will continue working on this. Regards, Hans
[PATCH 5/7] staging: atomisp: Add OVTI2680 ACPI id to ov2680 driver
Signed-off-by: Hans de Goede
---
drivers/staging/media/atomisp/i2c/ov2680.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c
b/drivers/staging/media/atomisp/i2c/ov2680.c
index 566091035c64..449aa2aa276f 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -1521,6 +1521,7 @@ static int ov2680_probe(struct i2c_client *client,
static struct acpi_device_id ov2680_acpi_match[] = {
{"XXOV2680"},
+ {"OVTI2680"},
{},
};
MODULE_DEVICE_TABLE(acpi, ov2680_acpi_match);
--
2.13.0
[PATCH 4/7] staging: atomisp: Add INT0310 ACPI id to gc0310 driver
Signed-off-by: Hans de Goede
---
drivers/staging/media/atomisp/i2c/gc0310.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/i2c/gc0310.c
b/drivers/staging/media/atomisp/i2c/gc0310.c
index 1ec616a15086..350fd7fd5b86 100644
--- a/drivers/staging/media/atomisp/i2c/gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/gc0310.c
@@ -1455,6 +1455,7 @@ static int gc0310_probe(struct i2c_client *client,
static struct acpi_device_id gc0310_acpi_match[] = {
{"XXGC0310"},
+ {"INT0310"},
{},
};
--
2.13.0
[PATCH 1/7] staging: atomisp: Fix calling efivar_entry_get() with unaligned arguments
efivar_entry_get has certain alignment requirements and the atomisp
platform code was not honoring these, causing an oops by triggering the
WARN_ON in arch/x86/platform/efi/efi_64.c: virt_to_phys_or_null_size().
This commit fixes this by using the members of the efivar struct embedded
in the efivar_entry struct we kzalloc as arguments to efivar_entry_get(),
which is how all the other callers of efivar_entry_get() do this.
Signed-off-by: Hans de Goede
---
.../atomisp/platform/intel-mid/atomisp_gmin_platform.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 5b4506a71126..104fea2f8697 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -623,9 +623,7 @@ int gmin_get_config_var(struct device *dev, const char
*var, char *out, size_t *
char var8[CFG_VAR_NAME_MAX];
efi_char16_t var16[CFG_VAR_NAME_MAX];
struct efivar_entry *ev;
- u32 efiattr_dummy;
int i, j, ret;
- unsigned long efilen;
if (dev && ACPI_COMPANION(dev))
dev = &ACPI_COMPANION(dev)->dev;
@@ -684,15 +682,18 @@ int gmin_get_config_var(struct device *dev, const char
*var, char *out, size_t *
return -ENOMEM;
memcpy(&ev->var.VariableName, var16, sizeof(var16));
ev->var.VendorGuid = GMIN_CFG_VAR_EFI_GUID;
+ ev->var.DataSize = *out_len;
- efilen = *out_len;
- ret = efivar_entry_get(ev, &efiattr_dummy, &efilen, out);
+ ret = efivar_entry_get(ev, &ev->var.Attributes,
+ &ev->var.DataSize, ev->var.Data);
+ if (ret == 0) {
+ memcpy(out, ev->var.Data, ev->var.DataSize);
+ *out_len = ev->var.DataSize;
+ } else {
+ dev_warn(dev, "Failed to find gmin variable %s\n", var8);
+ }
kfree(ev);
- *out_len = efilen;
-
- if (ret)
- dev_warn(dev, "Failed to find gmin variable %s\n", var8);
return ret;
}
--
2.13.0
[PATCH 3/7] staging: atomisp: Set step to 0 for mt9m114 menu control
menu controls are not allowed to have a step size, set step to 0 to
fix an oops from the WARN_ON in v4l2_ctrl_new_custom() triggering
because of this.
Signed-off-by: Hans de Goede
---
drivers/staging/media/atomisp/i2c/mt9m114.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c
b/drivers/staging/media/atomisp/i2c/mt9m114.c
index ced175c268d1..3fa915313e53 100644
--- a/drivers/staging/media/atomisp/i2c/mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
@@ -1499,7 +1499,7 @@ static struct v4l2_ctrl_config mt9m114_controls[] = {
.type = V4L2_CTRL_TYPE_MENU,
.min = 0,
.max = 3,
-.step = 1,
+.step = 0,
.def = 1,
.flags = 0,
},
--
2.13.0
[PATCH 2/7] staging: atomisp: Do not call dev_warn with a NULL device
Do not call dev_warn with a NULL device, this silence the following 2
warnings:
[ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO
[ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO
We could switch to using pr_warn for dev == NULL instead, but as comments
in the source indicate, the check for these 2 special gmin variables with
a NULL device is a workaround for 2 specific evaluation boards, so
completely silencing the missing warning for these actually is a good
thing.
Signed-off-by: Hans de Goede
---
.../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 104fea2f8697..3fea81ea5dbd 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -689,7 +689,7 @@ int gmin_get_config_var(struct device *dev, const char
*var, char *out, size_t *
if (ret == 0) {
memcpy(out, ev->var.Data, ev->var.DataSize);
*out_len = ev->var.DataSize;
- } else {
+ } else if (dev) {
dev_warn(dev, "Failed to find gmin variable %s\n", var8);
}
--
2.13.0
[PATCH 7/7] staging: atomisp: Make ov2680 driver less chatty
There is no reason for all this printk spamming and certainly
not at an error log level.
Signed-off-by: Hans de Goede
---
drivers/staging/media/atomisp/i2c/ov2680.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c
b/drivers/staging/media/atomisp/i2c/ov2680.c
index 6dd466558701..3cabfe54c669 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -1191,9 +1191,8 @@ static int ov2680_detect(struct i2c_client *client)
OV2680_SC_CMMN_SUB_ID, &high);
revision = (u8) high & 0x0f;
- dev_err(&client->dev, "sensor_revision id = 0x%x\n", id);
- dev_err(&client->dev, "detect ov2680 success\n");
- dev_err(&client->dev, "5##\n");
+ dev_info(&client->dev, "sensor_revision id = 0x%x\n", id);
+
return 0;
}
@@ -1448,8 +1447,6 @@ static int ov2680_probe(struct i2c_client *client,
void *pdata;
unsigned int i;
- printk("ov2680_probe\n");
- dev_info(&client->dev, "ov2680_probe\n");
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev) {
dev_err(&client->dev, "out of memory\n");
--
2.13.0
[PATCH 6/7] staging: atomisp: Ignore errors from second gpio in ov2680 driver
As the existing comment in the driver indicates the sensor has only 1 pin,
but some boards may have 2 gpios defined and we toggle both as we we don't
know which one is the right one. However if the ACPI resources table
defines only 1 gpio (as expected) the gpio1_ctrl call will always fail,
causing the probing of the driver to file.
This commit ignore the return value of the gpio1_ctrl call, fixing this.
Signed-off-by: Hans de Goede
---
drivers/staging/media/atomisp/i2c/ov2680.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c
b/drivers/staging/media/atomisp/i2c/ov2680.c
index 449aa2aa276f..6dd466558701 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -885,11 +885,12 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
if (flag) {
ret = dev->platform_data->gpio0_ctrl(sd, 1);
usleep_range(1, 15000);
- ret |= dev->platform_data->gpio1_ctrl(sd, 1);
+ /* Ignore return from second gpio, it may not be there */
+ dev->platform_data->gpio1_ctrl(sd, 1);
usleep_range(1, 15000);
} else {
- ret = dev->platform_data->gpio1_ctrl(sd, 0);
- ret |= dev->platform_data->gpio0_ctrl(sd, 0);
+ dev->platform_data->gpio1_ctrl(sd, 0);
+ ret = dev->platform_data->gpio0_ctrl(sd, 0);
}
return ret;
}
--
2.13.0
Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
Hi, On 28-05-17 19:08, Alan Cox wrote: On Sun, 28 May 2017 14:30:35 +0200 Hans de Goede wrote: Do not call dev_warn with a NULL device, this silence the following 2 warnings: [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO We could switch to using pr_warn for dev == NULL instead, but as comments in the source indicate, the check for these 2 special gmin variables with a NULL device is a workaround for 2 specific evaluation boards, so completely silencing the missing warning for these actually is a good thing. At which point real missing variables won't get reported so NAK. I think the right fix is to make the offending callers pass subdev->dev The code for the special v1p8 / v2p8 gpios is ugly as sin, it operates on a global v2p8_gpio value rather then storing info in the gmin_subdev struct, as such passing the subdev->dev pointer would be simply wrong. AFAICT the v1p8 / v2p8 gpio code is the only caller passing in a NULL pointer and as said since thisv1p8 / v2p8 gpio code is only for some special evaluation boards, silencing the error when these variables are not present actually is the right thing to do. which if my understanding of the subdevices is correct should pass the right valid device field from the atomisp. Please also cc me if you are proposing patches this driver - and also linux-media. Sorry about that, I messed up my git send-email foo and send this to a wrong set of addresses (and also added v5 in the subject which should not be there) I did send out a fresh-copy with the full 7 patch patch-set directly after CTRL+c-ing this wrong send-email (which only got the first 3 patches send). Regards, Hans
[PATCH] staging: atomisp: Fix endless recursion in hmm_init
hmm_init calls hmm_alloc to set dummy_ptr, hmm_alloc calls hmm_init when dummy_ptr is not yet set, which is the case in the call from hmm_init, so it calls hmm_init again, this continues until we have a stack overflow due to the recursion. This commit fixes this by adding a separate flag for tracking if hmm_init has been called. Not pretty, but it gets the job done, eventually we should be able to remove the hmm_init call from hmm_alloc. Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c index 5729539..e79ca3c 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c @@ -43,6 +43,7 @@ struct hmm_bo_device bo_device; struct hmm_pooldynamic_pool; struct hmm_poolreserved_pool; static ia_css_ptr dummy_ptr; +static bool hmm_initialized; struct _hmm_mem_stat hmm_mem_stat; /* p: private @@ -186,6 +187,8 @@ int hmm_init(void) if (ret) dev_err(atomisp_dev, "hmm_bo_device_init failed.\n"); + hmm_initialized = true; + /* * As hmm use NULL to indicate invalid ISP virtual address, * and ISP_VM_START is defined to 0 too, so we allocate @@ -217,6 +220,7 @@ void hmm_cleanup(void) dummy_ptr = 0; hmm_bo_device_exit(&bo_device); + hmm_initialized = false; } ia_css_ptr hmm_alloc(size_t bytes, enum hmm_bo_type type, @@ -229,7 +233,7 @@ ia_css_ptr hmm_alloc(size_t bytes, enum hmm_bo_type type, /* Check if we are initialized. In the ideal world we wouldn't need this but we can tackle it once the driver is a lot cleaner */ - if (!dummy_ptr) + if (!hmm_initialized) hmm_init(); /*Get page number from size*/ pgnr = size_to_pgnr_ceil(bytes); -- 2.9.4
Re: Firmware for staging atomisp driver
Hi, On 05/28/2017 02:30 PM, Hans de Goede wrote: Hi All, I've been trying to get the atomisp driver from staging to work on a couple of devices I have. I started with an Asus T100TA after fixing 2 oopses in the sensor driver there I found out that the BIOS does not allow to put the ISP in PCI mode and that there is no code to drive it in ACPI enumerated mode. So I moved to a generic Insyde T701 tablet which does allow this. After fixing some more sensor driver issues there I was ready to actually load the atomisp driver, but I could not find the exact firmware required, I did find a version which is close: "irci_stable_candrpv_0415_20150423_1753" and tried that but that causes the atomisp driver to explode in a backtrace which contains atomisp_load_firmware() so that one seems no good. Ok, so it turns out that the explosion was not a probem with a wrong firmware version, but rather another atomisp code bug. According to this patch: https://github.com/01org/ProductionKernelQuilts/blob/master/uefi/cht-m1stable/patches/cam-0418-atomisp2-css2401-and-2401_legacy-irci_stable_candrpv.patch The irci_stable_candrpv_0415_20150423_1753 version I have and the irci_stable_candrpv_0415_20150521_0458 version expected are fully compatible. So I'e focussed on fixing the crash and that was easy, see the patch I just send. Then I hit another bunch of crashes which all turn out to be due to races with udev opening /dev/video# nodes for probing before the driver is ready to handle this because the driver registers the v4l2 devices too soon. I've hacked around this for now: https://github.com/jwrdegoede/linux-sunxi/commit/88c9c248e6e0f86d547ea8441e16b0e8b4c951c8 And with this hack the driver loads without issues, giving me 10 /dev/video# devices. So I tried this app: https://github.com/jfwells/linux-asus-t100ta/tree/master/webcam/atomisp_testapp On a number of the v4l devices which leads to yet more oopses. So I'm starting to wonder, does anyone have this driver working (in its current form in 4.12-rc3 drivers/staging) at all ? I'm asking because that is hard to believe given e.g. the recursion bug I've just fixed. Can someone provide step-by-step instructions for how to get this driver working? I'm not expecting all userspace apps to just work, but at least a userspace example given a picture from the camera would be very helpful in further developing this driver. Regards, Hans
[PATCH] libv4lconvert: We support more then 32 bit src fmts now, so use 64 bit bitmasks
We support more then 32 bit src fmts now, so we can no longer re-use
struct v4l2_frmsizeenum.pixel_format to store a bitmask of all the
supported src-formats for a given frame-size.
This fixes a subtile bug where we would try to use SE401 as src fmt
instead of YUYV under certain circumstances.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1508706
Signed-off-by: Hans de Goede
---
lib/libv4lconvert/libv4lconvert-priv.h | 2 ++
lib/libv4lconvert/libv4lconvert.c | 9 -
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/libv4lconvert/libv4lconvert-priv.h
b/lib/libv4lconvert/libv4lconvert-priv.h
index e2389347..9a467e10 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -66,6 +66,8 @@ struct v4lconvert_data {
int cinfo_initialized;
#endif // HAVE_JPEG
struct v4l2_frmsizeenum framesizes[V4LCONVERT_MAX_FRAMESIZES];
+ /* Bitmask of all supported src_formats which can do for a size */
+ int64_t framesize_supported_src_formats[V4LCONVERT_MAX_FRAMESIZES];
unsigned int no_framesizes;
int bandwidth;
int fps;
diff --git a/lib/libv4lconvert/libv4lconvert.c
b/lib/libv4lconvert/libv4lconvert.c
index 1a5ccec2..d666bd97 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -434,7 +434,8 @@ static int v4lconvert_do_try_format_uvc(struct
v4lconvert_data *data,
for (i = 0; i < ARRAY_SIZE(supported_src_pixfmts); i++) {
/* is this format supported? */
- if (!(data->framesizes[best_framesize].pixel_format & (1 << i)))
+ if (!(data->framesize_supported_src_formats[best_framesize] &
+ (1ULL << i)))
continue;
/* Note the hardcoded use of discrete is based on this function
@@ -1647,9 +1648,7 @@ static void v4lconvert_get_framesizes(struct
v4lconvert_data *data,
return;
}
data->framesizes[data->no_framesizes].type =
frmsize.type;
- /* We use the pixel_format member to store a bitmask of
all
- supported src_formats which can do this size */
- data->framesizes[data->no_framesizes].pixel_format = 1
<< index;
+
data->framesize_supported_src_formats[data->no_framesizes] = 1ULL << index;
switch (frmsize.type) {
case V4L2_FRMSIZE_TYPE_DISCRETE:
@@ -1662,7 +1661,7 @@ static void v4lconvert_get_framesizes(struct
v4lconvert_data *data,
}
data->no_framesizes++;
} else {
- data->framesizes[j].pixel_format |= 1 << index;
+ data->framesize_supported_src_formats[j] |= 1ULL <<
index;
}
}
}
--
2.14.3
Re: RFC: Move the deprecated et61x251 and sn9c102 to staging
Hi,
On 01/02/2011 12:25 PM, Hans Verkuil wrote:
On Sunday, January 02, 2011 11:41:31 Mauro Carvalho Chehab wrote:
Em 01-01-2011 17:53, Hans Verkuil escreveu:
The subject says it all:
If there are no objections, then I propose that the deprecated et61x251 and
sn9c102 are moved to staging for 2.6.38 and marked for removal in 2.6.39.
Nack.
There are several USB ID's on sn9c102 not covered by gspca driver yet.
Why are these drivers marked deprecated then?
You'll have to look at me for the deprecation marking.
I did this because they are unmaintained and buggy in some areas and thus really
should go away. Also note that looking at usb-id's is *not* useful with these
drivers as when Luca wrote them he simply included large lists of usb-id's
without
the drivers actually being tested with cams with those id's. Usually when a
bridge
supports a range of configurable id's, this is used to have one generic (win32)
driver and the usb product id indicates which sensor is used.
I did a patch removing a whole bunch of usb-id's from the sn9c102 driver in the
past as we knew which sensors those id's corresponded with and Luca's driver
never
supported these sensors, so the claiming of the usb-id's was bogus.
However for many usb-id's claimed by the sn9c102 driver we don't know what
sensor
they belong to (or if any devices with that id exists out there at all), so I
left
them in to not cause regressions.
So if we want to see where we stand wrt replacing Luca's old drivers with gspca
subdrivers, you should look at the list of supported sensors.
I've made a list for all sn9c102 supported bridge + sensor combinations and
marked the ones which are not yet supported by gscpa:
sn9c101/102:
hv7131d
mi0343 *
ov7630
pas106b
pas202bcb
tas5110c1b
tas5110d
tas5130d1b
sn9c103:
hv7131r *
mi0360 *
ov7630
pas202bcb
sn9c105/120:
hv7131r
mi0360
mt9v111 *
ov7630
ov7660
So we have 3 models not yet supported in the sonixb driver (and sonixb
cams are quite rare now a days). And 1 model in the sonixj driver.
Note btw that I've disabled all Luca's drivers (et61x251, sn9c102 and
zc0301) in Fedora kernels for several Fedora releases already and sofar
I've received one bug report about this, which is resolved now as I
recently added support for the hv7131d sensor to the sonixb driver
(thanks to said bug report).
So all in all I believe that moving the sn9c102 driver to staging, or
at least remove all usb-id's which are a doublure with gspca's sonixb
and sonixj drivers is the right thing to do.
It seems to me that et61x251 will also stay there for a long time, as there are
just two devices supported by gspca driver, while et61x251 supports 25.
Btw, we currently have a conflict with this USB ID:
USB_DEVICE(0x102c, 0x6151),
Both etoms and et61x251 support it, and there's no #if to disable it on one
driver, if both drivers are compiled. We need to disable it either at
gspca_etoms
or at et61x251, in order to avoid users of having a random experience with this
device.
Surely such devices should be removed from et61x251 or sn9c102 as soon as they
are
added to gspca?
The problem is that the initial gspcav2 core + subdrivers as it entered the
mainline
is derived from / a partial rewrite of the out of tree v4l1 gspca drivers /
framework
as such the register init sequences for bridges + sensors were not all tested
when
gspca entered the mainline so in the case of untested bridge + sensor combo's
which
were already supported by Luca's mainline drivers, we added #ifdef's to use
Luca's
drivers in case both are compiled in. As more and more people tested the gspca
drivers
most of these #ifdef's were reversed to prefer the gspca sub drivers if both
were compiled in, one ubs-id at a time.
Looking at both drivers, the gspca one supports both the tas5130 and pas106
sensors,
where as the et61x251 driver only supports the tas5130 sensor. The conflicting
usb id
102c:6151 is for a camera with the pas106 sensor, so the solution is to remove
this
usb id from the et61x251 driver, as it does not support this id, despite
claiming it.
Looking closer at the et61x251 driver, one finds this beauty inside
et61x251_tas5130d1b.c, which is the only sensor module for the et61x251 driver:
int et61x251_probe_tas5130d1b(struct et61x251_device* cam)
{
const struct usb_device_id tas5130d1b_id_table[] = {
{ USB_DEVICE(0x102c, 0x6251), },
{ }
};
/* Sensor detection is based on USB pid/vid */
if (!et61x251_match_id(cam, tas5130d1b_id_table))
return -ENODEV;
...
IOW the et61x251 driver, in classical Luca style if I may say so, only supports
usb id
102c:6251, despite claiming many many more. So moving forward we should:
1) Remove all bogus usb id's from the et61x251 driver, as trying to use any of
these
devices with it will fail, as it won't be able to find a working sensor module
(I thought I did a patch for this once before, but either my memory is fai
Re: RFC: Move the deprecated et61x251 and sn9c102 to staging
Hi, One small correction to the sn9c102 sensor table, the mt9v111 sensor is handled by sonixj, so the table of bridge/sensor combi's supported by sn9c102, looks like this: sn9c101/102: hv7131d mi0343 * ov7630 pas106b pas202bcb tas5110c1b tas5110d tas5130d1b sn9c103: hv7131r * mi0360 * ov7630 pas202bcb sn9c105/120: hv7131r mi0360 mt9v111 ov7630 ov7660 So only 3 raw bayer + custom compression models supported by sn9c102 are not supported by gspca_sonixb, and all jpeg models are supported by gspca_sonixj. Porting the 3 remaining models over should be relatively easy, but I (I more or less maintain the sonixb driver) really need hardware access to ensure things stay working. Second correction, I was looking at an old tree and failed to notice that the zc0301 driver has already bitten the dust (good!). Regards, Hans On 01/02/2011 05:34 PM, Hans de Goede wrote: Hi, On 01/02/2011 12:25 PM, Hans Verkuil wrote: On Sunday, January 02, 2011 11:41:31 Mauro Carvalho Chehab wrote: Em 01-01-2011 17:53, Hans Verkuil escreveu: The subject says it all: If there are no objections, then I propose that the deprecated et61x251 and sn9c102 are moved to staging for 2.6.38 and marked for removal in 2.6.39. Nack. There are several USB ID's on sn9c102 not covered by gspca driver yet. Why are these drivers marked deprecated then? You'll have to look at me for the deprecation marking. I did this because they are unmaintained and buggy in some areas and thus really should go away. Also note that looking at usb-id's is *not* useful with these drivers as when Luca wrote them he simply included large lists of usb-id's without the drivers actually being tested with cams with those id's. Usually when a bridge supports a range of configurable id's, this is used to have one generic (win32) driver and the usb product id indicates which sensor is used. I did a patch removing a whole bunch of usb-id's from the sn9c102 driver in the past as we knew which sensors those id's corresponded with and Luca's driver never supported these sensors, so the claiming of the usb-id's was bogus. However for many usb-id's claimed by the sn9c102 driver we don't know what sensor they belong to (or if any devices with that id exists out there at all), so I left them in to not cause regressions. So if we want to see where we stand wrt replacing Luca's old drivers with gspca subdrivers, you should look at the list of supported sensors. I've made a list for all sn9c102 supported bridge + sensor combinations and marked the ones which are not yet supported by gscpa: sn9c101/102: hv7131d mi0343 * ov7630 pas106b pas202bcb tas5110c1b tas5110d tas5130d1b sn9c103: hv7131r * mi0360 * ov7630 pas202bcb sn9c105/120: hv7131r mi0360 mt9v111 * ov7630 ov7660 So we have 3 models not yet supported in the sonixb driver (and sonixb cams are quite rare now a days). And 1 model in the sonixj driver. Note btw that I've disabled all Luca's drivers (et61x251, sn9c102 and zc0301) in Fedora kernels for several Fedora releases already and sofar I've received one bug report about this, which is resolved now as I recently added support for the hv7131d sensor to the sonixb driver (thanks to said bug report). So all in all I believe that moving the sn9c102 driver to staging, or at least remove all usb-id's which are a doublure with gspca's sonixb and sonixj drivers is the right thing to do. It seems to me that et61x251 will also stay there for a long time, as there are just two devices supported by gspca driver, while et61x251 supports 25. Btw, we currently have a conflict with this USB ID: USB_DEVICE(0x102c, 0x6151), Both etoms and et61x251 support it, and there's no #if to disable it on one driver, if both drivers are compiled. We need to disable it either at gspca_etoms or at et61x251, in order to avoid users of having a random experience with this device. Surely such devices should be removed from et61x251 or sn9c102 as soon as they are added to gspca? The problem is that the initial gspcav2 core + subdrivers as it entered the mainline is derived from / a partial rewrite of the out of tree v4l1 gspca drivers / framework as such the register init sequences for bridges + sensors were not all tested when gspca entered the mainline so in the case of untested bridge + sensor combo's which were already supported by Luca's mainline drivers, we added #ifdef's to use Luca's drivers in case both are compiled in. As more and more people tested the gspca drivers most of these #ifdef's were reversed to prefer the gspca sub drivers if both were compiled in, one ubs-id at a time. Looking at both drivers, the gspca one supports both the tas5130 and pas106 sensors, where as the et61x251 driver only supports the tas5130 sensor. The conflicting usb id 102c:6151 is for a camera with the pas106 sensor, so the solution is to r
Re: [PATCH] Philips SPC315NC is vertically flipped
Hi,
On 01/03/2011 11:51 AM, Mauro Carvalho Chehab wrote:
Signed-off-by: Mauro Carvalho Chehab
diff --git a/lib/libv4lconvert/control/libv4lcontrol.c
b/lib/libv4lconvert/control/libv4lcontrol.c
index f32ef7b..a182efe 100644
--- a/lib/libv4lconvert/control/libv4lcontrol.c
+++ b/lib/libv4lconvert/control/libv4lcontrol.c
@@ -46,6 +46,8 @@ static const struct v4lcontrol_flags_info v4lcontrol_flags[]
= {
{ 0x0471, 0x0326, 0, NULL, NULL, V4LCONTROL_HFLIPPED |
V4LCONTROL_VFLIPPED },
/* Philips SPC210NC */
{ 0x0471, 0x032d, 0, NULL, NULL, V4LCONTROL_HFLIPPED |
V4LCONTROL_VFLIPPED },
+ /* Philips SPC315NC */
+ { 0x0471, 0x032e, 0, NULL, NULL, V4LCONTROL_VFLIPPED },
/* Genius E-M 112 (also want whitebalance by default) */
{ 0x093a, 0x2476, 0, NULL, NULL,
V4LCONTROL_HFLIPPED|V4LCONTROL_VFLIPPED | V4LCONTROL_WANTS_WB,
1500 },
Are you sure it is only vertically flipped ? I've have the Philips SPC 200NC
(0471:0325)
myself and it simply has the sensor upside down (so both h and v flipped).
Did you happen to use skype to test this? Skype is known to decide to mirror
the image
before showing it in some cases (I don't know when / why skype does this).
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.38] gspca core fixes
Hi Mauro, Please pull from my tree for a number of gspca core fixes. While looking into the issue you were seeing with qv4l2 I also found a bug in the gspca core which made it impossible to switch from userptr to mmap mode with qv4l2 and gspca. While fixing that I also ended up fixing some locking issues. All the gspca core patches have been reviewed by J.F. Moine. This pull request also includes a patch for removing all the bogus usb-ids from the et61x251 driver as discussed. The following changes since commit 187134a5875df20356f4dca075db29f294115a47: [media] DVB: IR support for TechnoTrend CT-3650 (2010-12-31 09:10:24 -0200) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git for_v2.6.38 Hans de Goede (9): gspca_main: Locking fixes 1 gspca_main: Locking fixes 2 gspca_main: Update buffer flags even when user_copy fails gspca_main: Remove no longer used users variable gspca_main: Set memory type to GSPCA_MEMORY_NO on buffer release gspca_main: Simplify read mode memory type checks gspca_main: Allow switching from read to mmap / userptr mode gspca_main: wake wq on streamoff et61x251: remove wrongly claimed usb ids drivers/media/video/et61x251/et61x251.h | 24 drivers/media/video/gspca/gspca.c | 208 ++- drivers/media/video/gspca/gspca.h |2 - 3 files changed, 94 insertions(+), 140 deletions(-) Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Move the deprecated et61x251 and sn9c102 to staging
Hi, On 01/02/2011 09:13 PM, Hans Verkuil wrote: Hi Hans, On Sunday, January 02, 2011 19:33:31 Hans de Goede wrote: Hi, One small correction to the sn9c102 sensor table, the mt9v111 sensor is handled by sonixj, so the table of bridge/sensor combi's supported by sn9c102, looks like this: sn9c101/102: hv7131d mi0343 * ov7630 pas106b pas202bcb tas5110c1b tas5110d tas5130d1b sn9c103: hv7131r * mi0360 * ov7630 pas202bcb sn9c105/120: hv7131r mi0360 mt9v111 ov7630 ov7660 So only 3 raw bayer + custom compression models supported by sn9c102 are not supported by gspca_sonixb, and all jpeg models are supported by gspca_sonixj. Porting the 3 remaining models over should be relatively easy, but I (I more or less maintain the sonixb driver) really need hardware access to ensure things stay working. Second correction, I was looking at an old tree and failed to notice that the zc0301 driver has already bitten the dust (good!). Thank you for your very helpful answer. Can you make a patch removing all the bogus usb IDs from these drivers? I've just send a pull request including removal of all the bogus id's from the et61x251 driver. The sn9x102 driver is much harder to do, this would require hunting for windows drivers and then looking inside the .inf files to figure out what ids which are currently not in gspca could be added. More over the real gain would be removing support for the jpeg sn9c1xx chips (the 105 and 120 bridge support in sn9c102) as that is completely covered by gspca_sonixj. But that is not worth the effort given that we want the entire driver to go away sooner rather then later. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Add plugin support to libv4l
Hi, First of all many thanks for working on this! I've several remarks which I would like to see addressed before merging this. Since most remarks are rather high level remarks I've opted to just make a bulleted list of them rather then inserting them inline. * The biggest problem with your current implementation is that for each existing libv4l2_foo function you check if there is a plugin attached to the fd passed in and if that plugin wants to handle the call. Now lets assume that there is a plugin and that it wants to handle all calls. That means that you've now effectively replaced all libv4l2_foo calls with calling the corresponding foo function from the plugin and returning its result. This means that for this fd / device you've achieved the same result as completely replacing libv4l2.so.0 with a new library containing the plugin code. IOW you've not placed then plugin between libv4l2 and the device (as intended) but completely short-circuited / replaced libv4l2. This means for example for a device which only supports yuv output, that libv4l2 will no longer do format emulation and conversion and an app which only supports devices which deliver rgb data will no longer work. To actually place the plugin between libv4l2 (and libv4lconvert) and the device, you should replace all the SYS_FOO calls in libv4l2. The SYS_FOO calls are the calls to the actual device, so be replacing those with calls to the plugin you actual place the plugin between libv4l and the device as intended. * Currently you add a loop much like the one in the v4l2_get_index function to each libv4l2_plugin function. Basically you add an array of v4l2_plugin_info structs in libv4l2-plugin. Which gets searched by fd, much like the v4l2_dev_info struct array. Including needing similar locking. I would like you to instead just store the plugin info for a certain fd directly into the v4l2_dev_info struct. This way the separate array, looping and locking can go away. * Next I would also like to see all the libv4l2_plugin_foo functions except for libv4l2_plugin_open go away. Instead libv4l2.c can call the plugin functions directly. Let me try to explain what I have in mind. Lets say we store the struct libv4l2_plugin_data pointer to the active plugin in the v4l2_dev_info struct and name it dev_ops (short for device operations). Then we can replace all SYS_FOO calls inside libv4l2 (except the ones were v4l2_get_index returns -1), with a call to the relevant devop functions, for example: result = SYS_IOCTL(devices[index].fd, VIDIOC_REQBUFS, req); Would become: result = devices[index].dev_ops->v4l2_plugin_ioctl( devices[index].fd, VIDIOC_REQBUFS, req); Note that the plugin_used parameter of the v4l2_plugin_ioctl is gone, it should simply do a normal SYS_IOCTL and return the return value of that if it is not interested in intercepting the ioctl (you could move the definition of the SYS_FOO macros to libv4l2-plugin.h to make them availables to plugins). Also I think it would be better to rename the function pointers inside the libv4l2_plugin_data struct from v4l2_plugin_foo to just foo, so that the above code would become: result = devices[index].dev_ops->v4l2_plugin_ioctl( devices[index].fd, VIDIOC_REQBUFS, req); * The above means that need to always have a dev_ops pointer, so we need to have a default_dev_ops struct to use when no plugin wants to talk to the device. * You've put the v4l2_plugin_foo functions (of which only v4l2_plugin_foo will remain in my vision) in lib/include/libv4l2.h I don't think these functions should be public, their prototypes should be moved to lib/libv4l2/libv4l2-priv.h, and they should not be declared LIBV4L_PUBLIC. * There is one special case in all this, files under libv4lconvert also use SYS_IOCTL in various places. Since this now need to go through the plugin we need to take some special measures here. There are 2 options: 1) Break the libv4lconvert ABI (very few programs use it) and pass a struct libv4l2_plugin_data pointer to the v4lconvert_create function. *And* export the default_dev_ops struct from libv4l2. 2) Add a libv4l2_raw_ioctl method, which just gets the index and then does devices[index].dev_ops->v4l2_plugin_ioctl Except that this is not really an option as libv4lconvert should not depend on libv4l2 My vote personally goes to 1. * I think that once we do 1) from above it would be good to rename libv4l2_plugin_data to libv4l2_dev_ops, as that makes the public API more clear and dev_ops is in essence what a plugin provides. * Note that were I wrote: "like to see all the libv4l2_plugin_foo functions except for libv4l2_plugin_open go away" I did so for simplicity, in reality the wrappers around mmap and munmap need to stay too, but they should use data directly stored inside the v4l2_dev_info struct. This means that we need to either: mv the mmap and munmap
[GIT PATCHES FOR 2.6.38-rc1] gspca locking fixes and moving over usb-ids to sonix? drivers
Hi Mauro, Here is a new pull request including all the patches from my previous request: "Please pull from my tree for a number of gspca core fixes. While looking into the issue you were seeing with qv4l2 I also found a bug in the gspca core which made it impossible to switch from userptr to mmap mode with qv4l2 and gspca. While fixing that I also ended up fixing some locking issues." On top of that there are patches removing non supported bogus / non supported usb-ids from Luca's et61x251 and sn9xc102 drivers. Combined with some fixes / bridge variant unification patches to the gspca sonixb and sonixj drivers allowing moving most usb-ids to use gspca sonix? by default. I've asked J.F. Moine to review all these patches and he has and responded to me with an Acked-By email hence the Acked-By tag on all of them. Except for the last one which I did after he reviewed the 2 patch sets. The following changes since commit 0a97a683049d83deaf636d18316358065417d87b: [media] cpia2: convert .ioctl to .unlocked_ioctl (2011-01-06 11:34:41 -0200) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git for_2.6.38-rc1 Hans de Goede (19): gspca_main: Locking fixes 1 gspca_main: Locking fixes 2 gspca_main: Update buffer flags even when user_copy fails gspca_main: Remove no longer used users variable gspca_main: Set memory type to GSPCA_MEMORY_NO on buffer release gspca_main: Simplify read mode memory type checks gspca_main: Allow switching from read to mmap / userptr mode gspca_main: wake wq on streamoff et61x251: remove wrongly claimed usb ids sn9c102: Remove not supported and non existing usb ids gspca_sonixb: Refactor to unify bridge handling gspca_sonixb: Adjust autoexposure window for vga cams so that it is centered gspca_sonixb: Fix TAS5110D sensor gain control gspca_sonixb: TAS5130C brightness control really is a gain control gspca_sonixb: Add usb ids for known sn9c103 cameras gspca_sonixj: Enable more usb ids when sn9c102 gets compiled too gspca_sonixj: Probe sensor type independent of bridge type gspca_sonixj: Add one more commented out usb-id gspca_sonixb: Fix mirrored image with ov7630 drivers/media/video/et61x251/et61x251.h| 24 -- drivers/media/video/gspca/gspca.c | 208 +-- drivers/media/video/gspca/gspca.h |2 - drivers/media/video/gspca/sonixb.c | 266 ++-- drivers/media/video/gspca/sonixj.c | 63 +++--- drivers/media/video/sn9c102/sn9c102_devtable.h | 74 +++ 6 files changed, 311 insertions(+), 326 deletions(-) Thanks & Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Move the deprecated et61x251 and sn9c102 to staging
Hi, On 01/02/2011 09:13 PM, Hans Verkuil wrote: Hi Hans, On Sunday, January 02, 2011 19:33:31 Hans de Goede wrote: So only 3 raw bayer + custom compression models supported by sn9c102 are not supported by gspca_sonixb, and all jpeg models are supported by gspca_sonixj. Porting the 3 remaining models over should be relatively easy, but I (I more or less maintain the sonixb driver) really need hardware access to ensure things stay working. Second correction, I was looking at an old tree and failed to notice that the zc0301 driver has already bitten the dust (good!). Thank you for your very helpful answer. Can you make a patch removing all the bogus usb IDs from these drivers? I've managed to make some time to also sort out the sn9c1xx usb ids situation. I've just send a pull request which includes patches cleaning things up. After this there are only 5 usb-ids left which will default to sn9c102 when both are compiled in, and only 3 of those are not supported by gspca. So if we move the sn9c102 driver to staging we will loose support for only 3 usb-ids. IOW I think it is time to move it to staging :) Note I can write a patch to add untested support for these 3 to the sonixb driver, given my experience with adding support for the hv7131d based on the sn9c102 code, that should be doable. But it will be completely untested :( Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Move the deprecated et61x251 and sn9c102 to staging
Hi, On 01/10/2011 02:33 AM, Mauro Carvalho Chehab wrote: Em 09-01-2011 10:02, Hans de Goede escreveu: I've managed to make some time to also sort out the sn9c1xx usb ids situation. I've just send a pull request which includes patches cleaning things up. After this there are only 5 usb-ids left which will default to sn9c102 when both are compiled in, and only 3 of those are not supported by gspca. Good! So if we move the sn9c102 driver to staging we will loose support for only 3 usb-ids. IOW I think it is time to move it to staging :) This would be a regression. Yes, although I wonder if anyone will notice. Fedora has had the sn9c102 driver disabled for 3 releases now and I've received (and fixed) a single bug in all that time about a cam not supported by gspca_sonixb which was supported by sn9c102 Note I can write a patch to add untested support for these 3 to the sonixb driver, given my experience with adding support for the hv7131d based on the sn9c102 code, that should be doable. But it will be completely untested :( I think that the better would be to add support for it at gspca, but wait for some feedback before considering it working. Well I've never seen these cams in the wild. sonixb cams with vga sensors are quite rare because they cannot do more then 7.5-10 fps. So most cam makers did the smart thing and went with a sonixj bridge for vga sensors. Anyways I'll do a gspca patch for adding support for the missing 3 models (as time permits). And then we can ship that (and make it the default if both are compiled in) for 1 or 2 cycles before moving the sn9c102 driver to staging. Assuming we don't receive any negative feedback in those 2 cycles (or manage to fix found bugs). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][rfc] media, video, stv06xx, pb0100: Don't potentially deref NULL in pb0100_start().
Hi, On 01/13/2011 11:05 PM, Jesper Juhl wrote: usb_altnum_to_altsetting() may return NULL. If it does we'll dereference a NULL pointer in drivers/media/video/gspca/stv06xx/stv06xx_pb0100.c::pb0100_start(). As far as I can tell there's not really anything more sensible than -ENODEV that we can return in that situation, but I'm not at all intimate with this code so I'd like a bit of review/comments on this before it's applied. Anyway, here's a proposed patch. Hi, On 01/13/2011 11:05 PM, Jesper Juhl wrote: > usb_altnum_to_altsetting() may return NULL. If it does we'll dereference a > NULL pointer in > drivers/media/video/gspca/stv06xx/stv06xx_pb0100.c::pb0100_start(). > As far as I can tell there's not really anything more sensible than > -ENODEV that we can return in that situation, but I'm not at all intimate > with this code so I'd like a bit of review/comments on this before it's > applied. > Anyway, here's a proposed patch. > pb0100_start gets called from stv06xx_start, which also does a usb_altnum_to_altsetting(intf, sd->gspca_dev.alt); and does contain the NULL check before calling pb0100_start. So I left out the check on purpose, to keep the code compact in IMHO better readable. Still I agree this is a bit tricky. So not NACK but not ACK either. What do others think? Regards, Hans Signed-off-by: Jesper Juhl --- stv06xx_pb0100.c |2 ++ 1 file changed, 2 insertions(+) compile tested only. diff --git a/drivers/media/video/gspca/stv06xx/stv06xx_pb0100.c b/drivers/media/video/gspca/stv06xx/stv06xx_pb0100.c index ac47b4c..75a5b9c 100644 --- a/drivers/media/video/gspca/stv06xx/stv06xx_pb0100.c +++ b/drivers/media/video/gspca/stv06xx/stv06xx_pb0100.c @@ -217,6 +217,8 @@ static int pb0100_start(struct sd *sd) intf = usb_ifnum_to_if(sd->gspca_dev.dev, sd->gspca_dev.iface); alt = usb_altnum_to_altsetting(intf, sd->gspca_dev.alt); + if (!alt) + return -ENODEV; packet_size = le16_to_cpu(alt->endpoint[0].desc.wMaxPacketSize); /* If we don't have enough bandwidth use a lower framerate */ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v2] Add plugin support to libv4l
Hi, Thanks for the new patch, it looks much better. Unfortunately I found what I can only describe as a bug in the plugin rfc (which as you probasbly know I wrote). The problem is 2 fold: 1) The existence of v4l2_fd_open (and its active use by multiple applications) means that we cannot let he plugin open the fd, since it may already be opened by the app. So the plugin function dev_open should be replaced by a dev_init taking an already open fd. Note we could probably change the API and remove v4l2_fd_open, but ... The reason for actually passing the open call with a file path (ie "/dev/video0") to the plugin was to allow creating a fully fake device (which would be created by a plugin when an open tries to open "fakevideo0" for example). However I now realize that doing fake (or userspace originating) device is best left to a loopback v4l2 kernel driver, because: 2) The fd needs to be a real file descriptor, which also really links to the v4l2 buffer for the device, as the application may use poll or select on the fd. Note that 2 also means when working with subdevices that the application actually needs to open the subdevice which will be the exit node for the format which the apps want (raw bayer for example has a different exit node then YUV data). This is part of the unsolved enumeration problem which we discussed in Helsinki last year. So summarizing, I believe that: 1) dev_open should be replaced by a dev_init which gets passed in an already open fd for the main / exit /dev/video node for the device 2) dev_close should be replaced by dev_cleanup, which frees allocated resources but does not close the fd (for consistency) This means this patch will need to be reworked a bit more, sorry about this. ### Looking at / thinking about the mmap / munmap code for plugins, I'm wondering if we should allow plugins to intercept mmap at all (given the above where we have given up on fully fake devices). Intercepting mmap is only useful for creating fakebuffers, which in turn is only necessary for format conversion which libv4l2 already handles. So I think it would be best to remove mmap interception, if we ever get a use case scenario where plugins need to modify frame data, they can create and manage their own mapping of the buffers and modify the data inline (after a dqbuf), or we can add a separate plugin mechanism to libv4lconvert for registering new format conversion plugins. Given that the main use case is for plugins which control settings inside the "webcam bridge" part of soc's related to 3a I don't think that dropping mmap interception will cause any problems and it will significantly simplify the code, removing some nasty corner cases. Which are hard to get right (and the current code does not get right). ### More detailed comments inline. Note quite a few remarks have to do with mmap / munmap. If we decide to drop this, they can be ignored :) On 01/14/2011 06:18 PM, Yordan Kamenov wrote: A libv4l2 plugin will sit in between libv4l2 itself and the actual /dev/video device node a fd refers to. It will be called each time libv4l2 wants to do an operation (read/write/ioctl/mmap/munmap) on the actual /dev/video node in question. Signed-off-by: Yordan Kamenov --- lib/include/libv4l2-plugin.h | 43 +++ lib/include/libv4l2.h |4 +- lib/include/libv4lconvert.h|5 +- lib/libv4l1/libv4l1.c |2 +- lib/libv4l2/Makefile |4 +- lib/libv4l2/libv4l2-priv.h | 24 ++ lib/libv4l2/libv4l2.c | 128 +++--- lib/libv4l2/v4l2-plugin.c | 344 lib/libv4l2/v4l2convert.c | 23 ++- lib/libv4lconvert/control/libv4lcontrol-priv.h |3 + lib/libv4lconvert/control/libv4lcontrol.c | 26 +- lib/libv4lconvert/control/libv4lcontrol.h |5 +- lib/libv4lconvert/libv4lconvert-priv.h |1 + lib/libv4lconvert/libv4lconvert.c | 25 +- 14 files changed, 573 insertions(+), 64 deletions(-) create mode 100644 lib/include/libv4l2-plugin.h create mode 100644 lib/libv4l2/v4l2-plugin.c diff --git a/lib/include/libv4l2-plugin.h b/lib/include/libv4l2-plugin.h new file mode 100644 index 000..3971735 --- /dev/null +++ b/lib/include/libv4l2-plugin.h @@ -0,0 +1,43 @@ +/* +* Copyright (C) 2010 Nokia Corporation + +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU Lesser General Public License as published by +* the Free Software Foundation; either version 2.1 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* Lesser General Public License for mor
Re: Upstreaming syntek driver
Hi, On 01/20/2011 12:35 PM, Luca Tettamanti wrote: On Tue, Jan 18, 2011 at 11:45 PM, Hans Verkuil wrote: [...] After a quick scan through the sources in svn I found the following (in no particular order): - Supports easycap model with ID 05e1:0408: a driver for this model is now in driver/staging/easycap. Can you elaborate? Is this the same hardware? - format conversion must be moved to libv4lconvert (if that can't already be used out of the box). Ditto for software brightness correction. - kill off the sysfs bits - kill off V4L1 - use the new control framework for the control handling - use video_ioctl2 instead of the current ioctl function - use unlocked_ioctl instead of ioctl Ok, major surgery then :) But probably the first step should be to see if this can't be made part of the gspca driver. I can't help thinking that that would be the best approach. But I guess the gspca developers can give a better idea of how hard that is. I've looked at the framework provided by gspca, it would probably allow to drop most of the USB support code from the driver. Yeah, that is the whole idea :) I give a big +1 to the Hans' suggestion to convert this to a gspca driver! I'm looking into frame handling. Let me know if you need any help / explanation about how certain things are done in gspca. Regards, Hans G (aka the other Hans). -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] v4l2-ctrls: add v4l2_ctrl_auto_cluster to simplify autogain/gain scenarios
Hi,
On 01/22/2011 12:06 PM, Hans Verkuil wrote:
It is a bit tricky to handle autogain/gain type scenerios correctly. Such
controls need to be clustered and the V4L2_CTRL_FLAG_UPDATE should be set on
the non-auto controls. If you set a non-auto control without setting the
auto control at the same time, then the auto control should switch to manual
mode. And usually the non-auto controls must be marked volatile, but this should
only be in effect if the auto control is set to auto.
The chances of drivers doing all these things correctly are pretty remote.
So a new v4l2_ctrl_auto_cluster function was added that takes care of these
issues.
I like the concept of this, but I'm not so happy with the automatically put
the auto control in manual mode. We've discussed this before and iirc we
came to the conclusion then that the proper thing to do is to mark the
controls as read only, when the related auto control is in auto mode.
This is what the UVC spec for example mandates and what the current UVC driver
does. Combining this with an app which honors the update and the read only
flag (try gtk-v4l), results in a nice experience. User enables auto exposure
-> exposure control gets grayed out, put exposure back manual -> control
is ungrayed.
So this new auto_cluster behavior would be a behavioral change (for both the
uvc driver and several gspca drivers), and more over an unwanted one IMHO
setting one control should not change another as a side effect.
Regards,
Hans
Signed-off-by: Hans Verkuil
---
drivers/media/video/v4l2-ctrls.c | 27 -
include/media/v4l2-ctrls.h | 48 +-
2 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 983e287..b5da617 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -1178,6 +1178,25 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct
v4l2_ctrl **controls)
}
EXPORT_SYMBOL(v4l2_ctrl_cluster);
+void v4l2_ctrl_auto_cluster(unsigned ncontrols, struct v4l2_ctrl **controls,
+ u8 manual_val, bool set_volatile)
+{
+ int i;
+
+ v4l2_ctrl_cluster(ncontrols, controls);
+ controls[0]->is_auto = true;
+ controls[0]->manual_mode_value = manual_val;
+
+ for (i = 1; i< ncontrols; i++) {
+ if (controls[i]) {
+ controls[i]->flags |= V4L2_CTRL_FLAG_UPDATE;
+ if (set_volatile)
+ controls[i]->is_volatile = true;
+ }
+ }
+}
+EXPORT_SYMBOL(v4l2_ctrl_auto_cluster);
+
/* Activate/deactivate a control. */
void v4l2_ctrl_activate(struct v4l2_ctrl *ctrl, bool active)
{
@@ -1605,7 +1624,8 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
struct v4l2_ext_controls *cs
v4l2_ctrl_lock(master);
/* g_volatile_ctrl will update the current control values */
- if (ctrl->is_volatile&& master->ops->g_volatile_ctrl)
+ if (ctrl->is_volatile&& master->ops->g_volatile_ctrl&&
+ (!master->is_auto || master->cur.val !=
master->manual_mode_value))
ret = master->ops->g_volatile_ctrl(master);
/* If OK, then copy the current control values to the caller */
if (!ret)
@@ -1717,6 +1737,11 @@ static int try_or_set_control_cluster(struct v4l2_ctrl
*master, bool set)
/* Don't set if there is no change */
if (!ret&& set&& cluster_changed(master)) {
+ if (master->is_auto&& !master->is_new&&
+ master->cur.val != master->manual_mode_value) {
+ master->is_new = 1;
+ master->val = master->manual_mode_value;
+ }
ret = master->ops->s_ctrl(master);
/* If OK, then make the new values permanent. */
if (!ret)
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index a2b2d58..e715f4e 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -66,7 +66,16 @@ struct v4l2_ctrl_ops {
* retrieved through the g_volatile_ctrl op. Drivers can set
* this flag.
* @is_enabled: If 0, then this control is disabled and will be hidden for
- *applications. Controls are always enabled by default.
+ *applications. Controls are always enabled by default. Drivers
+ *should never set this flag.
+ * @is_auto: If set, then this control selects whether the other cluster
+ *members are in 'automatic' mode or 'manual' mode. This is
+ *used for autogain/gain type clusters. Drivers should never
+ *set this flag.
+ * @manual_mode_value: If the is_auto flag is set, then this is the value
+ *of the auto control that determines if that control is in
+ *
Announcing v4l-utils-0.8.2
Hi, I'm happy to announce the release of v4l-utils-0.8.2 This release features many small fixes, esp. to the ir-keytable util (which now comes with udev rules and a manpage), qv4l2, v4l2-ctl and v4l2-compliance. v4l-utils-0.8.2 --- * Utils changes: * Various ir-keytable improvements (mchehab) * Various qv4l2 fixes (hverkuil, hdegoede) * Various v4l2-ctl fixes (hverkuil) * Add parsers for cx231xx i2c, saa7134 pci, sn9c201 usb and generic usb logs (mchehab) * v4l2-compliance: lots of new tests (hverkuil) * libv4l changes * Add many more laptop models to the upside down devices table (hdegoede) * Add support for 8-bits grey format (V4L2_PIX_FMT_GREY) (mchehab) Go get it here: http://linuxtv.org/downloads/v4l-utils/v4l-utils-0.8.2.tar.bz2 You can always find the latest developments here: http://git.linuxtv.org/v4l-utils.git Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] v4l2-ctrls: add v4l2_ctrl_auto_cluster to simplify autogain/gain scenarios
Hi, On 01/23/2011 05:13 PM, Hans Verkuil wrote: On Sunday, January 23, 2011 16:15:03 Hans de Goede wrote: This is what the UVC spec for example mandates and what the current UVC driver does. Combining this with an app which honors the update and the read only flag (try gtk-v4l), results in a nice experience. User enables auto exposure -> exposure control gets grayed out, put exposure back manual -> control is ungrayed. So this new auto_cluster behavior would be a behavioral change (for both the uvc driver and several gspca drivers), and more over an unwanted one IMHO setting one control should not change another as a side effect. Actually, I've been converting a whole list of subdev drivers recently (soc_camera, ov7670) and they all behaved like this. So I didn't change anything. Hmm, interesting. There is nothing preventing other drivers from doing something different. That said, changing the behavior to your proposal may not be such a bad idea. Yes and AFAIK this is what we agreed on when we discussed auto control a couple of months ago. But then I need the OK from all driver authors involved, since this would mean a change of behavior for them. The good news is that once they use the new framework function I only need to change what that function does and I don't need to change any of those drivers. So I will proceed for now by converting those drivers to use this new function, and at the same time I can contact the authors and ask what their opinion is of this change. I'm hoping for more feedback as well from what others think. Yes, contacting the authors to discuss this further sounds like a good idea. BTW, if I understand the gspca code correctly then it seems that if an e.g. autogain control is set to 1, then the gain control effectively disappears. I think queryctrl will just never return it. That can't be right. Erm, it should not disappear, but just get disabled. But this may have (accidentally) changed with the drivers which were converted to the new control framework. Anyways, we should discuss this with involved driver authors and agree on a standard way to handle this. Once we have agreement on how to handle this converting the drivers should be relatively easy. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What to do with videodev.h
Hi All, With v4l1 support going completely away, the question is raised what to do with linux/videodev.h . Since v4l1 apps can still use the old API through libv4l1, these apps will still need linux/videodev.h to compile. So I see 3 options: 1) Keep videodev.h in the kernel tree even after we've dropped the API support at the kernel level (seems like a bad idea to me) 2) Copy videodev.h over to v4l-utils as is (under a different name) and modify the #include in libv4l1.h to include it under the new name 3) Copy the (needed) contents of videodev.h over to libv4l1.h I'm not sure where I stand wrt 2 versus 3. Comments anyone? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Announcing v4l-utils-0.8.3
Hi, I'm happy to announce the release of v4l-utils-0.8.3. The main feature this release is that it will actually compile on a system with the kernel headers version >= 2.6.38. v4l-utils-0.8.3 --- * Utils changes: * Various ir-keytable improvements (mchehab) * Various cx231xx parser improvements (mchehab) * libv4l changes * Add a few more laptop models to the upside down devices table (hdegoede) * Make libv4l1 compile with kernels >= 2.6.38, which no longer have the v4l1 linux/videodev.h header (hdegoede) Go get it here: http://linuxtv.org/downloads/v4l-utils/v4l-utils-0.8.3.tar.bz2 You can always find the latest developments here: http://git.linuxtv.org/v4l-utils.git Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l-utils: Add the JPEG Lite decoding function
Hi, On 02/14/2011 01:36 PM, Jean-Francois Moine wrote: Hi Hans, JPEG Lite images are created by the DivIO nw80x chips. The gspca subdriver nw80x will be ready soon. Decoding to RGB24 and BGR24 are ok. Decoding to YUV420 and YVU420 are not tested. You're working on nw80x support? Cool! I've a cam with one of those chipsets lying around, it is a Dynalink 06be:d001. I'll definitely give the driver a try when it hits your git tree, or send it my way if you wanted it tested with this cam earlier. Now about the libv4l patch, unfortunately it cannot go in as is. The problem is that the code you derived it from seems to be GPL not LGPL (judging from the copyright header you put on top of it), and libv4l is LGPL, and has to be to to allow it to be used with for example skype and flash. The thing to do here is to try and contact the original author and get permission to relicense under the LGPL (version 2 or later). In the mean time the code can still go in but as an external helper, see for example the ov511 and ov518 decompression code. There is a generic external helper framework in libv4l, so the needed code changes should be minimal. Thanks & Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l-utils: Add the JPEG Lite decoding function
Hi,
On 02/15/2011 10:26 AM, Jean-Francois Moine wrote:
Hi Hans,
I got the permission to relicense the JPEG Lite decoding to the LGPL.
Ah, good, patch applied and pushed to git.
If you want to test the nw80x driver, get the gspca tarball from my web
page (2.12.12). I added your webcam which should directly work (p35u -
chip nw801).
I've tested it and it works :) I also tested the JPGL -> YUV path.
I did find 2 bugs, the "if (gspca_dev->curr_mode)" test in sd_start,
needs to be inverted. In general it is a good idea to do a test
on gspca_dev->width, rather then curr_mode IMHO, it is more
readable and less error prone.
Talking about readability, I also found the
if (sd->bridge == BRIDGE_NW800) {
...
} else {
...
if (sd->bridge == BRIDGE_NW802) {
...
} else {
...
}
}
part in sd_init a bit hard to grok, can this be changed
to a switch case on sd->bridge ?
The other bug was a divide by zero -> kernel panic, in
do_autogain when sd->ae_res == 0, note this was when I was
messing around with the driver a bit (before I found the
issue with the inverted curr_mode check), but I think this
could happen in real life to depending on register values
and we should protect against this.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.39] gspca_sn9c20x fixes
Hi Mauro, Please pull from my gspca tree, for some gspca_sn9c20x fixes I've been doing. The following changes since commit 5ed4bbdae09d207d141759e013a0f3c24ae76ecc: [media] tuner-core: Don't touch at standby during tuner_lookup (2011-02-15 10:31:01 -0200) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git gspca-for_v2.6.39 Hans de Goede (5): gspca_sn9c20x: Fix colored borders with ov7660 sensor gspca_sn9c20x: Add hflip and vflip controls for the ov7660 sensor gspca_sn9c20x: Add LED_REVERSE flag for 0c45:62bb gspca_sn9c20x: Make buffers slightly larger for JPEG frames gspca_sn9c20x: Add another MSI laptop to the sn9c20x upside down list drivers/media/video/gspca/sn9c20x.c | 40 ++ 1 files changed, 30 insertions(+), 10 deletions(-) Thanks, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v4l-utils-0.8.3 and KVDR
Hi, On 02/20/2011 12:48 AM, Mike Booth wrote: My understanding of the "wrappers"contained in this library is that v4l applications should work with kernels from 2.6.36 onwards if the compat.so is preloaded. I use KVDR for watching and controlling VDR on my TV. Xine and Xineliboutput or not options as they don't provide TV out and TV out fronm the video card is also not an option because of where things are in the house. KVDR fails with Xv-VIDIOCGCAP: Invalid argument Xv-VIDIOCGMBUF: Invalid argument works perfectly fine on linux-2.6.35 Anyone have any ideas First of all make sure you load kdvr with the appropriote LD_PRELOAD, ie: LD_PRELOAD=/usr/lib/libv4l/v4l1compat.so kvdr If that does not help, do the following before launching kvdr: export LIBV4L1_LOG_FILENAME=/tmp/log So the total sequence of commands becomes: export LIBV4L1_LOG_FILENAME=/tmp/log LD_PRELOAD=/usr/lib/libv4l/v4l1compat.so kvdr Then do what ever you want to do and fails, and send another mail with /tmp/log attached. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v4l-utils-0.8.3 and KVDR
Hi, On 02/22/2011 10:03 AM, Mike Booth wrote: KVDR has a number of different parameters including -xforce xv-mode on startup and disable overlay-mod -ddont switch modeline during xv with kernel 2.6.35 I run KVDR with -x as I have an NVIDIA graphics. Running on 2.6.38 KVDR -x doesn't produce any log. The display appears and immediately disappears although there is a process running. So with 2.6.35 and v4l-utils-0.8.3 things work ? Then this is not a libv4l problem, as libv4l will no longer use the kernels v4l1 compat independent of the kernels version. Also in the log I see nothing indicating this is a v4l1 app (I'm not familiar with kvdr), so I think something else may have changed in the new kernel causing your issue. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.39] new vicam driver + gspca_sn9c20x fixes
Hi Mauro, Please pull from my gspca tree, for a new driver for the vicam, removal of the old vicam driver from staging, some gspca_sn9c20x fixes and a gspca_cpia1 fix. Note this pull request supersedes my previous one. The following changes since commit 5ed4bbdae09d207d141759e013a0f3c24ae76ecc: [media] tuner-core: Don't touch at standby during tuner_lookup (2011-02-15 10:31:01 -0200) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git gspca-for_v2.6.39 Hans de Goede (8): gspca_sn9c20x: Fix colored borders with ov7660 sensor gspca_sn9c20x: Add hflip and vflip controls for the ov7660 sensor gspca_sn9c20x: Add LED_REVERSE flag for 0c45:62bb gspca_sn9c20x: Make buffers slightly larger for JPEG frames gspca_sn9c20x: Add another MSI laptop to the sn9c20x upside down list gspca: Add new vicam subdriver gspca_cpia1: Don't allow the framerate divisor to go above 2 staging-usbvideo: remove drivers/media/video/gspca/Kconfig | 10 + drivers/media/video/gspca/Makefile |2 + drivers/media/video/gspca/cpia1.c |6 +- drivers/media/video/gspca/sn9c20x.c | 40 +- drivers/media/video/gspca/vicam.c | 381 ++ drivers/staging/Kconfig |2 - drivers/staging/Makefile|1 - drivers/staging/usbvideo/Kconfig| 15 - drivers/staging/usbvideo/Makefile |2 - drivers/staging/usbvideo/TODO |5 - drivers/staging/usbvideo/usbvideo.c | 2230 --- drivers/staging/usbvideo/usbvideo.h | 395 --- drivers/staging/usbvideo/vicam.c| 952 --- drivers/staging/usbvideo/videodev.h | 318 - 14 files changed, 426 insertions(+), 3933 deletions(-) create mode 100644 drivers/media/video/gspca/vicam.c delete mode 100644 drivers/staging/usbvideo/Kconfig delete mode 100644 drivers/staging/usbvideo/Makefile delete mode 100644 drivers/staging/usbvideo/TODO delete mode 100644 drivers/staging/usbvideo/usbvideo.c delete mode 100644 drivers/staging/usbvideo/usbvideo.h delete mode 100644 drivers/staging/usbvideo/vicam.c delete mode 100644 drivers/staging/usbvideo/videodev.h Thanks, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/1 v3] libv4l: Add plugin support
Hi, On 03/10/2011 02:46 PM, Yordan Kamenov wrote: Hi Hans, any comments on that? I looked at globally immediately after you send it, summary of that look: looks good, but I still spotted some minor issues. I'm still trying to make some time for a more detailed look. I hope to do that real soon now, esp. as I would like to have this reviewed before the Warschau meeting. Thanks & Regards, Hans Regards Yordan Yordan Kamenov wrote: Hi Hans, here is third version of plugin support for libv4l2. Changes in v3: * Pass opened fd to the plugin instead of filename * Plugin private data is returned by init call and is passed as argument in ioctl/read/close (remove libv4l2_set/get_plugindata functions) * Plugin do not handle mmap/munmap -- Changes in v2: * Remove calls of v4l2_plugin_foo functions in the beginning of coresponding v4l2_foo functions and instead replace SYS_FOO calls. * Add to v4l2_dev_info device operation structure which can hold plugin callbacks or dyrect syscall(SYS_foo, ...) calls. * Under libv4lconvert also replace SYS_FOO cals with device operations. This required also to add dev_ops field to v4lconvert_data and v4lcontrol_data. --- v1: Here is initial version of plugin support for libv4l, based on your RFC. It is provided by functions v4l2_plugin_[open,close,etc]. When open() is called libv4l dlopens files in /usr/lib/libv4l/plugins 1 at a time and call open() callback passing through the applications parameters unmodified. If a plugin is relevant for the specified device node, it can indicate so by returning a value other then -1 (the actual file descriptor). As soon as a plugin returns another value then -1 plugin loading stops and information about it (fd and corresponding library handle) is stored. For each function v4l2_[ioctl,read,close,etc] is called corresponding v4l2_plugin_* function which looks if there is loaded plugin for that file and call it's callbacks. v4l2_plugin_* functions indicate by their first argument if plugin was used, and if it was not then v4l2_* functions proceed with their usual behavior. Yordan Kamenov (1): libv4l: Add plugin support to libv4l lib/include/libv4l2-plugin.h | 36 ++ lib/include/libv4lconvert.h | 5 +- lib/libv4l2/Makefile | 4 +- lib/libv4l2/libv4l2-priv.h | 10 ++ lib/libv4l2/libv4l2.c | 90 ++ lib/libv4l2/v4l2-plugin.c | 160 lib/libv4l2/v4l2convert.c | 9 -- lib/libv4lconvert/control/libv4lcontrol-priv.h | 4 + lib/libv4lconvert/control/libv4lcontrol.c | 35 -- lib/libv4lconvert/control/libv4lcontrol.h | 5 +- lib/libv4lconvert/libv4lconvert-priv.h | 2 + lib/libv4lconvert/libv4lconvert.c | 34 -- 12 files changed, 333 insertions(+), 61 deletions(-) create mode 100644 lib/include/libv4l2-plugin.h create mode 100644 lib/libv4l2/v4l2-plugin.c -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the driver for "0c45:6413 Microdia" webcam?
Hi, On 03/11/2011 07:04 PM, Chen, Xianwen wrote: Hi there, I'm having a hard time locating the proper driver for "0c45:6413 Microdia" webcam. I consulted "Documentation/video4linux/gspca.txt", but didn't find such a device. The 6413 is a UVC compatible camera / controller from Microdia, as such it should use the uvcvideo driver. Basically on any modern Linux distribution, it should just work as soon as you plug it in. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.39] Add support for cpia1 camera button
Hi Mauro, Please pull from my gspca tree, for a single patch adding support for the button on cpia1 based cameras. The following changes since commit 41f3becb7bef489f9e8c35284dd88a1ff59b190c: [media] V4L DocBook: update V4L2 version (2011-03-11 18:09:02 -0300) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git gspca-for_v2.6.39 Hans de Goede (1): gspca_cpia1: Add support for button drivers/media/video/gspca/cpia1.c | 31 ++- 1 files changed, 26 insertions(+), 5 deletions(-) Thanks, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Missing V4L2_PIX_FMT_JPGL
Hi, On 03/14/2011 11:16 AM, Hans Verkuil wrote: Hi Hans, I just copied the latest videobuf2.h to v4l-utils and tried to compile, but it fails with: make[2]: Entering directory `/home/hve/work/src/v4l/v4l-utils/lib/libv4lconvert' cc -Wp,-MMD,"libv4lconvert.d",-MQ,"libv4lconvert.o",-MP -c -I../include -fvisibility=hidden -fPIC -DLIBDIR=\"/usr/local/lib\" -DLIBSUBDIR=\"libv4l\" -I../../include -I../../lib/include -D_GNU_SOURCE -DV4L_UTILS_VERSION='"0.8.4-test"' -g -O1 -Wall -Wpointer-arith -Wstrict-prototypes -Wmissing-prototypes -o libv4lconvert.o libv4lconvert.c libv4lconvert.c:73: error: 'V4L2_PIX_FMT_JPGL' undeclared here (not in a function) It seems V4L2_PIX_FMT_JPGL was removed. I guess the support for this format can be removed from libv4lconvert.c? Actually it is not removed, it is not yet added. This is a new format for devices using the (ancient) nw80x chipset. Jean Francois Moine has been working on adding support for those to the gspca driver. I expect a pull request from him soon, to add this driver to 2.6.39, and thus the define to videodev2.h . In the mean time please leave the define in v4l-utils's videodev2.h copy (insert it manually after syncing). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Announcing v4l-utils-0.8.1
Hi, I'm happy to announce the second stable release of v4l-utils, with as highlight that libv4l1 no longer needs the kernel v4l1 compat code, so that can be removed from the kernel (jay!). New this release: v4l-utils-0.8.1 --- * Utils changes: * Various v4l-keytable improvements (mchehab) * Various qv4l2 fixes (hverkuil) * v4l2-ctl: Added support for s/g_dv_timings (Mats Randgaard) * libv4l changes (hdegoede): * Add many more laptop models to the upside down devices table * Detect short frames (and retry upto 3 times to get a non short frame) * Support (uvc) cameras with more then 16 framesizes properly (Balint Reczey) * libv4l1 no longer relies on the kernel v4l1 compat ioctl handling, many thanks to Huzaifa Sidhpurwala for his work on this! * Add support for Xirlink C-It YYVYUY * Add support for konica yuv420 format Go get it here: http://linuxtv.org/downloads/v4l-utils/v4l-utils-0.8.1.tar.bz2 You can always find the latest developments here: http://git.linuxtv.org/v4l-utils.git Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ibmcam (xrilink_cit) and konica webcam driver porting to gspca update
Hi, On 08/28/2010 12:54 AM, Jonathan Isom wrote: On Sun, Jul 4, 2010 at 6:29 AM, Hans de Goede wrote: Hi all, I've finished porting the usbvideo v4l1 ibmcam and konicawc drivers to gspcav2. The ibmcam driver is replaced by gspca_xirlink_cit, which also adds support for 2 new models (it turned out my testing cams where not supported by the old driver). This one could use more testing. I just tried using your driver. I get no video. using 2.6.35.3. Had to patch usb_buffer_[alloc& free] otherwise no changes to your tree. /usr/bin/qv4l2 /dev/video2 Start Capture: Input/output error VIDIOC_STREAMON: Input/output error Start Capture: Input/output error VIDIOC_STREAMON: Input/output error Thanks for testing! Ok, so a couple of things: 1) For the new xirlink cit driver to work you need the latest libv4l from v4l-utils (use the just released 0.8.1 for example), but that does not explain the io errors. 2) Make sure you the old usbvideo ibmcam driver is no used (remove the .ko file from /lib/modules/... 3) Do echo 63 > /sys/module/gspca_main/parameters/debug (as root, note sudo does not work with redirects) And then try some application again and after trying do dmesg > dmesg.txt and attach dmesg.txt to your next reply. 4) What did you need to change exactly, can you share the changes in question with me in the form of a patch ? (I'm waiting for Douglas to sync the main hg v4l-dvb tree with the latest upstream / mauro's tree and then I'll rebase my ibmcam tree. 5) Do lsusb -v > lsusb.log and attach that to your next mail too. Thanks & Regards, Hans -- info Model 2 KSX-X9903 0x0545 0x8080 3.0a Old, cheaper model Xirlink C-It /usr/sbin/v4l2-dbg -d /dev/video2 -D Driver info: Driver name : xirlink-cit Card type : USB IMAGING DEVICE Bus info : usb-:00:12.2-6.1 Driver version: 133376 Capabilities : 0x0501 Video Capture Read/Write Streaming Any Ideas Jonathan The konicawc driver is replaced by gspca_konica which is pretty much finished. You can get them both here: http://linuxtv.org/hg/~hgoede/ibmcam Once Douglas updates the hg v4l-dvb tree to be up2date with the latest and greatest from Mauro, then I'll rebase my tree (the ibmcam driver needs a very recent gspca core patch), and send a pull request. Regards, Hans p.s. 1) Many thanks to Patryk Biela for providing me a konica driver using camera. 2) Still to do the se401 driver. 3) I'll be on vacation the coming week and not reading email. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] LED control
Hi all, On 09/04/2010 01:10 PM, Jean-Francois Moine wrote: Some media devices may have one or many lights (LEDs, illuminators, lamps..). This patch makes them controlable by the applications. Signed-off-by: Jean-Francois Moine -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ led.patch diff --git a/Documentation/DocBook/v4l/controls.xml b/Documentation/DocBook/v4l/controls.xml index 8408caa..c9b8ca5 100644 --- a/Documentation/DocBook/v4l/controls.xml +++ b/Documentation/DocBook/v4l/controls.xml @@ -312,6 +312,13 @@ minimum value disables backlight compensation. information and bits 24-31 must be zero. + V4L2_CID_LEDS + integer + Switch on or off the LED(s) or illuminator(s) of the device. + The control type and values depend on the driver and may be either + a single boolean (0: off, 1:on) or the index in a menu type. + I think that using one control for both status leds (which is what we are usually talking about) and illuminator(s) is a bad idea. I'm fine with standardizing these, but can we please have 2 CID's one for status lights and one for the led. Esp, as I can easily see us supporting a microscope in the future where the microscope itself or other devices with the same bridge will have a status led, so then we will need 2 separate controls anyways. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gspca_cpia1: Add lamp control for Intel Play QX3 microscope
Hi,
On 09/03/2010 03:09 AM, Andy Walls wrote:
# HG changeset patch
# User Andy Walls
# Date 1283475832 14400
# Node ID 0d251a2976b46e11cc817207de191896718b93a3
# Parent a4c762698bcb138982b81cf59e5bc4b7155866a9
gspca_cpia1: Add lamp cotrol for Intel Play QX3 microscope
From: Andy Walls
Add a v4l2 control to get the lamp control code working for the Intel Play
QX3 microscope. My daughter in middle school thought it was cool, and is now
examining the grossest specimens she can find.
Hehe, cool I'm very happy to hear the cpia1 driver actually being used in a
"productive" manner, that shows it is worth all the time and effort I've put
into
cleaning up / rewriting old v4l1 drivers :)
About the patch: first of all thanks. wrt lamps versus lights I'm indifferent.
The only thing I've notices is that you've made the controls instand apply.
Normally
controls setting changes when not streaming are just remembered and then applied
when the stream is initialized.
However your code sends the lamp settings to the device as soon as they are
changed, and does not send them on sd_start. The sending as soon as changes
makes sense. But did you check that this actually works, iow did you play with
the lamps control while not streaming ? and then tried to stream and see if
the settings stuck.
Also the not sending at sd_start, nor sd_init means that you assume that the
defaults in the driver (both lamps off) ar the same as of the device as you
never force that the device <-> driver settings are synced on driver load
or stream start. This may not be the case when resuming from suspend or
the driver is rmmod-ed insmod-ed. So assuming that the instant apply
of this control does not cause issues, you should add a call to
command_setlamps(gspca_dev); at the end of sd_init.
Regards,
Hans
Priority: normal
Signed-off-by: Andy Walls
diff -r a4c762698bcb -r 0d251a2976b4 linux/drivers/media/video/gspca/cpia1.c
--- a/linux/drivers/media/video/gspca/cpia1.c Wed Aug 25 16:13:54 2010 -0300
+++ b/linux/drivers/media/video/gspca/cpia1.c Thu Sep 02 21:03:52 2010 -0400
@@ -333,8 +333,8 @@
} format;
struct {/* Intel QX3 specific data */
u8 qx3_detected;/* a QX3 is present */
- u8 toplight;/* top light lit , R/W */
- u8 bottomlight; /* bottom light lit, R/W */
+ u8 toplamp; /* top lamp lit , R/W */
+ u8 bottomlamp; /* bottom lamp lit, R/W */
u8 button; /* snapshot button pressed (R/O) */
u8 cradled; /* microscope is in cradle (R/O) */
} qx3;
@@ -373,6 +373,8 @@
static int sd_getfreq(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setcomptarget(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getcomptarget(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setlamps(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getlamps(struct gspca_dev *gspca_dev, __s32 *val);
static const struct ctrl sd_ctrls[] = {
{
@@ -447,6 +449,20 @@
.set = sd_setcomptarget,
.get = sd_getcomptarget,
},
+ {
+ {
+#define V4L2_CID_LAMPS (V4L2_CID_PRIVATE_BASE+1)
+ .id = V4L2_CID_LAMPS,
+ .type= V4L2_CTRL_TYPE_MENU,
+ .name= "Lamps",
+ .minimum = 0,
+ .maximum = 3,
+ .step= 1,
+ .default_value = 0,
+ },
+ .set = sd_setlamps,
+ .get = sd_getlamps,
+ },
};
static const struct v4l2_pix_format mode[] = {
@@ -766,8 +782,8 @@
params->compressionTarget.targetQ = 5; /* From windows driver */
params->qx3.qx3_detected = 0;
- params->qx3.toplight = 0;
- params->qx3.bottomlight = 0;
+ params->qx3.toplamp = 0;
+ params->qx3.bottomlamp = 0;
params->qx3.button = 0;
params->qx3.cradled = 0;
}
@@ -1059,17 +1075,16 @@
0, sd->params.streamStartLine, 0, 0);
}
-#if 0 /* Currently unused */ /* keep */
-static int command_setlights(struct gspca_dev *gspca_dev)
+static int command_setlamps(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
- int ret, p1, p2;
+ int ret, p;
if (!sd->params.qx3.qx3_detected)
return 0;
- p1 = (sd->params.qx3.bottomlight == 0)<< 1;
- p2 = (sd->params.qx3.toplight == 0)<< 3;
+ p = (sd->params.qx3.toplamp== 0) ? 0x8 : 0;
+ p |= (sd->params.qx3.bottomlamp == 0) ? 0x2 : 0;
ret = do_command(gspca_dev, CPIA_COMMAND_WriteVCReg,
0x90, 0x8F, 0x50, 0);
@@ -1077,9 +1092,8 @@
return ret;
return do_command(gspca_dev, CPIA_COMMAND_WriteMCPort, 2, 0,
- p1
Re: [PATCH] LED control
Hi, On 09/05/2010 10:04 AM, Peter Korsgaard wrote: "Hans" == Hans de Goede writes: Hi, >> + V4L2_CID_LEDS >> + integer >> + Switch on or off the LED(s) or illuminator(s) of the device. >> + The control type and values depend on the driver and may be either >> + a single boolean (0: off, 1:on) or the index in a menu type. >> + Hans> I think that using one control for both status leds (which is Hans> what we are usually talking about) and illuminator(s) is a bad Hans> idea. I'm fine with standardizing these, but can we please have 2 Hans> CID's one for status lights and one for the led. Esp, as I can Hans> easily see us supporting a microscope in the future where the Hans> microscope itself or other devices with the same bridge will have Hans> a status led, so then we will need 2 separate controls anyways. Why does this need to go through the v4l2 api and not just use the standard LED (sysfs) api in the first place? Quoting from the reply by Jean-Francois Moine to a patch adding illuminator control support to the cpia1 driver where this proposal is a result of: ### As many gspca users are waiting for a light/LED/illuminator/lamp control, I tried to define a standard one in March 2009: http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/3095 A second, but more restrictive, attempt was done by Németh Márton in February 2010: http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/16705 The main objection to that proposals was that the sysfs LED interface should be used instead: http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/3114 A patch in this way was done by Németh Márton in February 2010: http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/16670 but it was rather complex, and there was no consensus http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/17111 ### So using the sysfs interface for this is non trivial. Most cameras don't offer any hardware dimming / blinking features, but we do want to do an auto setting where the led goes on when streaming and goes off again when not streaming. So the sysfs interface is not a good match to what we need. A more important argument IMHO however is that the LED control is just one element of many things one can control on (some) webcams, things like focus, pan and tilt for the more fancy ones also come into play. Not to mention contrast, brightness etc. settings. Currently we have one central API for this which is the v4l2 ctrl API, and we have several apps which dynamically build up a UI for this depending on the ctrls advertised by the device. Adding a LED ctrl to the v4l2 API will make this automatically show up in these apps and give the user one central place to control everything related to the camera. Where as using the led sysfs API would mean that the led control will basically stay invisible to the end user unless we start patching all apps to also use support this API, requiring all v4l2 apps to grow code to support a whole new api just to turn on / off a led does not seem like a good idea to me. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gspca_cpia1: Add lamp control for Intel Play QX3 microscope
Hi,
p.s. (forgot to mention this in my previous mail)
On 09/03/2010 03:09 AM, Andy Walls wrote:
@@ -447,6 +449,20 @@
.set = sd_setcomptarget,
.get = sd_getcomptarget,
},
+ {
+ {
+#define V4L2_CID_LAMPS (V4L2_CID_PRIVATE_BASE+1)
+ .id = V4L2_CID_LAMPS,
+ .type= V4L2_CTRL_TYPE_MENU,
+ .name= "Lamps",
+ .minimum = 0,
+ .maximum = 3,
+ .step= 1,
+ .default_value = 0,
+ },
+ .set = sd_setlamps,
+ .get = sd_getlamps,
+ },
};
static const struct v4l2_pix_format mode[] = {
We only want this control to be available on the qx3 and not on
all cpia1 devices, so you need to add something like the following to
sd_config:
if (!(id->idVendor == 0x0813 && id->idProduct == 0x0001))
gspca_dev->ctrl_dis = 1 << LAMPS_IDX;
Where LAMPS_IDX is a define giving the index of V4L2_CID_LAMPS in the
sd_ctrls array, see the ov519 gspca driver for example.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ibmcam (xrilink_cit) and konica webcam driver porting to gspca update
Hi, On 08/31/2010 11:43 PM, David Ellingsworth wrote: Hans, I haven't had any success with this driver as of yet. My camera is shown here: http://www.amazon.com/IBM-Net-Camera-Pro-camera/dp/B0009MH25U The part number listed on the bottom is 22P5086. It's also labeled as being an IBM Net Camera Pro. Ah ok, so you have the same one as I have, that model was never supported by the old ibmcam driver, so I take it you never had it working with the old ibmcam driver ? > When I plug the camera in, it is detected by the driver but it does not seem to function in this mode. Every attempt to obtain video from it using qv4l2 results in a black or green image. If I use the ibm_netcam_pro module option Given that is the same camera as I have using the ibm_netcam_pro module option is definitely the right thing to do. I noticed in your lsusb -v output that you're doing this from within vmware? I think that is the cause of things not working. This camera will not even work when connected through a real hub, let alone through a virtual one. The only way this camera works for me is when it is connected to a usb port directly on the motherboard, running Linux directly on the hardware, can you please try that ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] LED control
Hi, On 09/05/2010 10:56 AM, Jean-Francois Moine wrote: On Sun, 05 Sep 2010 09:56:54 +0200 Hans de Goede wrote: I think that using one control for both status leds (which is what we are usually talking about) and illuminator(s) is a bad idea. I'm fine with standardizing these, but can we please have 2 CID's one for status lights and one for the led. Esp, as I can easily see us supporting a microscope in the future where the microscope itself or other devices with the same bridge will have a status led, so then we will need 2 separate controls anyways. Hi Hans, I was not thinking about the status light (I do not see any other usage for it), but well about illuminators which I saw only in microscopes. Ah, ok thanks for clarifying. For some more on this see p.s. below. So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS? I think that V4L2_CID_ILLUMINATORS together with a comment in the .h and explanation in the spec that this specifically applies to microscopes would be good. Regards, Hans p.s. I think it would be good to have a V4L2_CID_STATUS_LED too. In many drivers we are explicitly controlling the led by register writes. Some people may very well prefer the led to always be off. I know that uvc logitech cameras have controls for the status led through the extended uvc controls. Once we have a standardized LED control, we can move the logitech uvc cams over from using their own private one to this one. Once this is in place I would like to build some framework in to gspca for supporting this control in gspca (the control would be handled by the core, and sub drivers would have an sd_set_led function). While at it could you write a proposal / patch for adding this control to the spec as well ? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] LED control
Hi, On 09/05/2010 08:43 PM, Andy Walls wrote: On Sun, 2010-09-05 at 15:54 +0200, Hans de Goede wrote: Hi, On 09/05/2010 10:56 AM, Jean-Francois Moine wrote: On Sun, 05 Sep 2010 09:56:54 +0200 Hans de Goede wrote: I think that using one control for both status leds (which is what we are usually talking about) and illuminator(s) is a bad idea. I'm fine with standardizing these, but can we please have 2 CID's one for status lights and one for the led. Esp, as I can easily see us supporting a microscope in the future where the microscope itself or other devices with the same bridge will have a status led, so then we will need 2 separate controls anyways. Hi Hans, I was not thinking about the status light (I do not see any other usage for it), but well about illuminators which I saw only in microscopes. Ah, ok thanks for clarifying. For some more on this see p.s. below. So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS? I think that V4L2_CID_ILLUMINATORS together with a comment in the .h and explanation in the spec that this specifically applies to microscopes would be good. I concur with ILLUMINATORS. The word makes it very clear the control is about actively putting light on a subject. A quick Goggle search shows that the term 'illuminator" appears to apply to photography and IR cameras as well. Regards, Hans p.s. I think it would be good to have a V4L2_CID_STATUS_LED too. In many drivers we are explicitly controlling the led by register writes. Some people may very well prefer the led to always be off. I know that uvc logitech cameras have controls for the status led through the extended uvc controls. Once we have a standardized LED control, we can move the logitech uvc cams over from using their own private one to this one. I saw two use cases mentioned for status LEDs: 1. always off 2. driver automatically controls the LEDs. Can't that choice be handled with a module option Sure, just like all other v4l2 controls could be a module option, that is not very userfriendly though. , is there a case where one needs more control? Not really. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pull request: http://linuxtv.org/hg/~hgoede/ibmcam3 : new xirlink_cit + konica drivers
Hi Mauro, Please pull from: http://linuxtv.org/hg/~hgoede/ibmcam3 Which is my gspca tree which features 2 new (rewritten old v4l1 drivers) gspca subdrivers for xirlink cit and konica chipset webcams. The complete pull consists of the following commits: 4 minutes Hans de Goede gspca_xirlink_cit: adjust ibm netcam pro framerate for available bandwidthdefault tip 12 hoursHans de Goede gspca_konica: New gspca subdriver for konica chipset using cams 22 hoursHans de Goede gspca_*: correct typo in my email address in various subdrivers 2 monthsHans de Goede Mark usbvideo ibmcam driver as deprecated 22 hoursHans de Goede gspca_xirlink_cit: support bandwidth changing for devices with 1 alt setting 6 days Hans de Goede gspca_xirlink_cit: Use alt setting -> fps formula for model 1 cams too 13 hours Hans de Goede gspca_xirlink_cit: Add support for camera with a bcd version of 0.01 2 months Hans de Goede gspca_xirlink_cit: Add support for Model 1, 2 & 4 cameras 13 hours Hans de Goede gspca_xirlink_cit: New gspca subdriver replacing v4l1 usbvideo/ibmcam.c Thanks & Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi,
Looks good to me.
Acked-by: Hans de Goede
Regards,
Hans
On 09/06/2010 08:11 PM, Jean-Francois Moine wrote:
Hi,
This new proposal cancels the previous 'LED control' patch.
Cheers.
-- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
led.patch
Some media devices (microscopes) may have one or many illuminators,
and most webcams have a status LED which is normally on when capture is active.
This patch makes them controlable by the applications.
Signed-off-by: Jean-Francois Moine
diff --git a/Documentation/DocBook/v4l/controls.xml
b/Documentation/DocBook/v4l/controls.xml
index 8408caa..77f87ad 100644
--- a/Documentation/DocBook/v4l/controls.xml
+++ b/Documentation/DocBook/v4l/controls.xml
@@ -312,10 +312,27 @@ minimum value disables backlight compensation.
information and bits 24-31 must be zero.
+ V4L2_CID_ILLUMINATORS
+ integer
+ Switch on or off the illuminator(s) of the device
+ (usually a microscope).
+ The control type and values depend on the driver and may be either
+ a single boolean (0: off, 1:on) or defined by a menu type.
+
+
+ V4L2_CID_STATUS_LED
+ integer
+ Set the status LED behaviour. Possible values for
+enum v4l2_status_led are:
+V4L2_STATUS_LED_AUTO (0),
+V4L2_STATUS_LED_ON (1),
+V4L2_STATUS_LED_OFF (2).
+
+
V4L2_CID_LASTP1
End of the predefined control IDs (currently
-V4L2_CID_BG_COLOR + 1).
+V4L2_CID_STATUS_LED + 1).
V4L2_CID_PRIVATE_BASE
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 61490c6..75e8869 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1045,8 +1045,16 @@ enum v4l2_colorfx {
#define V4L2_CID_CHROMA_GAIN(V4L2_CID_BASE+36)
+#define V4L2_CID_ILLUMINATORS (V4L2_CID_BASE+37)
+#define V4L2_CID_STATUS_LED(V4L2_CID_BASE+38)
+enum v4l2_status_led {
+ V4L2_STATUS_LED_AUTO= 0,
+ V4L2_STATUS_LED_ON = 1,
+ V4L2_STATUS_LED_OFF = 2,
+};
+
/* last CID + 1 */
-#define V4L2_CID_LASTP1 (V4L2_CID_BASE+37)
+#define V4L2_CID_LASTP1 (V4L2_CID_BASE+39)
/* MPEG-class control IDs defined by V4L2 */
#define V4L2_CID_MPEG_BASE(V4L2_CTRL_CLASS_MPEG | 0x900)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/07/2010 09:30 AM, Hans Verkuil wrote: On Monday, September 06, 2010 20:11:05 Jean-Francois Moine wrote: Hi, This new proposal cancels the previous 'LED control' patch. Cheers. Hi Jean-Francois, You must also add support for these new controls in v4l2-ctrls.c in v4l2_ctrl_get_menu(), v4l2_ctrl_get_name() and v4l2_ctrl_fill(). How is CID_ILLUMINATORS supposed to work in the case of multiple lights? Wouldn't a bitmask type be more suitable to this than a menu type? There isn't a bitmask type at the moment, but this seems to be a pretty good candidate for a type like that. Actually, for the status led I would also use a bitmask since there may be multiple leds. I guess you would need two bitmasks: one to select auto vs manual, and one for the manual settings. So far I've not seen cameras with multiple status leds, I do have seen camera which have the following settings for their 1 led (logitech uvc cams): auto on off blinking So I think a menu type is better suited, and that is what the current (private) uvc control uses. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Replying to myself. On 09/07/2010 11:42 AM, Hans de Goede wrote: Hi, On 09/07/2010 09:30 AM, Hans Verkuil wrote: On Monday, September 06, 2010 20:11:05 Jean-Francois Moine wrote: Hi, This new proposal cancels the previous 'LED control' patch. Cheers. Hi Jean-Francois, You must also add support for these new controls in v4l2-ctrls.c in v4l2_ctrl_get_menu(), v4l2_ctrl_get_name() and v4l2_ctrl_fill(). How is CID_ILLUMINATORS supposed to work in the case of multiple lights? Wouldn't a bitmask type be more suitable to this than a menu type? There isn't a bitmask type at the moment, but this seems to be a pretty good candidate for a type like that. Actually, for the status led I would also use a bitmask since there may be multiple leds. I guess you would need two bitmasks: one to select auto vs manual, and one for the manual settings. So far I've not seen cameras with multiple status leds, I do have seen camera which have the following settings for their 1 led (logitech uvc cams): auto on off blinking So I think a menu type is better suited, and that is what the current (private) uvc control uses. The same argument more or less goes for the CID_ILLIMUNATORS controls. Also given that we currently don't have a bitmask type I think introducing one without a really really good reason is a bad idea as any exiting apps won't know how to deal with it. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi all, On 09/07/2010 11:47 AM, Hans Verkuil wrote: On Tuesday, September 07, 2010 11:44:18 Hans de Goede wrote: Replying to myself. On 09/07/2010 11:42 AM, Hans de Goede wrote: Hi, On 09/07/2010 09:30 AM, Hans Verkuil wrote: On Monday, September 06, 2010 20:11:05 Jean-Francois Moine wrote: Hi, This new proposal cancels the previous 'LED control' patch. Cheers. Hi Jean-Francois, You must also add support for these new controls in v4l2-ctrls.c in v4l2_ctrl_get_menu(), v4l2_ctrl_get_name() and v4l2_ctrl_fill(). How is CID_ILLUMINATORS supposed to work in the case of multiple lights? Wouldn't a bitmask type be more suitable to this than a menu type? There isn't a bitmask type at the moment, but this seems to be a pretty good candidate for a type like that. Actually, for the status led I would also use a bitmask since there may be multiple leds. I guess you would need two bitmasks: one to select auto vs manual, and one for the manual settings. So far I've not seen cameras with multiple status leds, I do have seen camera which have the following settings for their 1 led (logitech uvc cams): auto on off blinking So I think a menu type is better suited, and that is what the current (private) uvc control uses. The same argument more or less goes for the CID_ILLIMUNATORS controls. Also given that we currently don't have a bitmask type I think introducing one without a really really good reason is a bad idea as any exiting apps won't know how to deal with it. But I can guarantee that we will get video devices with multiple leds in the future. So we need to think *now* about how to do this. One simple option is of course to name the controls CID_ILLUMINATOR0 and CID_LED0. That way we can easily add LED1, LED2, etc. later without running into weird inconsistent control names. Naming them LED0 and ILLUMINATOR0 works for me. Note about the illuminator one, if you look at the patch it made the illuminator control a menu with the following options: Both off Top on, Bottom off Top off, Bottom on Both on Which raises the question do we leave this as is, or do we make this 2 boolean controls. I personally would like to vote for keeping it as is, as both lamps illuminate the same substrate in this case, and esp. switching between Top on, Bottom off to Top off, Bottom on in one go is a good feature to have UI wise (iow switch from top to bottom lighting or visa versa. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/07/2010 04:50 PM, Hans Verkuil wrote: On Tuesday, September 07, 2010 13:59:19 Hans de Goede wrote: Hi all, On 09/07/2010 11:47 AM, Hans Verkuil wrote: On Tuesday, September 07, 2010 11:44:18 Hans de Goede wrote: Replying to myself. On 09/07/2010 11:42 AM, Hans de Goede wrote: Hi, On 09/07/2010 09:30 AM, Hans Verkuil wrote: On Monday, September 06, 2010 20:11:05 Jean-Francois Moine wrote: Hi, This new proposal cancels the previous 'LED control' patch. Cheers. Hi Jean-Francois, You must also add support for these new controls in v4l2-ctrls.c in v4l2_ctrl_get_menu(), v4l2_ctrl_get_name() and v4l2_ctrl_fill(). How is CID_ILLUMINATORS supposed to work in the case of multiple lights? Wouldn't a bitmask type be more suitable to this than a menu type? There isn't a bitmask type at the moment, but this seems to be a pretty good candidate for a type like that. Actually, for the status led I would also use a bitmask since there may be multiple leds. I guess you would need two bitmasks: one to select auto vs manual, and one for the manual settings. So far I've not seen cameras with multiple status leds, I do have seen camera which have the following settings for their 1 led (logitech uvc cams): auto on off blinking So I think a menu type is better suited, and that is what the current (private) uvc control uses. The same argument more or less goes for the CID_ILLIMUNATORS controls. Also given that we currently don't have a bitmask type I think introducing one without a really really good reason is a bad idea as any exiting apps won't know how to deal with it. But I can guarantee that we will get video devices with multiple leds in the future. So we need to think *now* about how to do this. One simple option is of course to name the controls CID_ILLUMINATOR0 and CID_LED0. That way we can easily add LED1, LED2, etc. later without running into weird inconsistent control names. Naming them LED0 and ILLUMINATOR0 works for me. Note about the illuminator one, if you look at the patch it made the illuminator control a menu with the following options: Where in the patch? Am I missing something? Both off Top on, Bottom off Top off, Bottom on Both on Which raises the question do we leave this as is, or do we make this 2 boolean controls. I personally would like to vote for keeping it as is, as both lamps illuminate the same substrate in this case, and esp. switching between Top on, Bottom off to Top off, Bottom on in one go is a good feature to have UI wise (iow switch from top to bottom lighting or visa versa. The problem with having one control is that while this makes sense for this particular microscope, it doesn't make sense in general. Actual it does atleast for microscopes in general a substrate under a microscope usually is either illuminated from above or below. Standard controls such as proposed by this patch should have a fixed type Although I agree with that sentiment in general I don't see this as an absolute need, apps should get the type by doing a query ctrl not by assuming they know it based on the cid. And esp. for menu controls this is not true, for example most devices have a light freq filter menu of: off 50 hz 60 hz Which matches what is documented in videodev2.h Where as some have: off 50 hz 60 hz auto hz Because they can (and default to) detect the light frequency automatically. consistent behavior. Note that I am also wondering whether it wouldn't be a good idea to use a menu for this, just as for the LEDs. I do agree that the illuminator ctrls should be a menu, that way the driver author can also choose wether to group 2 together in a single control or not we simply should not specify the menu values in the spec (the same can be said for the led case). Regards, Hams -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi,
On 09/07/2010 05:30 PM, Hans Verkuil wrote:
On Tuesday, September 07, 2010 15:04:55 Hans de Goede wrote:
Hi,
On 09/07/2010 04:50 PM, Hans Verkuil wrote:
Both off
Top on, Bottom off
Top off, Bottom on
Both on
Which raises the question do we leave this as is, or do we make this 2 boolean
controls. I personally would like to vote for keeping it as is, as both lamps
illuminate the same substrate in this case, and esp. switching between
Top on, Bottom off to Top off, Bottom on in one go is a good feature to have
UI wise (iow switch from top to bottom lighting or visa versa.
The problem with having one control is that while this makes sense for this
particular microscope, it doesn't make sense in general.
Actual it does atleast for microscopes in general a substrate under a microscope
usually is either illuminated from above or below.
Standard controls such as proposed by this patch should have a fixed type
Although I agree with that sentiment in general I don't see this as an absolute
need, apps should get the type by doing a query ctrl not by assuming they
know it based on the cid.
And esp. for menu controls this is not true, for example
most devices have a light freq filter menu of:
off
50 hz
60 hz
Which matches what is documented in videodev2.h
Where as some have:
off
50 hz
60 hz
auto hz
Because they can (and default to) detect the light frequency automatically.
The v4l2 api allows drivers to selectively enable items from a menu. So this
control has a fixed menu type and a fixed menu contents. Some of the menu
choices are just not available for some drivers.
This is not possible:
Quoting from:
http://www.linuxtv.org/downloads/v4l-dvb-apis/re61.html
"Menu items are enumerated by calling VIDIOC_QUERYMENU with successive index values
from struct v4l2_queryctrl minimum (0) to maximum, inclusive."
And many apps are coded this way, so we cannot simply skip values in a
menu enum just because a driver does not support them, this would
break apps as they (rightfully) don't expect an error when
calling VIDIOC_QUERYMENU with an index between minimum and
maximum, so given for example:
enum v4l2_led {
V4L2_LED_AUTO = 0,
V4L2_LED_BLINK = 1,
V4L2_LED_OFF = 2,
V4L2_LED_ON = 3,
};
Drivers which do not support blink would still need to report a minimum
and maximum of 0 and 3, making any control apps expect 4 menu items not
3 !
And this example is exactly why I'm arguing against defining standard
meanings for standard controls with a menu type.
Also note that at least with the uvc driver that due to how extension
unit controls are working (the uvcvideo driver gets told about these
vendor specific controls from a userspace helper), the menu index is
the value which gets written to the hardware! So one cannot simply
make this match some random enum.
There are several advantages of sticking to one standard menu for standard
controls:
1) The contents of the menu can be defined centrally in v4l2-ctrls.c, which
ensures consistency. Not only of the menu texts, but also of how the
control behaves in various drivers.
No they cannot as v4l2-ctrls.c will not know when to return -EINVAL to
indicate that in the example case the driver does not support blink, and
moreover an app will not expect this and maybe decide to not show the
menu at all, or ...
2) It makes it possible to set the control directly from within a program.
This is currently true for *all* standard controls
No this is not true, programs still need to know minimum and maximum values
for all integer standard controls, brightness may be 0-15 on one device
and 0-65535 on another, so they cannot simply bang in any value they need to
take into account the query ctrl results.
This looks pretty decent IMHO:
enum v4l2_illuminator {
V4L2_ILLUMINATOR_OFF = 0,
V4L2_ILLUMINATOR_ON = 1,
};
#define V4L2_CID_ILLUMINATOR_0 (V4L2_CID_BASE+37)
#define V4L2_CID_ILLUMINATOR_1 (V4L2_CID_BASE+38)
I don't like this, as explained before most microscopes have a top
and a bottom light, and you want to switch between them, or to
all off, or to all on. So having a menu with 4 options for this
makes a lot more sense then having 2 separate controls. Defining
these values as standard values would take away the option for drivers
to do something other then a simple on / off control here. Again
what is wrong with with not defining standard meanings for standard
controls with a menu type. This means less stuff in videodev2.h
and more flexibility wrt using these control ids.
I think we should not even define a type for this one. If we
get microscopes with pwm control for the lights we will want this
to be an integer using one control per light.
We have this excellent infrastructure to automatically discover
control types, ranges and menu item meaning. Why would it be
forbidden to use this for standard controls.
Either we ne
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/09/2010 08:55 AM, Hans Verkuil wrote: On Tuesday, September 07, 2010 23:14:10 Hans de Goede wrote: How about a compromise, we add a set of standard defines for menu index meanings, with a note that these are present as a way to standardize things between drivers, but that some drivers may deviate and that apps should always use VIDIOC_QUERYMENU ? Let's use boolean for these illuminator controls instead. Problem solved :-) Erm, no. If you take a look at the current qx5 microscope support code in the cpia2 driver it currently is a menu with the following possible values: Off Top Bottom Both So now lets say we create standard controls for illuminators and make them booleans and use 2 booleans. And then modify the cpia2 driver to follow the new standard. The user behavior then goes from: - user things lets switch from top to bottom lighting - go to control - click menu drops down select top / bottom -> easy To: - user things lets switch from top to bottom lighting - go to control - heuh 2 checkboxes ? - click one check box off - clock other check box on -> not easy If I were a user I would call this change a regression, and as such I find the boolean proposal unacceptable. Maybe we should call the control V4L2_CID_MICROSCOPE_ILLUMINATOR To make it more clear that the menu variant of this is meant for microscopes (which typically have either only a bottom illuminator or both a bottom and a top one). And if we then ever need to support some other kind of illuminator we can add a separate cid for that/ Otherwise I think it might be best to just keep this as a private control. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/09/2010 08:55 AM, Peter Korsgaard wrote: "Hans" == Hans Verkuil writes: Hi, >> - the status LED should be controlled by the LED interface. Hans> I originally was in favor of controlling these through v4l as Hans> well, but people made some good arguments against that. The main Hans> one being: why would you want to show these as a control? What is Hans> the end user supposed to do with them? It makes little sense. Hans> Frankly, why would you want to expose LEDs at all? Shouldn't this Hans> be completely hidden by the driver? No generic application will Hans> ever do anything with status LEDs anyway. So it should be the Hans> driver that operates them and in that case the LEDs do not need Hans> to be exposed anywhere. It's not that it *HAS* to be exposed - But if we can, then it's nice to do so as it gives flexibility to the user instead of hardcoding policy in the kernel. Reading this whole thread I have to agree that if we are going to expose camera status LEDs it would be done through the sysfs API. I think this can be done nicely for gspca based drivers (as we can put all the "crud" in the gspca core having to do it only once), but that is a low priority nice to have thingy. This does leave us with the problem of logitech uvc cams where the LED currently is exposed as a v4l2 control. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/09/2010 03:29 PM, Hans Verkuil wrote: Hi, On 09/09/2010 08:55 AM, Peter Korsgaard wrote: "Hans" == Hans Verkuil writes: Hi, >> - the status LED should be controlled by the LED interface. Hans> I originally was in favor of controlling these through v4l as Hans> well, but people made some good arguments against that. The main Hans> one being: why would you want to show these as a control? What is Hans> the end user supposed to do with them? It makes little sense. Hans> Frankly, why would you want to expose LEDs at all? Shouldn't this Hans> be completely hidden by the driver? No generic application will Hans> ever do anything with status LEDs anyway. So it should be the Hans> driver that operates them and in that case the LEDs do not need Hans> to be exposed anywhere. It's not that it *HAS* to be exposed - But if we can, then it's nice to do so as it gives flexibility to the user instead of hardcoding policy in the kernel. Reading this whole thread I have to agree that if we are going to expose camera status LEDs it would be done through the sysfs API. I think this can be done nicely for gspca based drivers (as we can put all the "crud" in the gspca core having to do it only once), but that is a low priority nice to have thingy. This does leave us with the problem of logitech uvc cams where the LED currently is exposed as a v4l2 control. Is it possible for the uvc driver to detect and use a LED control? That's how I would expect this to work, but I know that uvc is a bit of a strange beast. Unfortunately no, some uvc cameras have "proprietary" controls. The uvc driver knows nothing about these but offers an API to map these to v4l2 controls (where userspace tells it the v4l2 cid, type, min, max, etc.). Currently on logitech cameras the userspace tools if installed will map the led control to a private v4l2 menu control with the following options: On Off Auto Blink The cameras default to auto, where the led is turned on when video is being streamed and off when there is no streaming going on. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/09/2010 04:14 PM, Andy Walls wrote: I'm of the mind that independent boolean illuminator controls are Ok. I think that scales better. Not that I could imagine many in use for 1 camera anyway, but some may be colors other than white. Illuminator0 should always correspond to the most common default application of the device. Ok, booleans it is then. JF can you do a new rfc / documentation + videodev2.h patch and then lets get the qx3 light control patch Andy did modified to match and merge it. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Illuminators and status LED controls
Hi, On 09/09/2010 04:41 PM, Andy Walls wrote: Hans de Goede, The uvc API that creates v4l2 ctrls on behalf of userspace could intercept those calls and create an LED interface instead of, or in addition to, the v4l2 ctrl. That would mean special casing certain extension controls which I don't think is something which we want to do. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] gspca_cpia1: Add basic v4l2 illuminator controls for the Intel Play QX3
Ack.
Acked-by: Hans de Goede
On 09/12/2010 03:51 AM, Andy Walls wrote:
gspca_cpia1: Add basic v4l2 illuminator controls for the Intel Play QX3
This patch add basic V4L2 controls for the illuminators on the Intel
Play QX3 microscope.
Signed-off-by: Andy Walls
diff -r 6e0befab696a -r d165649ca8a0 linux/drivers/media/video/gspca/cpia1.c
--- a/linux/drivers/media/video/gspca/cpia1.c Fri Sep 03 00:28:05 2010 -0300
+++ b/linux/drivers/media/video/gspca/cpia1.c Sat Sep 11 14:15:26 2010 -0400
@@ -373,6 +373,10 @@
static int sd_getfreq(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setcomptarget(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getcomptarget(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setilluminator1(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getilluminator1(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setilluminator2(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getilluminator2(struct gspca_dev *gspca_dev, __s32 *val);
static const struct ctrl sd_ctrls[] = {
{
@@ -434,6 +438,34 @@
},
{
{
+ .id = V4L2_CID_ILLUMINATORS_1,
+ .type= V4L2_CTRL_TYPE_BOOLEAN,
+ .name= "Illuminator 1",
+ .minimum = 0,
+ .maximum = 1,
+ .step= 1,
+#define ILLUMINATORS_1_DEF 0
+ .default_value = ILLUMINATORS_1_DEF,
+ },
+ .set = sd_setilluminator1,
+ .get = sd_getilluminator1,
+ },
+ {
+ {
+ .id = V4L2_CID_ILLUMINATORS_2,
+ .type= V4L2_CTRL_TYPE_BOOLEAN,
+ .name= "Illuminator 2",
+ .minimum = 0,
+ .maximum = 1,
+ .step= 1,
+#define ILLUMINATORS_2_DEF 0
+ .default_value = ILLUMINATORS_2_DEF,
+ },
+ .set = sd_setilluminator2,
+ .get = sd_getilluminator2,
+ },
+ {
+ {
#define V4L2_CID_COMP_TARGET V4L2_CID_PRIVATE_BASE
.id = V4L2_CID_COMP_TARGET,
.type= V4L2_CTRL_TYPE_MENU,
@@ -1059,7 +1091,6 @@
0, sd->params.streamStartLine, 0, 0);
}
-#if 0 /* Currently unused */ /* keep */
static int command_setlights(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
@@ -1079,7 +1110,6 @@
return do_command(gspca_dev, CPIA_COMMAND_WriteMCPort, 2, 0,
p1 | p2 | 0xE0, 0);
}
-#endif
static int set_flicker(struct gspca_dev *gspca_dev, int on, int apply)
{
@@ -1932,6 +1962,72 @@
return 0;
}
+static int sd_setilluminator(struct gspca_dev *gspca_dev, __s32 val, int n)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+ int ret;
+
+ if (!sd->params.qx3.qx3_detected)
+ return -EINVAL;
+
+ switch (n) {
+ case 1:
+ sd->params.qx3.bottomlight = val ? 1 : 0;
+ break;
+ case 2:
+ sd->params.qx3.toplight = val ? 1 : 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = command_setlights(gspca_dev);
+ if (ret&& ret != -EINVAL)
+ ret = -EBUSY;
+
+ return ret;
+}
+
+static int sd_setilluminator1(struct gspca_dev *gspca_dev, __s32 val)
+{
+ return sd_setilluminator(gspca_dev, val, 1);
+}
+
+static int sd_setilluminator2(struct gspca_dev *gspca_dev, __s32 val)
+{
+ return sd_setilluminator(gspca_dev, val, 2);
+}
+
+static int sd_getilluminator(struct gspca_dev *gspca_dev, __s32 *val, int n)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ if (!sd->params.qx3.qx3_detected)
+ return -EINVAL;
+
+ switch (n) {
+ case 1:
+ *val = sd->params.qx3.bottomlight;
+ break;
+ case 2:
+ *val = sd->params.qx3.toplight;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int sd_getilluminator1(struct gspca_dev *gspca_dev, __s32 *val)
+{
+ return sd_getilluminator(gspca_dev, val, 1);
+}
+
+static int sd_getilluminator2(struct gspca_dev *gspca_dev, __s32 *val)
+{
+ return sd_getilluminator(gspca_dev, val, 2);
+}
+
static int sd_querymenu(struct gspca_dev *gspca_dev,
struct v4l2_querymenu *menu)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] gspca_cpia1: Disable illuminator controls if not an Intel Play QX3
Hi,
On 09/12/2010 03:51 AM, Andy Walls wrote:
gspca_cpia1: Disable illuminator controls if not an Intel Play QX3
The illuminator controls should only be available to the user for the Intel
Play QX3 microscope.
Signed-off-by: Andy Walls
diff -r d165649ca8a0 -r 32d5c323c541 linux/drivers/media/video/gspca/cpia1.c
--- a/linux/drivers/media/video/gspca/cpia1.c Sat Sep 11 14:15:26 2010 -0400
+++ b/linux/drivers/media/video/gspca/cpia1.c Sat Sep 11 21:15:03 2010 -0400
@@ -1743,6 +1743,22 @@
do_command(gspca_dev, CPIA_COMMAND_GetCameraStatus, 0, 0, 0, 0);
}
+static void sd_disable_qx3_ctrls(struct gspca_dev *gspca_dev)
+{
+ int i, n;
+ __u32 id;
+
+ n = ARRAY_SIZE(sd_ctrls);
+ for (i = 0; i< n; i++) {
+ id = sd_ctrls[i].qctrl.id;
+
+ if (id == V4L2_CID_ILLUMINATORS_1 ||
+ id == V4L2_CID_ILLUMINATORS_2) {
+ gspca_dev->ctrl_dis |= (1<< i);
+ }
+ }
+}
+
/* this function is called at probe and resume time */
static int sd_init(struct gspca_dev *gspca_dev)
{
Hmm, this deviates from how all other gspca subdrivers do this, they
define indexes for ctrls together with the sd_ctrls intializer and
then use these, so instead of the above blurb there would be
a
#define ILLUMINATORS_1_IDX x
#define ILLUMINATORS_2_IDX x
Where these ctrls get "defined" (see for example ov519.c)
And then:
+ if (!sd->params.qx3.qx3_detected)
+ sd_disable_qx3_ctrls(gspca_dev);
+
Would become:
if (!sd->params.qx3.qx3_detected)
gspca_dev->ctrl_dis |= (1 << ILLUMINATORS_1_IDX) |
(1 << ILLUMINATORS_2_IDX);
I think it would be good to use the same construction in the cpia1
driver for consistency between all the gspca subdrivers.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] gspca_cpia1: Restore QX3 illuminators' state on resume
Hi, On 09/12/2010 03:51 AM, Andy Walls wrote: gspca_cpia1: Restore QX3 illuminators' state on resume Turn the lights of the QX3 on (or off) as needed when resuming and at module load. Signed-off-by: Andy Walls diff -r 32d5c323c541 -r c2e7fb2d768e linux/drivers/media/video/gspca/cpia1.c --- a/linux/drivers/media/video/gspca/cpia1.c Sat Sep 11 21:15:03 2010 -0400 +++ b/linux/drivers/media/video/gspca/cpia1.c Sat Sep 11 21:32:35 2010 -0400 @@ -1772,6 +1772,10 @@ if (ret) return ret; + /* Ensure the QX3 illuminators' states are restored upon resume */ + if (sd->params.qx3.qx3_detected) + command_setlights(gspca_dev); + sd_stopN(gspca_dev); if (!sd->params.qx3.qx3_detected) Notice the: if (sd->params.qx3.qx3_detected) command_setlights(gspca_dev); sd_stopN(gspca_dev); if (!sd->params.qx3.qx3_detected) Given that at least the order of execution of the second if statement does not matter wrt to the sd_stopN(gspca_dev), can we please make this: if (sd->params.qx3.qx3_detected) command_setlights(gspca_dev); else sd_stopN(gspca_dev); Thanks, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] gspca_cpia1: Restore QX3 illuminators' state on resume
Ack! Acked-by: Hans de Goede On 09/12/2010 07:45 PM, Andy Walls wrote: Turn the lights of the QX3 on (or off) as needed when resuming and at module load. Signed-off-by: Andy Walls diff -r f09faf8dd85d -r 5e576066eeaf linux/drivers/media/video/gspca/cpia1.c --- a/linux/drivers/media/video/gspca/cpia1.c Sun Sep 12 12:43:45 2010 -0400 +++ b/linux/drivers/media/video/gspca/cpia1.c Sun Sep 12 12:47:00 2010 -0400 @@ -1756,6 +1756,10 @@ if (ret) return ret; + /* Ensure the QX3 illuminators' states are restored upon resume */ + if (sd->params.qx3.qx3_detected) + command_setlights(gspca_dev); + sd_stopN(gspca_dev); PDEBUG(D_PROBE, "CPIA Version: %d.%02d (%d.%d)", -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] gspca_cpia1: Disable illuminator controls if not an Intel Play QX3
Ack,
Acked-by: Hans de Goede
p.s.
Jean-Francois, since your tree also has the needed videodev2.h changes I assume
you'll take these patches in your tree and thus I won't add them to mine.
Regards,
Hans
On 09/12/2010 07:45 PM, Andy Walls wrote:
The illuminator controls should only be available to the user for the Intel
Play QX3 microscope. The implementation to inhibit the controls is intended to
be consistent with the other gspca driver implementations.
Signed-off-by: Andy Walls
diff -r 5e576066eeaf -r 8a9732bd1548 linux/drivers/media/video/gspca/cpia1.c
--- a/linux/drivers/media/video/gspca/cpia1.c Sun Sep 12 12:47:00 2010 -0400
+++ b/linux/drivers/media/video/gspca/cpia1.c Sun Sep 12 13:13:33 2010 -0400
@@ -380,6 +380,7 @@
static const struct ctrl sd_ctrls[] = {
{
+#define BRIGHTNESS_IDX 0
{
.id = V4L2_CID_BRIGHTNESS,
.type= V4L2_CTRL_TYPE_INTEGER,
@@ -394,6 +395,7 @@
.set = sd_setbrightness,
.get = sd_getbrightness,
},
+#define CONTRAST_IDX 1
{
{
.id = V4L2_CID_CONTRAST,
@@ -408,6 +410,7 @@
.set = sd_setcontrast,
.get = sd_getcontrast,
},
+#define SATURATION_IDX 2
{
{
.id = V4L2_CID_SATURATION,
@@ -422,6 +425,7 @@
.set = sd_setsaturation,
.get = sd_getsaturation,
},
+#define POWER_LINE_FREQUENCY_IDX 3
{
{
.id = V4L2_CID_POWER_LINE_FREQUENCY,
@@ -436,6 +440,7 @@
.set = sd_setfreq,
.get = sd_getfreq,
},
+#define ILLUMINATORS_1_IDX 4
{
{
.id = V4L2_CID_ILLUMINATORS_1,
@@ -450,6 +455,7 @@
.set = sd_setilluminator1,
.get = sd_getilluminator1,
},
+#define ILLUMINATORS_2_IDX 5
{
{
.id = V4L2_CID_ILLUMINATORS_2,
@@ -464,6 +470,7 @@
.set = sd_setilluminator2,
.get = sd_getilluminator2,
},
+#define COMP_TARGET_IDX 6
{
{
#define V4L2_CID_COMP_TARGET V4L2_CID_PRIVATE_BASE
@@ -1756,9 +1763,13 @@
if (ret)
return ret;
- /* Ensure the QX3 illuminators' states are restored upon resume */
+ /* Ensure the QX3 illuminators' states are restored upon resume,
+ or disable the illuminator controls, if this isn't a QX3 */
if (sd->params.qx3.qx3_detected)
command_setlights(gspca_dev);
+ else
+ gspca_dev->ctrl_dis |=
+ ((1<< ILLUMINATORS_1_IDX) | (1<< ILLUMINATORS_2_IDX));
sd_stopN(gspca_dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Move ivtv utilities and ivtv X video extension to v4l-utils
Hi, On 09/17/2010 12:58 AM, Andy Walls wrote: Hi Hans and Hans, I'd like to move the source code maintained here: http://ivtvdriver.org/svn/ to someplace where it may be less likely to suffer bit rot. I was hoping the v4l-utils git repo would be an appropriate place. Do either of you have any opinions on this? Moving this to v4l-utils.git is fine with me, assuming it is ok with the owner of the code too (H. Verkuil I think ?). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libv4l conversion problem
Hi, On 10/14/2010 01:16 PM, Gary Thomas wrote: Hans, Please forgive the direct email; try as I might, I could not find any other vehicle to discuss this (feel free to steer me to the proper place). There indeed is a lack of a mailinglist or forum for v4l-utils. This has been discussed before and it was decided that given the low amount of discussion around v4l-utils we will just use the linux-media mailing list for this (added to the CC). I'm working with the latest code (0.8.1) on an embedded ARM system which has a camera that can only deliver UYVY422 data. Ok, so when you say UYVY422, I assume that this is packed data, right, so not some planar format, right? libv4l supports converting UYVY422 packed data to: RGB24 BGR24 YUV420 (planar) YVU420 (planar) The problem I have is that most everything else, e.g. I'm trying to run cheese, wants YUYV422. cheese specifically should be happy with almost any YUV or RGB format as it uses gstreamer. I know for a fact that it works happily with libv4l's YUV420 (planar) output. Should the library be able to handle this case (device only does UYUV and application wants YUYV)? It does not support converting to packed yuv formats, but it does support conversion to planar yuv formats. At least for cheese this should work fine. Any suggestions how I move forward? Make sure that your gstreamer is compiled with libv4l support. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l-utils: libv4l1: When asked for RGB, return RGB and not BGR
Hi, NACK The byte ordering in v4l1's VIDEO_PALETTE_RGB24 was never really clear, but the kernel v4l1 compatibility ioctl handling has been mapping VIDEO_PALETTE_RGB24 <-> V4L2_PIX_FMT_BGR24 for ever and many v4l1 apps actually expect VIDEO_PALETTE_RGB24 to be BGR24. The only one I know of to get this wrong is camorama and the solution there is to: 1) not use camorama 2) if you use camorama anyway, fix it, there is a list of patches fixing various issues available here: http://pkgs.fedoraproject.org/gitweb/?p=camorama.git;a=tree Regards, Hans On 10/18/2010 02:44 PM, Marc Deslauriers wrote: libv4l1: When asked for RGB, return RGB and not BGR Signed-off-by: Marc Deslauriers --- lib/libv4l1/libv4l1.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/libv4l1/libv4l1.c b/lib/libv4l1/libv4l1.c index cb53899..202f020 100644 --- a/lib/libv4l1/libv4l1.c +++ b/lib/libv4l1/libv4l1.c @@ -87,9 +87,9 @@ static unsigned int palette_to_pixelformat(unsigned int palette) case VIDEO_PALETTE_RGB565: return V4L2_PIX_FMT_RGB565; case VIDEO_PALETTE_RGB24: - return V4L2_PIX_FMT_BGR24; + return V4L2_PIX_FMT_RGB24; case VIDEO_PALETTE_RGB32: - return V4L2_PIX_FMT_BGR32; + return V4L2_PIX_FMT_RGB32; case VIDEO_PALETTE_YUYV: return V4L2_PIX_FMT_YUYV; case VIDEO_PALETTE_YUV422: @@ -118,9 +118,9 @@ static unsigned int pixelformat_to_palette(unsigned int pixelformat) return VIDEO_PALETTE_RGB555; case V4L2_PIX_FMT_RGB565: return VIDEO_PALETTE_RGB565; - case V4L2_PIX_FMT_BGR24: + case V4L2_PIX_FMT_RGB24: return VIDEO_PALETTE_RGB24; - case V4L2_PIX_FMT_BGR32: + case V4L2_PIX_FMT_RGB32: return VIDEO_PALETTE_RGB32; case V4L2_PIX_FMT_YUYV: return VIDEO_PALETTE_YUYV; -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC, PATCH] gspca pac7302: add support for camera button
Hi,
Thanks for working on this! I think it would be great if we could
get support for camera buttons in general into gspca.
I've not looked closely at your code yet, have you looked at
the camera button code in the gspca sn9c20x.c driver? Also I would really
like to see as much of the button handling code as possible go into
the gspca core. AFAIK many many camera's use an usb interrupt ep for this, so
I would like to see the setting up and cleanup of this interrupt ep be in
the core (as said before see the sn9c20x driver for another driver which
does such things).
Regards,
Hans
On 11/15/2009 09:47 AM, Németh Márton wrote:
From: Márton Németh
Add support for snapshot button found on Labtec Webcam 2200.
Signed-off-by: Márton Németh
---
Hi,
this is the first trial to add support for the snapshot button. This
code is working only before the streaming is started. When the streaming
is started the alternate number of the interface 0 is changed and the
interrupt URB is no longer usable. I guess the interrupt URB is to
be reconstructed every time when the alternate number is changed.
When I disconnect the device I get the following error:
uhci_hcd :00:1d.1: dma_pool_free buffer-32, f58ba168/358ba168 (bad dma)
I guess something is wrong in this patch with the cleanup routine.
Regards,
Márton Németh
---
diff -r 09c1284de47d linux/drivers/media/video/gspca/gspca.h
--- a/linux/drivers/media/video/gspca/gspca.h Sat Nov 14 08:58:12 2009 +0100
+++ b/linux/drivers/media/video/gspca/gspca.h Sun Nov 15 10:40:54 2009 +0100
@@ -138,6 +138,7 @@
struct module *module; /* subdriver handling the device */
struct usb_device *dev;
struct file *capt_file; /* file doing video capture */
+ struct input_dev *input_dev;
struct cam cam; /* device information */
const struct sd_desc *sd_desc; /* subdriver description */
@@ -147,6 +148,7 @@
#define USB_BUF_SZ 64
__u8 *usb_buf; /* buffer for USB exchanges */
struct urb *urb[MAX_NURBS];
+ struct urb *int_urb;
__u8 *frbuf;/* buffer for nframes */
struct gspca_frame frame[GSPCA_MAX_FRAMES];
diff -r 09c1284de47d linux/drivers/media/video/gspca/pac7302.c
--- a/linux/drivers/media/video/gspca/pac7302.c Sat Nov 14 08:58:12 2009 +0100
+++ b/linux/drivers/media/video/gspca/pac7302.c Sun Nov 15 10:40:54 2009 +0100
@@ -68,6 +68,7 @@
#define MODULE_NAME "pac7302"
+#include
#include
#include "gspca.h"
@@ -1220,6 +1221,50 @@
}
#endif
+#if LINUX_VERSION_CODE< KERNEL_VERSION(2, 6, 19)
+static void int_irq(struct urb *urb, struct pt_regs *regs)
+#else
+static void int_irq(struct urb *urb)
+#endif
+{
+ struct gspca_dev *gspca_dev = (struct gspca_dev *) urb->context;
+ int ret;
+ int i;
+ __u8 data0, data1;
+
+ printk(KERN_DEBUG "int_irq()\n");
+ printk(KERN_DEBUG "urb->status: %i\n", urb->status);
+ if (urb->status == 0) {
+ printk(KERN_DEBUG "urb->actual_length: %u\n",
urb->actual_length);
+ for (i = 0; i< urb->actual_length; i++) {
+ printk(KERN_DEBUG "urb->transfer_buffer[%i]=0x%x\n",
+ i, ((__u8*)urb->transfer_buffer)[i]);
+ }
+ if (urb->actual_length == 2) {
+ data0 = ((__u8*)urb->transfer_buffer)[0];
+ data1 = ((__u8*)urb->transfer_buffer)[1];
+ if ((data0 == 0x00&& data1 == 0x11) ||
+ (data0 == 0x22&& data1 == 0x33) ||
+ (data0 == 0x44&& data1 == 0x55) ||
+ (data0 == 0x66&& data1 == 0x77) ||
+ (data0 == 0x88&& data1 == 0x99) ||
+ (data0 == 0xaa&& data1 == 0xbb) ||
+ (data0 == 0xcc&& data1 == 0xdd) ||
+ (data0 == 0xee&& data1 == 0xff)) {
+ input_report_key(gspca_dev->input_dev,
KEY_CAMERA, 1);
+ input_sync(gspca_dev->input_dev);
+ input_report_key(gspca_dev->input_dev,
KEY_CAMERA, 0);
+ input_sync(gspca_dev->input_dev);
+ } else
+ printk(KERN_DEBUG "Unknown packet received\n");
+ }
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+ printk(KERN_DEBUG "resubmit urb: %i\n", ret);
+ }
+
+}
+
+
/* sub-driver description for pac7302 */
static struct sd_desc sd_desc = {
.name = MODULE_NAME,
@@ -1254,19 +1299,132 @@
};
MODULE_DEVICE_TABLE(usb, device_table);
+static int init_camera_input(struct gspca_dev *gspca_dev, const struct
usb_device_id *id)
+{
+ struct input_dev *input_dev;
+ int err;
+
+ printk(KERN_DEBUG "allocating input
Re: [RFC, PATCH] gspca pac7302: add support for camera button
Hi, On 11/16/2009 07:58 AM, Németh Márton wrote: Hi, Hans de Goede wrote: Hi, Thanks for working on this! I think it would be great if we could get support for camera buttons in general into gspca. I've not looked closely at your code yet, have you looked at the camera button code in the gspca sn9c20x.c driver? Also I would really As you proposed I had a look on sn9c20x. It seems that sn9c20x uses register read via USB control message. The pac7302 uses interrupt endpoint. So it looks like quite different to me. Currently I see the common point in the connection to input subsystem only. Ah you are right, oops, most camera's use an interrupt end point so I assumed sn9c20x would be the same, my bad. like to see as much of the button handling code as possible go into the gspca core. AFAIK many many camera's use an usb interrupt ep for this, so I would like to see the setting up and cleanup of this interrupt ep be in the core (as said before see the sn9c20x driver for another driver which does such things). Unfortunately I do not know how the USB descriptors of other webcams look like. I have access to two webcams which are handled by gspca: No problem, just put all the input code in pac7302.c for now, we will abstract it later when we add support for the button on other camera's too. Comparing these two endpoints shows the common and different points: Common: interface class, endpoint direction, endpoint type. Different: interface number, sub class, protocol, endpoint address, max packet size, interval. Maybe the second example is not a good one because I don't know whether the interrupt endpoint is used for buttons or not. Do you have access to webcams equipped with button? Could you please send the device descriptor (lsusb -v) about these devices in order the common points can be identified for interrupt endpoints? As the author/maintainer of quite a few drivers and libv4l author I have build up quite a test camera collection, I'll send you the lsusb -v output of a few in a private mail. But as said before, for now I think you can just put the input code inside pac7302.c, then later on we can try to abstract it. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
