Re: [linux-uvc-devel] [PATCH 0/2] uvcvideo: Support Logitech RGB Bayer formats

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 07.01.2013 23:45, schrieb Laurent Pinchart:
Hi Oleksij,

(CC'ing linux-media)

On Wednesday 02 January 2013 11:50:51 Oleksij Rempel wrote:
Am 02.01.2013 10:03, schrieb Peter Ross:
On Tue, Jan 01, 2013 at 05:16:44PM +0100, Oleksij Rempel wrote:
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.xml

Using 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.

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.

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 ?

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.


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 over
uvc_xu_control_query interface or some other way.

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.

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 :)
--
Regards,
Oleksij
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux