| Message ID | 20221204215227.4186-5-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 261cd929e06b716b3bcc944c96fdf94371b3a7ed |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/binkaudio: clear pts when returning more than one frame per input packet | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
James Almer: > This ensures the video stream duration is not lost after decoding. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/h263dec.c | 13 +++++++++++++ > libavcodec/mpegvideo.h | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c > index ac7a8521e5..0a2d7487a8 100644 > --- a/libavcodec/h263dec.c > +++ b/libavcodec/h263dec.c > @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict, > return ret; > s->next_picture_ptr = NULL; > > + *got_frame = 1; > + } else if (s->skipped_last_frame && s->current_picture_ptr) { > + /* Output the last picture we decoded again if the stream ended with > + * an NVOP */ > + if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0) > + return ret; > + /* Copy props from the last input packet. Otherwise, props from the last > + * returned picture would be reused */ > + if ((ret = ff_decode_frame_props(avctx, pict)) < 0) > + return ret; > + s->current_picture_ptr = NULL; > + > *got_frame = 1; > } > > @@ -500,6 +512,7 @@ retry: > s->extradata_parsed = 1; > } > ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0); > + s->skipped_last_frame = (ret == FRAME_SKIPPED); > } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) { > ret = ff_intel_h263_decode_picture_header(s); > } else if (CONFIG_FLV_DECODER && s->h263_flv) { > diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h > index 6440b906b1..42275953b9 100644 > --- a/libavcodec/mpegvideo.h > +++ b/libavcodec/mpegvideo.h > @@ -175,6 +175,7 @@ typedef struct MpegEncContext { > Picture *last_picture_ptr; ///< pointer to the previous picture. > Picture *next_picture_ptr; ///< pointer to the next picture (for bidir pred) > Picture *current_picture_ptr; ///< pointer to the current picture > + int skipped_last_frame; > int last_dc[3]; ///< last DC values for MPEG-1 > int16_t *dc_val_base; > int16_t *dc_val[3]; ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous Can you give an example where this matters? And what does the spec say about this? Is "output the last frame again" really the appropriate response upon encountering a NVOP? - Andreas
On 12/6/2022 6:17 AM, Andreas Rheinhardt wrote: > James Almer: >> This ensures the video stream duration is not lost after decoding. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/h263dec.c | 13 +++++++++++++ >> libavcodec/mpegvideo.h | 1 + >> 2 files changed, 14 insertions(+) >> >> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c >> index ac7a8521e5..0a2d7487a8 100644 >> --- a/libavcodec/h263dec.c >> +++ b/libavcodec/h263dec.c >> @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict, >> return ret; >> s->next_picture_ptr = NULL; >> >> + *got_frame = 1; >> + } else if (s->skipped_last_frame && s->current_picture_ptr) { >> + /* Output the last picture we decoded again if the stream ended with >> + * an NVOP */ >> + if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0) >> + return ret; >> + /* Copy props from the last input packet. Otherwise, props from the last >> + * returned picture would be reused */ >> + if ((ret = ff_decode_frame_props(avctx, pict)) < 0) >> + return ret; >> + s->current_picture_ptr = NULL; >> + >> *got_frame = 1; >> } >> >> @@ -500,6 +512,7 @@ retry: >> s->extradata_parsed = 1; >> } >> ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0); >> + s->skipped_last_frame = (ret == FRAME_SKIPPED); >> } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) { >> ret = ff_intel_h263_decode_picture_header(s); >> } else if (CONFIG_FLV_DECODER && s->h263_flv) { >> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h >> index 6440b906b1..42275953b9 100644 >> --- a/libavcodec/mpegvideo.h >> +++ b/libavcodec/mpegvideo.h >> @@ -175,6 +175,7 @@ typedef struct MpegEncContext { >> Picture *last_picture_ptr; ///< pointer to the previous picture. >> Picture *next_picture_ptr; ///< pointer to the next picture (for bidir pred) >> Picture *current_picture_ptr; ///< pointer to the current picture >> + int skipped_last_frame; >> int last_dc[3]; ///< last DC values for MPEG-1 >> int16_t *dc_val_base; >> int16_t *dc_val[3]; ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous > > Can you give an example where this matters? And what does the spec say > about this? Is "output the last frame again" really the appropriate > response upon encountering a NVOP? > The reference decoder returns a frame for every packet, nvop or otherwise. If nvop, it will be the last decoded frame. Doing that here was rejected some years ago as it would make the decoder unconditionally cfr, and that's undesired. Say you have a video stream that's 6 minutes and 30 seconds long, where the last thirty seconds worth of packets are NVOPs. The decoder will consume them and generate nothing, as expected, but then at EOF the last returned frame was that with a pts at the 6 minutes mark, resulting in the decoded stream being 6 minutes, and as such stream length information is lost. By returning the last frame again at the end with the last input packet's pts, you will get the correct stream length with no difference in video output while remaining vfr.
On 12/4/2022 6:52 PM, James Almer wrote: > This ensures the video stream duration is not lost after decoding. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/h263dec.c | 13 +++++++++++++ > libavcodec/mpegvideo.h | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c > index ac7a8521e5..0a2d7487a8 100644 > --- a/libavcodec/h263dec.c > +++ b/libavcodec/h263dec.c > @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict, > return ret; > s->next_picture_ptr = NULL; > > + *got_frame = 1; > + } else if (s->skipped_last_frame && s->current_picture_ptr) { > + /* Output the last picture we decoded again if the stream ended with > + * an NVOP */ > + if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0) > + return ret; > + /* Copy props from the last input packet. Otherwise, props from the last > + * returned picture would be reused */ > + if ((ret = ff_decode_frame_props(avctx, pict)) < 0) > + return ret; > + s->current_picture_ptr = NULL; > + > *got_frame = 1; > } > > @@ -500,6 +512,7 @@ retry: > s->extradata_parsed = 1; > } > ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0); > + s->skipped_last_frame = (ret == FRAME_SKIPPED); > } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) { > ret = ff_intel_h263_decode_picture_header(s); > } else if (CONFIG_FLV_DECODER && s->h263_flv) { > diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h > index 6440b906b1..42275953b9 100644 > --- a/libavcodec/mpegvideo.h > +++ b/libavcodec/mpegvideo.h > @@ -175,6 +175,7 @@ typedef struct MpegEncContext { > Picture *last_picture_ptr; ///< pointer to the previous picture. > Picture *next_picture_ptr; ///< pointer to the next picture (for bidir pred) > Picture *current_picture_ptr; ///< pointer to the current picture > + int skipped_last_frame; > int last_dc[3]; ///< last DC values for MPEG-1 > int16_t *dc_val_base; > int16_t *dc_val[3]; ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous Will apply.
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c index ac7a8521e5..0a2d7487a8 100644 --- a/libavcodec/h263dec.c +++ b/libavcodec/h263dec.c @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict, return ret; s->next_picture_ptr = NULL; + *got_frame = 1; + } else if (s->skipped_last_frame && s->current_picture_ptr) { + /* Output the last picture we decoded again if the stream ended with + * an NVOP */ + if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0) + return ret; + /* Copy props from the last input packet. Otherwise, props from the last + * returned picture would be reused */ + if ((ret = ff_decode_frame_props(avctx, pict)) < 0) + return ret; + s->current_picture_ptr = NULL; + *got_frame = 1; } @@ -500,6 +512,7 @@ retry: s->extradata_parsed = 1; } ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0); + s->skipped_last_frame = (ret == FRAME_SKIPPED); } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) { ret = ff_intel_h263_decode_picture_header(s); } else if (CONFIG_FLV_DECODER && s->h263_flv) { diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h index 6440b906b1..42275953b9 100644 --- a/libavcodec/mpegvideo.h +++ b/libavcodec/mpegvideo.h @@ -175,6 +175,7 @@ typedef struct MpegEncContext { Picture *last_picture_ptr; ///< pointer to the previous picture. Picture *next_picture_ptr; ///< pointer to the next picture (for bidir pred) Picture *current_picture_ptr; ///< pointer to the current picture + int skipped_last_frame; int last_dc[3]; ///< last DC values for MPEG-1 int16_t *dc_val_base; int16_t *dc_val[3]; ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous
This ensures the video stream duration is not lost after decoding. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/h263dec.c | 13 +++++++++++++ libavcodec/mpegvideo.h | 1 + 2 files changed, 14 insertions(+)