[FFmpeg-devel] [PATCH] Missing Theora 3.2.1 granule support

[Måns Rullgård]

Henrik Gulbrandsen <henrik at gulbra.net> writes:

> On Fri, 2008-04-18 at 09:27 +0100, M?ns Rullg?rd wrote:
>> Henrik Gulbrandsen <henrik at gulbra.net> writes:
>> 
>> > Given the modified granule semantics for Theora bitstreams, which was
>> > mentioned in my previous email, the theora_gptopts function needs to
>> > change if it should still map from granule positions to PTS when the
>> > bitstream version is 3.2.1 or higher.
>> >
>> > I haven't actually tested this on a new bitstream, but with some good
>> > code review and a little luck, this may actually work as intended :-)
>> >
>> > /Henrik
>> >
>> >
>> > Index: libavformat/oggparsetheora.c
>> > ===================================================================
>> > --- libavformat/oggparsetheora.c	(revision 12887)
>> > +++ libavformat/oggparsetheora.c	(working copy)
>> > @@ -29,6 +29,7 @@
>> >  #include "oggdec.h"
>> >  
>> >  typedef struct theora_params {
>> > +    unsigned version;
>> >      int gpshift;
>> >      int gpmask;
>> >  } theora_params_t;
>> > @@ -95,6 +96,7 @@ theora_header (AVFormatContext * s, int 
>> >          if (version >= 0x304000)
>> >              skip_bits(&gb, 2);
>> >  
>> > +        thp->version = version;
>> >          thp->gpshift = get_bits(&gb, 5);
>> >          thp->gpmask = (1 << thp->gpshift) - 1;
>> >  
>> > @@ -124,6 +126,11 @@ theora_gptopts(AVFormatContext *ctx, int
>> >      uint64_t iframe = gp >> thp->gpshift;
>> >      uint64_t pframe = gp & thp->gpmask;
>> >  
>> > +    // We want PTS (beginning of display interval)
>> > +    // rather than the count of frames seen so far.
>> > +    if (thp->version >= 0x030201 && iframe > 0)
>> > +        iframe--;
>> 
>> What if iframe == 0?
>
> For Theora bitstream versions >= 3.2.1, that means that the current
> packet doesn't contain any frames. It would probably be some kind of
> header packet. In any case, we don't want the PTS to go negative...

We also don't want two packets with the same PTS.

-- 
M?ns Rullg?rd
mans at mansr.com