[FFmpeg-devel] [PATCH] v4l2: read the correct time per frame from devices that support a standard
Stefano Sabatini
stefasab at gmail.com
Wed Jan 30 00:09:46 CET 2013
On date Monday 2013-01-28 18:09:30 +0100, Giorgio Vazzana encoded:
> 2013/1/25 Stefano Sabatini <stefasab at gmail.com>:
[...]
> >> + /* 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
>
> Ok, changed.
>
> >> + 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
>
> Hmm, if the user does not request a standard on the command line, then
> we need to get the current standard, and enumerate the supported
> standards one by one until one matches. When we find a match, we can
> retrieve the time per frame. So, it should not be redundant.
My command was about the line "standard.index++;".
Since you set standard.index = i there's little point into assigning
it at the end of the loop.
[...]
> New patch attached.
> From 6e582c0699e6ced4715aaa19726c276940db48ea Mon Sep 17 00:00:00 2001
> From: Giorgio Vazzana <mywing81 at gmail.com>
> Date: Mon, 28 Jan 2013 17:57:52 +0100
> Subject: [PATCH] lavd/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.
BTW tell if this is fixing a known ticket.
>
> [1] http://linuxtv.org/downloads/v4l-dvb-apis/standard.html
> ---
> libavdevice/v4l2.c | 119 ++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 74 insertions(+), 45 deletions(-)
>
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index 5f01027..879201f 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -658,12 +658,10 @@ static int v4l2_set_parameters(AVFormatContext *s1)
> struct video_data *s = s1->priv_data;
> 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 };
> 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",
> @@ -672,57 +670,88 @@ 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 (s->std_id) {
> + av_log(s1, AV_LOG_DEBUG, "Setting standard: %s\n", s->standard);
> + /* set tv standard */
> + for(i = 0; ; i++) {
nit++: for_(
> + standard.index = i;
> + ret = v4l2_ioctl(s->fd, VIDIOC_ENUMSTD, &standard);
> + if (ret < 0 || !av_strcasecmp(standard.name, s->standard))
> + break;
> + }
Isn't the standard associated to an input fixed? Does it still make
sense to enumerate all the standards when you already get a std_id (or
can that be changed)?
> + if (ret < 0) {
> + ret = errno;
> + av_log(s1, AV_LOG_ERROR, "Unknown standard '%s'\n", s->standard);
Nit: unknown or unsupported ...
> + return AVERROR(ret);
> + }
> +
> + if (v4l2_ioctl(s->fd, VIDIOC_S_STD, &standard.id) < 0) {
> + ret = errno;
> + av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_S_STD): %s\n", strerror(errno));
> + return AVERROR(ret);
> + }
> + } else {
> + av_log(s1, AV_LOG_WARNING,
> + "This device does not support any standard\n");
> + }
> + }
> +
> + /* get standard */
> + if (v4l2_ioctl(s->fd, VIDIOC_G_STD, &s->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) {
> + ret = errno;
> + av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_ENUMSTD): %s\n", strerror(errno));
> + return AVERROR(ret);
> + }
Same here, couldn't you just set standard.index to the set standard
id? Why do you need to enumerate the standards again?
> + if (standard.id == s->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++;
> }
> - if (ret < 0) {
> - av_log(s1, AV_LOG_ERROR, "Unknown standard '%s'\n", s->standard);
> - return ret;
> - }
> + } else {
> + 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) {
> + ret = errno;
> + av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_G_PARM): %s\n", strerror(errno));
> + return AVERROR(ret);
> }
Note (unrelated): this could be replaced by a macro once and for all
[...]
--
FFmpeg = Fantastic and Fucking Minimalistic Philosophical Elastic Geek
More information about the ffmpeg-devel
mailing list