ipatix/agbplay

CGB Channel Fix

Closed this issue · 4 comments

In CGBChannel.cpp, the following lines are inside CGBChannel::SetVol():

envPeak = clip<uint8_t>(0, uint8_t((note.velocity * vol) >> 10), 15);
envSustain = clip<uint8_t>(0, uint8_t((envPeak * env.sus + 15) >> 4), 15);

The second line should be:

envSustain = clip<uint8_t>(0, uint8_t((envPeak * env.sus + 16) >> 4), 15);

Assuming envPeak and env.sus were max value (0xF), this would not affect the upper limit. However, this fixes CGB instruments which have 0 sustain that are played at a low volume.

For example, Tiny Woods (song 125 in Pokemon Mystery Dungeon: Red Rescue Team) has a noise track which plays at volume 16. The noise itself has ADSR: {0, 5, 0, 0}. In agbplay, this track would play silently because of these two lines (envPeak would end up as 1, but envSustain would end up as 0), but in-game the track does produce sound. Changing the 15 to 16 in this line would fix this situation and set it to the correct level (envPeak 1, envSustain 1).

That's interesting. I generally assumed that everything with sustain = 0 would eventually get silent.

But in general, the CGB ADSR code definitely IS flawed. The combination of "better than hardware" implementation (with linear envelope instead of rough steps) and my lack of completely understanding the original code produces wrong results in quite a lot of cases.

I'll investigate this. Since the code probably needs a complete rewrite, I'll have to check how it works EXACTLY to prevent any of these problems. Since I'm not sure how correct your change would be, I'll wait before applying the change.

Sounds good. After making the change, you start to very rarely notice certain noises lasting too long for example. Definitely seems like something is incorrect somewhere else.

Now that I see this issue, I actually have changed a bunch of the CGB envelope code. So testing this once again would be quite useful.

I recently did some changes on the envelopes and the volume calculation for CGB instruments. I think I managed to get them 100% accurate in terms of volume levels. If you still experience this issue, feel free to reopen.