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

[Michael Niedermayer]

On Wed, Jun 25, 2008 at 11:26:18AM -0700, Baptiste Coudurier wrote:
> 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.

The vorbis way causes the first few frames to disapear somewhere and
pages of "vp3: first frame not a keyframe" to be printed.

I see no such issues with my patch.

Note that is ogg->nut stream copy

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080625/1b5ca4e2/attachment.pgp>