[FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

Michael Niedermayer michaelni at gmx.at
Sat Feb 9 00:37:44 EET 2019


On Fri, Feb 08, 2019 at 05:07:03PM -0300, James Almer wrote:
> On 2/8/2019 12:17 AM, Chris Cunningham wrote:
> > On Wed, Feb 6, 2019 at 6:03 PM James Almer <jamrial at gmail.com> wrote:
> > 
> >> Can a valid ogm stream contain more than one header packet?
> > 
> > 
> > Looking at ogg_packet oggdec.c, we read headers until hitting the first
> > non-header (counted in ogg stream field nb_headers), so multiple headers
> > per stream is probably ok.
> 
> Probably ok as in our code currently allows it? Because if the spec
> disallows it, then that should be changed.
> 
> The ogg demuxer currently looks for at most an specific amount of
> headers per stream. In general that means two, parameters and Vorbis
> comment metadata, and when the first non header packet is found it stops
> trying to read headers.
> 
> What i mean with more than one header packet is for example two or more
> headers of a given type in the bitstream. I'm fairly sure only one of
> each is expected, so parsing any of them a second time, which is what in
> this sample of yours is resulting in codec_id NONE being propagated, is
> probably not correct.
> 
> > But multiple codecs in a given stream seems
> > supsect to me. Someone with more ogg expertise should chime in.
> > 
> > Because the
> >> issue here as far as i can see is that ogm_header() is not validating
> >> the output of ff_codec_get_id() and is happily accepting and propagating
> >> AV_CODEC_ID_NONE as derived from it in a late packet, long after the
> >> parser and everything else was already initialized.
> >>
> >> No other ogg module sets st->internal->need_context_update and all of
> >> them may also end up calling the respective header reading function more
> >> than once on invalid files, but unlike the ogm one, all of them hardcode
> >> the codec_id instead of blindly accepting a potentially bogus value
> >> derived from the bitstream.
> >>
> > 
> > I'm open to hard-coding codec for gsm.
> 
> No, ogm can have all kinds of codecs, hence it being defined in the
> bitstream. It should nonetheless have a check to make sure what's read
> is not AV_CODEC_ID_NONE.
> 
> > One thing I notice is that the codec
> > is just one of several codecpar fields that are potentially changing.
> > If any of those change without need_context_update, it seems like values in
> > the internal avctx could become stale?
> 
> That's why i was wondering if more than one header was allowed. I'm
> fairly sure it's not, and the demuxer should ignore any further header
> packet if it contains a header of a type already parsed.

ogg allows chaining streams when they have differing serial numbers
https://xiph.org/ogg/doc/oggstream.html

i think ive seen actual files doing this

ogg_replace_stream() might assign these into existing avstreams i think

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190208/4b2c1e99/attachment.sig>


More information about the ffmpeg-devel mailing list