Strange Envelope Issue in Sonic Advance 3
Closed this issue · 10 comments
In Chaos Angel Act 1 (song 44) in Sonic Advance 3, on actual hardware the square on track 10 has a low sustain value that continues playing for a bit, but in agbplay, the notes seem to decay to 0, giving them a very staccato sound; an example comparison can be found in this discord message. There also seem to be notes that get cut off or not played on hardware but do play in agbplay; not sure if this is related or not.
Update: Just found another example of this: the Battle Arena theme from Pokemon Emerald (song 458) has the same issue with its squares (channels 7 and 8).
Can you please test the latest version of branch psg_debugging?
There'll be some debug output but I hope the results generated are more accurate than before.
Sorry, just saw this; will test in the next few days
Okay, it has been a long time and I have good and not so good news:
The good news is that I found the problem, the not so good news is that it's fundamentally impossible to fix this accurately. So due to what I'd consider a bug in the mp2k code volume changes during the middle of a sustaining CGB note (except channel 3) won't update the hardware channel correctly.
I'll explain this based on the pokeemerald decomp of mp2k. When a track volume event is being processed it set's the channel volume of all sound channels that belong to that track. Now, the important part is that if a sound channel is a CGB channel it will also set that channel's modify to CGB_CHANNEL_MO_VOL which can not trivially be found here:
https://github.com/pret/pokeemerald/blob/003d7d7e2e399e121e6b0dfff5d4628763b7e85d/src/m4a_1.s#L1398
Setting this modify field will cause the CgbSound() function to update the hardware channels volume based on the channel variables envelope volume (!) and not it's channel volume (!). This is an important distinction because the envelope volume is only ever update when CgbModVol is called and when a note is in sustain state, it will only do so every 7 frames (!!!):
https://github.com/pret/pokeemerald/blob/003d7d7e2e399e121e6b0dfff5d4628763b7e85d/src/m4a.c#L1094
So let's assume a track volume change happens while the envelopeCounter is not zero. The associated channel variable modify will be set to CGB_CHANNEL_MO_VOL. Because envelopeCounter is not zero, CgbModVol() won't be called that frame and this will cause CgbSound() to erroneously apply the old non-updated sustain volume to the hardware channel. This will cause notes in Pokemon's Battle Arena to not turn off because during the fade out an old/wrong sustain volume is applied to the hardware channel.
However, because every 7 frames during a sustained CGB (non channel 3) note CgbModVol is actually called, there may be a case where the volume is actually correctly applied. If you consider this a random event (which depends on the time when the note was started), you have a 1 in 7 chance that the volume is actually applied correctly.
Okay, where's the catch? Why can agbplay not mimic this behavior? Can agbplay not correctly count the frames of the envelopes to see at which frame exactly it should update the hardware volume to which level? Unfortunately that's not the case because the mp2k CGB envelopes occur with 64 Hz while agbplay does so in sync with the console framerate of 60 Hz.
Actually, because the sound code of mp2k also only really runs at 60 Hz, every 15 frames it has to update the envelopes twice to stay in sync with the hardware envelopes. This big problem we now have is that these double updates every 15 frames are not easily predicted since on console they are in no way synced to when the song actually starts. So worst case you may listen to the same song on console multiple times and it'll sound different.
To conclude this already overly detailed writeup, what do we do about it? Clearly the current agbplay behavior is off to a non satisfactory extend. As explained in the previous paragraph, even changing the framerate of the envelopes to 64 Hz won't cause 100% consistent results. And because this is not solvable my idea is to take the approach to just assume that this 1 in 7 chance (where the volume is actually updated properly) never occurs. This would very likely fix most of the reported cases but may cause rare circumstances where it doesn't sound right. Though, these cases then would probably not even be deterministic on hardware.
Any comments or ideas how I should proceed?
As discussed on discord, I think the idea of just assuming it never hits the 1 in 7 chance is the best we can realistically do without incorporating an entire GBA emulator into the program, so that sounds like the best idea.
Okay, I have a version that fixed Battle Arena! Anyway, now I've already noticed some regressions which I'll investigate.
@Kurausukun Please try branch cgb_battlearenafix to see if it fixes the issue. I'm very confident many of the issues are fixed now. The behavior to fix this sustain bug is currently an option with default off and you have to set it to on via simulate-cgb-sustain-bug (see README).
Okay, now this is one of what I'd call final attempts to solve it.
Do you happen to know if Sonic uses a non standard song volume? Does the recording you provided to me also sound like that in ithe GSF?
I may be doing something wrong, but the square melody in Battle Arena still cuts off too short for me. Referring to the melody that starts at 20sec here: https://youtu.be/TPWtoOFX_hA?si=d7WxY3f2UdT5q0dH
There is an option called simulate-cgb-sustain-bug in the JSON you have to enable to fix this.