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

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

[Ming Qian]

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);
-        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);
-- 
2.33.0

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

[FFmpeg-devel] [PATCH v5 2/7] avcodec/v4l2_m2m: remove ff_v4l2_m2m_codec_full_reinit

[Ming Qian]

ff_v4l2_m2m_codec_full_reinit is not used any more,
so remove it.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 libavcodec/v4l2_m2m.c | 76 -------------------------------------------
 libavcodec/v4l2_m2m.h | 10 ------
 2 files changed, 86 deletions(-)

diff --git a/libavcodec/v4l2_m2m.c b/libavcodec/v4l2_m2m.c
index cdfd579810f2..a0ee5a201360 100644
--- a/libavcodec/v4l2_m2m.c
+++ b/libavcodec/v4l2_m2m.c
@@ -245,82 +245,6 @@ int ff_v4l2_m2m_codec_reinit(V4L2m2mContext *s)
     return 0;
 }
 
-int ff_v4l2_m2m_codec_full_reinit(V4L2m2mContext *s)
-{
-    void *log_ctx = s->avctx;
-    int ret;
-
-    av_log(log_ctx, AV_LOG_DEBUG, "%s full reinit\n", s->devname);
-
-    /* wait for pending buffer references */
-    if (atomic_load(&s->refcount))
-        while(sem_wait(&s->refsync) == -1 && errno == EINTR);
-
-    ret = ff_v4l2_context_set_status(&s->output, VIDIOC_STREAMOFF);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_ERROR, "output VIDIOC_STREAMOFF\n");
-        goto error;
-    }
-
-    ret = ff_v4l2_context_set_status(&s->capture, VIDIOC_STREAMOFF);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_ERROR, "capture VIDIOC_STREAMOFF\n");
-        goto error;
-    }
-
-    /* release and unmmap the buffers */
-    ff_v4l2_context_release(&s->output);
-    ff_v4l2_context_release(&s->capture);
-
-    /* start again now that we know the stream dimensions */
-    s->draining = 0;
-    s->reinit = 0;
-
-    ret = ff_v4l2_context_get_format(&s->output, 0);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_DEBUG, "v4l2 output format not supported\n");
-        goto error;
-    }
-
-    ret = ff_v4l2_context_get_format(&s->capture, 0);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_DEBUG, "v4l2 capture format not supported\n");
-        goto error;
-    }
-
-    ret = ff_v4l2_context_set_format(&s->output);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_ERROR, "can't set v4l2 output format\n");
-        goto error;
-    }
-
-    ret = ff_v4l2_context_set_format(&s->capture);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_ERROR, "can't to set v4l2 capture format\n");
-        goto error;
-    }
-
-    ret = ff_v4l2_context_init(&s->output);
-    if (ret) {
-        av_log(log_ctx, AV_LOG_ERROR, "no v4l2 output context's buffers\n");
-        goto error;
-    }
-
-    /* decoder's buffers need to be updated at a later stage */
-    if (s->avctx && !av_codec_is_decoder(s->avctx->codec)) {
-        ret = ff_v4l2_context_init(&s->capture);
-        if (ret) {
-            av_log(log_ctx, AV_LOG_ERROR, "no v4l2 capture context's buffers\n");
-            goto error;
-        }
-    }
-
-    return 0;
-
-error:
-    return ret;
-}
-
 static void v4l2_m2m_destroy_context(void *opaque, uint8_t *context)
 {
     V4L2m2mContext *s = (V4L2m2mContext*)context;
diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h
index b67b21633109..16e0a6d6b90f 100644
--- a/libavcodec/v4l2_m2m.h
+++ b/libavcodec/v4l2_m2m.h
@@ -119,14 +119,4 @@ int ff_v4l2_m2m_codec_end(V4L2m2mPriv *priv);
  */
 int ff_v4l2_m2m_codec_reinit(V4L2m2mContext *ctx);
 
-/**
- * Reinitializes the V4L2m2mContext when the driver cannot continue processing
- * with the  any of the current V4L2Contexts (ie, changes in output and capture).
- *
- * @param[in] ctx The V4L2m2mContext instantiated by the encoder/decoder.
- *
- * @returns 0 in case of success, negative number otherwise
- */
-int ff_v4l2_m2m_codec_full_reinit(V4L2m2mContext *ctx);
-
 #endif /* AVCODEC_V4L2_M2M_H */
-- 
2.33.0

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

[FFmpeg-devel] [PATCH v5 3/7] avcodec/v4l2_context: add v4l2_start_decode

[Ming Qian]

on dynamic resolution change,
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.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 libavcodec/v4l2_context.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index dda5157698c3..a181f884d2a6 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
@@ -190,6 +205,9 @@ static int v4l2_handle_event(V4L2Context *ctx)
         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;
     }
 
     if (reinit)
-- 
2.33.0

_______________________________________________
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 v5 3/7] avcodec/v4l2_context: add v4l2_start_decode

[Andriy Gelman]

On Tue, 04. Jan 17:08, Ming Qian wrote:
> on dynamic resolution change,
> 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.
> 

For the patch title, I think something like the following would be better:
avcodec/v4l2_context: send start decode command after dynamic resolution change
event

I'll 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 | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index dda5157698c3..a181f884d2a6 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
> @@ -190,6 +205,9 @@ static int v4l2_handle_event(V4L2Context *ctx)
>          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;
>      }
>  
>      if (reinit)

lgtm otherwise.

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

[FFmpeg-devel] [PATCH v5 4/7] avcodec/v4l2_context: set resolution change if decoded format changed

[Ming Qian]

The dynamic resoltuion change occurs when he decoder detects a coded
frame with one or more of the following parameters different from those
previously established (and reflected by corresponding queries):
1. coded resolution (OUTPUT width and height)
2. visible resolution (selection rectangles)
3. the minimum number of buffers needed for decoding
4. bit-depth of the bitstream has been changed

when the bit-depth is changed, the pixel format will be changed,
so the format is changed after a source change event,
we should handle it and reinit the capture queue.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 libavcodec/v4l2_context.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index a181f884d2a6..0a0f5b226960 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -85,9 +85,11 @@ static inline unsigned int v4l2_resolution_changed(V4L2Context *ctx, struct v4l2
 {
     struct v4l2_format *fmt1 = &ctx->format;
     int ret =  V4L2_TYPE_IS_MULTIPLANAR(ctx->type) ?
+        fmt1->fmt.pix_mp.pixelformat != fmt2->fmt.pix_mp.pixelformat ||
         fmt1->fmt.pix_mp.width != fmt2->fmt.pix_mp.width ||
         fmt1->fmt.pix_mp.height != fmt2->fmt.pix_mp.height
         :
+        fmt1->fmt.pix.pixelformat != fmt2->fmt.pix.pixelformat ||
         fmt1->fmt.pix.width != fmt2->fmt.pix.width ||
         fmt1->fmt.pix.height != fmt2->fmt.pix.height;
 
-- 
2.33.0

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

[FFmpeg-devel] [PATCH v5 5/7] avcodec/v4l2_context: resume the decoding process after source change event received.

[Ming Qian]

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.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 libavcodec/v4l2_context.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index 0a0f5b226960..b7b584dfbef6 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -180,7 +180,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) {
@@ -202,8 +202,7 @@ 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);
@@ -212,28 +211,20 @@ static int v4l2_handle_event(V4L2Context *ctx)
         return 0;
     }
 
-    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;
 }
 
-- 
2.33.0

_______________________________________________
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 v5 5/7] avcodec/v4l2_context: resume the decoding process after source change event received.

[Andriy Gelman]

On Tue, 04. Jan 17:08, 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.
> 

This is just a clean up to remove the reinit variable so the commit title and
description are wrong.

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 v5 5/7] avcodec/v4l2_context: resume the decoding process after source change event received.

[Ming Qian]

On Tue, 04. Jan 17:41, Andriy Gelman wrote:
> On Tue, 04. Jan 17:08, 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.
> > 
> 
> This is just a clean up to remove the reinit variable so the commit title and
> description are wrong.
> 

Updated the commit message and pushed.

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

[FFmpeg-devel] [PATCH v5 6/7] avcodec/v4l2_context: sync v4l2 context status with driver.

[Ming Qian]

Check v4l2 context before call VIDIOC_STREAMON() or
VIDIOC_STREAMOFF().

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 libavcodec/v4l2_context.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index b7b584dfbef6..8910ae08d3a5 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -562,6 +562,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;
+
     ret = ioctl(ctx_to_m2mctx(ctx)->fd, cmd, &type);
     if (ret < 0)
         return AVERROR(errno);
-- 
2.33.0

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

[FFmpeg-devel] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Ming Qian]

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 b0c3d30ac8ae..e67758531ace 100644
--- a/libavcodec/v4l2_m2m_dec.c
+++ b/libavcodec/v4l2_m2m_dec.c
@@ -153,6 +153,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;
@@ -161,16 +169,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:
-- 
2.33.0

_______________________________________________
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] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Ming Qian]

Hi Andriy,

    What do you think of this patch?

    The Initialization flow defined in linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
4. This step only applies to coded formats that contain resolution information in the stream. Continue queuing/dequeuing bytestream buffers to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The buffers will be processed and returned to the client in order, until required metadata to configure the CAPTURE queue are found. This is indicated by the decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set to V4L2_EVENT_SRC_CH_RESOLUTION.
	Note: A client capable of acquiring stream parameters from the bytestream on its own may attempt to set the width and height of the OUTPUT format to non-zero values matching the coded size of the stream, skip this step and continue with the Capture Setup sequence.

5. Continue with the Capture Setup sequence.

    In ffmpeg's implementation, ffmpeg will set non-zero width and height on output queue, so the step 4 should be skipped, and setup the capture queue directly.
    So the flow should be:
1. Set the coded format and valid resolution on OUTPUT via VIDIOC_S_FMT().
2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
5. Continue with the Capture Setup sequence.

And this patch is just following the above flow.

Ming


> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of
> Ming Qian
> Sent: Tuesday, January 4, 2022 5:09 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [EXT] [FFmpeg-devel] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup
> capture queue before enqueue the first frame
> 
> Caution: EXT Email
> 
> 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
> b0c3d30ac8ae..e67758531ace 100644
> --- a/libavcodec/v4l2_m2m_dec.c
> +++ b/libavcodec/v4l2_m2m_dec.c
> @@ -153,6 +153,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;
> @@ -161,16 +169,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:
> --
> 2.33.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmpeg.
> org%2Fmailman%2Flistinfo%2Fffmpeg-devel&amp;data=04%7C01%7Cming.qi
> an%40nxp.com%7C6b9621a0804c45f9da9108d9cf61fb87%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C1%7C637768841997715387%7CUnknown%
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000&amp;sdata=E7V4kwlJtxbh6jnisMV8FWAwwDzksS
> qLp82kapSW4FA%3D&amp;reserved=0
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
_______________________________________________
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] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Andriy Gelman]

Hi Ming,

On Mon, 21. Mar 07:27, Ming Qian wrote:
> Hi Andriy,
> 
>     What do you think of this patch?
> 
>     The Initialization flow defined in linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
> 1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
> 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> 4. This step only applies to coded formats that contain resolution information in the stream. Continue queuing/dequeuing bytestream buffers to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The buffers will be processed and returned to the client in order, until required metadata to configure the CAPTURE queue are found. This is indicated by the decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set to V4L2_EVENT_SRC_CH_RESOLUTION.
> 	Note: A client capable of acquiring stream parameters from the bytestream on its own may attempt to set the width and height of the OUTPUT format to non-zero values matching the coded size of the stream, skip this step and continue with the Capture Setup sequence.
> 
> 5. Continue with the Capture Setup sequence.
> 
>     In ffmpeg's implementation, ffmpeg will set non-zero width and height on output queue, so the step 4 should be skipped, and setup the capture queue directly.
>     So the flow should be:
> 1. Set the coded format and valid resolution on OUTPUT via VIDIOC_S_FMT().
> 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> 5. Continue with the Capture Setup sequence.
> 
> And this patch is just following the above flow.
> 
> Ming

The v4 version didn't work for me on odroid xu4
http://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/290679.html

I haven't had time retest but don't think anything has changed.

-- 
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] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Ming Qian]

> From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> Sent: Saturday, March 26, 2022 11:43 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: Ming Qian <ming.qian@nxp.com>
> Subject: Re: [FFmpeg-devel] [EXT] [PATCH v5 7/7] avcodec/v4l2_m2m_dec:
> setup capture queue before enqueue the first frame
> 
> Caution: EXT Email
> 
> Hi Ming,
> 
> On Mon, 21. Mar 07:27, Ming Qian wrote:
> > Hi Andriy,
> >
> >     What do you think of this patch?
> >
> >     The Initialization flow defined in
> linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > 1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
> > 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> > 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> > 4. This step only applies to coded formats that contain resolution
> information in the stream. Continue queuing/dequeuing bytestream buffers
> to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The
> buffers will be processed and returned to the client in order, until required
> metadata to configure the CAPTURE queue are found. This is indicated by the
> decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set to
> V4L2_EVENT_SRC_CH_RESOLUTION.
> >       Note: A client capable of acquiring stream parameters from the
> bytestream on its own may attempt to set the width and height of the
> OUTPUT format to non-zero values matching the coded size of the stream, skip
> this step and continue with the Capture Setup sequence.
> >
> > 5. Continue with the Capture Setup sequence.
> >
> >     In ffmpeg's implementation, ffmpeg will set non-zero width and height
> on output queue, so the step 4 should be skipped, and setup the capture
> queue directly.
> >     So the flow should be:
> > 1. Set the coded format and valid resolution on OUTPUT via VIDIOC_S_FMT().
> > 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> > 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> > 5. Continue with the Capture Setup sequence.
> >
> > And this patch is just following the above flow.
> >
> > Ming
> 
> The v4 version didn't work for me on odroid xu4
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fffmpeg.o
> rg%2Fpipermail%2Fffmpeg-devel%2F2022-January%2F290679.html&amp;dat
> a=04%7C01%7Cming.qian%40nxp.com%7Cca988d592253473cbac408da0f3f4
> 801%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637839061704
> 093567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=I80bpE70S6JI
> 8A%2BaXQGq4JyxL0eBbtLN58p5SpqlA6s%3D&amp;reserved=0
> 
> I haven't had time retest but don't think anything has changed.
> 
> --
> Andriy

Hi Andriy,
    I check the S5P MFC Video Codec driver, and indeed it requires the client enqueue the output buffer which contains the sequence header. And in vidioc_g_fmt(), driver will block and wait until the header is parsed.
    And indeed my patch will not work on it.
    But it seems don't meet the specification.
    For S5P MFC, the step 4 can't be skipped, and the client should wait the V4L2_EVENT_SOURCE_CHANGE event before the step 5 setting up capture queue. 
    The ffmpeg v4l2 decoder don't skip step 4, but also doesn't wait the V4L2_EVENT_SOURCE_CHANGE event.

So in current, we should make it work on more devices instead of more spec?

    The Initialization flow defined in linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
	1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
	2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
	3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
	4. This step only applies to coded formats that contain resolution information in the stream. Continue queuing/dequeuing bytestream buffers to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The buffers will be processed and returned to the client in order, until required metadata to configure the CAPTURE queue are found. This is indicated by the decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set to V4L2_EVENT_SRC_CH_RESOLUTION.
	Note: A client capable of acquiring stream parameters from the bytestream on its own may attempt to set the width and height of the OUTPUT format to non-zero values matching the coded size of the stream, skip this step and continue with the Capture Setup sequence.
	5. Continue with the Capture Setup sequence.
	
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] [EXT] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Andriy Gelman]

Hi Ming,

Sorry for the late reply.

On Tue, 29. Mar 06:51, Ming Qian wrote:
> > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > Sent: Saturday, March 26, 2022 11:43 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: Ming Qian <ming.qian@nxp.com>
> > Subject: Re: [FFmpeg-devel] [EXT] [PATCH v5 7/7] avcodec/v4l2_m2m_dec:
> > setup capture queue before enqueue the first frame
> > 
> > Caution: EXT Email
> > 
> > Hi Ming,
> > 
> > On Mon, 21. Mar 07:27, Ming Qian wrote:
> > > Hi Andriy,
> > >
> > >     What do you think of this patch?
> > >
> > >     The Initialization flow defined in
> > linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > > 1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
> > > 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> > > 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> > > 4. This step only applies to coded formats that contain resolution
> > information in the stream. Continue queuing/dequeuing bytestream buffers
> > to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The
> > buffers will be processed and returned to the client in order, until required
> > metadata to configure the CAPTURE queue are found. This is indicated by the
> > decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set to
> > V4L2_EVENT_SRC_CH_RESOLUTION.
> > >       Note: A client capable of acquiring stream parameters from the
> > bytestream on its own may attempt to set the width and height of the
> > OUTPUT format to non-zero values matching the coded size of the stream, skip
> > this step and continue with the Capture Setup sequence.
> > >
> > > 5. Continue with the Capture Setup sequence.
> > >
> > >     In ffmpeg's implementation, ffmpeg will set non-zero width and height
> > on output queue, so the step 4 should be skipped, and setup the capture
> > queue directly.
> > >     So the flow should be:
> > > 1. Set the coded format and valid resolution on OUTPUT via VIDIOC_S_FMT().
> > > 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> > > 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> > > 5. Continue with the Capture Setup sequence.
> > >
> > > And this patch is just following the above flow.
> > >
> > > Ming
> > 
> > The v4 version didn't work for me on odroid xu4
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fffmpeg.o
> > rg%2Fpipermail%2Fffmpeg-devel%2F2022-January%2F290679.html&amp;dat
> > a=04%7C01%7Cming.qian%40nxp.com%7Cca988d592253473cbac408da0f3f4
> > 801%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637839061704
> > 093567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> > uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=I80bpE70S6JI
> > 8A%2BaXQGq4JyxL0eBbtLN58p5SpqlA6s%3D&amp;reserved=0
> > 
> > I haven't had time retest but don't think anything has changed.
> > 
> > --
> > Andriy
> 
> Hi Andriy,
>     I check the S5P MFC Video Codec driver, and indeed it requires the client enqueue the output buffer which contains the sequence header. And in vidioc_g_fmt(), driver will block and wait until the header is parsed.
>     And indeed my patch will not work on it.
>     But it seems don't meet the specification.
>     For S5P MFC, the step 4 can't be skipped, and the client should wait the V4L2_EVENT_SOURCE_CHANGE event before the step 5 setting up capture queue. 
>     The ffmpeg v4l2 decoder don't skip step 4, but also doesn't wait the V4L2_EVENT_SOURCE_CHANGE event.

