[FFmpeg-devel] [EXT] Re: [PATCH v4 1/3] avcodec/v4l2_context: don't reinit output queue when dynamic resolution change

[Ming Qian]

> -----Original Message-----
> From: Andriy Gelman [mailto:andriy.gelman at gmail.com]
> Sent: Monday, January 3, 2022 12:24 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Cc: Ming Qian <ming.qian at nxp.com>
> Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 1/3] avcodec/v4l2_context: don't
> reinit output queue when dynamic resolution change
> 
> Caution: EXT Email
> 
> On Thu, 19. Aug 16:55, Ming Qian wrote:
> > in the v4l2 stateful video document, we can see the following
> > description:
> >     During the resolution change sequence, the OUTPUT queue must
> remain
> >     streaming. Calling VIDIOC_STREAMOFF() on the OUTPUT queue would
> >     abort the sequence and initiate a seek.
> >
> >     In principle, the OUTPUT queue operates separately from the CAPTURE
> >     queue and this remains true for the duration of the entire
> >     resolution change sequence as well.
> >
> > so don't reinit the output queue when handling the resolution change
> > event
> >
> > Signed-off-by: Ming Qian <ming.qian at nxp.com>
> > ---
> >  libavcodec/v4l2_context.c | 27 ++-------------------------
> >  1 file changed, 2 insertions(+), 25 deletions(-)
> >
> > diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> > index ff1ea8e57b08..dda5157698c3 100644
> > --- a/libavcodec/v4l2_context.c
> > +++ b/libavcodec/v4l2_context.c
> > @@ -162,9 +162,8 @@ static int v4l2_handle_event(V4L2Context *ctx)  {
> >      V4L2m2mContext *s = ctx_to_m2mctx(ctx);
> >      struct v4l2_format cap_fmt = s->capture.format;
> > -    struct v4l2_format out_fmt = s->output.format;
> >      struct v4l2_event evt = { 0 };
> > -    int full_reinit, reinit, ret;
> > +    int reinit, ret;
> >
> >      ret = ioctl(s->fd, VIDIOC_DQEVENT, &evt);
> >      if (ret < 0) {
> > @@ -180,25 +179,12 @@ static int v4l2_handle_event(V4L2Context *ctx)
> >      if (evt.type != V4L2_EVENT_SOURCE_CHANGE)
> >          return 0;
> >
> > -    ret = ioctl(s->fd, VIDIOC_G_FMT, &out_fmt);
> > -    if (ret) {
> > -        av_log(logger(ctx), AV_LOG_ERROR, "%s VIDIOC_G_FMT\n",
> s->output.name);
> > -        return 0;
> > -    }
> > -
> >      ret = ioctl(s->fd, VIDIOC_G_FMT, &cap_fmt);
> >      if (ret) {
> >          av_log(logger(ctx), AV_LOG_ERROR, "%s VIDIOC_G_FMT\n",
> s->capture.name);
> >          return 0;
> >      }
> >
> > -    full_reinit = v4l2_resolution_changed(&s->output, &out_fmt);
> > -    if (full_reinit) {
> > -        s->output.height = v4l2_get_height(&out_fmt);
> > -        s->output.width = v4l2_get_width(&out_fmt);
> > -        s->output.sample_aspect_ratio = v4l2_get_sar(&s->output);
> > -    }
> > -
> >      reinit = v4l2_resolution_changed(&s->capture, &cap_fmt);
> >      if (reinit) {
> >          s->capture.height = v4l2_get_height(&cap_fmt);
> > @@ -206,18 +192,9 @@ static int v4l2_handle_event(V4L2Context *ctx)
> >          s->capture.sample_aspect_ratio = v4l2_get_sar(&s->capture);
> >      }
> >
> > -    if (full_reinit || reinit)
> > +    if (reinit)
> >          s->reinit = 1;
> >
> > -    if (full_reinit) {
> 
> > -        ret = ff_v4l2_m2m_codec_full_reinit(s);
> 
> This is the only use of the function ff_v4l2_m2m_codec_full_reinit(). An option
> is to remove the function in the commit. I can see how this private
> function could be useful in the future though.. Do we then remove the function
> and add
> it back in the future or just let it be?
> 
> Suggestion on IRC was to remove, but I also want to check on ML.

Hi Andriy,
  I think we can just keep it, if you really need to remove it, I think we can submit a single patch that remove it.

> 
> > -        if (ret) {
> > -            av_log(logger(ctx), AV_LOG_ERROR,
> "v4l2_m2m_codec_full_reinit\n");
> > -            return AVERROR(EINVAL);
> > -        }
> > -        goto reinit_run;
> > -    }
> > -
> >      if (reinit) {
> >          if (s->avctx)
> >              ret = ff_set_dimensions(s->avctx, s->capture.width,
> s->capture.height);
> 
> Otherwise, the patch looks good to me.
> Sorry for the delay.
> 
> Thanks,
> --
> Andriy