uvcvideo: Support Logitech RGB Bayer formats
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): 20130102090346.GA2243@b9d1a1848204a80b27c5d574b483f38a
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Oleksij,
(CC'ing linux-media)
Quoted 28 lines · On Wednesday 02 January 2013 11:50:51 Oleksij Rempel wrote:
Am 02.01.2013 10:03, schrieb Peter Ross:
Quoted 27 lines · Oleksij Rempel
Quoted 5 lines
Hi Peter,
thank you for your work, but most of it belongs to uvcdynctrl. You will need to add it here:
/usr/share/uvcdynctrl/data/046d/logitech.xmlUsing uvcdynctrl is problematic for two reasons.
1. Setting the Logitech 'disable video processing' and 'raw data bpp' controls independently from uvcvideo causes the driver to *always* drop frames.
uvc_video_decode_isoc()performs a sanity check on the size of incoming uncompressed frames. It expects to receive frame sizes reported by the 8-bit yuyv 4:2:2 format description. The check fails when video processing is disabled, because the 8-/10-bit RGB Bayer frame sizes are always smaller.2. Userspace applications need to distinguish the yuyv 4:2:2 format from the the additional RGB Bayer formats.
Currently, when video processing is disabled, user applications expect to receive yuyv 4:2:2 pixels, because thats what it and uvcvideo has agreed to (mediate by V4L2). Instead the application receives RGB Bayer pixels and attempts to process them as yuyv resulting in visual garbage.
One could argue that an application that explicitly disables processing would be able to handle that. However, I agree that point 1 is problematic, and point 2 is definitely not clean.
Quoted 8 lines
Quoted 7 lines
Some of this has been discussed previously, see
<http://article.gmane.org/gmane.linux.drivers.uvc.devel/3061/>.The patch tries to solve both of these problems: 1) making uvcvideo aware of the Logitech controls and recalculating the expected frame sizes, and
2) presenting the RGB Bayer formats through the V4L2 interface, so user application can request them.
Quoted 4 lines
If, the problem seems to be bigger.
We have:
1. Formats provided by usb descriptor and controlled over uvc protocol.
2. Formats provided by usb descriptor and controlled over XU
Do we really have devices that expose formats through the standard UVC descriptors but require XU access to select them ?
Quoted 9 lines
3. Formats controlled over XU and provided by documentation other kind of knowledge.
Last case in not properly handled by uvcvideo.ko, but IMHO it is not good way to have all possible XU in driver.
Both problems you described should be fixed, but not in this way. If it is possible - uvcdynctl should provide/update format descriptor overuvc_xu_control_queryinterface or some other way.
Quoted 6 lines
Beside, i was working on kernel space plugin system for uvcvideo driver, which was probably not the best idea. How about to do some part of it in userspace? For example, uvcdynctl can provide bandwidth information too. This way we can fix many buggy cams without needing to permanently update kernel driver.
Laurent?
I'm really undecided here.
On one hand I agree with you, from a theoretical point of view the driver should not know about all possible XUs. This just can't scale.
On the other hand, it's pretty clear that we don't need to scale. We only have a single public dynamic controls XML file, and looking at the descriptors of most devices it's pretty clear that they reuse the same XUs, as they are based on only a dozen or so different chips.
The dynamic controls API brings additional complexity to the driver, and I think that it was a good design decision. However, in some cases, the implications of changing an XU control value go well beyond what is usually expected of a control. I'm thus tempted to allow handling of XU controls in the kernel when there's a good reason to do so. Of course I want the code to be clean, without lots of hooks all over the place that would make the driver sources impossible to read and understand :-)
Comments and thoughts will be appreciated.
Signature · 3 lines
Regards, Laurent Pinchart
Am 07.01.2013 23:45, schrieb Laurent Pinchart:
Quoted 46 lines
Hi Oleksij,
(CC'ing linux-media)
Quoted 28 lines · On Wednesday 02 January 2013 11:50:51 Oleksij Rempel wrote:
Am 02.01.2013 10:03, schrieb Peter Ross:
Quoted 27 lines · Oleksij Rempel
Quoted 5 lines
Hi Peter,
thank you for your work, but most of it belongs to uvcdynctrl. You will need to add it here:
/usr/share/uvcdynctrl/data/046d/logitech.xmlUsing uvcdynctrl is problematic for two reasons.
1. Setting the Logitech 'disable video processing' and 'raw data bpp' controls independently from uvcvideo causes the driver to *always* drop frames.
uvc_video_decode_isoc()performs a sanity check on the size of incoming uncompressed frames. It expects to receive frame sizes reported by the 8-bit yuyv 4:2:2 format description. The check fails when video processing is disabled, because the 8-/10-bit RGB Bayer frame sizes are always smaller.2. Userspace applications need to distinguish the yuyv 4:2:2 format from the the additional RGB Bayer formats.
Currently, when video processing is disabled, user applications expect to receive yuyv 4:2:2 pixels, because thats what it and uvcvideo has agreed to (mediate by V4L2). Instead the application receives RGB Bayer pixels and attempts to process them as yuyv resulting in visual garbage.
One could argue that an application that explicitly disables processing would be able to handle that. However, I agree that point 1 is problematic, and point 2 is definitely not clean.
Quoted 8 lines
Quoted 7 lines
Some of this has been discussed previously, see
<http://article.gmane.org/gmane.linux.drivers.uvc.devel/3061/>.The patch tries to solve both of these problems: 1) making uvcvideo aware of the Logitech controls and recalculating the expected frame sizes, and
2) presenting the RGB Bayer formats through the V4L2 interface, so user application can request them.
Quoted 7 lines
Quoted 4 lines
If, the problem seems to be bigger.
We have:
1. Formats provided by usb descriptor and controlled over uvc protocol.
2. Formats provided by usb descriptor and controlled over XUDo we really have devices that expose formats through the standard UVC descriptors but require XU access to select them ?
h264? Can be started over uvc, but mostly use less without XU. I'll bet, if you will accept this XUs, some body will ask to add XU for h264.
Quoted 35 lines
Quoted 10 lines
3. Formats controlled over XU and provided by documentation other kind of knowledge.
Last case in not properly handled by uvcvideo.ko, but IMHO it is not good way to have all possible XU in driver.
Both problems you described should be fixed, but not in this way. If it is possible - uvcdynctl should provide/update format descriptor overuvc_xu_control_queryinterface or some other way.Quoted 6 lines
Beside, i was working on kernel space plugin system for uvcvideo driver, which was probably not the best idea. How about to do some part of it in userspace? For example, uvcdynctl can provide bandwidth information too. This way we can fix many buggy cams without needing to permanently update kernel driver.
Laurent?I'm really undecided here.
On one hand I agree with you, from a theoretical point of view the driver should not know about all possible XUs. This just can't scale.
On the other hand, it's pretty clear that we don't need to scale. We only have a single public dynamic controls XML file, and looking at the descriptors of most devices it's pretty clear that they reuse the same XUs, as they are based on only a dozen or so different chips.
The dynamic controls API brings additional complexity to the driver, and I think that it was a good design decision. However, in some cases, the implications of changing an XU control value go well beyond what is usually expected of a control. I'm thus tempted to allow handling of XU controls in the kernel when there's a good reason to do so. Of course I want the code to be clean, without lots of hooks all over the place that would make the driver sources impossible to read and understand :-)
Quoted 1 line
Comments and thoughts will be appreciated.
I had my doubts in that because:
- we don't know if same XU was used for some thing different in ... older version of some hardware.
- we can't guarantee anything here. This XUs are not documented and not a part of sail strategy. If some thing will not work, or will brake because of it - only we will be responsible.
- In this case we will not have any win in case of bandwidth[1]. To enable this stream we should ask YUV and webcam wil allocate bandwidth for it.
- but all this is just FUD. I always happy to see, if some body goes beyond it and revers undocumented XUs and brings patches to make use of it for all :) Peter thx!
[1] - bandwidth seems to be number one in our "top 3" of always coming problems :)
Signature · 2 lines
Regards, Oleksij
Am 07.01.2013 23:45, schrieb Laurent Pinchart:
Quoted 62 lines
Hi Oleksij,
(CC'ing linux-media)
Quoted 2 lines · On Wednesday 02 January 2013 11:50:51 Oleksij Rempel wrote:
Am 02.01.2013 10:03, schrieb Peter Ross:
Quoted 1 line · Oleksij Rempel
One could argue that an application that explicitly disables processing would be able to handle that. However, I agree that point 1 is problematic, and point 2 is definitely not clean.
Quoted 8 lines
Quoted 7 lines
Some of this has been discussed previously, see
<http://article.gmane.org/gmane.linux.drivers.uvc.devel/3061/>.The patch tries to solve both of these problems: 1) making uvcvideo aware of the Logitech controls and recalculating the expected frame sizes, and
2) presenting the RGB Bayer formats through the V4L2 interface, so user application can request them.Quoted 4 lines
If, the problem seems to be bigger.
We have:
1. Formats provided by usb descriptor and controlled over uvc protocol.
2. Formats provided by usb descriptor and controlled over XUDo we really have devices that expose formats through the standard UVC descriptors but require XU access to select them ?
Quoted 9 lines
3. Formats controlled over XU and provided by documentation other kind of knowledge.
Last case in not properly handled by uvcvideo.ko, but IMHO it is not good way to have all possible XU in driver.
Both problems you described should be fixed, but not in this way. If it is possible - uvcdynctl should provide/update format descriptor overuvc_xu_control_queryinterface or some other way.Quoted 6 lines
Beside, i was working on kernel space plugin system for uvcvideo driver, which was probably not the best idea. How about to do some part of it in userspace? For example, uvcdynctl can provide bandwidth information too. This way we can fix many buggy cams without needing to permanently update kernel driver.
Laurent?I'm really undecided here.
On one hand I agree with you, from a theoretical point of view the driver should not know about all possible XUs. This just can't scale.
On the other hand, it's pretty clear that we don't need to scale. We only have a single public dynamic controls XML file, and looking at the descriptors of most devices it's pretty clear that they reuse the same XUs, as they are based on only a dozen or so different chips.
The dynamic controls API brings additional complexity to the driver, and I think that it was a good design decision. However, in some cases, the implications of changing an XU control value go well beyond what is usually expected of a control. I'm thus tempted to allow handling of XU controls in the kernel when there's a good reason to do so. Of course I want the code to be clean, without lots of hooks all over the place that would make the driver sources impossible to read and understand :-)
Quoted 2 lines
Comments and thoughts will be appreciated.
Hello Peter, Laurent,
Peter do have time to update this patches for latest kernel? Laurent would you accept them?
Signature · 2 lines
Regards, Oleksij