Skip to content
This repository has been archived by the owner on Jun 23, 2023. It is now read-only.

Use non-linear scaling for high volume levels #532

Merged
merged 2 commits into from
Sep 17, 2022
Merged

Use non-linear scaling for high volume levels #532

merged 2 commits into from
Sep 17, 2022

Conversation

ceski-1
Copy link
Contributor

@ceski-1 ceski-1 commented Sep 17, 2022

As discussed in #531, there is a good argument in favor of the music volume slider behaving like vanilla Doom but modified to smoothly transition when approaching maximum volume. This is so the user can perceive differences between higher volume slider settings which was a shortcoming of vanilla Doom. Source ports that prefer staying faithful to vanilla Doom should continue using the vanilla Doom approach. Other than that, both solutions are pretty similar.

Here's a plot I made from the previous PR. PrBoom+ targets the yellow line now:

chart

Here's a capture of the MIDI output as the music volume slider is adjusted from 0 to 15:

F0 7F 7F 04 01 00 00 F7    0
F0 7F 7F 04 01 1E 0A F7    1310
F0 7F 7F 04 01 3D 14 F7    2621
F0 7F 7F 04 01 5B 1E F7    3931
F0 7F 7F 04 01 7A 28 F7    5242
F0 7F 7F 04 01 19 33 F7    6553
F0 7F 7F 04 01 37 3D F7    7863
F0 7F 7F 04 01 56 47 F7    9174
F0 7F 7F 04 01 75 51 F7    10485
F0 7F 7F 04 01 13 5C F7    11795
F0 7F 7F 04 01 32 66 F7    13106
F0 7F 7F 04 01 6C 6E F7    14188
F0 7F 7F 04 01 7A 75 F7    15098
F0 7F 7F 04 01 74 7A F7    15732
F0 7F 7F 04 01 03 7E F7    16131
F0 7F 7F 04 01 7F 7F F7    16383

So all SysEx messages are correct. As before, if a MIDI file tries to change the master volume, the message will be intercepted and scaled appropriately to the current volume slider and now with a correction factor applied as well.

I'll leave this as a draft so others can provide feedback.

@fabiangreffrath fabiangreffrath marked this pull request as ready for review September 17, 2022 15:09
Copy link
Collaborator

@kraflab kraflab left a comment

Choose a reason for hiding this comment

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

Thanks 😎

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants