Fix custom music corruption past 256 sequences (#5989) (#6736)

The resolved replacement id (which can exceed 255) rode a single
per-player seqToPlay slot, written at enqueue but consumed
asynchronously on the audio thread; back-to-back starts and
priority-queue promotions clobbered it. sSeqFlags[0x6F] was also indexed
by raw id, reading out of bounds past the authentic range.

- func_800F9280 resolves the replacement and packs the full 16-bit id
  into the 0x82/0x85 play command; the handler reads opArgs & 0xFFFF.
  Audio_QueueSeqCmd no longer pre-writes the shared slot.
- SyncInitSeqPlayerInternal uses the command-carried id and bounds-checks
  it against the calloc'd sequence map (+0xF headroom for reserved-range
  skips).
- Route sSeqFlags reads through a bounded Audio_GetSeqFlags helper.
- Warn and skip gracefully past the 16-bit id limit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
David Racine
2026-06-12 23:36:25 -04:00
committed by GitHub
parent f3413bfd26
commit eb4142835e
4 changed files with 53 additions and 35 deletions
+15 -8
View File
@@ -580,20 +580,17 @@ s32 AudioLoad_SyncInitSeqPlayerInternal(s32 playerIdx, s32 seqId, s32 arg2) {
s32 index;
s32 numFonts;
s32 fontId;
s8 authCachePolicy = -1; // since 0 is a valid cache policy value
AudioSeq_SequencePlayerDisable(seqPlayer);
fontId = 0xFF;
if (gAudioContext.seqReplaced[playerIdx]) {
authCachePolicy = seqCachePolicyMap[seqId];
seqId = gAudioContext.seqToPlay[playerIdx];
// seqId is the resolved 16-bit id from func_800F9280(). Reject ids with no loaded sequence; the
// map has sequenceMapSize + 0xF slots (custom ids skip the reserved 129-135 range).
if (seqId < 0 || (size_t)seqId >= sequenceMapSize + 0xF || sequenceMap[seqId] == NULL) {
return 0;
}
SequenceData seqData2 = ResourceMgr_LoadSeqByName(sequenceMap[seqId]);
if (authCachePolicy != -1) {
seqData2.cachePolicy = authCachePolicy;
}
for (int i = 0; i < seqData2.numFonts; i++) {
fontId = seqData2.fonts[i];
@@ -1342,7 +1339,8 @@ void AudioLoad_Init(void* heap, size_t heapSize) {
char** seqList = ResourceMgr_ListFiles("audio/sequences*", &seqListSize);
char** customSeqList = ResourceMgr_ListFiles("custom/music/*", &customSeqListSize);
sequenceMapSize = (size_t)(seqListSize + customSeqListSize);
sequenceMap = malloc((sequenceMapSize + 0xF) * sizeof(char*));
// calloc: unassigned slots stay NULL for the guard in AudioLoad_SyncInitSeqPlayerInternal().
sequenceMap = calloc(sequenceMapSize + 0xF, sizeof(char*));
gAudioContext.seqLoadStatus = malloc(sequenceMapSize);
memset(gAudioContext.seqLoadStatus, 5, sequenceMapSize);
@@ -1433,6 +1431,15 @@ void AudioLoad_Init(void* heap, size_t heapSize) {
seqNum++;
}
// Sequence ids are carried in 16 bits; fail gracefully past the limit.
if (seqNum > 0xFFFF) {
Messagebox_ShowErrorBox("Too Many Sequences",
"The number of custom sequences exceeds the supported limit (65535). Some custom "
"music will not be available. Please reduce the size of your music pack(s).");
LUSLOG_ERROR("Custom sequence limit (0xFFFF) exceeded; remaining custom sequences skipped.");
break;
}
AudioCollection_AddToCollection(customSeqList[j], seqNum);
sDat->seqNumber = seqNum;
+3 -2
View File
@@ -247,11 +247,12 @@ void func_800E5584(AudioCmd* cmd) {
AudioLoad_SyncLoadSeqParts(cmd->arg1, cmd->arg2);
return;
case 0x82:
AudioLoad_SyncInitSeqPlayer(cmd->arg0, cmd->arg1, cmd->arg2);
// 16-bit seqId packed in opArgs bits 0-15. See func_800F9280().
AudioLoad_SyncInitSeqPlayer(cmd->arg0, cmd->opArgs & 0xFFFF, 0);
func_800E59AC(cmd->arg0, cmd->data);
return;
case 0x85:
AudioLoad_SyncInitSeqPlayerSkipTicks(cmd->arg0, cmd->arg1, cmd->data);
AudioLoad_SyncInitSeqPlayerSkipTicks(cmd->arg0, cmd->opArgs & 0xFFFF, cmd->data);
return;
case 0x83:
if (gAudioContext.seqPlayers[cmd->arg0].enabled) {
+21 -13
View File
@@ -257,6 +257,14 @@ u8 sSeqFlags[0x6F] = {
1, // NA_BGM_CUSTOM_SEQ
};
// Returns 0 for ids past the authentic range (custom sequences have no vanilla flags).
static u8 Audio_GetSeqFlags(u16 seqId) {
if (seqId >= ARRAY_COUNT(sSeqFlags)) {
return 0;
}
return sSeqFlags[seqId];
}
s8 sSpecReverbs[20] = { 0, 0, 0, 0, 0, 0, 0, 40, 0, 15, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
NatureAmbienceDataIO sNatureAmbienceDataIO[20] = {
@@ -4518,7 +4526,7 @@ void func_800F5550(u16 seqId) {
Audio_QueueCmdS32(0xF8000000, 0);
}
if ((sSeqFlags[D_80130630] & 0x20) && sSeqFlags[(seqId & 0xFF) & 0xFF] & 0x10) {
if ((Audio_GetSeqFlags(D_80130630) & 0x20) && Audio_GetSeqFlags((seqId & 0xFF) & 0xFF) & 0x10) {
if ((D_8013062C & 0x3F) != 0) {
sp27 = 0x1E;
@@ -4528,9 +4536,9 @@ void func_800F5550(u16 seqId) {
D_8013062C = 0;
} else {
nv = (sSeqFlags[(seqId & 0xFF) & 0xFF] & 0x40) ? 1 : 0xFF;
nv = (Audio_GetSeqFlags((seqId & 0xFF) & 0xFF) & 0x40) ? 1 : 0xFF;
func_800F5E18(SEQ_PLAYER_BGM_MAIN, seqId, 0, 7, nv);
if (!(sSeqFlags[seqId] & 0x20)) {
if (!(Audio_GetSeqFlags(seqId) & 0x20)) {
D_8013062C = 0xC0;
}
}
@@ -4544,7 +4552,7 @@ void func_800F56A8(void) {
temp_v0 = func_800FA0B4(SEQ_PLAYER_BGM_MAIN);
bvar = temp_v0 & 0xFF;
if ((temp_v0 != NA_BGM_DISABLED) && (sSeqFlags[bvar] & 0x10)) {
if ((temp_v0 != NA_BGM_DISABLED) && (Audio_GetSeqFlags(bvar) & 0x10)) {
if (D_8013062C != 0xC0) {
D_8013062C = gAudioContext.seqPlayers[SEQ_PLAYER_BGM_MAIN].soundScriptIO[3];
} else {
@@ -4577,9 +4585,9 @@ void func_800F5918(void) {
void func_800F595C(u16 arg0) {
u8 arg0b = arg0 & 0xFF;
if (sSeqFlags[arg0b] & 2) {
if (Audio_GetSeqFlags(arg0b) & 2) {
Audio_PlayFanfare(arg0);
} else if (sSeqFlags[arg0b] & 4) {
} else if (Audio_GetSeqFlags(arg0b) & 4) {
Audio_StartSeq(SEQ_PLAYER_FANFARE, 0, arg0);
} else {
@@ -4591,9 +4599,9 @@ void func_800F595C(u16 arg0) {
void func_800F59E8(u16 arg0) {
u8 arg0b = arg0 & 0xFF;
if (sSeqFlags[arg0b] & 2) {
if (Audio_GetSeqFlags(arg0b) & 2) {
Audio_SeqCmd1(SEQ_PLAYER_FANFARE, 0);
} else if (sSeqFlags[arg0b] & 4) {
} else if (Audio_GetSeqFlags(arg0b) & 4) {
Audio_SeqCmd1(SEQ_PLAYER_FANFARE, 0);
} else {
Audio_SeqCmd1(SEQ_PLAYER_BGM_MAIN, 0);
@@ -4603,9 +4611,9 @@ void func_800F59E8(u16 arg0) {
s32 func_800F5A58(u8 arg0) {
u8 phi_a1 = 0;
if (sSeqFlags[arg0 & 0xFF] & 2) {
if (Audio_GetSeqFlags(arg0 & 0xFF) & 2) {
phi_a1 = 1;
} else if (sSeqFlags[arg0 & 0xFF] & 4) {
} else if (Audio_GetSeqFlags(arg0 & 0xFF) & 4) {
phi_a1 = 1;
}
@@ -4656,7 +4664,7 @@ void PreviewSequence(u16 seqId) {
*/
void func_800F5B58(void) {
if ((func_800FA0B4(SEQ_PLAYER_BGM_MAIN) != NA_BGM_DISABLED) && (sPrevMainBgmSeqId != NA_BGM_DISABLED) &&
(sSeqFlags[func_800FA0B4(SEQ_PLAYER_BGM_MAIN) & 0xFF] & 8)) {
(Audio_GetSeqFlags(func_800FA0B4(SEQ_PLAYER_BGM_MAIN) & 0xFF) & 8)) {
if (sPrevMainBgmSeqId == NA_BGM_DISABLED) {
Audio_SeqCmd1(SEQ_PLAYER_BGM_MAIN, 0);
} else {
@@ -4773,7 +4781,7 @@ void Audio_SetSequenceMode(u8 seqMode) {
seqMode = SEQ_MODE_IGNORE;
}
if ((seqId == NA_BGM_DISABLED) || (sSeqFlags[(u8)(seqId & 0xFF)] & 1) ||
if ((seqId == NA_BGM_DISABLED) || (Audio_GetSeqFlags((u8)(seqId & 0xFF)) & 1) ||
((sPrevSeqMode & 0x7F) == SEQ_MODE_ENEMY)) {
if (seqMode != (sPrevSeqMode & 0x7F)) {
if (seqMode == SEQ_MODE_ENEMY) {
@@ -5204,7 +5212,7 @@ void Audio_PlayNatureAmbienceSequence(u8 natureAmbienceId) {
u8 val;
if ((gActiveSeqs[SEQ_PLAYER_BGM_MAIN].seqId == NA_BGM_DISABLED) ||
!(sSeqFlags[((u8)gActiveSeqs[SEQ_PLAYER_BGM_MAIN].seqId) & 0xFF] & 0x80)) {
!(Audio_GetSeqFlags(((u8)gActiveSeqs[SEQ_PLAYER_BGM_MAIN].seqId) & 0xFF) & 0x80)) {
Audio_StartNatureAmbienceSequence(sNatureAmbienceDataIO[natureAmbienceId].playerIO,
sNatureAmbienceDataIO[natureAmbienceId].channelMask);
+14 -12
View File
@@ -39,15 +39,26 @@ u8 D_80133418 = 0;
void func_800F9280(u8 playerIdx, u8 seqId, u8 arg2, u16 fadeTimer) {
u8 i;
u16 dur;
u16 resolvedSeqId;
s32 pad;
if (D_80133408 == 0 || playerIdx == SEQ_PLAYER_SFX) {
// Resolve here so the full 16-bit id rides in the command (bits 0-15) rather than the shared
// seqToPlay slot. seqReplaced is set out-of-band by preview/slow load.
// See AudioEditor_GetReplacementSeq().
if (gAudioContext.seqReplaced[playerIdx]) {
resolvedSeqId = gAudioContext.seqToPlay[playerIdx];
gAudioContext.seqReplaced[playerIdx] = 0;
} else {
resolvedSeqId = AudioEditor_GetReplacementSeq(seqId);
}
arg2 &= 0x7F;
if (arg2 == 0x7F) {
dur = (fadeTimer >> 3) * 60 * gAudioContext.audioBufferParameters.updatesPerFrame;
Audio_QueueCmdS32(0x85000000 | _SHIFTL(playerIdx, 16, 8) | _SHIFTL(seqId, 8, 8), dur);
Audio_QueueCmdS32(0x85000000 | _SHIFTL(playerIdx, 16, 8) | (resolvedSeqId & 0xFFFF), dur);
} else {
Audio_QueueCmdS32(0x82000000 | _SHIFTL(playerIdx, 16, 8) | _SHIFTL(seqId, 8, 8),
Audio_QueueCmdS32(0x82000000 | _SHIFTL(playerIdx, 16, 8) | (resolvedSeqId & 0xFFFF),
(fadeTimer * (u16)gAudioContext.audioBufferParameters.updatesPerFrame) / 4);
}
@@ -372,16 +383,7 @@ extern f32 D_80130F24;
extern f32 D_80130F28;
void Audio_QueueSeqCmd(u32 cmd) {
u8 op = cmd >> 28;
if (op == 0 || op == 2 || op == 12) {
u8 seqId = cmd & 0xFF;
u8 playerIdx = GET_PLAYER_IDX(cmd);
u16 newSeqId = AudioEditor_GetReplacementSeq(seqId);
gAudioContext.seqReplaced[playerIdx] = (seqId != newSeqId);
gAudioContext.seqToPlay[playerIdx] = newSeqId;
cmd |= (seqId & 0xFF);
}
// Replacement is resolved per-command in func_800F9280().
sAudioSeqCmds[sSeqCmdWrPos++] = cmd;
}