[FFmpeg-devel] [PATCH 1/5] vorbis: handle special packets in the middle of a stream

[Ben Boeckel]

On Sat, Oct 19, 2013 at 21:15:10 +0200, Michael Niedermayer wrote:
> On Sat, Oct 19, 2013 at 09:40:42AM -0400, Ben Boeckel wrote:
> >  int avpriv_vorbis_parse_frame(VorbisParseContext *s, const uint8_t *buf,
> > -                              int buf_size)
> > +                              int buf_size, int *flags)
> >  {
> 
> breaks ABI between libs
> you can neither remove nor change a function that is used from outside
> the lib that you change. This affects all functions with name starting
> with "av"

Would avpriv_vorbis_parse_frame_flags work? (Then just reimplement the
old name as calling the new one with NULL).

> this looks quite wrong
> vorbis_header() can change codec values like channels and extradata
> these values can be set initially by the demuxer but not changed
> afterwards in that way as the decoder can be run in a seperate thread
> and using them and there can be a packet que between demuxer and
> decoder.
> Also the demuxer already passes these packets to the decoder which
> should update extradata accordingly

Ok. I'll refactor out '\003vorbis' handling and make that get called
from here and vorbis_header().

> I see some subsequent patch from you replaces the vorbis_header()
> calls again. I dont know if that solves the problem or not as its
> hard to follow what gets replaced and whats there then.
> Possibly you could split the changes in a more logic way so theres
> no code added that just gets removed or replaced later
> also each patch should generally only touch one lib, this reduces
> the chances of breaking ABI

Will do.

wm4 mentioned that another side_data type should be used instead of
riding on the current one which is used for AVFrame metadata currently.
Would something like AV_PKT_METADATA_UPDATE work (with the same data
format as the current one)

I'll probably get a chance to take another stab this weekend at the
earliest.

--Ben