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

[Henrik Gulbrandsen]

On Tue, 2008-04-22 at 19:56 +0100, M?ns Rullg?rd wrote:
> Henrik Gulbrandsen <henrik at gulbra.net> writes:
> 
> > On Sun, 2008-04-20 at 13:28 +0100, M?ns Rullg?rd wrote:
> >> Henrik Gulbrandsen <henrik at gulbra.net> writes:
> >> 
> >> > On Fri, 2008-04-18 at 00:45 -0700, Baptiste Coudurier wrote: 
> >> >> I think the "start" value is incorrect. According to specs, ogg
> >> >> granule stores the pts plus the duration of the ogg packet, so last
> >> >> packet granule is the stream duration.
> >> >
> >> > As mentioned previously, Ogg Theora bitstreams prior to version 3.2.1
> >> > don't follow this rule. Since libtheora uses 3.2.0 for versions before
> >> > 1.0beta1, I think there is a good case for being backwards compatible.
> >> 
> >> How many such files exist in the wild?  Given that I've never seen a
> >> wild Theora file, other than Xiph promotional stuff, I reckon there
> >> can't be that many.
> >
> > Well, more to the point: My test system happens to have libtheora alpha7
> 
> That's a very bad argument.

It's a statement of the facts. What is there to argue about? Surely,
you're not suggesting that the bug should be left unfixed just because
it's minor and only occurs under special circumstances?

[...]

> >> I still don't understand.  Why do you seek back in the file after
> >> reading the headers?  It doesn't make any sense to me.
> >
> > Have a look at ogg_get_headers()! We're looping over an unknown number
> > of header packets for each stream and must hand them over to the codec
> > to figure out if they are actually headers or not. This means that we
> > are not only reading the headers, but also the first data packet, which
> > is currently discarded. I just want it back in stream for later reading.
> 
> Read the code again.  The packet stays in the buffer and is returned
> by the next call to ogg_packet().

I must suffer from brain atrophy! Thanks! In that case, the core issue
is that the 5814 check-in didn't take this into account. It assumes that
the next page read will belong to the first data packet. Or at least, I
think that's the assumption. It looks like whoever wrote this was happy
with a quick approximation anyway. I was hoping for a minimal update to
the code, but... I hope the attached version is a reasonable approach.

/Henrik

-------------- next part --------------
A non-text attachment was scrubbed...
Name: theora_granule_3.patch
Type: text/x-patch
Size: 968 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080426/0a3fab24/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: theora_duration_3.patch
Type: text/x-patch
Size: 1330 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080426/0a3fab24/attachment-0001.bin>