Skip to content

Movie Vorbis: a Flush closes the decoder, so the sound comes back - #787

Closed
oliverames wants to merge 1 commit into
seedds:mainfrom
oliverames:fix-vorbis-flush
Closed

oliverames wants to merge 1 commit into
seedds:mainfrom
oliverames:fix-vorbis-flush

Conversation

@oliverames

Copy link
Copy Markdown
Contributor

What and why

Smash's opening movie on an iPhone lost its sound for good once the game flushed and restarted it. In my run it happened after the app lost focus once, and the movie came back without its audio.

I recorded what the game sends to its audio output with DM_AUDIO_DUMP. It is real audio (RMS about 1,500 to 2,000) for the first 16 seconds of the movie, and exact zeros from the flush to the end of the 52 seconds I pulled. The host's AudioUnit reported 0 underruns the whole time, so the phone was not dropping sound; the host stopped delivering the movie's sound. The log shows [movie] Flush, Flush, Start, Start at that point. An earlier silent run (Oct 30, no focus change) had the same Flush and Start about 6.6 s in.

h_movie_Flush called stb_vorbis_flush_pushdata on the Vorbis decoder. I fed the host's own md_vorbis path a generated stream and flushed after 100 packets: none of the next 100 packets gave a sample, whether the stream went on or started again from its first packet. Closing the decoder and opening it again gave samples from the next packet but one (the first only primes the overlap).

A Flush now closes the Vorbis decoder, and the next packet opens it again from the headers (md_vorbis_open), as at the start of a stream.

Related: #505 (the same silence on an iPhone, closed after a reinstall). This does not touch the crackle and the SIGSEGV in #768.

Checked

  • Tests: make -C dm/host/tests GAME=01006A800016E000 build/t_movie_vpx build/t_movie_avc build/t_vorbis, all three pass on an Apple-silicon Mac. The new Vorbis case in t_movie_vpx builds a small stream, flushes twice and checks that the same samples come again. It fails with stb_vorbis_flush_pushdata (assertion at the first check after the Flush) and passes with this change. dm/tools/tests/test_docs.py passes (11 tests).
  • Games run: none with this change yet. The failure was found on Smash 13.0.5 (commit 2006d5ed) on an iPhone (iPhone19,4). I am putting a build with this change on the same phone and will comment here with what it shows.
  • Devices and OS: Apple-silicon Mac for the tests.
  • scripts/regress.py impact origin/main..HEAD finds no usage manifest for Smash (116 profiles are unknown) and the 4 games with one are not affected, so I did not run check --games.

Docs

  • The finding is in docs/reference/movies.md ("Vorbis sound stops for good after a Flush") and linked from Smash's note, and python3 dm/tools/docs_symptoms.py was run (the symptom index)
  • A chain tool changed: not applicable
  • No game files, keys or data extracted from a game in the change or the attachments

On an iPhone, Super Smash Bros. Ultimate's opening movie lost its sound for good once the game flushed and restarted it (after the app lost focus and came back). DM_AUDIO_DUMP showed the game's mix with real audio up to the flush and exact zeros after it, with the AudioUnit at 0 underruns, so the host was no longer delivering the movie's sound.

h_movie_Flush called stb_vorbis_flush_pushdata on the Vorbis decoder. In a test on a generated stream, none of the next 100 packets gave a sample afterwards, whether the stream went on or started again from its first packet. A Flush now closes the decoder and the next packet opens it again from the headers, as at the start of a stream.

t_movie_vpx builds a small Vorbis stream, flushes twice and checks that the same samples come again. It fails with stb_vorbis_flush_pushdata and passes with the fix. docs/reference/movies.md has the entry (symptom index regenerated) and the Smash note links it. dm/tools/tests/test_docs.py passes. scripts/regress.py impact has no usage manifest for Smash, so no game was run on the Mac.
seedds added a commit that referenced this pull request Oct 31, 2026
Flush put stb_vorbis into Ogg resynchronization, which retains trailing
bytes for the next call. The host passes complete packet pages and drops
those unconsumed bytes, so decoding never recovered after a seek or loop.
Close the decoder on Flush and reopen it from retained headers at the next
packet, resetting overlap and synthetic Ogg state together.

Keep the synthetic repeated-Flush regression and the documented finding.
The regression fails with the old implementation and passes with this
change; the movie VP8/VP9, AVC, Vorbis and documentation checks pass.
Confirmation on the affected iPhone remains pending.

Co-authored-by: Oliver Ames <7366664+oliverames@users.noreply.github.com>
@seedds

seedds commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Landed on main in ec9c3aa6, with Oliver Ames credited as co-author for the diagnosis, decoder fix, regression test and documentation. Thank you.

I reviewed the full diff, decoder lifecycle, locking and stb's push-data implementation. The failure mechanism is concrete: stb's resynchronizer retains three trailing bytes for the next call, while md_vorbis drops each page's unconsumed tail. Closing and reopening from the retained headers correctly resets overlap and synthetic Ogg state at this complete-packet boundary. This is a general fix for the Flush path, with no game-specific rule or workaround. Confidence in the decoder bug and fix is high; no blocking code findings.

Verified on an Apple-silicon Mac:

  • The new regression fails at the first post-Flush assertion with the old implementation and passes with the fix, including repeated Flush.
  • t_movie_vpx, t_movie_avc and t_vorbis pass.
  • All 11 documentation checks pass, including after applying the change to the current main tree.

One minor coverage clarification: the regression compares the number of packets producing output, rather than PCM sample values or lengths. It catches the permanent loss of output.

Regression risk is low because the runtime change is confined to Vorbis Flush, so no regress.py check --games run was needed. impact identifies four profiles as unaffected and 116 without manifests as unknown; it does not establish that other games cannot reach this path.

Confirmation on the affected iPhone is still pending. Please add the result here after testing the same movie and focus-loss/Flush sequence. The crackle and SIGSEGV tracked in #768 remain outside this change.

Closing because the reviewed change is now incorporated on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants