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

Add per-sound channel individual volume core options #223

Closed
eadmaster opened this issue Nov 29, 2020 · 11 comments
Closed

Add per-sound channel individual volume core options #223

eadmaster opened this issue Nov 29, 2020 · 11 comments

Comments

@eadmaster
Copy link

eadmaster commented Nov 29, 2020

This is meant to "fix" games having unbalanced SFX vs the BGM.

Will need to add separate options for the PSG and Yamaha chips.

@eadmaster
Copy link
Author

eadmaster commented Nov 30, 2020

I've checked the code and I am going to try implementing this myself starting with the SMS PSG.
Simply adding an on/off switch for the singular channels seems trivial, but i'll try with proper volume scaling as well. If you have any suggestion please post here.

eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Nov 30, 2020
@eadmaster
Copy link
Author

eadmaster commented Nov 30, 2020

UPDATE: first addition made for the PSG chip.
The noise channel 3 seem to work as toggle only though (any suggestion?)

@eadmaster eadmaster changed the title [FR] add per-sound channel individual volume core options Add per-sound channel individual volume core options (WIP) Nov 30, 2020
eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Nov 30, 2020
@eadmaster
Copy link
Author

eadmaster commented Nov 30, 2020

UPDATE2: added options for the MegaDrive/Genesis FM chip too!

The Master System FM (YM2413) chip is still left, but this has a lower priority for me.

Please review the code in my fork before i sumbit the first PR.

@eadmaster eadmaster changed the title Add per-sound channel individual volume core options (WIP) Add per-sound channel individual volume core options (WIP, please review my fork) Nov 30, 2020
@eadmaster
Copy link
Author

UPDATE3: still need some help to scale the volume of the noise channel on the PSG chip.

@ekeeke
Copy link

ekeeke commented Dec 2, 2020

The reason it does not work properly for PSG channels is that, since config.psg_ch_volumes[i] are unsigned int, not float, setting it to a value inferior to 100 and dividing it by 100 will always gives you 0 as a result. So basically, anytime you set one psg channel below 100%, you are actually muting it.

The modified code in psg_config should be corrected like this for it to work as you expect:
psg.chanAmp[i][0] = ((preamp * config.psg_ch_volumes[i]) / 100) * ((panning >> (i + 4)) & 1);

so that the division occurs after multiplying individual channel volume by preamp value.
EDIT: actually the same should be done for the FM core modifications, as your use of parentheses cause the division to occur before the multiplication.

Also, forcing delta value update whatever the current output level when your channel volume is below 100 is incorrect and should be removed for correct emulation behavior.

Finally, if you want this to be merged upstream, you will need to isolate and enclose your core modifications using #IFDEF USE_PER_SOUND_CHANNELS_CONFIG / #ENDIF for example, so the core still compile fines in non-libretro ports that don't have your additional config data defined and don't need this hack.

eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 3, 2020
@eadmaster
Copy link
Author

eadmaster commented Dec 3, 2020

So basically, anytime you set one psg channel below 100%, you are actually muting it.

For some reason the tone PSG channels were scaling correctly the volume, and since i couldn't figure out the reason why the noise channel didn't i started making experiments...

Finally, if you want this to be merged upstream, you will need to isolate and enclose your core modifications using #IFDEF USE_PER_SOUND_CHANNELS_CONFIG / #ENDIF for example, so the core still compile fines in non-libretro ports that don't have your additional config data defined and don't need this hack.

Actually my initial idea was to init the volume arrays with 100 values and then do the volume scaling only if the frontend overrides these values instead of using prepoc vars.

Btw i've done all the changes you suggested and everything seems to work fine.
Since you already confirmed there should be no side-effects with these changes i'll send a PR soon.

eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 3, 2020
eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 3, 2020
@eadmaster
Copy link
Author

UPDATE: i've just pushed a first implementation of the SMS FM (YM2413) channel volume scaler, please review!

