[FFmpeg-devel] [PATCH] v4l2: read the correct time per frame from devices that support a standard

Stefano Sabatini stefasab at gmail.com
Fri Jan 25 18:21:41 CET 2013


On date Thursday 2013-01-24 22:09:45 +0100, Giorgio Vazzana encoded:
> Hi,
> 
> this is the second patch I had in my queue. Please review.
> 
> Giorgio Vazzana

> From a3b03f9013b24a92b9f92ce9ded9fed78b8dfaca Mon Sep 17 00:00:00 2001
> From: Giorgio Vazzana <mywing81 at gmail.com>
> Date: Thu, 24 Jan 2013 21:54:45 +0100
> Subject: [PATCH] v4l2: read the correct time per frame from devices that support a standard
> 
> Generally speaking, there are two types of v4l2 devices [1]:
> 
> 1) devices that support a standard, like PAL or NTFS (tv cards, for example). For
> this class of devices the framerate is fixed by the standard (for example PAL uses
> 25 fps) and the v4l2 driver cannot usually negotiate a different framerate (unless
> it can skip frames on the driver side, to save I/O bandwidth).
> 
> 2) devices for which the notion of standard does not make sense (webcams, for example).
> For these devices it is usually possibile to request a desidered framerate.
> 
> In either case, the desidered frame rate can be requested when the VIDIOC_G_PARM
> ioctl returns the V4L2_CAP_TIMEPERFRAME flag in the capability field.
> 
> Currently the code does not check for V4L2_CAP_TIMEPERFRAME and supports only the
> second category of devices, returning a time per frame of 0/0 for devices in the
> first group that do not permit to negotiate the framerate.
> 
> This patch add support to read the correct framerate in all cases.
> 
> [1] http://linuxtv.org/downloads/v4l-dvb-apis/standard.html

Bravo! This is what I call a good commit message :-).

> ---
>  libavdevice/v4l2.c |  117 ++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 72 insertions(+), 45 deletions(-)
> 
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index fdabe62..27a03af 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -658,12 +658,11 @@ static int v4l2_set_parameters(AVFormatContext *s1)
>      struct v4l2_input input = { 0 };
>      struct v4l2_standard standard = { 0 };
>      struct v4l2_streamparm streamparm = { 0 };
> -    struct v4l2_fract *tpf = &streamparm.parm.capture.timeperframe;
> +    struct v4l2_fract *tpf;
>      AVRational framerate_q = { 0 };
> +    v4l2_std_id std_id;
>      int i, ret;
>  
> -    streamparm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -
>      if (s->framerate &&
>          (ret = av_parse_video_rate(&framerate_q, s->framerate)) < 0) {
>          av_log(s1, AV_LOG_ERROR, "Could not parse framerate '%s'.\n",
> @@ -679,57 +678,85 @@ static int v4l2_set_parameters(AVFormatContext *s1)
>      }
>  
>      if (s->standard) {
> -        av_log(s1, AV_LOG_DEBUG, "The V4L2 driver set standard: %s\n",
> -               s->standard);
> -        /* set tv standard */
> -        for(i=0;;i++) {
> +        if (input.std) {
> +            av_log(s1, AV_LOG_DEBUG, "Setting V4L2 driver standard: %s\n",
> +                   s->standard);
> +            /* set tv standard */
> +            for(i = 0; ; i++) {
> +                standard.index = i;
> +                ret = v4l2_ioctl(s->fd, VIDIOC_ENUMSTD, &standard);

v4l2_ioctl() returns -1 in case of error, to get a meaningful error
code (which is propagated to the caller function), you need to do:
ret = AVERROR(errno);
just after the call.

> +                if (ret < 0 || !av_strcasecmp(standard.name, s->standard))
> +                    break;
> +            }
> +            if (ret < 0) {
> +                av_log(s1, AV_LOG_ERROR, "Unknown standard '%s'\n", s->standard);
> +                return ret;
> +            }
> +

> +            if (v4l2_ioctl(s->fd, VIDIOC_S_STD, &standard.id) < 0) {
> +                av_log(s1, AV_LOG_ERROR, "The V4L2 driver ioctl set standard failed\n");
> +                return AVERROR(EIO);

Here you can return AVERROR(errno) to provide a more meaningful error
code (you should also set ret before av_log(), or alternatively:

v4l2_ioctl(s->fd, VIDIOC_S_STD, &standard.id);
if ((ret = AVERROR(errno)) < 0) {
...
return ret;
}

> +            }
> +        } else {
> +            av_log(s1, AV_LOG_WARNING,
> +                   "The V4L2 driver does not allow to change standard\n");

Not sure if we should rather fail here, leave as is if in doubt.

> +        }
> +    }
> +
> +    /* get standard */
> +    if (v4l2_ioctl(s->fd, VIDIOC_G_STD, &std_id) == 0) {
> +        tpf = &standard.frameperiod;
> +        for (i = 0; ; i++) {
>              standard.index = i;
>              ret = v4l2_ioctl(s->fd, VIDIOC_ENUMSTD, &standard);
> -            if (ret < 0 || !av_strcasecmp(standard.name, s->standard))
> +            if (ret < 0) {
> +                av_log(s1, AV_LOG_ERROR, "The V4L2 driver ioctl enum standard failed\n");
> +                return ret;
> +            }

ditto for errno

> +            if (standard.id == std_id) {
> +                av_log(s1, AV_LOG_DEBUG,
> +                       "Current standard: %s, id: %"PRIu64", frameperiod: %d/%d\n",
> +                       standard.name, standard.id, tpf->numerator, tpf->denominator);
>                  break;
> +            }

> +            standard.index++;

seems redundant

>          }
> -        if (ret < 0) {
> -            av_log(s1, AV_LOG_ERROR, "Unknown standard '%s'\n", s->standard);
> -            return ret;
> -        }
> +    } else {
> +        std_id = 0;
> +        tpf = &streamparm.parm.capture.timeperframe;
> +    }
>  
> -        av_log(s1, AV_LOG_DEBUG,
> -               "The V4L2 driver set standard: %s, id: %"PRIu64"\n",
> -               s->standard, (uint64_t)standard.id);
> -        if (v4l2_ioctl(s->fd, VIDIOC_S_STD, &standard.id) < 0) {
> -            av_log(s1, AV_LOG_ERROR,
> -                   "The V4L2 driver ioctl set standard(%s) failed\n",
> -                   s->standard);
> -            return AVERROR(EIO);
> -        }

> +    streamparm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +    if (v4l2_ioctl(s->fd, VIDIOC_G_PARM, &streamparm) != 0) {
> +        av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_G_PARM): %s\n", strerror(errno));
> +        return AVERROR(errno);

usual considerations on errno

>      }
>  
>      if (framerate_q.num && framerate_q.den) {
> -        av_log(s1, AV_LOG_DEBUG, "Setting time per frame to %d/%d\n",
> -               framerate_q.den, framerate_q.num);
> -        tpf->numerator   = framerate_q.den;
> -        tpf->denominator = framerate_q.num;
> -
> -        if (v4l2_ioctl(s->fd, VIDIOC_S_PARM, &streamparm) != 0) {
> -            av_log(s1, AV_LOG_ERROR,
> -                   "ioctl set time per frame(%d/%d) failed\n",
> +        if (streamparm.parm.capture.capability & V4L2_CAP_TIMEPERFRAME) {
> +            tpf = &streamparm.parm.capture.timeperframe;
> +
> +            av_log(s1, AV_LOG_DEBUG, "Setting time per frame to %d/%d\n",
>                     framerate_q.den, framerate_q.num);
> -            return AVERROR(EIO);
> -        }
> +            tpf->numerator   = framerate_q.den;
> +            tpf->denominator = framerate_q.num;
>  
> -        if (framerate_q.num != tpf->denominator ||
> -            framerate_q.den != tpf->numerator) {
> -            av_log(s1, AV_LOG_INFO,
> -                   "The driver changed the time per frame from "
> -                   "%d/%d to %d/%d\n",
> -                   framerate_q.den, framerate_q.num,
> -                   tpf->numerator, tpf->denominator);
> -        }
> -    } else {
> -        if (v4l2_ioctl(s->fd, VIDIOC_G_PARM, &streamparm) != 0) {
> -            av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_G_PARM): %s\n",
> -                   strerror(errno));
> -            return AVERROR(errno);

> +            if (v4l2_ioctl(s->fd, VIDIOC_S_PARM, &streamparm) != 0) {
> +                av_log(s1, AV_LOG_ERROR, "ioctl set time per frame failed\n");
> +                return AVERROR(EIO);
> +            }

ditto about error code (alternatively return always EIO and we'll fix
it later globally)

[...]
-- 
FFmpeg = Fantastic and Fiendish Murdering Puristic Everlasting Generator


More information about the ffmpeg-devel mailing list