forked from Ports/goldeneye-pc-port
fix(D77): in-level music silent — missing return in sub_GAME_7F0C0BF0 (D6-class UB)
Root cause: src/game/mp_music.c's sub_GAME_7F0C0BF0() is a one-line wrapper around get_mTrack2Vol() with no return statement. On N64/IDO the value happened to already sit in $v0; at the PC release build's -O2 the return register is genuinely undefined (docs/porting-notes.md D6 class, same as D187). Every in-level music trigger goes through set_missionstate()'s MISSION_STATE_1/4 cases (the only path that starts a level's background track), which feeds this garbage/zero value straight into musicTrack1ApplySeqpVol()/musicTrack3ApplySeqpVol() right before musicTrack1Play()/musicTrack3Play() — silencing the whole track for its entire duration while note-on processing continues normally. Front-end/menu music bypasses set_missionstate() entirely (front.c calls musicTrack1Play() directly), which is why it was never affected. Fix: return the tail call under #ifdef AVOID_UB, #else keeps the N64 body verbatim. One line, no game-logic change. Diagnostic: added a temporary, cheap (~1/s, gated on GE_AUDIOTRACE) [AUDIOLVL] peak/RMS probe of the exact buffer audioSetNextBuffer() hands to SDL (port/src/audio.c), interleaved into the same audiotrace.log as the existing csplayer.c [MUSICNOTE] probe via the shared geTracePrintf lock. Before the fix: 173 [MUSICNOTE] events over a 28s BUNKER1 capture, but every single [AUDIOLVL] reading (29 samples spanning the full run) read peak=0 rms=0.0. After the fix: same note count, every [AUDIOLVL] reading sustained non-zero (peak ~2700-10300, rms ~800-2900). tools_pc/verify.sh bunker1: PASS, no visual regression. Docs: docs/dev/findings.md D77 updated from stale "nothing reaches PC speakers" framing to the actual root cause + verification; new D6 instance logged in docs/porting-notes.md. Claude-Session: https://claude.ai/code/session_016TaKy6AAsVUTfjkhrX6CjG
This commit is contained in:
1 parent
7c437c0ed6
commit
199182d1d9
4 files changed
+148
-15
No files matched your search
+82
-11
@@ -380,7 +380,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
|
||||
| D59–D68 | intro render: blood-RLE clobber, DMA validate, OSMesgQueue, HEADS/BODIES sentinels, romCopy width, image_entry, Globalimagetable BE→LE | resolved |
|
||||
| D69 · D78–D84 | stage load (`load_bg_file`): bg/stan offline sidecar (`d69_emit.py`) + StandTile/bg_room_data ABI | resolved |
|
||||
| D70–D74 | intro-logo pixels: C-array bswap, UV path, sinf/cosf `DVAL()`, texture-import truncation | resolved |
|
||||
| D75 · D77 | front-end 3D model transforms · audio | OPEN (parked). **M-32 triage (see §F "D75 ADDENDUM"): (a) D73-scope-gap RULED OUT (gu tree fully endian-clean). Splits in two: Bug 1 = logo/photo transform = the parked D114/D116 fast3d viewport mirror (not game code). Bug 2 = absent animated models = category (b). M-32b runtime probe (`GE_D75=1`): `render_pos`/`dynAllocate`-arena hypothesis RULED OUT (render_pos valid + fresh each frame), model instances valid (nMtx 21/1), zero fast3d DL warnings — failure is downstream in `drawjointlist`/`dotube` vtx/node-DL resolution or an off-screen `basemtx`. Needs a drawjointlist-level probe. M-33 (upright captures, D168 fixed): "Bug 1 = D114/D116 mirror" retracted; **gun-barrel Bond RENDERS fine** (walk + fire, upright) — the "absent" reports were the flipped capture; **Nintendo logo genuinely broken** (renders as 2 white blobs, shifted left — a real `logoinst` transform/geometry bug, not a flip); cast roll not re-captured. See §F "D75 Bug 2 — M-33 UPDATE".** |
|
||||
| D75 | front-end 3D model transforms (D77 audio split out below — resolved, unrelated cause) | OPEN (parked). **M-32 triage (see §F "D75 ADDENDUM"): (a) D73-scope-gap RULED OUT (gu tree fully endian-clean). Splits in two: Bug 1 = logo/photo transform = the parked D114/D116 fast3d viewport mirror (not game code). Bug 2 = absent animated models = category (b). M-32b runtime probe (`GE_D75=1`): `render_pos`/`dynAllocate`-arena hypothesis RULED OUT (render_pos valid + fresh each frame), model instances valid (nMtx 21/1), zero fast3d DL warnings — failure is downstream in `drawjointlist`/`dotube` vtx/node-DL resolution or an off-screen `basemtx`. Needs a drawjointlist-level probe. M-33 (upright captures, D168 fixed): "Bug 1 = D114/D116 mirror" retracted; **gun-barrel Bond RENDERS fine** (walk + fire, upright) — the "absent" reports were the flipped capture; **Nintendo logo genuinely broken** (renders as 2 white blobs, shifted left — a real `logoinst` transform/geometry bug, not a flip); cast roll not re-captured. See §F "D75 Bug 2 — M-33 UPDATE".** |
|
||||
| D76 · D164 | disclaimer/legal screen only draws line 1 — **root-caused (M-31)**: `constructor_menu00_legalscreen` text loop bounds `legal_text_end` on `&legalscreen_MRD`, a linker-adjacency assumption that mingw breaks (`legalscreen_MRD` links 0x60 *before* `legalpage_text_array`) → `do{}while(ptr<end)` runs once. NOT an image-table bug (screen references zero `sImageTableEntry`). | fix proposed (not applied — `front.c` owned by another agent) |
|
||||
| D159 | front-end wallet-Bond photo "interlaced"/combed (RC1 / D149) — `texSwapAltRowBytes` odd-row 8-byte pre-swap (N64 RDP odd-line TMEM XOR compensation) not reversed by fast3d | FIXED (`#ifdef PORT` no-op the swap in `image.c`) |
|
||||
| D161 | Depot (`-level_30`) ceiling = bright-blue speckle + radial rays (B2). A CI8 tile drawn with `gsDPSetTextureLUT(G_TT_NONE)` was decoded against the stale `rdp.palette` → garbage. Fix: `#ifdef`-free narrow route CI→I when `palette_fmt == G_TT_NONE` in `gfx_pc.cpp import_texture()`. | FIXED (`port/fast3d/gfx_pc.cpp`) |
|
||||
@@ -2365,16 +2365,87 @@ The image-table/D68 hypothesis is **wrong**: the legal screen (`constructor_menu
|
||||
D75 family, parked). The "only 2 lines render" is a **linker-adjacency bug** in the text loop — see
|
||||
**D164**.
|
||||
|
||||
**D77 (OPEN: audio — music runs in code but no audible output on PC).**
|
||||
Intro music is processed without fault, but **nothing reaches the PC speakers.** This is
|
||||
distinct from the earlier audio work: D54/D54b only stopped the audio-thread SIGSEGVs
|
||||
(`__getTrackByte`, `alLoadParam`); they do not imply the mixed output is routed to a device.
|
||||
Per §6 Audio the intended path is libaudio (CPU synth) → PD's `audio.c` (SDL device) +
|
||||
`mixer.c`; the likely gap is that the SDL audio device / mixer queue is never opened or fed —
|
||||
synthesis runs but the AI-DMA→device handoff never happens. Files to check: `port/src/audio.c`,
|
||||
`port/src/mixer.c`, `src/audi.c` (`OUTPUT_RATE`), and the AI shim in `port/src/libultra.c`.
|
||||
Verify: confirm an SDL audio device is opened and the mix buffer is written/pushed; check
|
||||
`OUTPUT_RATE` match (PD = 22020 Hz stereo s16) and that the AI-DMA shim feeds it.
|
||||
**D77 (RESOLVED — in-level background music was silent; the general audio
|
||||
pipeline was never at fault).** Original framing ("nothing reaches the PC
|
||||
speakers") is **stale/wrong** — superseded by the M-78 audio work
|
||||
(D198–D208): the SDL device opens, the software mixer runs, and menu/intro
|
||||
music is audibly correct. The actual, narrower symptom (still present until
|
||||
this fix): **in-level** background music never plays, while the SAME
|
||||
`musicTrack1Play()` API called directly from the front end (menu/intro)
|
||||
worked fine.
|
||||
|
||||
**Root cause: D6-class missing-`return` bug (see `docs/porting-notes.md`
|
||||
§D6), NOT a mixer/trigger-plumbing gap.** `sub_GAME_7F0C0BF0()`
|
||||
(`src/game/mp_music.c`) is a one-line wrapper `{ get_mTrack2Vol(); }` with no
|
||||
`return` — on N64/IDO the value happened to already sit in `$v0`; at the PC
|
||||
release build's `-O2` the return register is genuinely undefined. Every
|
||||
in-level music trigger goes through `set_missionstate()`'s
|
||||
MISSION_STATE_1/4 cases (the ONLY call path that starts a level's
|
||||
background track — `lvlStageLoad()` → `sub_GAME_7F0C11FC()` →
|
||||
`set_missionstate()`), which calls
|
||||
`musicTrack1ApplySeqpVol(sub_GAME_7F0C0BF0())` /
|
||||
`musicTrack3ApplySeqpVol(sub_GAME_7F0C0C10())` immediately before
|
||||
`musicTrack1Play()`/`musicTrack3Play()`. The front-end/menu music
|
||||
(`front.c`'s several direct `musicTrack1Play(M_INTRO/M_FOLDERS/...)` calls)
|
||||
never goes through `set_missionstate()` at all, which is exactly why it
|
||||
worked while level music didn't — different call path, not a different
|
||||
pipeline. With the garbage/zero volume applied, `musicTrack1Play()`'s own
|
||||
tail call `musicTrack1ApplySeqpVol(musicTrack1GetVolume())` re-applies the
|
||||
same zeroed value, so the whole track plays at (effectively) zero gain for
|
||||
its entire duration.
|
||||
|
||||
**Why this was hard to see from a note-level trace alone:** the compact-seq
|
||||
player keeps working correctly downstream of the bug — `csplayer.c`'s
|
||||
existing `GE_AUDIOTRACE` `[MUSICNOTE]` probe showed ~170-250 real-looking
|
||||
note-on events (sane key/velocity) firing continuously through a whole
|
||||
BUNKER1 capture. The bug is a *track/channel-level gain* fed once per
|
||||
mission-state transition, not a per-note field, so a per-note trace looks
|
||||
perfectly healthy; only a trace of the actual mixed output distinguishes
|
||||
"notes are being processed" from "notes are audible".
|
||||
|
||||
**Verification (this session, `chore/golden-bunker1-rebaseline-d215`
|
||||
worktree, `-level_09`/BUNKER1):**
|
||||
- Added a temporary, cheap (~1/s, gated on `GE_AUDIOTRACE`) `[AUDIOLVL]`
|
||||
peak/RMS probe of the exact buffer `audioSetNextBuffer()` hands to SDL
|
||||
(`port/src/audio.c`), writing into the SAME `audiotrace.log` (same
|
||||
`geTracePrintf` lock) as the existing `[MUSICNOTE]` line so the two
|
||||
interleave in true call order without needing per-line timestamps.
|
||||
- **Before the fix:** 173 `[MUSICNOTE]` events over a 28 s capture; **every
|
||||
single `[AUDIOLVL]` reading (29 samples spanning the full run) read
|
||||
`peak=0 rms=0.0`** — literally zero output reaches the mixed buffer for
|
||||
the entire run despite continuous note-on activity.
|
||||
- **After the fix** (`return` added under `#ifdef AVOID_UB`, `#else` keeps
|
||||
the N64 body verbatim): same 28 s BUNKER1 capture, 173 `[MUSICNOTE]`
|
||||
events, and **every `[AUDIOLVL]` reading from the first note onward is
|
||||
strongly non-zero and sustained** (`peak` ~2700–10300, `rms` ~800–2900,
|
||||
continuously, not brief bursts) — a categorical before/after difference,
|
||||
not a marginal one.
|
||||
- `tools_pc/verify.sh bunker1`: **PASS**, `frames=3 worst_cell=0` — no
|
||||
visual regression (this is a one-line audio-only fix; nothing touches
|
||||
rendering).
|
||||
- Runtime audible confirmation (human ears on real hardware) is still owed
|
||||
— the capture above proves samples reach the SDL queue with real
|
||||
amplitude, which is the strongest evidence obtainable headless, but a
|
||||
human should do a by-ear BUNKER1 pass to close this out completely.
|
||||
|
||||
**Fix:** `src/game/mp_music.c`, `sub_GAME_7F0C0BF0()` — `return
|
||||
get_mTrack2Vol();` under `#ifdef AVOID_UB` (`#else` keeps the N64 body
|
||||
verbatim, matching the D187/D6 precedent). One line. No game-logic change
|
||||
(same value the N64 build always effectively returned); ABI/UB-class fix,
|
||||
not the D3x pointer-width family (no pointer/struct-layout involved here).
|
||||
|
||||
**Probe left in the tree:** `port/src/audio.c`, the `[AUDIOLVL]` block,
|
||||
gated on `getenv("GE_AUDIOTRACE")` (same env var as the pre-existing
|
||||
`[MUSICNOTE]` probe in `csplayer.c`, not touched this session). Cheap
|
||||
(~1 peak/RMS scan per second of audio, only when the env var is set) — safe
|
||||
to leave for future audio work reusing the same trace file. Remove whenever
|
||||
`GE_AUDIOTRACE`'s other temporary probes are cleaned up.
|
||||
|
||||
**Confidence: HIGH.** Root cause matches an already-catalogued bug class
|
||||
(D6/D187) exactly; the fix is a single `return`; and the before/after
|
||||
`[AUDIOLVL]` measurement is a direct, categorical (zero → sustained
|
||||
non-zero) reading of the exact buffer handed to the audio device, not an
|
||||
inference from upstream state.
|
||||
|
||||
**Cross-cutting (Q1 — shared blocker?):** before ordering D75 vs D77, spend one check on whether
|
||||
any single root cause touches both audio and rendering. Current evidence says **independent**
|
||||
|
||||
+22
-4
@@ -702,10 +702,28 @@ silently disappears.
|
||||
Invisible under `-Og`; vanished the day release switched to `-O2`
|
||||
(`34885535`). Fix: `return` the tail call under `#ifdef AVOID_UB`
|
||||
(`#else` keeps the N64 body verbatim).
|
||||
- The one currently flagged in-source is `grep -rn "missing a \"return\"" src/`
|
||||
(just `gunfire.c` today). Others may exist without the banner — suspect this
|
||||
class whenever an `-O2` build drops a small on-screen element that an `-Og`
|
||||
build shows. One-line `#ifdef AVOID_UB` fix each.
|
||||
- Instance: **D77** — `sub_GAME_7F0C0BF0()` (`src/game/mp_music.c`), a
|
||||
one-line wrapper around `get_mTrack2Vol()` with no `return`. Every
|
||||
in-level music trigger (`set_missionstate()`'s MISSION_STATE_1/4 cases —
|
||||
i.e. the only path that starts a level's background track; the front-end
|
||||
menu/intro music bypasses this and calls `musicTrack1Play()` directly,
|
||||
which is why menu music worked while level music didn't) feeds this
|
||||
garbage/zero return straight into `musicTrack1ApplySeqpVol()` /
|
||||
`musicTrack3ApplySeqpVol()`. The result isn't a missing on-screen
|
||||
primitive but a **silent audio channel that still runs**: the compact-seq
|
||||
player keeps processing note-on events (visible in a `GE_AUDIOTRACE`
|
||||
`[MUSICNOTE]` capture with normal-looking key/velocity data) but every
|
||||
note's synthesized volume is scaled by the zeroed track volume, so nothing
|
||||
reaches the mixed output. A per-note trace alone looks completely healthy
|
||||
— the tell is a *track/channel-level* gain feed with a missing `return`
|
||||
upstream. This class isn't limited to display-list cursors; audit any
|
||||
`-O2`-only "quietly broken but not crashing" symptom for the same shape.
|
||||
Fix: `return` under `#ifdef AVOID_UB` (`#else` keeps the N64 body).
|
||||
- The one flagged in-source with an explicit comment is
|
||||
`grep -rn "missing a \"return\"" src/` (`gunfire.c`). Others (like D77's)
|
||||
exist without the banner — suspect this class whenever an `-O2` build
|
||||
silently drops or mutes a small on-screen/audio element that an `-Og`
|
||||
build shows/plays correctly. One-line `#ifdef AVOID_UB` fix each.
|
||||
- Not caught headless by a symbol/link check or a default-config framediff
|
||||
(the missing primitive is often small). Needs an `-O2` build + an eyeball
|
||||
or a targeted crop diff.
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
#include "system.h"
|
||||
#include "config.h"
|
||||
#include "audio.h"
|
||||
#include "audiotrace.h"
|
||||
|
||||
static SDL_AudioDeviceID dev = 0;
|
||||
|
||||
@@ -250,6 +251,35 @@ void audioSetNextBuffer(const s16 *buf, u32 len)
|
||||
fwrite(buf, 1, len, s_audioDumpFile);
|
||||
s_audioDumpBytesWritten += len;
|
||||
}
|
||||
|
||||
/* D77 diag (temporary): GE_AUDIOTRACE=1 already logs csplayer.c
|
||||
* [MUSICNOTE] note-on events into audiotrace.log. That proves the
|
||||
* level-music trigger fires and posts notes to the synth, but says
|
||||
* nothing about whether those notes actually reach the final mixed
|
||||
* buffer handed to the SDL device below. Interleave a cheap (~1/s)
|
||||
* peak/RMS reading of *this* buffer into the SAME file/lock
|
||||
* (geTracePrintf) so the two can be correlated by line order without
|
||||
* per-line timestamps. Remove once D77 closes. */
|
||||
if (buf && len && getenv("GE_AUDIOTRACE")) {
|
||||
static u64 lastReportUs = 0;
|
||||
u64 now = sysGetMicroseconds();
|
||||
if (now - lastReportUs >= 1000000ull) {
|
||||
const s16 *s = buf;
|
||||
u32 n = len / 2u;
|
||||
s32 peak = 0;
|
||||
s64 sumsq = 0;
|
||||
for (u32 i = 0; i < n; i++) {
|
||||
s32 v = s[i];
|
||||
if (v < 0) v = -v;
|
||||
if (v > peak) peak = v;
|
||||
sumsq += (s64)s[i] * (s64)s[i];
|
||||
}
|
||||
double rms = n ? __builtin_sqrt((double)sumsq / (double)n) : 0.0;
|
||||
geTracePrintf("audiotrace.log", "[AUDIOLVL] peak=%d rms=%.1f n=%u\n",
|
||||
(int)peak, rms, (unsigned)n);
|
||||
lastReportUs = now;
|
||||
}
|
||||
}
|
||||
if (dev && buf && len) {
|
||||
if (audioGetSamplesBuffered() < queueLimit) {
|
||||
SDL_QueueAudio(dev, buf, len);
|
||||
|
||||
@@ -37,9 +37,23 @@ s32 mission_state = MISSION_STATE_0;
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* D6-class bug (docs/porting-notes.md): missing "return" on a non-void
|
||||
* function whose body is a single tail call. The N64/IDO build happened to
|
||||
* leave get_mTrack2Vol()'s result in $v0, so the ASM ground truth "works" by
|
||||
* accident; GCC at -O2 (the PC release build) does not guarantee that, and
|
||||
* every caller of this function (set_missionstate()'s MISSION_STATE_1/4
|
||||
* transitions -- i.e. every in-level music-track-start trigger) fed
|
||||
* musicTrack1ApplySeqpVol()/musicTrack3ApplySeqpVol() a garbage/zero volume,
|
||||
* silencing level music while leaving note-on processing intact (D77).
|
||||
*/
|
||||
u16 sub_GAME_7F0C0BF0(void)
|
||||
{
|
||||
#ifdef AVOID_UB
|
||||
return get_mTrack2Vol();
|
||||
#else
|
||||
get_mTrack2Vol();
|
||||
#endif
|
||||
}
|
||||
|
||||
u16 sub_GAME_7F0C0C10(void)
|
||||
|
||||
Reference in new issue
Block a user