[Vorbis-dev] possible libogg bug holding up Ogg FLAC

[Josh Coalson]

I wish I would have come across this in time for the libogg-1.1.1
release... Maybe I'm doing something wrong but here it is.

One FLAC compressed frame becomes one packet when encapsulated in
Ogg, and FLAC packets can be much larger than the nominal 4k page
size.  For CD audio they are usually 10-15Kbytes.  Imagine this
Ogg stream where the lines denote page boundaries and the x's
are one big FLAC packet (preceding and following packets not
shown):

  5            6             7             8
 +------------+-------------+-------------+------------+
 |      xxxxxx|xxxxxxxxxxxxx|xxxxxxxxxxxxx|xxxxxxx     |

            ^
            |


Say I reset the sync/stream states and fseek to where the
arrow is.  Then I ogg_sync_pageout() and fill the sync
buffer until it syncs on page 6 and ogg_sync_pageout()
returns > 0.

Then I ogg_stream_pagein().  It recognizes that a reset
happened -- "if(pageno!=os->pageno){" -- then inside checks
the 'continued' flag and eats up all the segments of the
packet in page 6, then 'bodysize' is 0 and the function
returns.

Then I ogg_stream_packetout() and it returns 0 because there
is no packet data at all.

Then I ogg_sync_pageout() and ogg_stream_pagein() again to
get page 7.  But now, the logic in ogg_stream_pagein() that
eats up continued packet segments does not get triggered
because:

  if(pageno!=os->pageno){

is false.  So then bodysize is not 0 like it should be and
it gets treated as the beginning of a real packet.

Is this a bug?  It seems that ogg_stream_pagein() needs a
longer view of when it is continuing big packets, and it's
never come up because no one has put such big packets in
Ogg and then seeked to them just right.

I made a little change that added a flag to ogg_stream_state
that keeps track of it that seems to work.  But I don't know
if it's practical to mess with a non-opaque data structure
in a reasonably stable API.  But if this is OK I can submit a
patch.

It also just occurred to me that maybe a simpler fix is to
just not set os->pageno at the end of the function if it's
continuing a long packet, but I'm not sure of the wider
side-effects of that.

I was hours away from doing a FLAC release with much better
Ogg FLAC support (and first offical mapping, thanks Arc!)
when I ran into this.

The last FLAC release is 20 months old and it really needs a
new one.  If this fix is something that can be quickly rolled
into a libogg-1.1.2 that would be great, otherwise maybe I
would just turn off Ogg FLAC seeking until then.

Sorry for the long post, hope it was clear enough.

Josh



		
_______________________________
Do you Yahoo!?
Declare Yourself - Register online to vote today!
http://vote.yahoo.com