> 
> So in current, we should make it work on more devices instead of more spec?

Personally I'd prefer not to break users. Odroid xu4 is a popular board afaik.
But I'm also not the maintainer of v4l2m2m.

IMO it would be better the fix the problem in the driver first.

> 
>     The Initialization flow defined in linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
> 	1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
> 	2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on OUTPUT.
> 	3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> 	4. This step only applies to coded formats that contain resolution information in the stream. Continue queuing/dequeuing bytestream buffers to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The buffers will be processed and returned to the client in order, until required metadata to configure the CAPTURE queue are found. This is indicated by the decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set to V4L2_EVENT_SRC_CH_RESOLUTION.
> 	Note: A client capable of acquiring stream parameters from the bytestream on its own may attempt to set the width and height of the OUTPUT format to non-zero values matching the coded size of the stream, skip this step and continue with the Capture Setup sequence.
> 	5. Continue with the Capture Setup sequence.


-- 
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] [PATCH v5 7/7] avcodec/v4l2_m2m_dec: setup capture queue before enqueue the first frame

[Ming Qian]

> From: Andriy Gelman <andriy.gelman@gmail.com>
> Sent: Monday, April 4, 2022 6:29 AM
> To: Ming Qian <ming.qian@nxp.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [EXT] [PATCH v5 7/7] avcodec/v4l2_m2m_dec:
> setup capture queue before enqueue the first frame
> 
> Caution: EXT Email
> 
> Hi Ming,
> 
> Sorry for the late reply.
> 
> On Tue, 29. Mar 06:51, Ming Qian wrote:
> > > From: Andriy Gelman [mailto:andriy.gelman@gmail.com]
> > > Sent: Saturday, March 26, 2022 11:43 PM
> > > To: FFmpeg development discussions and patches
> > > <ffmpeg-devel@ffmpeg.org>
> > > Cc: Ming Qian <ming.qian@nxp.com>
> > > Subject: Re: [FFmpeg-devel] [EXT] [PATCH v5 7/7]
> avcodec/v4l2_m2m_dec:
> > > setup capture queue before enqueue the first frame
> > >
> > > Caution: EXT Email
> > >
> > > Hi Ming,
> > >
> > > On Mon, 21. Mar 07:27, Ming Qian wrote:
> > > > Hi Andriy,
> > > >
> > > >     What do you think of this patch?
> > > >
> > > >     The Initialization flow defined in
> > > linux/Documentation/userspace-api/media/v4l/dev-decoder.rst
> > > > 1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
> > > > 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on
> OUTPUT.
> > > > 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> > > > 4. This step only applies to coded formats that contain resolution
> > > information in the stream. Continue queuing/dequeuing bytestream
> > > buffers to/from the OUTPUT queue via VIDIOC_QBUF() and
> > > VIDIOC_DQBUF(). The buffers will be processed and returned to the
> > > client in order, until required metadata to configure the CAPTURE
> > > queue are found. This is indicated by the decoder sending a
> > > V4L2_EVENT_SOURCE_CHANGE event with changes set to
> V4L2_EVENT_SRC_CH_RESOLUTION.
> > > >       Note: A client capable of acquiring stream parameters from
> > > > the
> > > bytestream on its own may attempt to set the width and height of the
> > > OUTPUT format to non-zero values matching the coded size of the
> > > stream, skip this step and continue with the Capture Setup sequence.
> > > >
> > > > 5. Continue with the Capture Setup sequence.
> > > >
> > > >     In ffmpeg's implementation, ffmpeg will set non-zero width and
> > > > height
> > > on output queue, so the step 4 should be skipped, and setup the
> > > capture queue directly.
> > > >     So the flow should be:
> > > > 1. Set the coded format and valid resolution on OUTPUT via
> VIDIOC_S_FMT().
> > > > 2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on
> OUTPUT.
> > > > 3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> > > > 5. Continue with the Capture Setup sequence.
> > > >
> > > > And this patch is just following the above flow.
> > > >
> > > > Ming
> > >
> > > The v4 version didn't work for me on odroid xu4
> > >
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fffm
> > >
> peg.o%2F&amp;data=04%7C01%7Cming.qian%40nxp.com%7Cfa858e4a13434
> ee727
> > >
> 3508da15c1695b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C6378
> 4621
> > >
> 7684591813%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luM
> > >
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=wl%2BZaEBNIPZ
> TOyv
> > > 7dgmnHhSQyAs37PjZCkvhwPL%2B6xg%3D&amp;reserved=0
> > > rg%2Fpipermail%2Fffmpeg-devel%2F2022-
> January%2F290679.html&amp;dat
> > >
> a=04%7C01%7Cming.qian%40nxp.com%7Cca988d592253473cbac408da0f3f4
> > > 801%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637839061704
> > >
> 093567%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
> l
> > >
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=I80bpE70S6JI
> > > 8A%2BaXQGq4JyxL0eBbtLN58p5SpqlA6s%3D&amp;reserved=0
> > >
> > > I haven't had time retest but don't think anything has changed.
> > >
> > > --
> > > Andriy
> >
> > Hi Andriy,
> >     I check the S5P MFC Video Codec driver, and indeed it requires the client
> enqueue the output buffer which contains the sequence header. And in
> vidioc_g_fmt(), driver will block and wait until the header is parsed.
> >     And indeed my patch will not work on it.
> >     But it seems don't meet the specification.
> >     For S5P MFC, the step 4 can't be skipped, and the client should wait the
> V4L2_EVENT_SOURCE_CHANGE event before the step 5 setting up capture
> queue.
> >     The ffmpeg v4l2 decoder don't skip step 4, but also doesn't wait the
> V4L2_EVENT_SOURCE_CHANGE event.
> 
> >
> > So in current, we should make it work on more devices instead of more
> spec?
> 
> Personally I'd prefer not to break users. Odroid xu4 is a popular board afaik.
> But I'm also not the maintainer of v4l2m2m.
> 
> IMO it would be better the fix the problem in the driver first.
> 

