Handle end address alignments with the AI Buffers at boot time to prevent audio stuttering on inaccurate emulators

[gheskett]

ORIGINAL ISSUE: "Setting GFX_POOL_SIZE to 60000 causes severe audio playback stuttering"

There is no reason the GFX pool size should have any impact whatsoever on audio, so this is likely some sort of UB taking place. This is reproduceable in at least HackerSM64 2.4, all the way back to at least HackerSM64 2.1.2 (maybe further, more testing needed). Choice of GCC version / compiler also seems to not matter for this bug, so whatever behavior is likely taking place seems to be consistent. Hopefully this means bisecting for the issue won't be too difficult or inconclusive, however the severity of this issue likely correlates to some other unexplainable issues people may have run into with their hacks.

[gheskett]

UPDATE: Did some digging and can confirm it happens even prior to the release of HackerSM64 2.0.0. Ended up changing GFX_POOL_SIZE to 59408 to accommodate for changes to the thread stacks, and the stuttering still happens. We'll need to go even deeper on this one.

Interestingly though, the size of the audio heap also changed, but accommodating for that with the GFX buffer size did not result in the issue taking place. Therefore, I believe whatever corruption may be taking place is happening to something allocated on the audio heap (i.e. not some random buffer/variable pre-allocated in the audio code, also unlikely to be happening in the thread4 stack since that address shifted but audio heap didn't). It also doesn't appear that the address of gGfxPools has changed at all between the tested pre-HackerSM64 2.0.0 commit and HackerSM64 2.4.0. I have a hunch that this issue could've been the true source of the infamous patch_audio_bank audio crash™️, in which case there's some evidence that it may actually be a vanilla bug (though if it is vanilla, then we will not be able to bisect our way to the root of the issue...)

Oldest investigated commit: 79db39b06b2aeaf24ddcb96b0101c7f3d1eab456

---

MAP DATA COMPARISONS

HackerSM64 2.4.0:
#define GFX_POOL_SIZE 60000

 build/us_n64/src/buffers/buffers.o(.bss*)
 .bss.gGfxPools
                0x0000000080025c00    0xea6a0 build/us_n64/src/buffers/buffers.o
           -->  0x0000000080025c00                gGfxPools
 .bss.gSaveBuffer
                0x00000000801102a0      0x1d8 build/us_n64/src/buffers/buffers.o
                0x00000000801102a0                gSaveBuffer
 *fill*         0x0000000080110478        0x8 
 .bss.gGfxSPTaskYieldBuffer
                0x0000000080110480      0xc00 build/us_n64/src/buffers/buffers.o
                0x0000000080110480                gGfxSPTaskYieldBuffer
 .bss.gGfxSPTaskStack
                0x0000000080111080      0x400 build/us_n64/src/buffers/buffers.o
                0x0000000080111080                gGfxSPTaskStack
 .bss.gThread5Stack
                0x0000000080111480     0x2000 build/us_n64/src/buffers/buffers.o
                0x0000000080111480                gThread5Stack
 .bss.gThread4Stack
                0x0000000080113480     0x2000 build/us_n64/src/buffers/buffers.o
                0x0000000080113480                gThread4Stack
 .bss.gThread3Stack
                0x0000000080115480      0x200 build/us_n64/src/buffers/buffers.o
                0x0000000080115480                gThread3Stack
 .bss.gIdleThreadStack
                0x0000000080115680      0x100 build/us_n64/src/buffers/buffers.o
                0x0000000080115680                gIdleThreadStack
 .bss.gAudioHeap
                0x0000000080115780    0x7ab80 build/us_n64/src/buffers/buffers.o
           -->  0x0000000080115780                gAudioHeap
 build/us_n64/src/audio/globals_start.o(.bss*)
 .bss.gAudioGlobalsStartMarker
                0x0000000080190300        0x8 build/us_n64/src/audio/globals_start.o
                0x0000000080190300                gAudioGlobalsStartMarker

HackerSM64 pre-2.0.0:
#define GFX_POOL_SIZE 59408

 build/us/src/buffers/buffers.o(.bss*)
 .bss           0x0000000080025c00   0x1607d0 build/us/src/buffers/buffers.o
           -->  0x0000000080025c00                gGfxPools
                0x000000008010dda0                gSaveBuffer
                0x000000008010df78                gGfxSPTaskYieldBuffer
                0x000000008010eb78                gGfxSPTaskStack
                0x000000008010ef78                gThread5Stack
                0x0000000080110f78                gThread4Stack
                0x0000000080112f78                gThread3Stack
                0x0000000080114f78                gIdleThreadStack
           -->  0x0000000080115780                gAudioHeap
                0x00000000801793d0                gDecompressionHeap
 build/us/src/audio/globals_start.o(.bss*)
 .bss           0x00000000801863d0        0x8 build/us/src/audio/globals_start.o
                0x00000000801863d0                gAudioGlobalsStartMarker

[gheskett]

Brute force debugging idea for later:

Step 1: Allocate a useless buffer over what the audio heap would've been before, and fill it with static data, then iterate over it to see if any data changed. If change detected, assert and crash with address of change.

Step 2: Set up a watcher for problematic address and add checks throughout the code to identify when it changes and assert whenever it happens.

Step 3: Gradually shift the assertions in the direction of the detected change, until eventually converging onto the problematic function / code.

Step 4: Fix the bad code.

Step 5: Profit!

[gheskett]

Well this stinks...The brute force method above did not work.

Turns out declaring a useless buffer the same size as the audio heap and placing it before the audio heap does not result in corruption of either the audio heap or the useless buffer itself...HOWEVER, swapping the addresses of both and changing nothing else brings back audio corruption. This implies there must be something weird going on in which addresses on the audio heap are self-dependent and yet looking for something specific maybe? Or maybe audio corruption is happening in a way that only works based on relative addresses, yet not lenient to the point where it happens at all offsets? This can't be a caching issue because this is reproduceable on HLE emulators.

What in the world is going on here???

EDIT: It seems like shifting the address of the audio heap by multiples of 0x2000 results in reproduceable behavior. Smaller values result in no stutter. Audio stutter is reproduceable with GFX_POOL_SIZE set to 10848, outright confirming that this issue has absolutely nothing to do with GFX_POOL_SIZE itself.

Audio heap stutter should always be reproduceable at offsets of 0xXXXX<1,3,5,7,9>780.

Analysis of the output waveform implies that the signal is being completely cut. A proper audio dump is needed to properly determine if this is likely ADPCM data being lost or the AI buffer being cleared out, but it seems like it potentially affects an entire audio frame (60FPS), rather than just a single update (240FPS). As such, I think it's most likely the AI buffer that's ultimately getting cleared out here.

[gheskett]

I can now confirm the issue definitely has something to do with gAiBuffers...however the above brute forcing method STILL does not work. This is awfully peculiar, but it means there HAS to be something weird going on with the address of the gAiBuffers entries themselves.

[gheskett]

Further investigation has taken me all the way to libultra, and...........this looks awfully suspicious, I have to say.

Not only does it look suspicious, it also matches up with some of my observations, leading me to believe this is the problem. What it does mean though is not only is this issue outside the scope of HackerSM64, it also means there's no memory leak to worry about here in the first place. Therefore, I no longer deem this critical, and am closing the issue for good (although this probably could be fixed in hackerlibultra someday).

If anybody ever has playback stuttering issues with audio, I'm pretty sure at this point that simply moving the audio heap will solve the problem for good (and safely too!)