On Sun, Jan 15, 2017 at 5:35 PM, Michael Niedermayer <[email protected]> wrote:
> > --- a/libavformat/oggparsetheora.c > > +++ b/libavformat/oggparsetheora.c > > @@ -153,6 +153,10 @@ static uint64_t theora_gptopts(AVFormatContext > *ctx, int idx, uint64_t gp, > > if (!thp) > > return AV_NOPTS_VALUE; > > > > + // Fix for broken files; negative granulepos is invalid. > > Please add a reference to the spec and section that says this > > is this documented in some specification about ogg or theora ? > I cannot find any definite statment that granulepos is a signed > field, just that -1 as in twos compliment is a special value for it. > and that its interpretation is left to the codec, but i knew that > already. > > is something declaring the highest bit to be 0 for all frames of > all codecs in ogg ? > Monty says negative values other than the special -1 are forbidden, but we're still double-checking the spec to make sure we can source that correctly... :) In libogg's ogg_packet type, granulepos is stored and exposed as a signed ogg_int64_t: https://xiph.org/ogg/doc/libogg/ogg_packet.html Ogg Vorbis encoding does explicitly exclude other negative values for granulepos: https://www.xiph.org/vorbis/doc/Vorbis_I_spec.html#x1-132000A.2 The general bitstream documentation at https://xiph.org/vorbis/doc/framing.html only mentions -1 as a special value, but isn't clear on the meaning of the rest of the values... Further general bitstream documentation at https://www.xiph.org/ogg/doc/oggstream.html indicates that "Packets and pages must be arranged in ascending granule-position and time order", in which case a legitimate value of 0xffff'ffff'ffff'fffe (-2) would be wildly out of place; affected parts of the input file look roughly like: packet 2633: granulepos 2600<<6 + 33 packet 2634: granulepos -2 packet 2635: granulepos 2600<<6 + 35 Theora's granulepos combines a frame count for keyframe, bit-shifted, with a count of frames since the keyframe. This can be decoded into a total frame count, which increases by one on every frame, as well as the GP value's always-positive change. (See https://www.theora.org/doc/Theora.pdf section A.2.3) So I've got a bad input file one way or another -- either the -2 value is violating the spec by its existence, or it's violating the spec by not updating the total frame count by one. > > orthogonal to this, timestamps half as large would still cause problems > if software has problems with huge timestamps > We should reject invalid granulepos, if this is invalid but i think > this is not a sufficient fix if the problem is frame duplication > from large timestamp differences > *nod* similar problems would occur with an out of place large positive granulepos, which would still be invalid due to being out of place even if it's a valid number. But I've only seen it with -1s (expected) and this file with -2 (unexpected and seemingly invalid) so far. One further possibility is to validate Theora granulepos values against the previous frame's granulepos; we should always see the _decoded_ granulepos advance by 1 from frame to frame unless we're seeking. (Theora handles extra space between frames by encoding a special empty "duplicate" frame packet to take up space in the frame sequence.) -- brion > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > No snowflake in an avalanche ever feels responsible. -- Voltaire > > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
