[FFmpeg-devel] [PATCH] Incorrect Ogg Theora duration

Michael Niedermayer michaelni
Thu Jun 26 12:07:29 CEST 2008


On Sat, Apr 26, 2008 at 02:06:43PM +0200, Henrik Gulbrandsen wrote:
> 
> On Sat, 2008-04-26 at 12:52 +0100, M?ns Rullg?rd wrote:
> > Henrik Gulbrandsen <henrik at gulbra.net> writes:
> 
> [...]
> 
> > > Index: libavformat/oggdec.c
> > > ===================================================================
> > > --- libavformat/oggdec.c	(revision 12969)
> > > +++ libavformat/oggdec.c	(working copy)
> > > @@ -468,15 +468,34 @@ ogg_get_length (AVFormatContext * s)
> > >  
> > >      ogg->size = size;
> > >      ogg_restore (s, 0);
> > > +    if (idx == -1)
> > > +        return 0;
> > > +
> > >      ogg_save (s);
> > > +
> > > +    // Read the first stream packet if not already done
> > > +    if (idx != ogg->curidx) {
> > > +        while (!ogg_read_page (s, &i)) {
> > > +            if (i == idx && ogg->streams[i].granule != -1)
> > > +                break;
> > > +        }
> > > +    }
> > > +
> > > +    // Read the second stream packet
> > >      while (!ogg_read_page (s, &i)) {
> > >          if (i == idx && ogg->streams[i].granule != -1 && ogg->streams[i].granule != 0)
> > >              break;
> > >      }
> > > +
> > >      if (i == idx) {
> > > -        s->streams[idx]->start_time = ogg_gptopts (s, idx, ogg->streams[idx].granule);
> > > +        // Calulate start time assuming same duration for the two packets
> > 
> > That's one hell of an assumption.
> 
> ...but the best we can do without decoding the packet, if we're not
> allowed to rely on start_time being 0 in the first place...
> 
> > > +        ogg_stream_t *os = ogg->streams + idx;
> > > +        int64_t pkt2_pts = ogg_gptopts(s, idx, os->lastgp);
> > > +        int64_t duration = ogg_gptopts(s, idx, os->granule) - pkt2_pts;
> > > +        s->streams[idx]->start_time = FFMAX(0, pkt2_pts - duration);
> > >          s->streams[idx]->duration -= s->streams[idx]->start_time;
> > >      }
> > > +
> > >      ogg_restore (s, 0);
> > >  
> > >      return 0;
> > 
> > The Ogg spec actually mandates a start time of zero for all streams:
> 
> As far as I'm concerned, feel free to rip out this entire part of the
> code and set the start time to zero! That would solve all my problems.
> However, somebody went through a lot of effort to handle a case where
> it's apparently not guaranteed to be zero. I assume it involves audio
> extracted from somewhere in the middle of a streamed recording, but that
> is not explicitly stated anywhere. As long as this code is needed, I
> want it to also work correctly for the common case where start_time
> actually is zero, instead of always assuming non-zero as it does now.

I am also for riping the start_time calculation out,
mans? are you ok with removing the start_time guessing code?
The values it guesses are wrong and do not match the timestamp of the
first packet. And they happen to mess up stream copy as ffmpeg subtracts
the smallest start_time which leads to many packets with negative timestamps.

ill apply below in a few days if i hear no objections:

     ogg->size = size;
     ogg_restore (s, 0);
-    ogg_save (s);
-    while (!ogg_read_page (s, &i)) {
-        if (i == idx && ogg->streams[i].granule != -1 && ogg->streams[i].granule != 0)
-            break;
-    }
-    if (i == idx) {
-        s->streams[idx]->start_time = ogg_gptopts (s, idx, ogg->streams[idx].granule);
-        s->streams[idx]->duration -= s->streams[idx]->start_time;
-    }
-    ogg_restore (s, 0);

     return 0;


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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- 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/20080626/7e564cf7/attachment.pgp>



More information about the ffmpeg-devel mailing list