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

MIDI Notes above 0x50 (G# 5) all play the same wrong note 0x24 #3

Open
greigs opened this issue Feb 10, 2019 · 6 comments · Fixed by TonyTwoStep/mGB#1 · May be fixed by #6
Open

MIDI Notes above 0x50 (G# 5) all play the same wrong note 0x24 #3

greigs opened this issue Feb 10, 2019 · 6 comments · Fixed by TonyTwoStep/mGB#1 · May be fixed by #6

Comments

@greigs
Copy link

greigs commented Feb 10, 2019

I am finding building the rom using the master branch with the new make process using SDCC results in this issue. Notes above G# 5 all play C2 on channels 1 and 2. The same issue exists on channel 3 but at a different note. The newest rom in the releases directory works fine but I believe that was built using the old process.

Steps to reproduce, using Ubuntu 18.10:

  1. Cloned the repo including gbdk-2 submodule at specified commit ( 4c3abca0e2c75ae714bbd8efa01da2bd258dabd8 )
  2. make gbdk-n
  3. make mgb
  4. flash rom created at /Source to EMS cart
  5. Run on DMG

Rom is attached. mgb.zip

@greigs
Copy link
Author

greigs commented Feb 10, 2019

@zemzelett did you encounter this issue during the rewrite?

@zemzelett
Copy link

zemzelett commented Feb 10, 2019 via email

@trash80
Copy link
Owner

trash80 commented Feb 10, 2019

Just pushed some older versions I believe before I made changes for using the newer GBDK which seems to have a few different issues. Maybe this may help with investigation. Hopefully it can compile for you. I currently do not have the time to look into this, but I recall quirks when I made the GBDK changes.

@greigs
Copy link
Author

greigs commented Feb 10, 2019

For some reason the const keyword was removed from the UWORD freq[72] array in the new version. Adding this back in seems to fix the issue. This makes me think there must have been a reason for this, though. I'd also really like to know why this fixes this particular bug.

@trash80
Copy link
Owner

trash80 commented Feb 10, 2019

I do not know but that sort of wacky trickery generally indicates issues fighting with GBDK bugs/features.

dalton-tulou added a commit to dalton-tulou/mGB that referenced this issue Aug 16, 2020
@dalton-tulou
Copy link

I think the reason for this issue might be that the freq table lookups (in mGBASMSynthFunctions.s) don't use full 16-bit address addition. So if the table is located to close to the end of a 256-byte address boundary, the lookup will fail. I have prepared a fix but I haven't tested it yet: dalton-tulou@dd486d8

The significance of const is then coincidental. Having it will put the table in ROM rather than RAM, and the table just happens to end up at an address which doesn't trigger the bug, but any future change in the code or the compiler could make it reoccur.

I do think that the table should be const though, so there's a compilation error if there's an attempt to write to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants