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

[Stefano Sabatini]

On date Friday 2013-01-25 19:49:52 +0100, Giorgio Vazzana encoded:
> 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);
> }

yes (I assumed ioctl was setting errno = 0 in case of success)

> > 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.

+1
 
> > 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.

Whatever you think it's simpler.

[...]
-- 
FFmpeg = Formidable and Fanciful Marvellous Portable Enhancing Guide