[FFmpeg-devel] [PATCH 5/5] avcodec/mpeg4videodec: duplicate the last decoded frame when the last coded frame was skipped

[James Almer]

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