Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AX: make volume field signed #12276

Merged
merged 1 commit into from Nov 12, 2023
Merged

AX: make volume field signed #12276

merged 1 commit into from Nov 12, 2023

Conversation

Tilka
Copy link
Member

@Tilka Tilka commented Nov 5, 2023

This fixes overly loud sounds in Freestyle Metal X (issue 13120).

This fixes overly loud sounds in Freestyle Metal X (issue 13120).
@Pokechu22
Copy link
Contributor

This seems like a plausible fix; I haven't attempted to confirm it though (I don't own Freestyle Metal X and don't want to dig through AX right now). I am curious as to what value of cur_volume was the game setting here, though.


Looking at how it's implemented:

// Apply a global volume ramp using the volume envelope parameters.
for (u32 i = 0; i < count; ++i)
{
const s32 sample = ((s32)samples[i] * pb.vol_env.cur_volume) >> 15;
samples[i] = std::clamp(sample, -32767, 32767); // -32768 ?
pb.vol_env.cur_volume += pb.vol_env.cur_volume_delta;
}

the maximum value of 32767 or 0x7fff would be multiplied by the sample and then shifted by 15. Since 0x7fff >> 15 == 0 but 0x8000u >> 15 == 1, I'm pretty sure this means that the volume is always reduced slightly (e.g. (0x7fff * 0x7fff) >> 15 == 0x7ffe, (-0x8000 * 0x7fff) >> 15 == -0x7fff).

The clamping would only apply if the volume and sample were both -0x8000, as (-0x8000 * -0x8000) >> 15 == +0x8000 which would be clamped to +0x7fff. I don't think there's any cases where -0x8000 could be generated (and clamped to -0x7fff) anymore. Even still, if they are using the normal saturation functionality in the DSP, it would be clamped to -0x8000 through +0x7fff. In practice, this probably doesn't matter.

@Tilka
Copy link
Member Author

Tilka commented Nov 6, 2023

I am curious as to what value of cur_volume was the game setting here, though.

The volume is 0xFF64 aka -156 (delta is 0).

@Pokechu22
Copy link
Contributor

I am curious as to what value of cur_volume was the game setting here, though.

The volume is 0xFF64 aka -156 (delta is 0).

OK, so effectively -0.00476, which would behave basically the same as 0.00476, but was instead being treated as 1.99524 (or 419 times larger, though I don't know how much louder that would actually be perceived as).

@Tilka
Copy link
Member Author

Tilka commented Nov 6, 2023

Here is a comparison without the background music (careful, it's quite loud):

broken.mp4
fixed.mp4

@Pokechu22
Copy link
Contributor

That's a pretty significant difference and definitely seems better (though I can't really hear the chickens at all in the fixed version). For the chickens specifically, does the game try to change the volume based on distance, or are they at a constant volume? I'm curious if there's some kind of overflow or something there. (Though this would also depend on the chickens actually existing in the world, which I'm not 100% sure is the case.)

@Tilka
Copy link
Member Author

Tilka commented Nov 6, 2023

They do get louder when you get closer to a barn.

Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems plausible enough. I haven't done any testing though.

@Tilka Tilka merged commit 4285e9d into dolphin-emu:master Nov 12, 2023
11 checks passed
@Tilka Tilka deleted the ax_volume branch November 12, 2023 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants