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

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

[Andriy Gelman]

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

> -        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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [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@gmail.com]
> Sent: Monday, January 3, 2022 12:24 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: Ming Qian <ming.qian@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@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Andriy Gelman]

On Thu, 19. Aug 16:55, Ming Qian wrote:
> client need to resume the decoding process
> after it dequeues the source change event.
> no matter what's the return value of v4l2_resolution_changed().
> if the client doesn't resume the decoding process,
> the decoder may keep waiting
> 
> in documentation of v4l2 stateful decoder, we can see the following
> description:
> 	The client must continue the sequence as described below to
> 	continue the decoding process.
> 	1.  Dequeue the source change event.
> 		Important
> 		A source change triggers an implicit decoder drain,
> 		similar to the explicit Drain sequence. The decoder is
> 		stopped after it completes. The decoding process must be
> 		resumed with either a pair of calls to
> 		VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on the CAPTURE
> 		queue, or a call to VIDIOC_DECODER_CMD() with the
> 		V4L2_DEC_CMD_START command.
> 	2.  Continue with the Capture Setup sequence.

Please also add that this fixes decoding of
https://streams.videolan.org/ffmpeg/incoming/720p60.mp4 on RPi4.

> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
>  libavcodec/v4l2_context.c | 52 ++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index dda5157698c3..b08f0015c2e5 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -153,6 +153,21 @@ static inline void v4l2_save_to_context(V4L2Context* ctx, struct v4l2_format_upd
>      }
>  }
>  
> +static int v4l2_start_decode(V4L2Context *ctx)
> +{
> +    struct v4l2_decoder_cmd cmd = {
> +        .cmd = V4L2_DEC_CMD_START,
> +        .flags = 0,
> +    };
> +    int ret;
> +
> +    ret = ioctl(ctx_to_m2mctx(ctx)->fd, VIDIOC_DECODER_CMD, &cmd);
> +    if (ret)
> +        return AVERROR(errno);
> +
> +    return 0;
> +}
> +
>  /**
>   * handle resolution change event and end of stream event
>   * returns 1 if reinit was successful, negative if it failed
> @@ -163,7 +178,7 @@ static int v4l2_handle_event(V4L2Context *ctx)
>      V4L2m2mContext *s = ctx_to_m2mctx(ctx);
>      struct v4l2_format cap_fmt = s->capture.format;
>      struct v4l2_event evt = { 0 };
> -    int reinit, ret;
> +    int ret;
>  
>      ret = ioctl(s->fd, VIDIOC_DQEVENT, &evt);
>      if (ret < 0) {
> @@ -185,35 +200,29 @@ static int v4l2_handle_event(V4L2Context *ctx)
>          return 0;
>      }
>  
> -    reinit = v4l2_resolution_changed(&s->capture, &cap_fmt);
> -    if (reinit) {
> +    if (v4l2_resolution_changed(&s->capture, &cap_fmt)) {
>          s->capture.height = v4l2_get_height(&cap_fmt);
>          s->capture.width = v4l2_get_width(&cap_fmt);
>          s->capture.sample_aspect_ratio = v4l2_get_sar(&s->capture);

> +    } else {
> +        v4l2_start_decode(ctx);
> +        return 0;
>      }

You can minimize the diff just by adding this part and the
definition for v4l2_start_decode(). Then have a separate commit that cleans up
and removes the reinit variable.

>  
> -    if (reinit)
> -        s->reinit = 1;
> +    s->reinit = 1;
>  
> -    if (reinit) {
> -        if (s->avctx)
> -            ret = ff_set_dimensions(s->avctx, s->capture.width, s->capture.height);
> -        if (ret < 0)
> -            av_log(logger(ctx), AV_LOG_WARNING, "update avcodec height and width\n");
> +    if (s->avctx)
> +        ret = ff_set_dimensions(s->avctx, s->capture.width, s->capture.height);
> +    if (ret < 0)
> +        av_log(logger(ctx), AV_LOG_WARNING, "update avcodec height and width\n");
>  
> -        ret = ff_v4l2_m2m_codec_reinit(s);
> -        if (ret) {
> -            av_log(logger(ctx), AV_LOG_ERROR, "v4l2_m2m_codec_reinit\n");
> -            return AVERROR(EINVAL);
> -        }
> -        goto reinit_run;
> +    ret = ff_v4l2_m2m_codec_reinit(s);
> +    if (ret) {
> +        av_log(logger(ctx), AV_LOG_ERROR, "v4l2_m2m_codec_reinit\n");
> +        return AVERROR(EINVAL);
>      }
>  
> -    /* dummy event received */
> -    return 0;
> -
>      /* reinit executed */
> -reinit_run:
>      return 1;
>  }
>  
> @@ -551,6 +560,9 @@ int ff_v4l2_context_set_status(V4L2Context* ctx, uint32_t cmd)
>      int type = ctx->type;
>      int ret;

>  
> +    if (ctx->streamon == (cmd == VIDIOC_STREAMON))
> +        return 0;
> +

This change looks unrelated.

>      ret = ioctl(ctx_to_m2mctx(ctx)->fd, cmd, &type);
>      if (ret < 0)
>          return AVERROR(errno);

Thanks,
-- 
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Ming Qian]

> -----Original Message-----
> From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> Sent: Monday, January 3, 2022 12:41 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: Ming Qian <ming.qian@nxp.com>
> Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> resume the decoding process after source change event received.
> 
> Caution: EXT Email
> 
> On Thu, 19. Aug 16:55, Ming Qian wrote:
> > client need to resume the decoding process after it dequeues the
> > source change event.
> > no matter what's the return value of v4l2_resolution_changed().
> > if the client doesn't resume the decoding process, the decoder may
> > keep waiting
> >
> > in documentation of v4l2 stateful decoder, we can see the following
> > description:
> >       The client must continue the sequence as described below to
> >       continue the decoding process.
> >       1.  Dequeue the source change event.
> >               Important
> >               A source change triggers an implicit decoder drain,
> >               similar to the explicit Drain sequence. The decoder is
> >               stopped after it completes. The decoding process must be
> >               resumed with either a pair of calls to
> >               VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on the
> CAPTURE
> >               queue, or a call to VIDIOC_DECODER_CMD() with the
> >               V4L2_DEC_CMD_START command.
> >       2.  Continue with the Capture Setup sequence.
> 
> Please also add that this fixes decoding of
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstreams
> .videolan.org%2Fffmpeg%2Fincoming%2F720p60.mp4&amp;data=04%7C01%
> 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e
> a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=X4rKQX19MQg1gO3ILiBCQ
> qSLIvqovZLA95KKiyoVNzI%3D&amp;reserved=0 on RPi4.
> 

Hi Andriy,
    What's wrong with this stream? Everything is normal on my side when I play it using ffplay.

> >
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> >  libavcodec/v4l2_context.c | 52
> > ++++++++++++++++++++++++---------------
> >  1 file changed, 32 insertions(+), 20 deletions(-)
> >
> > diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> > index dda5157698c3..b08f0015c2e5 100644
> > --- a/libavcodec/v4l2_context.c
> > +++ b/libavcodec/v4l2_context.c
> > @@ -153,6 +153,21 @@ static inline void
> v4l2_save_to_context(V4L2Context* ctx, struct v4l2_format_upd
> >      }
> >  }
> >
> > +static int v4l2_start_decode(V4L2Context *ctx) {
> > +    struct v4l2_decoder_cmd cmd = {
> > +        .cmd = V4L2_DEC_CMD_START,
> > +        .flags = 0,
> > +    };
> > +    int ret;
> > +
> > +    ret = ioctl(ctx_to_m2mctx(ctx)->fd, VIDIOC_DECODER_CMD, &cmd);
> > +    if (ret)
> > +        return AVERROR(errno);
> > +
> > +    return 0;
> > +}
> > +
> >  /**
> >   * handle resolution change event and end of stream event
> >   * returns 1 if reinit was successful, negative if it failed @@
> > -163,7 +178,7 @@ static int v4l2_handle_event(V4L2Context *ctx)
> >      V4L2m2mContext *s = ctx_to_m2mctx(ctx);
> >      struct v4l2_format cap_fmt = s->capture.format;
> >      struct v4l2_event evt = { 0 };
> > -    int reinit, ret;
> > +    int ret;
> >
> >      ret = ioctl(s->fd, VIDIOC_DQEVENT, &evt);
> >      if (ret < 0) {
> > @@ -185,35 +200,29 @@ static int v4l2_handle_event(V4L2Context *ctx)
> >          return 0;
> >      }
> >
> > -    reinit = v4l2_resolution_changed(&s->capture, &cap_fmt);
> > -    if (reinit) {
> > +    if (v4l2_resolution_changed(&s->capture, &cap_fmt)) {
> >          s->capture.height = v4l2_get_height(&cap_fmt);
> >          s->capture.width = v4l2_get_width(&cap_fmt);
> >          s->capture.sample_aspect_ratio = v4l2_get_sar(&s->capture);
> 
> > +    } else {
> > +        v4l2_start_decode(ctx);
> > +        return 0;
> >      }
> 
> You can minimize the diff just by adding this part and the definition for
> v4l2_start_decode(). Then have a separate commit that cleans up and removes
> the reinit variable.
> 

OK, I'll split it into two parts.

> >
> > -    if (reinit)
> > -        s->reinit = 1;
> > +    s->reinit = 1;
> >
> > -    if (reinit) {
> > -        if (s->avctx)
> > -            ret = ff_set_dimensions(s->avctx, s->capture.width,
> s->capture.height);
> > -        if (ret < 0)
> > -            av_log(logger(ctx), AV_LOG_WARNING, "update avcodec
> height and width\n");
> > +    if (s->avctx)
> > +        ret = ff_set_dimensions(s->avctx, s->capture.width,
> s->capture.height);
> > +    if (ret < 0)
> > +        av_log(logger(ctx), AV_LOG_WARNING, "update avcodec height
> > + and width\n");
> >
> > -        ret = ff_v4l2_m2m_codec_reinit(s);
> > -        if (ret) {
> > -            av_log(logger(ctx), AV_LOG_ERROR,
> "v4l2_m2m_codec_reinit\n");
> > -            return AVERROR(EINVAL);
> > -        }
> > -        goto reinit_run;
> > +    ret = ff_v4l2_m2m_codec_reinit(s);
> > +    if (ret) {
> > +        av_log(logger(ctx), AV_LOG_ERROR, "v4l2_m2m_codec_reinit\n");
> > +        return AVERROR(EINVAL);
> >      }
> >
> > -    /* dummy event received */
> > -    return 0;
> > -
> >      /* reinit executed */
> > -reinit_run:
> >      return 1;
> >  }
> >
> > @@ -551,6 +560,9 @@ int ff_v4l2_context_set_status(V4L2Context* ctx,
> uint32_t cmd)
> >      int type = ctx->type;
> >      int ret;
> 
> >
> > +    if (ctx->streamon == (cmd == VIDIOC_STREAMON))
> > +        return 0;
> > +
> 
> This change looks unrelated.

Yes it is not directly related, it's just an enhancement that avoid driver can't handle the repeated streamon or streamoff ioctl.
Maybe I can remove it from this patch, and make a single patch.

> 
> >      ret = ioctl(ctx_to_m2mctx(ctx)->fd, cmd, &type);
> >      if (ret < 0)
> >          return AVERROR(errno);
> 
> Thanks,
> --
> Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Andriy Gelman]

Hi Ming,

On Tue, 04. Jan 07:51, Ming Qian wrote:
> 
> > -----Original Message-----
> > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > Sent: Monday, January 3, 2022 12:41 AM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: Ming Qian <ming.qian@nxp.com>
> > Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> > resume the decoding process after source change event received.
> > 
> > Caution: EXT Email
> > 
> > On Thu, 19. Aug 16:55, Ming Qian wrote:
> > > client need to resume the decoding process after it dequeues the
> > > source change event.
> > > no matter what's the return value of v4l2_resolution_changed().
> > > if the client doesn't resume the decoding process, the decoder may
> > > keep waiting
> > >
> > > in documentation of v4l2 stateful decoder, we can see the following
> > > description:
> > >       The client must continue the sequence as described below to
> > >       continue the decoding process.
> > >       1.  Dequeue the source change event.
> > >               Important
> > >               A source change triggers an implicit decoder drain,
> > >               similar to the explicit Drain sequence. The decoder is
> > >               stopped after it completes. The decoding process must be
> > >               resumed with either a pair of calls to
> > >               VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on the
> > CAPTURE
> > >               queue, or a call to VIDIOC_DECODER_CMD() with the
> > >               V4L2_DEC_CMD_START command.
> > >       2.  Continue with the Capture Setup sequence.
> > 
> > Please also add that this fixes decoding of
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstreams
> > .videolan.org%2Fffmpeg%2Fincoming%2F720p60.mp4&amp;data=04%7C01%
> > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e
> > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU
> > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=X4rKQX19MQg1gO3ILiBCQ
> > qSLIvqovZLA95KKiyoVNzI%3D&amp;reserved=0 on RPi4.
> > 

