stb_vorbis fails to handle ogg's containing skeleton metadata

[kcgen]

The library fails to open vorbis stream that contain ogg skeleton metadata (reference: https://xiph.org/ogg/doc/skeleton.html).

Specifically because the following check fails (as do all subsequent checks):

if (f->segments[0] != 30) return error(f, VORBIS_invalid_first_page);

Example vorbis files with and without skeleton metadata are attached, which comply with the specification.

• libVorbis-1_3_6__libOgg-1_3_3-no_skeleton.ogg.gz
• libVorbis-1_3_6__libOgg-1_3_3-with_skeleton.ogg.gz

To reproduce it, encode a wav file with oggenc -k audio.wav or use the latest version(s) of OggDrop and toggle skeleton stream as shown:

I think the best approach would be include enough code just to sanely skip over it:

1. Detect the Ogg Skeleton's fishead\0 identifier in the first segment:

00000000  4f 67 67 53 00 02 00 00  00 00 00 00 00 00 6f 00  |OggS..........o.|
00000010  00 00 00 00 00 00 b6 13  29 e2 01 40 66 69 73 68  |........)..@fish|
00000020  65 61 64 00 03 00 00 00  00 00 00 00 00 00 00 00  |ead.............|
00000030  e8 03 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000050  00 00 00 00 00 00 00 00  00 00 00 00 4f 67 67 53  |............OggS|
00000060  00 02 00 00 00 00 00 00  00 00 68 00 00 00 00 00  |..........h.....|
00000070  00 00 a0 be 0b 2e 01 1e  01 76 6f 72 62 69 73 00  |.........vorbis.|

1. If it's present, handle and skip subsequent fishbone\0 secondary segments, which have no actual content packets.

[nothings]

Is there a reason we should support Ogg Skeleton at all?

I mean, to be clear, stb_vorbis implements the specification on this page: https://xiph.org/vorbis/doc/

None of those documents (as far as I can recall) make any mention of Ogg Skeleton.

[kcgen]

Given it's a valid Ogg Vorbis file format and stb_vorbis is a compliant parser/decoder, I think the first order is to not fail when fed these streams.

[kcgen]

"None of those documents (as far as I can recall) make any mention of Ogg Skeleton."

The issue is Vorbis held in an container Ogg - so it's ultimately "Ogg" that stb_vorbis parses.
The folks at Xiph have bubbled-up common Ogg format documentation here:

https://xiph.org/ogg/

Technical documentation on the Ogg container is available for the following areas:

• General structural overview
• Detailed header and parsing specification
• Issues and practices for multiplexed Ogg streams
• The Ogg Skeleton Metadata Bitstream

[nothings]

Given it's a valid Ogg Vorbis file format and stb_vorbis is a compliant parser/decoder, I think the first order is to not fail when fed these streams.

stb_vorbis doesn't support ogg vorbis files containing a Floor0 encoding. So no, stb_vorbis doesn't parse all ogg vorbis files anyway. (There's a list of things that aren't supported at the top of the file.)

I'll accept a pull request that fixes this, but I don't think it's worth my effort to add support for unimportant extensions that Xiph adds to Ogg over the years. People can just avoid putting Ogg Skeleton metadata into their files. (Unless someone wants to tell me why it's beneficial.)

[DanielGibson]

as #678 has been merged I think this can be closed?