Re: libv4l2 misbehavior after calling S_STD or S_DV_PRESET

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/07/2011 11:06 AM, Hans Verkuil wrote:
> On Friday 07 October 2011 09:57:42 Hans de Goede wrote:
>> Hi,
>>
>> Hmm, nasty...
>>
>> On 10/06/2011 01:13 PM, Hans Verkuil wrote:
>>> Hi Hans!
>>>
>>> I've been looking into a problem with libv4l2 that occurs when you change
>>> TV standard or video preset using VIDIOC_S_STD or VIDIOC_S_DV_PRESET.
>>> These calls will change the format implicitly (e.g. if the current
>>> format is set for PAL at 720x576 and you select NTSC, then the format
>>> will be reset to 720x480).
>>>
>>> However, libv4l2 isn't taking this into account and will keep using the
>>> cached dest_fmt value. It is easy to reproduce this using qv4l2.
>>>
>>> The same problem is likely to occur with S_CROP (haven't tested that yet,
>>> though): calling S_CROP can also change the format.
>>>
>>> To be precise: S_STD and S_DV_PRESET can change both the crop rectangle
>>> and the format, and S_CROP can change the format.
>>
>> First of all it would be good to actually document this behavior of
>> VIDIOC_S_STD or VIDIOC_S_DV_PRESET, the current docs don't mention this at
>> all: http://linuxtv.org/downloads/v4l-dvb-apis/standard.html
>
> Odd, I'd have sworn that it was in the docs.
>
> The full list of ioctls that can change both the crop settings and the format
> is:
>
> VIDIOC_S_STD
> VIDIOC_S_DV_PRESET

The patch already handles these 2 :)

> VIDIOC_S_DV_TIMINGS
> VIDIOC_S_INPUT (can implicitly change standard/preset)

I'll add these 2.

> VIDIOC_S_OUTPUT (ditto)

libv4l2 only cares about capture, all the rest it just passes
through completely unmodified.

> VIDIOC_S_CROP

Hmm, can this also change the fmt? That would be an unexpected
side effect. But if it does I can handle it the same way, right?

> Note that I suspect that there are quite a few drivers that do not handle this
> correctly. After all, for normal SDTV capture cards you almost never change
> the TV standard once it is set up at the start so I doubt if this has been
> tested much. For DV_PRESET it is much more common to switch from e.g.
> 720p to 1080p. That is how I found this issue.
>
>> I've attached 2 patches which should make libv4l2 deal with this correctly.
>> I assume you've a reproducer for this and I would appreciate it if you
>> could test if these patches actually fix the issue you are seeing.
>
> Almost working. The second patch forgot to set src_fmt.type, so I got an error
> back. After initializing it to BUF_TYPE_VIDEO_CAPTURE it worked fine.

Thanks for testing, and duh wrt my G_FMT mistake. If you can answer my question
about S_CROP then I'll go add the other ioctls which need similar handling to
my 2nd patch and push both patches.

Regards,

Hans