> 
> Hi Andriy,
>     What's wrong with this stream? Everything is normal on my side when I play it using ffplay.
> 

I couldn't decode the file on the Raspberry Pi4. After enqueuing the first few packets
there was a dynamic resolution change event, and the start decode command
was not sent. This is fixed by your patch.

Thanks,
-- 
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Andriy Gelman]

On Tue, 04. Jan 17:48, Andriy Gelman wrote:
> Hi Ming,
> 
> On Tue, 04. Jan 07:51, Ming Qian wrote:
> > 
> > > -----Original Message-----
> > > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > > Sent: Monday, January 3, 2022 12:41 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Cc: Ming Qian <ming.qian@nxp.com>
> > > Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> > > resume the decoding process after source change event received.
> > > 
> > > Caution: EXT Email
> > > 
> > > On Thu, 19. Aug 16:55, Ming Qian wrote:
> > > > client need to resume the decoding process after it dequeues the
> > > > source change event.
> > > > no matter what's the return value of v4l2_resolution_changed().
> > > > if the client doesn't resume the decoding process, the decoder may
> > > > keep waiting
> > > >
> > > > in documentation of v4l2 stateful decoder, we can see the following
> > > > description:
> > > >       The client must continue the sequence as described below to
> > > >       continue the decoding process.
> > > >       1.  Dequeue the source change event.
> > > >               Important
> > > >               A source change triggers an implicit decoder drain,
> > > >               similar to the explicit Drain sequence. The decoder is
> > > >               stopped after it completes. The decoding process must be
> > > >               resumed with either a pair of calls to
> > > >               VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on the
> > > CAPTURE
> > > >               queue, or a call to VIDIOC_DECODER_CMD() with the
> > > >               V4L2_DEC_CMD_START command.
> > > >       2.  Continue with the Capture Setup sequence.
> > > 
> > > Please also add that this fixes decoding of
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstreams
> > > .videolan.org%2Fffmpeg%2Fincoming%2F720p60.mp4&amp;data=04%7C01%
> > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e
> > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU
> > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=X4rKQX19MQg1gO3ILiBCQ
> > > qSLIvqovZLA95KKiyoVNzI%3D&amp;reserved=0 on RPi4.
> > > 
> 
> > 
> > Hi Andriy,
> >     What's wrong with this stream? Everything is normal on my side when I play it using ffplay.
> > 

> 
> I couldn't decode the file on the Raspberry Pi4. After enqueuing the first few packets
> there was a dynamic resolution change event, and the start decode command
> was not sent. This is fixed by your patch.

Also you may have to upgrade kernel to reproduce. It may have been working fine
before this commit:
https://github.com/raspberrypi/linux/commit/b7e6b495eff31298ba4665f71b2414cc9a8f99c2#diff-93defb6da917ce9bb43cb195d0e61f81673c5183ac75d631f3e1ee475a810dd6

-- 
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Ming Qian]

