| Message ID | 20210819085533.1174-2-ming.qian@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v4,1/3] avcodec/v4l2_context: don't reinit output queue when dynamic resolution change | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
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,
> -----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&data=04%7C01% > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=X4rKQX19MQg1gO3ILiBCQ > qSLIvqovZLA95KKiyoVNzI%3D&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
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&data=04%7C01% > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=X4rKQX19MQg1gO3ILiBCQ > > qSLIvqovZLA95KKiyoVNzI%3D&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,
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&data=04%7C01% > > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e > > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU > > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=X4rKQX19MQg1gO3ILiBCQ > > > qSLIvqovZLA95KKiyoVNzI%3D&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
> -----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&data=04%7 > C01% > > > > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e > > > > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU > > > > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > > > > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=X4rKQX19MQg1gO3ILiBCQ > > > > qSLIvqovZLA95KKiyoVNzI%3D&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&data=04%7C01%7Cming.qian%40nxp.com%7C > 64fa84ae1325497b2b4408d9cfd5aabd%7C686ea1d3bc2b4c6fa92cd99c5c301 > 635%7C0%7C1%7C637769338865335050%7CUnknown%7CTWFpbGZsb3d8e > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D% > 7C3000&sdata=ZCuu2FL5EusXxilM%2BkdpLjQDXnvVhFIXsKeIVHPKWHY% > 3D&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
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&data=04%7C01% > > > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e > > > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU > > > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6 > > > > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=X4rKQX19MQg1gO3ILiBCQ > > > > qSLIvqovZLA95KKiyoVNzI%3D&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.
> -----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&data=04% > 7C01% > > > > > > 7Cming.qian%40nxp.com%7Cea94a9c4cc0643b0a41f08d9ce0eadc5%7C686e > > > > > > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637767384703207931%7CU > > > > > > nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI > > > > > 6 > Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=X4rKQX19MQg1gO3ILiBCQ > > > > > qSLIvqovZLA95KKiyoVNzI%3D&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&data=04%7C01%7Cming.qian%40nxp.com%7Ce6b24805 > 32c6458d > > > 185b08d9d2ca2a81%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7 > C63777258 > > > 8014789133%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzI > > > iLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=RtRg%2Fu1ZggLSEf > lGvxy > > MutZv6Q%2B%2BX89O%2BE8DvXK0RHk%3D&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
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; } - 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; + ret = ioctl(ctx_to_m2mctx(ctx)->fd, cmd, &type); if (ret < 0) return AVERROR(errno);
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 | 52 ++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 20 deletions(-)