Re: [PATCH RFC v2 7/7] v4l: define own IOCTL ops for SDR FMT

[Hans Verkuil <hverkuil@xs4all.nl>]

On 12/15/2013 12:23 PM, Mauro Carvalho Chehab wrote:
> Em Sat, 14 Dec 2013 18:24:37 +0200
> Antti Palosaari <crope@iki.fi> escreveu:
> 
>> Hello, Mauro, Hans,
>>
>> On 14.12.2013 18:15, Antti Palosaari wrote:
>>> Use own format ops for SDR data:
>>> vidioc_enum_fmt_sdr_cap
>>> vidioc_g_fmt_sdr_cap
>>> vidioc_s_fmt_sdr_cap
>>> vidioc_try_fmt_sdr_cap
>>
>> To be honest, I am a little bit against that patch. Is there any good 
>> reason we duplicate these FMT ops every-time when new stream format is 
>> added? For my eyes that is mostly just bloating the code without good 
>> reason.
> 
> The is one reason: when the same device can be used in both SDR and non
> SDR mode (radio, video, vbi), then either the driver or the core would
> need to select the right set of vidioc_*fmt_* ops.
> 
> In the past, all drivers had about the same logic for such tests.
> Yet, as the implementations weren't the same, several of them were
> implementing it wrong.
> 
> So, we ended by moving those validations to the core.

I do think there is room for improvement here, though. Rather than
passing v4l2_format to the ops I would have preferred passing the appropriate
struct of the union instead.

And I never really liked it that try and set were split up. A 'try' boolean
would reduce the number of ops.

The first improvement is something that can be done at some point. It's too
late (and probably not worth it) to do anything about the second.

Regards,

	Hans

PS: Antti, I'll review the code in more detail tomorrow.