[FFmpeg-devel] [PATCH] lavc: reset codec on receiving packet after EOF in compat_decode

[James Cowgill]

Hi,

On 09/11/17 14:02, Hendrik Leppkes wrote:
> On Thu, Nov 9, 2017 at 1:21 PM, James Cowgill <jcowgill at debian.org> wrote:
>> In commit 061a0c14bb57 ("decode: restructure the core decoding code"), the
>> deprecated avcodec_decode_* APIs were reworked so that they called into the
>> new avcodec_send_packet / avcodec_receive_frame API. This had the side effect
>> of prohibiting sending new packets containing data after a drain
>> packet, but in previous versions of FFmpeg this "worked" and some
>> applications relied on it.
>>
>> To restore some compatibility, reset the codec if we receive a new non-drain
>> packet using the old API after draining has completed. While this does
>> not give the same behaviour as the old API did, in the majority of cases
>> it works and it does not require changes to any other part of the decoding
>> code.
>>
>> Fixes ticket #6775
>> Signed-off-by: James Cowgill <jcowgill at debian.org>
>> ---
>>  libavcodec/decode.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index 86fe5aef52..2f1932fa85 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -726,6 +726,11 @@ static int compat_decode(AVCodecContext *avctx, AVFrame *frame,
>>
>>      av_assert0(avci->compat_decode_consumed == 0);
>>
>> +    if (avci->draining_done && pkt && pkt->size != 0) {
>> +        av_log(avctx, AV_LOG_WARNING, "Got unexpected packet after EOF\n");
>> +        avcodec_flush_buffers(avctx);
>> +    }
>> +
> 
> I don't think this is a good idea. Draining and not flushing
> afterwards is a bug in the calling code, and even before recent
> changes it would result in inconsistent behavior and even crashes
> (with select decoders).

I am fully aware that this will only trigger if the calling code is
buggy. I am trying to avoid silent breakage of those applications doing
this when upgrading to ffmpeg 3.4.

I was looking at the documentation of avcodec_decode_* recently because
of this and I had some trouble deciding if using the API this way was
incorrect. I expect the downstreams affected thought that what they were
doing was fine and then got angry when ffmpeg suddenly "broke" their
code. This patch at least allows some sort of "transitional period"
until downstreams update.

>From the perspective of Debian, I could either apply this patch to
ffmpeg, or I would have to go through over 100 reverse dependencies to
see if they abuse the API and then fix them. I currently know of two
(gst-libav1.0 and kodi), but there could be more - especially within
less used packages.

Thanks,
James