@ekeeke
Copy link

ekeeke commented Dec 3, 2020

Please do not use LIBRETrO specific defines in core sourcecode, so that non-libretro ports can still support this hack if they want by setting specific define in their Makefile. Using a more generic #ifdef as initially proposed is a better long term solution. You only need to had -Dxxxxx in Makefile.libretro core defines, along with other core specific defines (like the one allowing cpu overclocking or sprites limit removal hacks)

EDITED: removed psg code modification proposal as it would not work

I didn't looked at your ym2413 core modifications yet.

@eadmaster
Copy link
Author

eadmaster commented Dec 5, 2020

Please do not use LIBRETrO specific defines in core sourcecode, so that non-libretro ports can still support this hack if they want by setting specific define in their Makefile. Using a more generic #ifdef as initially proposed is a better long term solution. You only need to had -Dxxxxx in Makefile.libretro core defines, along with other core specific defines (like the one allowing cpu overclocking or sprites limit removal hacks)

I see, it definitively makes sense, i am changing them now.

I am also adding a switch to hide all the new audio core options like snes9x has.

eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 6, 2020
@eadmaster
Copy link
Author

eadmaster commented Dec 9, 2020

Please confirm me my latest changes are ok, so i can submit a PR for this...

@eadmaster eadmaster changed the title Add per-sound channel individual volume core options (WIP, please review my fork) Add per-sound channel individual volume core options Dec 13, 2020
eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 13, 2020
fixed PSG Noise channel volume scaling

reworked PSG volume scaling formula, cleanups (libretro#223)

reworked MD volume scaling formula

added preproc tests for libretro (libretro#223)

added first implementation of SMS FM (YM2413) channel volume scaling (libretro#223)

added core option Show Advanced Audio Volume Settings

use USE_PER_SOUND_CHANNELS_CONFIG CFLAG for the new core options

changed volumes array types to avoid casting (libretro#223)

minor fixes
@eadmaster
Copy link
Author

eadmaster commented Dec 13, 2020

nevermind, i've just sent the PR, comment there if you want.

eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 13, 2020
Added per-sound channel individual volume core options (libretro#223)

fixed PSG Noise channel volume scaling

reworked PSG volume scaling formula, cleanups (libretro#223)

reworked MD volume scaling formula

added preproc tests for libretro (libretro#223)

added first implementation of SMS FM (YM2413) channel volume scaling (libretro#223)

added core option Show Advanced Audio Volume Settings

use USE_PER_SOUND_CHANNELS_CONFIG CFLAG for the new core options

changed volumes array types to avoid casting (libretro#223)

minor fixes
eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 13, 2020
Added per-sound channel individual volume core options (libretro#223)

fixed PSG Noise channel volume scaling

reworked PSG volume scaling formula, cleanups (libretro#223)

reworked MD volume scaling formula

added preproc tests for libretro (libretro#223)

added first implementation of SMS FM (YM2413) channel volume scaling (libretro#223)

added core option Show Advanced Audio Volume Settings

use USE_PER_SOUND_CHANNELS_CONFIG CFLAG for the new core options

changed volumes array types to avoid casting (libretro#223)

minor fixes
eadmaster pushed a commit to eadmaster/Genesis-Plus-GX that referenced this issue Dec 13, 2020
Added per-sound channel individual volume core options (libretro#223)

fixed PSG Noise channel volume scaling

reworked PSG volume scaling formula, cleanups (libretro#223)

reworked MD volume scaling formula

added preproc tests for libretro (libretro#223)

added first implementation of SMS FM (YM2413) channel volume scaling (libretro#223)

added core option Show Advanced Audio Volume Settings

use USE_PER_SOUND_CHANNELS_CONFIG CFLAG for the new core options

changed volumes array types to avoid casting (libretro#223)

minor fixes
inactive123 added a commit that referenced this issue Dec 14, 2020
Added per-sound channel individual volume core options (#223)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants