diff mbox series

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

Message ID 20220104090836.31660-7-ming.qian@nxp.com
State New
Headers show
Series [FFmpeg-devel,v5,1/7] avcodec/v4l2_context: don't reinit output queue when dynamic resolution change | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

Ming Qian Jan. 4, 2022, 9:08 a.m. UTC 
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(-)

Comments

Ming Qian March 21, 2022, 7:27 a.m. UTC | #1 
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".
Andriy Gelman March 26, 2022, 3:42 p.m. UTC | #2 
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.
Ming Qian March 29, 2022, 6:51 a.m. UTC | #3 
> 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
Andriy Gelman April 3, 2022, 10:29 p.m. UTC | #4 
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.
Ming Qian April 4, 2022, 12:14 p.m. UTC | #5 
> 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
diff mbox series

Patch

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: