[FFmpeg-devel] [PATCH] issue251, xvid within .ogm will not remux to .avi try2

[Baptiste Coudurier]

Hi,

Michael Niedermayer wrote:
> On Wed, Jun 25, 2008 at 02:23:00PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer wrote:
>>> On Wed, Jun 25, 2008 at 12:44:59PM +0100, M?ns Rullg?rd wrote:
>>>> Michael Niedermayer wrote:
>>>>> Hi
>>>>>
>>>>> This patch fixes "according to spec" timestamp association for theora
>>>>> it also as a sideeffect fixes the file from issue 251
>>>>>
>>>>> quote of the theora spec: (A.2.2)
>>>>>    Frame data pages MUST be marked with a granule index corresponding to
>>>>> the display time of the last frame/packet that finishes in that page.
>>>>>
>>>>> Note ive not tested this extensively, i thought our users can do this
>>>> better
>>>>> Ill apply this in a few days if i hear no objections
>>>>>
>>>>>
>>>>> Index: libavformat/oggdec.c
>>>>> ===================================================================
>>>>> --- libavformat/oggdec.c	(revision 13799)
>>>>> +++ libavformat/oggdec.c	(working copy)
>>>>> @@ -523,11 +523,25 @@
>>>>>          return AVERROR(EIO);
>>>>>      pkt->stream_index = idx;
>>>>>      memcpy (pkt->data, os->buf + pstart, psize);
>>>>> +    if (s->streams[idx]->codec->codec_id == CODEC_ID_VORBIS){
>>>>>      if (os->lastgp != -1LL){
>>>>>          pkt->pts = ogg_gptopts (s, idx, os->lastgp);
>>>>>          os->lastgp = -1;
>>>>>      }
>>>>> +    }else{
>>>>> +        int segp= os->segp;
>>>>> +        int nsegs=os->nsegs;
>>>>> +        while (segp < nsegs){
>>>>> +            if (os->segments[segp] < 255)
>>>>> +                break;
>>>>> +            segp++;
>>>>> +        }
>>>>>
>>>>> +        if (segp == nsegs && os->granule != -1LL){
>>>>> +            pkt->pts = ogg_gptopts (s, idx, os->granule);
>>>>> +        }
>>>>> +    }
>>>>> +
>>>>>      pkt->flags = os->pflags;
>>>> Can you please explain a little more why this change is needed, and
>>>> why it doesn't apply to Vorbis?
>>> As already quoted theora says:
>>>>> (A.2.2)
>>>>>    Frame data pages MUST be marked with a granule index corresponding to
>>>>> the display time of the last frame/packet that finishes in that page.
>>> vorbis (http://www.xiph.org/vorbis/doc/framing.html) says:
>>> --------
>>> absolute granule position
>>>
>>> (This is packed in the same way the rest of Ogg data is packed; LSb of LSB
>>> first. Note that the 'position' data specifies a 'sample' number (eg, in a
>>> CD quality sample is four octets, 16 bits for left and 16 bits for right;
>>> in video it would likely be the frame number. It is up to the specific codec
>>> in use to define the semantic meaning of the granule position value). The
>>> position specified is the total samples encoded after including all packets
>>> finished on this page (packets begun on this page but continuing on to the
>>> next page do not count). The rationale here is that the position specified
>>> in the frame header of the last page tells how long the data coded by the
>>> bitstream is. A truncated stream will still return the proper number of
>>> samples that can be decoded fully.
>>>
>>> A special value of '-1' (in two's complement) indicates that no packets
>>> finish on this page.
>>> --------
>>>
>>> Thus with theora
>>>
>>> page0       page1
>>> AAABB       BBCCC
>>>
>>> with theora the last frame finishing on page0 is A thus the granule pos from
>>> page0 applies to A and for page1 its B
>>>
>>> Iam not 100% certain what is correct for vorbis (iam still working on getting
>>> timestamps consistant with the number of samples returned by the decoder)
>>>
>>> But the vorbis spec sounds like one should associate B with page0 and
>>> C or D (when C is completed on page1) with page1
>> The way I read it, both "specs" are saying the same thing: that the
>> timestamp on a page refers to the last packet ending on that page.
>>
>> It is quite possible that the code is wrong, but I don't think we need
>> special cases for these two codecs.
> 
> After various bugfixes, i now get timestamps which are consistant in
> relation to the number of samples output by the decoder with oggdec.c
> svn and vorbis. The theora timestamp code does break vorbis timestamps.
> 

Does it work with both old way and new way to handle theora granule ?
IIRC it depends on bitstream version, new way granule start at 1 while
old way start at 0.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA