[FFmpeg-devel] [PATCH] decode.c: Handle 0-size packets in compat_decode

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Jul 4 01:03:19 EEST 2017


On 03.07.2017, at 21:07, wm4 <nfxjfg at googlemail.com> wrote:

> On Mon,  3 Jul 2017 20:57:21 +0200
> Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> 
>> The old API did that just fine, and if we provide
>> a compatibility layer it should at least be compatible.
>> For the test-case (feeding AVParser output directly to
>> decoder, failing to discard 0-size packets) just discarding
>> 0-size packets at the start works, but not sure if in some
>> cases we might want to pass them on to e.g. allow retrieving
>> additional pending frames.
>> ---
>> libavcodec/decode.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index 052f93d82f..c63090f137 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -842,6 +842,7 @@ static int compat_decode(AVCodecContext *avctx, AVFrame *frame,
>>     avci->compat_decode = 1;
>> 
>>     if (avci->compat_decode_partial_size > 0 &&
>> +        pkt->size &&
>>         avci->compat_decode_partial_size != pkt->size) {
>>         av_log(avctx, AV_LOG_ERROR,
>>                "Got unexpected packet size after a partial decode\n");
>> @@ -902,7 +903,10 @@ finish:
>>             ret = FFMIN(avci->compat_decode_consumed, pkt->size);
>>     }
>>     avci->compat_decode_consumed = 0;
>> -    avci->compat_decode_partial_size = (ret >= 0) ? pkt->size - ret : 0;
>> +    // for compatibility with old API behaviour handle
>> +    // 0-size specially
>> +    if (pkt->size)
>> +        avci->compat_decode_partial_size = (ret >= 0) ? pkt->size - ret : 0;
>> 
>>     return ret;
>> }
> 
> I thought it was decided that 0-sized packets are always disallowed for
> encoding?

This function is for decoding, I have not checked encoding side...

> Also, 0-sized packets are drain packets even with the old
> API, and thus could never properly work. You just relied in implicit
> restart behavior (i.e. leaving the drained state by feeding a new
> non-0-sized packet), which also didn't work with all decoders.

I don't know, and I don't really care, I just noticed that we have a huge compatibility wrapper function that isn't even compatible, which is a bit ridiculous.
So I proposed to make it at least more compatible.
I don't mind changing it to be simpler and just discard them right at the start, if you think that makes more sense. Though even if there's just one decoder where 0-sized packets had some working practical use it might be better to pass them through...
Note that I don't disagree that this was probably a bug that 0-sized packets got in (though I didn't re-check the parser API), just saying I do not see much reason to not be backwards-compatible.
I know for sure that the ac3 and mp3 audio codecs at least had no issues with 0-size packets before.


More information about the ffmpeg-devel mailing list