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

[Andriy Gelman <andriy.gelman@gmail.com>]

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