Fix ADPCMA channel volume bit length - Githubissues

jotego / jt12

FM sound source written in Verilog, fully compatible with YM2612, YM3438 (JT12), YM2203 (JT03) and YM2610 (JT10)

GNU General Public License v3.0

120 stars 22 forks source link

Fix ADPCMA channel volume bit length #41

Closed greyrogue closed 5 years ago

greyrogue commented 5 years ago

The ADPCMA channel volume field is only 5 bits wide.
Fixing this will likely throw off audio balancing.
There are some experimental changes in MiSTer to address this, but they aren't based on actual hardware.

greyrogue commented 5 years ago

I have made several more fixes/changes here: https://github.com/greyrogue/NeoGeo_MiSTer/commit/b9158b8e1cea2bbe4ea44172fb2fc31ee6561a67 Some I'm sure are correct (octal values are labelled decimal in the LUT), others I'm uncertain of. Do you want to comment on any of them?

greyrogue commented 5 years ago

I've left comments with the changes to explain them.

jotego commented 5 years ago

I have been reviewing the documentation. I don't see why you claim the ADPCM-A volume setting to be 5 bits and not 6. Both for YM2608 and YM2610 available documents show 6 bits for ADPCM-A volume. Why do you think it should be 5?

greyrogue commented 5 years ago

https://wiki.neogeodev.org/index.php?title=ADPCM#Registers says ADPCMA is only 5 bits. Mame source says the same, but I don't know if these sources are correct or not. Thanks for reviewing.

greyrogue commented 5 years ago

(In the above link master volume is 6, but channel volume is only 5)

jotego commented 5 years ago

Yes, it is the same for YM2608 too. I was looking at the master volume (as I was probably doing when I introduced the bug. Thanks a lot for noticing it and fixing it :-)