[FFmpeg-devel] [PATCH] v4l2: Select input immediately after opening the device

[Giorgio Vazzana]

2013/1/25 Stefano Sabatini <stefasab at gmail.com>:
>> From e4434e5bbec6728b04c6c51dc06a63ee3de09a6e Mon Sep 17 00:00:00 2001
>> From: Giorgio Vazzana <mywing81 at gmail.com>
>> Date: Fri, 25 Jan 2013 13:01:29 +0100
>> Subject: [PATCH] v4l2: Select input immediately after opening the device
>>
>> After opening the device, the first thing we should do is selecting the input. This is
>> because the image formats (VIDIOC_ENUM_FMT ioctl) and the standards (VIDIOC_ENUMSTD ioctl)
>> supported may depend on the selected input ([1] and [2]).
>>
>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-enum-fmt.html
>> [2] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-enumstd.html
>> ---
>>  libavdevice/v4l2.c |   31 +++++++++++++++++++------------
>>  1 files changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
>> index 4ac6967..df904d2 100644
>> --- a/libavdevice/v4l2.c
>> +++ b/libavdevice/v4l2.c
>> @@ -671,20 +671,11 @@ static int v4l2_set_parameters(AVFormatContext *s1)
>>          return ret;
>>      }
>>
>> -    /* set tv video input */
>> +    /* enum tv video input */
>>      input.index = s->channel;
>>      if (v4l2_ioctl(s->fd, VIDIOC_ENUMINPUT, &input) < 0) {
>> -        av_log(s1, AV_LOG_ERROR, "The V4L2 driver ioctl enum input failed:\n");
>> -        return AVERROR(EIO);
>> -    }
>> -
>> -    av_log(s1, AV_LOG_DEBUG, "The V4L2 driver set input_id: %d, input: %s\n",
>> -            s->channel, input.name);
>> -    if (v4l2_ioctl(s->fd, VIDIOC_S_INPUT, &input.index) < 0) {
>> -        av_log(s1, AV_LOG_ERROR,
>> -               "The V4L2 driver ioctl set input(%d) failed\n",
>> -                s->channel);
>> -        return AVERROR(EIO);
>> +        av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_ENUMINPUT): %s\n", strerror(errno));
>> +        return AVERROR(errno);
>
> av_log() may change errno as a side effect. Thus a safer approach:

Oh, right.

> v4l2_ioctl(s->fd, VIDIOC_S_INPUT, &input.index);
> ret = AVERROR(errno);
> if (ret < 0) {
> ...
> }

Hmm, this way we are not checking if the ioctl fails or not (if it
does not then errno is undefined)... maybe you meant:

input.index = s->channel;
if (v4l2_ioctl(s->fd, VIDIOC_ENUMINPUT, &input) < 0) {
    ret = errno;
    av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_ENUMINPUT): %s\n", strerror(errno));
    return AVERROR(ret);
}

> BTW strerror() is not thread safe and we should rather use
> av_err2str(), but this is an unrelated issue.
>
> Note: or you can keep the current error reporting logic in order to
> minimize the diff (keeping the first v4l2_ioctl(ENUMINPUT) as is) and
> we will fix it later.

I'd say let's change as little as possible to minimize the diff and
fix it later.

> Also: given that you're doing ENUMINPUT and S_INPUT in
> v4l2_read_header(), isn't this ioclt(ENUMINPUT) redundant here?

Because we need input.std: if it is 0 then the device does not support
any standard. We can save that value inside the struct video_data if
you agree, and call ioclt(ENUMINPUT) only once.

>>      }
>>
>>      if (s->standard) {
>> @@ -792,6 +783,7 @@ static int v4l2_read_header(AVFormatContext *s1)
>>      uint32_t desired_format;
>>      enum AVCodecID codec_id = AV_CODEC_ID_NONE;
>>      enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE;
>> +    struct v4l2_input input = { 0 };
>>
>>      st = avformat_new_stream(s1, NULL);
>>      if (!st)
>> @@ -801,6 +793,21 @@ static int v4l2_read_header(AVFormatContext *s1)
>>      if (s->fd < 0)
>>          return s->fd;
>>
>> +    /* set tv video input */
>> +    av_log(s1, AV_LOG_DEBUG, "Selecting input_channel: %d\n", s->channel);
>> +    if (v4l2_ioctl(s->fd, VIDIOC_S_INPUT, &s->channel) < 0) {
>> +        av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_S_INPUT): %s\n", strerror(errno));
>> +        return AVERROR(errno);
>> +    }
>
> ditto about error code issues

I will change it in next patch.

>> +
>> +    input.index = s->channel;
>> +    if (v4l2_ioctl(s->fd, VIDIOC_ENUMINPUT, &input) < 0) {
>> +        av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_ENUMINPUT): %s\n", strerror(errno));
>> +        return AVERROR(errno);
>> +    }
>> +    av_log(s1, AV_LOG_DEBUG, "input_channel: %d, input_name: %s\n",
>> +           s->channel, input.name);
>
> ditto

This one too.

> (or you can simply move the code from one place to another, as you prefer).