[FFmpeg-devel] [PATCH] v4l2: read the correct time per frame from devices that support a standard
Giorgio Vazzana
mywing81 at gmail.com
Wed Jan 30 15:11:14 CET 2013
2013/1/30 Stefano Sabatini <stefasab at gmail.com>:
> On date Monday 2013-01-28 18:09:30 +0100, Giorgio Vazzana encoded:
>> 2013/1/25 Stefano Sabatini <stefasab at gmail.com>:
> [...]
>> >> + 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.
Oh sorry, of course, I misread. Fixed.
> [...]
>> 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.
Not yet, but after this patch is applied I think I know what causes bug #1570.
>>
>> [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_(
Fixed.
>> + 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)?
No, it's not fixed: when you call VIDIOC_ENUMINPUT you get input.std
which is a set of all supported standards (the supported standards are
OR-ed together). This means that a Composite input can support, for
example, both PAL and NTSC, and you can select any of the supported
standards for that input. Moreover, in ffmpeg the user specifies the
desired standard by name, so enumerating all the standards until you
find one that matches is necessary.
>> + if (ret < 0) {
>> + ret = errno;
>> + av_log(s1, AV_LOG_ERROR, "Unknown standard '%s'\n", s->standard);
>
> Nit: unknown or unsupported ...
Changed.
>> + 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?
Because, the std_id is not an index into the standard enumeration, it
is again a set of all supported standards OR-ed together (why a set
and not only one standard you might ask... because some cards can not
distinguish, for example, between PAL-B and PAL-G and can switch
automatically between them). So you need to get the standard set
std_id (which is of type v4l2_std_id) and then find the match in the
enumeration: at the point you have the time per frame.
All this, with several examples, is explained at
http://linuxtv.org/downloads/v4l-dvb-apis/standard.html
>
>> + 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
Ok, it can be done later.
Allright, new patch attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavd-v4l2-read-the-correct-time-per-frame-from-devic.patch
Type: application/octet-stream
Size: 7431 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130130/45490206/attachment.obj>
More information about the ffmpeg-devel
mailing list