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

[Stefano Sabatini]

On date Thursday 2013-01-24 16:58:59 +0100, Giorgio Vazzana encoded:
> Hi,
> 
> I'm working at fixing some problems with v4l2 and ffmpeg (such as
> ticket #1570) so here's my
> first patch (more to come!). Please comment.
> 
> Regards,
> Giorgio Vazzana

> From 9b7b74abe15aac6a5d686f65e426b6e8172e9f19 Mon Sep 17 00:00:00 2001
> From: Giorgio Vazzana <mywing81 at gmail.com>
> Date: Thu, 24 Jan 2013 16:51:40 +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 |   27 +++++++++++++++++----------
>  1 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index 4ac6967..fdabe62 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -671,22 +671,13 @@ 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);
>      }

maybe use:
        av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_ENUMINPUT): %s\n", strerror(errno)

like in the other places for consistency. Also you should return
AVERROR(errno) in order to provide more info to the user.

>  
> -    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);
> -    }
> -
>      if (s->standard) {
>          av_log(s1, AV_LOG_DEBUG, "The V4L2 driver set standard: %s\n",
>                 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 V4L2 input_id: %d\n", s->channel);
> +    if (v4l2_ioctl(s->fd, VIDIOC_S_INPUT, &s->channel) < 0) {
> +        av_log(s1, AV_LOG_ERROR, "The V4L2 driver ioctl set input failed\n");
> +        return AVERROR(EIO);
> +    }

same: unrelated but returning AVERROR(errno) is favored

> +
> +    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);
> +    }

ditto about return error code

> +    av_log(s1, AV_LOG_DEBUG, "The V4L2 driver set input_id: %d, input: %s\n",
> +            s->channel, input.name);
> +

Nit: all this "V4L2 driver" stuff is redundant and a bit annoying,
especially in DEBUG log messages, I suggest something along the line:
input_channel:%d input_name:%s

[...]
-- 
FFmpeg = Fantastic and Fancy Mythic Portable Ephemeral Guru