> -----Original Message-----
> From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> Sent: Wednesday, January 5, 2022 6:58 AM
> To: Ming Qian <ming.qian@nxp.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> resume the decoding process after source change event received.
> 
> Caution: EXT Email
> 
> On Tue, 04. Jan 17:48, Andriy Gelman wrote:
> > Hi Ming,
> >
> > On Tue, 04. Jan 07:51, Ming Qian wrote:
> > >
> > > > -----Original Message-----
> > > > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > > > Sent: Monday, January 3, 2022 12:41 AM
> > > > To: FFmpeg development discussions and patches
> > > > <ffmpeg-devel@ffmpeg.org>
> > > > Cc: Ming Qian <ming.qian@nxp.com>
> > > > Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> > > > resume the decoding process after source change event received.
> > > >
> > > > Caution: EXT Email
> > > >
> > > > On Thu, 19. Aug 16:55, Ming Qian wrote:
> > > > > client need to resume the decoding process after it dequeues the
> > > > > source change event.
> > > > > no matter what's the return value of v4l2_resolution_changed().
> > > > > if the client doesn't resume the decoding process, the decoder
> > > > > may keep waiting
> > > > >
> > > > > in documentation of v4l2 stateful decoder, we can see the
> > > > > following
> > > > > description:
> > > > >       The client must continue the sequence as described below to
> > > > >       continue the decoding process.
> > > > >       1.  Dequeue the source change event.
> > > > >               Important
> > > > >               A source change triggers an implicit decoder drain,
> > > > >               similar to the explicit Drain sequence. The decoder is
> > > > >               stopped after it completes. The decoding process must
> be
> > > > >               resumed with either a pair of calls to
> > > > >               VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on
> the
> > > > CAPTURE
> > > > >               queue, or a call to VIDIOC_DECODER_CMD() with the
> > > > >               V4L2_DEC_CMD_START command.
> > > > >       2.  Continue with the Capture Setup sequence.
> > > >
> > > > Please also add that this fixes decoding of
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > streams
> > > > .videolan.org%2Fffmpeg%2Fincoming%2F720p60.mp4&amp;data=04%7
> C01%
> > > >
> 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e
> > > >
> a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU
> > > >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> > > >
> Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=X4rKQX19MQg1gO3ILiBCQ
> > > > qSLIvqovZLA95KKiyoVNzI%3D&amp;reserved=0 on RPi4.
> > > >
> >
> > >
> > > Hi Andriy,
> > >     What's wrong with this stream? Everything is normal on my side
> when I play it using ffplay.
> > >
> 
> >
> > I couldn't decode the file on the Raspberry Pi4. After enqueuing the
> > first few packets there was a dynamic resolution change event, and the
> > start decode command was not sent. This is fixed by your patch.
> 
> Also you may have to upgrade kernel to reproduce. It may have been working
> fine before this commit:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Fraspberrypi%2Flinux%2Fcommit%2Fb7e6b495eff31298ba4665f71b2
> 414cc9a8f99c2%23diff-93defb6da917ce9bb43cb195d0e61f81673c5183ac75
> d631f3e1ee475a810dd6&amp;data=04%7C01%7Cming.qian%40nxp.com%7C
> 64fa84ae1325497b2b4408d9cfd5aabd%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C1%7C637769338865335050%7CUnknown%7CTWFpbGZsb3d8e
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C3000&amp;sdata=ZCuu2FL5EusXxilM%2BkdpLjQDXnvVhFIXsKeIVHPKWHY%
> 3D&amp;reserved=0
> 

I got it, but I didn't have a Raspberry Pi4 on hand right now, maybe I can test it again after I get one.
I make this patch according the v4l2 decoder specification and test it on NXP i.MX 8Q and i.MX8M

> --
> Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Andriy Gelman]

On Tue, 04. Jan 17:58, Andriy Gelman wrote:
> On Tue, 04. Jan 17:48, Andriy Gelman wrote:
> > Hi Ming,
> > 
> > On Tue, 04. Jan 07:51, Ming Qian wrote:
> > > 
> > > > -----Original Message-----
> > > > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > > > Sent: Monday, January 3, 2022 12:41 AM
> > > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > > Cc: Ming Qian <ming.qian@nxp.com>
> > > > Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> > > > resume the decoding process after source change event received.
> > > > 
> > > > Caution: EXT Email
> > > > 
> > > > On Thu, 19. Aug 16:55, Ming Qian wrote:
> > > > > client need to resume the decoding process after it dequeues the
> > > > > source change event.
> > > > > no matter what's the return value of v4l2_resolution_changed().
> > > > > if the client doesn't resume the decoding process, the decoder may
> > > > > keep waiting
> > > > >
> > > > > in documentation of v4l2 stateful decoder, we can see the following
> > > > > description:
> > > > >       The client must continue the sequence as described below to
> > > > >       continue the decoding process.
> > > > >       1.  Dequeue the source change event.
> > > > >               Important
> > > > >               A source change triggers an implicit decoder drain,
> > > > >               similar to the explicit Drain sequence. The decoder is
> > > > >               stopped after it completes. The decoding process must be
> > > > >               resumed with either a pair of calls to
> > > > >               VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on the
> > > > CAPTURE
> > > > >               queue, or a call to VIDIOC_DECODER_CMD() with the
> > > > >               V4L2_DEC_CMD_START command.
> > > > >       2.  Continue with the Capture Setup sequence.
> > > > 
> > > > Please also add that this fixes decoding of
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstreams
> > > > .videolan.org%2Fffmpeg%2Fincoming%2F720p60.mp4&amp;data=04%7C01%
> > > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e
> > > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU
> > > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> > > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=X4rKQX19MQg1gO3ILiBCQ
> > > > qSLIvqovZLA95KKiyoVNzI%3D&amp;reserved=0 on RPi4.
> > > > 
> > 
> > > 
> > > Hi Andriy,
> > >     What's wrong with this stream? Everything is normal on my side when I play it using ffplay.
> > > 
> 
> > 
> > I couldn't decode the file on the Raspberry Pi4. After enqueuing the first few packets
> > there was a dynamic resolution change event, and the start decode command
> > was not sent. This is fixed by your patch.

> 
> Also you may have to upgrade kernel to reproduce. It may have been working fine
> before this commit:
> https://github.com/raspberrypi/linux/commit/b7e6b495eff31298ba4665f71b2414cc9a8f99c2#diff-93defb6da917ce9bb43cb195d0e61f81673c5183ac75d631f3e1ee475a810dd6
> 

I confirmed the above.

I'll apply patches 1 and 3 from this series later tonight if no one objects.

-- 
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 2/3] avcodec/v4l2_context: resume the decoding process after source change event received.

[Ming Qian]

> -----Original Message-----
> From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> Sent: Sunday, January 9, 2022 1:13 AM
> To: Ming Qian <ming.qian@nxp.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_context:
> resume the decoding process after source change event received.
> 
> Caution: EXT Email
> 
> On Tue, 04. Jan 17:58, Andriy Gelman wrote:
> > On Tue, 04. Jan 17:48, Andriy Gelman wrote:
> > > Hi Ming,
> > >
> > > On Tue, 04. Jan 07:51, Ming Qian wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > > > > Sent: Monday, January 3, 2022 12:41 AM
> > > > > To: FFmpeg development discussions and patches
> > > > > <ffmpeg-devel@ffmpeg.org>
> > > > > Cc: Ming Qian <ming.qian@nxp.com>
> > > > > Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 2/3]
> avcodec/v4l2_context:
> > > > > resume the decoding process after source change event received.
> > > > >
> > > > > Caution: EXT Email
> > > > >
> > > > > On Thu, 19. Aug 16:55, Ming Qian wrote:
> > > > > > client need to resume the decoding process after it dequeues
> > > > > > the source change event.
> > > > > > no matter what's the return value of v4l2_resolution_changed().
> > > > > > if the client doesn't resume the decoding process, the decoder
> > > > > > may keep waiting
> > > > > >
> > > > > > in documentation of v4l2 stateful decoder, we can see the
> > > > > > following
> > > > > > description:
> > > > > >       The client must continue the sequence as described below to
> > > > > >       continue the decoding process.
> > > > > >       1.  Dequeue the source change event.
> > > > > >               Important
> > > > > >               A source change triggers an implicit decoder drain,
> > > > > >               similar to the explicit Drain sequence. The decoder is
> > > > > >               stopped after it completes. The decoding process
> must be
> > > > > >               resumed with either a pair of calls to
> > > > > >               VIDIOC_STREAMOFF() and VIDIOC_STREAMON() on
> the
> > > > > CAPTURE
> > > > > >               queue, or a call to VIDIOC_DECODER_CMD() with the
> > > > > >               V4L2_DEC_CMD_START command.
> > > > > >       2.  Continue with the Capture Setup sequence.
> > > > >
> > > > > Please also add that this fixes decoding of
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Fstreams
> > > > > .videolan.org%2Fffmpeg%2Fincoming%2F720p60.mp4&amp;data=04%
> 7C01%
> > > > >
> 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e
> > > > >
> a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU
> > > > >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> > > > > 6
> Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=X4rKQX19MQg1gO3ILiBCQ
> > > > > qSLIvqovZLA95KKiyoVNzI%3D&amp;reserved=0 on RPi4.
> > > > >
> > >
> > > >
> > > > Hi Andriy,
> > > >     What's wrong with this stream? Everything is normal on my side
> when I play it using ffplay.
> > > >
> >
> > >
> > > I couldn't decode the file on the Raspberry Pi4. After enqueuing the
> > > first few packets there was a dynamic resolution change event, and
> > > the start decode command was not sent. This is fixed by your patch.
> 
> >
> > Also you may have to upgrade kernel to reproduce. It may have been
> > working fine before this commit:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> >
> ub.com%2Fraspberrypi%2Flinux%2Fcommit%2Fb7e6b495eff31298ba4665f71
> b2414
> >
> cc9a8f99c2%23diff-93defb6da917ce9bb43cb195d0e61f81673c5183ac75d63
> 1f3e1
> >
> ee475a810dd6&amp;data=04%7C01%7Cming.qian%40nxp.com%7Ce6b24805
> 32c6458d
> >
> 185b08d9d2ca2a81%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7
> C63777258
> >
> 8014789133%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzI
> >
> iLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=RtRg%2Fu1ZggLSEf
> lGvxy
> > MutZv6Q%2B%2BX89O%2BE8DvXK0RHk%3D&amp;reserved=0
> >
> 
> I confirmed the above.
> 
> I'll apply patches 1 and 3 from this series later tonight if no one objects.
> 
> --
> Andriy

