[FFmpeg-devel] [PATCH] v4l2: Old or cheap cams will _not_ accept parms query or modification. ioctl(VIDIOC_G_PARM) or ioctl(VIDIOC_S_PARM)

[Luca Abeni]

Hi all,

On 02/23/2012 05:45 AM, Michael Niedermayer wrote:
> On Wed, Feb 22, 2012 at 09:51:35PM -0300, Lucas F. Ottaviano wrote:
>> Hello! I think you should not ignore this, if you want ffmpeg to be as
>> universally accessible as possible...
>> It's a shame that because of this simplicity some users will not be able to
>> use ffmpeg or ffplay.
>>
>> + Some old or cheap cams will _not_ accept to set or get paramaters,
>>     so returning an error when this is not possible is not good.
>
> There are some problems in the patch
> simply ignoring the error would end up with a random timebase

In particular, here is a mail I previously sent about this issue (I hope
I am not misunderstanding something; please correct me if I misunderstood
some details of the patch).

I'll reply later with more details (Michael reminded me that ffmpeg could
in theory cope with unknown/default timebases, but there could be issues...
Moreover, right now I am less inclined to mark as "broken" a driver that
does not support VIDIOC_G_PARM - we should probably fall back to VIDIOC_G_STD
to read the frame period, if VIDIOC_G_PARM fails)


				Luca

-----------8<-------------8<-----------------8<------------------
[...]
While I think I understand the need for it (supporting
broken drivers), I do not think that decreasing the verbosity from
ERROR to WARNING is a good idea: to me, a broken driver is still an
error... Moreover, if the ioctl() fails ffmpeg risks to go on with a
completely unreliable frame rate.

Also, I do not think that both the VIDIOC_S_PARM and VIDIOC_G_PARM
codepaths need to be modified: IMHO a broken driver can be supported
by asking the user not to set the frame rate (so, the "return AVERROR(EIO)"
would need to be removed only in the VIDIOC_G_PARM case), or by asking
the user to set the correct frame rate by hand (so, the "return AVERROR(EIO)"
would need to be removed only in the VIDIOC_S_PARM case).

I do not remember if ffmpeg can cope with input devices that do not
set the codec time_base... If yes, I'd simply modify the VIDIOC_G_PARM
codepath to cope with a failing ioctl() (by resetting "tpf" to a sane
value or to - unspecified if such an option is possible). And I would
leave the other codepath unmodified (maybe changing the error message
to "ioctl set time per frame(%d/%d) failed - please do not set a frame rate").
With this patch, I suspect s1->streams[0]->codec->time_base risks to be
set to a random value.

If, instead, ffmpeg wants the input device to set a time_base, then
I would change the VIDIOC_S_PARM codepath (again, the code must be
modified to cope with a failing ioctl(): if the ioctl() fails, tpf
must be reset to the framerate_q values). In this case, I would leave
the other codepath unchanged (adding a "please set a frame rate" to
its error message).