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

Bug in NATV implementation #2

Open
pachuco opened this issue Sep 27, 2023 · 6 comments
Open

Bug in NATV implementation #2

pachuco opened this issue Sep 27, 2023 · 6 comments

Comments

@pachuco
Copy link

pachuco commented Sep 27, 2023

Stray bass on the loose in crestest.mid, channel 1, with Electric Piano 1 patch from bnk_common.bin.

https://files.catbox.moe/si7y9x.mid#crestest.mid
https://files.catbox.moe/eea12y.ogg#subbass_massage.ogg

First half is implementation as is in esfmbank(probably the driver as well).
Second half is implementation in XP x86 midi driver.

@pachuco
Copy link
Author

pachuco commented Sep 27, 2023

https://github.com/leecher1337/esfmbank/blob/main/src/essplaymid/NATV.C#L717

Changing to:

if (gBankMem[offset + 5] & 0x10) // signed?
transpose |= ~0x7F;

seems to fix things. Inb4 I've broken something somewhere else.

@pachuco
Copy link
Author

pachuco commented Sep 27, 2023

Ok, the above fix is alright, but I am also encountering my old friend, the broken channel volume.
https://files.catbox.moe/c33xgn.mid#meteor_herd.mid the fade at the end.

Maybe somewhere along the path of
MidiMessage() ->
case 0xb0: ->
case 7: ->
NATV_CalcNewVolume() ->
NATV_CalcVolume()

@leecher1337
Copy link
Owner

Yeah, you are definitely right about first fix. About second problem:
I think

https://github.com/leecher1337/esfmbank/blob/main/src/essplaymid/NATV.C#L430

fmwrite(i * 32 + j * 8 + 1, NATV_CalcVolume(voice->reg1[j], (voice->bVelocity & 3), voice->bChannel));

should be

fmwrite(i * 32 + j * 8 + 1, NATV_CalcVolume(voice->reg1[j], (voice->bVelocity >> (j*2)) & 3, voice->bChannel));

Does it help?

@pachuco
Copy link
Author

pachuco commented Sep 27, 2023

Yes it does. It also fixes intro here.

https://files.catbox.moe/mb52gl.MID#NOSTALGY.MID

@pachuco
Copy link
Author

pachuco commented Sep 28, 2023

There MIGHT also be a bug in voice stealing. Maybe-probably-perhaps, I'm not sure. I need to faff around and check.

Also, sorry about that overdue pull request. I wish not touch GIT with a 10 foot pole lately.

Edit: probs not

@pachuco
Copy link
Author

pachuco commented Apr 25, 2024

https://files.catbox.moe/9x1rze.mid#ttd03.mid

In this midi, there is a bug that results in large detuning of the bass channels here, on aarch64(termux) and I think x86-64.
HOWEVER...
I've been able to get aarch64 to compile as a result of a refactoring/stdint-ification, which might have introduced the bug.
That, or there is a subtle bug of the integers that breaks porting.

To that end, I've recorded a bunch of reg-value dumps of above midi. Maybe diffing them helps to tell what borked.
https://files.catbox.moe/x7oq0y.log#ttd03.log (normal or slightly broken)
https://files.catbox.moe/rtitao.log#ttd03_jank.log (very broken)

Edit: Poking through the NATV code, I can only think of pitchbend.

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