[FFmpeg-devel] [FFmpeg-cvslog] mjpegdec: convert to receive_frame()

[James Almer]

On 12/10/2020 6:44 PM, Michael Niedermayer wrote:
> On Thu, Dec 10, 2020 at 09:11:44AM +0000, Anton Khirnov wrote:
>> ffmpeg | branch: master | Anton Khirnov <anton at khirnov.net> | Tue Dec  1 11:34:27 2020 +0100| [e9a2a8777317d91af658f774c68442ac4aa726ec] | committer: Anton Khirnov
>>
>> mjpegdec: convert to receive_frame()
>>
>> This will be useful in the following commit.
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=e9a2a8777317d91af658f774c68442ac4aa726ec
>> ---
>>
>>   libavcodec/jpeglsdec.c |  5 ++--
>>   libavcodec/mjpegdec.c  | 77 ++++++++++++++++++++++++++++++++++++--------------
>>   libavcodec/mjpegdec.h  |  8 ++++--
>>   libavcodec/sp5xdec.c   | 34 +++++++++++-----------
>>   4 files changed, 80 insertions(+), 44 deletions(-)
> 
> This breaks:
> ./ffmpeg -i ~/tickets/2655/48-06-28_FrankChandlerReturns-14.opus -y file.bmp
> 
> after this commit no make bmp file is written
> 
> similar happens with pictures in mp3
> 
> thx

frame->pts is AV_NOPTS_VALUE, whereas frame->dts is 0 (flac_picture.c in 
lavf doesn't set any timestamps for the attached pic, though). On 
decoders using the AVCodec->decode() API, decode_simple_internal() would 
set frame->best_effort_timestamp to either pts or dts as chosen by 
guess_correct_pts(). In this case, it would choose the dts one.

The following change

> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 5a1849f944..3a07eb7155 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -572,6 +572,9 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>          ret = avctx->codec->receive_frame(avctx, frame);
>          if (ret != AVERROR(EAGAIN))
>              av_packet_unref(avci->last_pkt_props);
> +        frame->best_effort_timestamp = guess_correct_pts(avctx,
> +                                                         frame->pts,
> +                                                         frame->pkt_dts);
>      } else
>          ret = decode_simple_receive_frame(avctx, frame);

Seems to fix the issue, meaning my suggestion to call 
guess_correct_pts() on AVCodec->receive_frame() decoders to set 
best_effort_timestamp was probably a better idea than simply setting it 
to pts within the decoder itself.

There's in fact a lot of logic in decode_simple_internal() that may have 
to be shared with AVCodec->receive_frame() decoders sooner or later. 
Especially audio ones.