Hi Andriy,

    What to do with the rest of the patches?

Ming
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 3/3] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Andriy Gelman]

On Thu, 19. Aug 16:55, Ming Qian wrote:
> there are two proper ways to setup capture queue.
> 1. client wait the source change event,
>    then setup the capture queue and streamon
> 2. client setup the capture queue in advance,
>    but to avoid time issues, client should start
>    the capture queue before it enqueue the sequence
>    header to decoder driver through output queue.
>    and the sequence header is always in the first
>    frame, so client should start capture before
>    enqueue the first frame.
> 
> ffmpeg use the method 2 to setup capture queue,
> but currently ffmpeg enqueue the first frame
> before starting the capture queue.
> so in driver side, there are time issues.
> when driver has parsed the resolution from sequence header,
> but the client may not finished setup the capture.
> so driver can't decide whether to notify a source change event to
> client. and the following flow may be chaotic.
> 
> And it's OK that client setup capture queue first, then enqueue the
> first frame.
> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
>  libavcodec/v4l2_m2m_dec.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> index 224eb3d5e7be..6b936b6df2a9 100644
> --- a/libavcodec/v4l2_m2m_dec.c
> +++ b/libavcodec/v4l2_m2m_dec.c
> @@ -149,6 +149,14 @@ static int v4l2_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>      if (s->draining)
>          goto dequeue;
>  
> +    ret = v4l2_try_start(avctx);
> +    if (ret) {
> +        /* can't recover */
> +        if (ret != AVERROR(ENOMEM))
> +            ret = 0;
> +        goto fail;
> +    }
> +
>      ret = ff_v4l2_context_enqueue_packet(output, &s->buf_pkt);
>      if (ret < 0 && ret != AVERROR(EAGAIN))
>          goto fail;
> @@ -157,16 +165,6 @@ static int v4l2_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>      if (ret != AVERROR(EAGAIN))
>          av_packet_unref(&s->buf_pkt);
>  
> -    if (!s->draining) {
> -        ret = v4l2_try_start(avctx);
> -        if (ret) {
> -            /* cant recover */
> -            if (ret != AVERROR(ENOMEM))
> -                ret = 0;
> -            goto fail;
> -        }
> -    }
> -
>  dequeue:
>      return ff_v4l2_context_dequeue_frame(capture, frame, -1);
>  fail:

Unfortunately this does not work on odroid xu4.
It needs to parse the parameter sets before VIDIOC_G_FMT ioctl is called on the
capture queue:
https://github.com/hardkernel/linux/blob/odroid-5.4.y/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c#L317

Thanks,
-- 
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".