[FFmpeg-devel] [PATCH] Add support to video4linux2 framerate auto-set.

[Stefano Sabatini]

On date Wednesday 2011-01-05 12:47:55 +0100, Luca Abeni encoded:
> On 01/05/2011 12:26 PM, Stefano Sabatini wrote:
> >On date Wednesday 2011-01-05 08:46:05 +0100, Luca Abeni encoded:
> >>Hi Stefano,
> >>
> >>On 01/04/2011 09:52 PM, Stefano Sabatini wrote:
> >>>In particular, fix ffmpeg grabbing timestamps when the timebase value
> >>>is not set through the CLI.
> >>
> >>The idea looks good, but, can you move the frame rate autodetection to
> >>v4l2_set_parameters(), after the call to VIDIOC_S_PARM? (more precisely,
> >>after the "if (ap->time_base.num&&  ap->time_base.den)" block)
> >>Does it work, with such a change? (I think so, but, can you verify?)
> >
> >This would require another change:
> >if (ap->time_base.num&&  ap->time_base.den) {
> >    // set the timebase in the driver
> >}
> >if (!ap->time_base.num || !ap->time_base.den) {
> >    // auto-select timebase and set it in the driver
> 
> Sorry, but why the "set" part? AFAIK, VIDIOC_G_PARM returns the current
> frame period, and VIDIOC_S_PARM sets it. So, there is no point in reading
> a value and then setting it.
> 
> My point is that we want to read the current frame period only if we did
> not set it through VIDIOC_S_PARM.
> So, I'd do
> 	if (ap->time_base.num&&  ap->time_base.den) {
> 		// set the timebase in the driver
> 	} else {
> 		// read the current frame period, and use it as a timebase
> 	}
> 
> BTW, I am wondering if VIDIOC_G_PARM is the right ioctl, here, or if the
> frame rate should be detected by looking at the video standard description.
> 
> >I put the framerate auto-select code just after the size auto-select
> >code, as that looked more symmetrical and sounded more consistent with
> >the overall design.
> >
> >As for v4l2_set_parameters(), I suppose it should be used for setting
> >parameters, so the auto-selection phase should be already done when
> >the function is called. Currently the logic implemented is:
> >
> >v4l2_read_header()
> >   1. check the size and frame rate values, if unset (i.e. if they are
> >      set to 0) then use the auto-select feature of the v4l driver for
> >      selecting the values to set
> >
> >v4l2_set_parameters()
> >   2. set the size and frame rate values (they need to be non-0 values)
> 
> v4l2_set_parameters() can set the video standard, which can change the
> video size and frame rate... So, it should be done before autodetecting
> the values. And, yes, looking at the code this seems to be currently
> broken (regarding the frame size).
> 
> I suppose we should first call v4l2_set_parameters(), and then do the
> rest of the stuff...

Got it, check the attached patches.
-- 
FFmpeg = Fiendish and Fast Meaningful Ponderous Efficient Gadget