[Vorbis] metadata

[Ian Malone]

Ralph Giles wrote:
> On Tue, Oct 11, 2005 at 03:56:55PM +0100, Ian Malone wrote:
> 
> 
>>From what I can see in vorbisfile, ( while(i<3){ogg_stream_pagein...
>>in _fetch_headers) the problem arises in the failure to push a
>>page into the vorbis stream (because of the different serial number).
> 
> 
> Hmm. So ogg_stream_pagein() just returns an error if it's called with
> an ogg_page that doesn't match the serial number in the stream struct.
> The code in vorbisfile doesn't check that return code, so it should just
> be silently discarding the xml pages.
> 
> So I guess I don't know why it fails when the second XML page comes 
> immediately after the bos. The only increments the header count when
> ogg_stream_packetout() returns another packet, so it should keep pulling
> data until it has three packets from the first logical stream.
> 

I've just looked at vorbisfile.c, vorbisfile_example and the vorbisgain
source.  ov_read returns OV_EBADLINK when it encounters the XML page
(not the bos page though; it has already been eaten when reading
headers) because, as far as I can see:
_fetch_and_process_packet sees a change in the serial number and calls
_decode clear.  This leads to _fetch_and_process_packet entering
if(vf->ready_state!=INITSET), where it realises the serial number
doesn't match a known link.

Vorbisgain copies the returned PCM address without checking the return
value of ov_read first (a bug).  This sometimes leads to a segfault,
but not always: UB, seemingly irrelevant modifications change behaviour.
What happens with Winamp and the directshow filters I don't know.

When there are a few vorbis pages between the bos pages and the XML page
vorbisfile_example produces an output which is glitched near the start
(as does oggdec, which complains of a hole in the data with or without
this 'padding').  It looks like this is because of the _decode_clear
call (we don't get STREAMSET till we come round again to the next page,
I don't know what effect that has but it does something).

The following works for non-seekable and (presumably) non-chained
seekable:
--- vorbis/lib/vorbisfile.c     (revision 10174)
+++ vorbis/lib/vorbisfile.c     (working copy)
@@ -555,7 +555,7 @@

        /* has our decoding just traversed a bitstream boundary? */
        if(vf->ready_state==INITSET){
-       if(vf->current_serialno!=ogg_page_serialno(&og)){
+       if(vf->current_serialno!=ogg_page_serialno(&og) && 
ogg_page_bos(&og)){
           if(!spanp)
             return(OV_EOF);

Fixing chained seekable probably requires keeping the serial numbers
in each link, and replacing the above serial number comparisson and
the one in the bisection search with comparisons against that list.

-- 
imalone