search

trash80 / mGB

mGB - Nintendo Gameboy MIDI control for Arduinoboy
GNU General Public License v2.0
233 stars 31 forks source link

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

Open greigs opened 5 years ago

greigs commented 5 years ago

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 commented 5 years ago

@zemzelett did you encounter this issue during the rewrite?

zemzelett commented 5 years ago

Yes. During my rewrite i encountered something like this. I had random notes playing above a certain note value. It was due to the lookup table being restricted. I think i implemented something to prevent playing notes beyond the actual possible range of the Gameboy.

i guess something similar is happening for you. The code probably wraps to the lowest possible note or it goes beyond the lookup picking up „random values“ for the frequency. You might find something looking into the section of the code, where the frequency value is being looked up. (e.g.: https://github.com/trash80/mGB/blob/master/Source/mGBASMSynthFunctions.s#L429)

I’m currently on the move, so i’m not able to go into too much detail.

Am 10.02.2019 um 15:07 schrieb Greig Stewart notifications@github.com:

@zemzelett https://github.com/zemzelett did you encounter this issue during the rewrite?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/trash80/mGB/issues/3#issuecomment-462135922, or mute the thread https://github.com/notifications/unsubscribe-auth/ALBq918h_o--mpfjlWHidA4OfGdvspZzks5vMCeugaJpZM4azEh9.

trash80 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

dalton-tulou commented 4 years ago

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: https://github.com/dalton-tulou/mGB/commit/dd486d81fd526942554fc3c77cfe6451e55e77a6

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.