Hi Andriy,
    Thanks for your reply. Maybe I can try to work on the driver first.

Ming.

> >
> >     The Initialization flow defined in linux/Documentation/userspace-
> api/media/v4l/dev-decoder.rst
> >       1. Set the coded format on OUTPUT via VIDIOC_S_FMT().
> >       2. Allocate source (bytestream) buffers via VIDIOC_REQBUFS() on
> OUTPUT.
> >       3. Start streaming on the OUTPUT queue via VIDIOC_STREAMON().
> >       4. This step only applies to coded formats that contain resolution
> information in the stream. Continue queuing/dequeuing bytestream buffers
> to/from the OUTPUT queue via VIDIOC_QBUF() and VIDIOC_DQBUF(). The
> buffers will be processed and returned to the client in order, until required
> metadata to configure the CAPTURE queue are found. This is indicated by the
> decoder sending a V4L2_EVENT_SOURCE_CHANGE event with changes set
> to V4L2_EVENT_SRC_CH_RESOLUTION.
> >       Note: A client capable of acquiring stream parameters from the
> bytestream on its own may attempt to set the width and height of the
> OUTPUT format to non-zero values matching the coded size of the stream,
> skip this step and continue with the Capture Setup sequence.
> >       5. Continue with the Capture Setup sequence.
> 
> 
> --
> 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 v5 1/7] avcodec/v4l2_context: don't reinit output queue when dynamic resolution change

[Andriy Gelman]

On Tue, 04. Jan 17:08, 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);
> -        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);
> -- 
> 2.33.0

lgtm

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