Repository navigation
feat(Ogg): add Ogg Opus playback support - #1
Closed
hischampion wants to merge 3 commits into
Closed
hischampion wants to merge 3 commits into
hischampion wants to merge 3 commits into
Conversation
added 3 commits
August 25, 2026 14:54
AudioFileStreamProcessor routed every kAudioFileOggType stream to the libvorbisfile processor. Ogg is a container, not a codec, and audio/ogg is shared by Vorbis, Opus, Speex and FLAC-in-Ogg, so Opus reached libvorbisfile and was rejected. Navidrome, for one, serves Opus transcodes as audio/ogg. Codec selection cannot happen at openFileStream time, because both the file hint and the Content-Type say only "Ogg". It now happens on first data, by reading the first Ogg page. - OggVorbisStreamProcessor becomes OggStreamProcessor driving an injected OggAudioDecoder. The renderer loop, ring-buffer waits and packet accounting are unchanged; only decoder references were renamed. - OggCodecSniffer reads the first page to tell OpusHead from \x01vorbis, accounting for the lacing table when locating the payload. - OpusFileBridge wraps libopusfile, mirroring VorbisFileBridge. - Unsupported Ogg codecs now report kAudioFileStreamError_UnsupportedDataFormat instead of failing silently inside libvorbisfile. Two details worth flagging for review: Opus always decodes at 48 kHz. OpusHead.input_sample_rate describes the material that was encoded, not the output, so it must not be used as the output rate; a fixture that declares 44100 guards against that regression. OFOpen rewinds the ring buffer when op_open_callbacks fails. A failed open is the normal case while the header is still arriving, but it consumes bytes first, so without the rewind every retry starts mid-stream and returns OP_ENOTFORMAT forever. This is reachable with a header larger than the 16 KB open threshold - an Opus file with embedded cover art, for instance. The same flaw exists in the Vorbis path today and is left alone here. Tests cover the sniffer and the C bridge, both of which are pure computation and need no audio hardware. OggStreamProcessor itself still requires a device.
OpusFileBridge.c arrived with a near-verbatim copy of the ring buffer in VorbisFileBridge.c: the same struct, the same rb_read/rb_write, and the same create/destroy/push/available/mark-EOF wrappers under a different prefix. Two copies drift, so this moves the buffer into OggRingBuffer.c and leaves both bridges holding only their codec-specific callbacks. Behaviour is unchanged. Decoding the test fixtures through both bridges produces byte-for-byte identical PCM before and after (705600 bytes of Vorbis, 768000 of Opus), and the codec bridges keep their existing public API, so nothing on the Swift side moves. This commit is self-contained and can be dropped if you would rather not have the Vorbis bridge touched in the same PR as the Opus addition.
The existing job runs swift build and swift test, which compile the macOS slice. This PR adds a binary xcframework dependency and a second C bridge, so two gaps are worth closing: - build-ios compiles against the iOS slices of the ogg/vorbis/opus xcframeworks, which swift build never touches. - test-asan runs the suite under Address Sanitizer. The codec bridges are C operating on raw pointers, and ASan is what makes an overrun there fail loudly instead of silently corrupting audio. Both are additive; the existing job is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds Ogg Opus playback.
AudioFileStreamProcessorcurrently routes everykAudioFileOggTypestream to the libvorbisfile processor, but Ogg is a container:audio/oggis shared by Vorbis, Opus, Speex and FLAC-in-Ogg. Navidrome serves Opus transcodes asaudio/ogg, so Opus reaches libvorbisfile and is rejected.Codec selection can't happen at
openFileStreamtime — the file hint and the Content-Type both say only "Ogg". It now happens on first data, by reading the first Ogg page.Shape
OggVorbisStreamProcessor→OggStreamProcessor, driving an injectedOggAudioDecoder. Renderer loop, ring-buffer waits and packet accounting are unchanged; only decoder references were renamed. Pushed as a rename so the diff reads as one.OggCodecSniffer— reads the first page, tellsOpusHeadfrom\x01vorbis, accounts for the lacing table when locating the payload.OpusFileBridge— libopusfile wrapper mirroringVorbisFileBridge.sbooth/opus-binary-xcframework0.3.0 — same author and packaging as the ogg/vorbis xcframeworks already inPackage.resolved; ships opusfile, not just libopus.Two things I'd want a second opinion on
A failed open used to be unrecoverable.
op_open_callbacksconsumes bytes through the read callback before it discovers the header is short, so the retry starts mid-stream and returnsOP_ENOTFORMATforever.OFOpennow snapshots the ring buffer position and rewinds on failure.This is reachable whenever the headers exceed the 16 KB open threshold — an Opus file with embedded cover art, for instance. The same flaw is in the Vorbis path today; I left it alone rather than widen the PR, but it reproduces: feeding a
.oggin 512-byte chunks,VFOpennever succeeds.seek_cbinVorbisFileBridge.cis dead code — defined but never passed toov_open_callbacks(it warns under-Wunused-function). I mention it because it looks like the mechanism that would have made failed opens recoverable, and it isn't wired up.Deliberate choices
OpusHead.input_sample_ratedescribes the material that was encoded, not the output; using it as the output rate is the classic pitch-shift bug. A fixture that declares 44100 guards it.seek = NULLfor opusfile. A working seek callback would make opusfile probe as seekable and then fail on the end-seek, since only the head of a stream is buffered.openIfNeeded()is mirrored from the Vorbis path rather than redesigned.kAudioFileStreamError_UnsupportedDataFormatinstead of failing silently inside libvorbisfile. Slight behaviour change, arguably an improvement.Commits
feat(Ogg)— the Opus support, self-contained.refactor(AudioCodecs)— one shared ring buffer instead of two copies. Byte-for-byte identical PCM out of both bridges before and after. Droppable if you'd rather not have the Vorbis bridge touched here.ci— iOS-destination build and an ASan run. Also droppable.Testing
OggCodecSnifferand the C bridge are pure computation, so both are covered in CI: 13 sniffer cases and 9 bridge cases, against generated tone fixtures.The bridge was additionally compiled and run against libopusfile on Linux, outside this package, before submission: decode is bit-identical to ffmpeg's (peak 0.126652 on both), the stereo deinterleave lands 440 Hz and 660 Hz in the right channels, and valgrind reports no leaks or invalid accesses.
Not tested on a device. I don't have a Mac.
OggStreamProcessorneedsAudioRendererContextand a liveAVAudioEngine, so it isn't CI-testable, and per CONTRIBUTING audio behaves differently off-hardware. Treat this as "compiles, decoder verified, needs device verification".