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

[Oleksij Rempel <linux@rempel-privat.de>]

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:
> 
> 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 ?
> 
>> 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.
> 

Hello Peter, Laurent,

Peter do have time to update this patches for latest kernel? Laurent
would you accept them?

-- 
Regards,
Oleksij