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

[wm4]

On Tue, 4 Jul 2017 20:20:38 +0200
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On 04.07.2017, at 10:42, wm4 <nfxjfg at googlemail.com> wrote:
> > Not really comfortable with the current patch. Why does it even touch
> > the compat_decode_partial_size handling path?  
> 
> Because that is the code that gets utterly confused by 0-size packets and breaks everything.
> For ac3 what happens is that after the first 0-sized packets all following packets fail to decode.

0-sized packets do not exist in AC3 in particular and have no meaning.

> Probably that means the whole partial_size thing is broken and not robust, but I haven't been able to really understand it.

This is for the mechanism where you passed a packet to the decode API,
and it only decoded a few bytes of it, and you had to feed the rest of
the packet to the decoder again next time to get all audio. (Plus
confusing special cases about some decoders consuming 0 bytes in the
first call, or consuming more bytes than what was provided as input...)

The new API doesn't have that, and will just require multiple receive
calls in those cases.

> > If you really want this, then I think you should go all the way and
> > implement the implicit drain-and-flush behavior that the old API
> > exposed with most decoders. (Meaning you can drain the decoder, and
> > then it's implicitly flushed, so that sending new packets will restart
> > decoding.)  
> 
> Yeah, I just don't feel particularly confident implementing that, since this compat wrapper has all kinds of weird code and no comments...

Shouldn't be too hard if you understand the old API. The new API is
well-documented.

> > Either that, or skip empty packets on the API user side. If a
> > libavformat demuxer or libavcodec parser is producing empty packets,
> > these should probably be fixed.  
> 
> Skipping it on user side is of course what I did, but it doesn't provide compatibility.
> Any distribution e.g. updating FFmpeg more often than MPlayer will break.

Well, it was broken before anyway. Decoding 0 sized packets usually
will start draining the decoder, and sending new data after that
(without flush) resulted in arbitrary behavior.