search

flamewing / flamedriver

Upgraded S3&K sound driver
https://github.com/flamewing/flamedriver-skdisasm
Other
14 stars 5 forks source link

DACs not mapped correctly in the latest release #10

Closed AngelKOR64 closed 3 years ago

AngelKOR64 commented 3 years ago

Hello, I just wanted to notify that in the latest release of the flamedriver, apparently the DACs aren't mapped correctly in the code... causing songs and sfx to not sound right. I don't know what's causing it, but I had to switch to an earlier version if the flamedriver for them to work properly. Thanks for the driver!

flamewing commented 3 years ago

Uhhh... SFX do not use the DAC, only music does. Can you tell me which version you switched to?

flamewing commented 3 years ago

Can you give me a case that reproduces the issue you are describing? Because I stuffed the latest Flamedriver on both S3&K and on Big's Fishing Derby and they both sound perfectly fine.

AngelKOR64 commented 3 years ago

I tried building the example disassembly, and it was broken there. It first said that the size was too big and it built a broken rom, then, I changed the size to 1200 in the size guess, it built and some music and sfx sounded broken. I switched to a version from around the 29th of june of last year. Thanks for responding.

flamewing commented 3 years ago

Have you tried looking at the reference disassembly? Specifically, I think that your issue(s) might be:

  1. Not using fdp2bin (see changes in Build Scripts/build.py, Build Scripts/buildS3.bat, Build Scripts/buildS3Complete.bat, and Build Scripts/buildSK.bat, as well as the Windows binary in AS/Win32/fdp2bin.exe)
  2. The SndDrvInit function.
AngelKOR64 commented 3 years ago

That's the issue I'm having, they seem to be broken in the reference disassembly, which is what was perplexing me, I honestly have no idea why.

flamewing commented 3 years ago

But the reference disassembly is the one I specifically built and tested... Please make sure you are on the latest version. Anyway, I updated it just in the off chance I had a different fdp2bin in the path before the one included in the disassembly. Can you try again?

AngelKOR64 commented 3 years ago

Hmm... I just downloaded the latest version of the reference it and tried to build it, but I ended up with a 894KB unreadable build, then I changed the size to $1200 and it gave me a readable build, but still with the issue. But if everything is okay with the driver, it could be on how my computer handles the building of this version of the driver. Thanks for replying!

flamewing commented 3 years ago

Do you mind sharing the build so I can take a look? You can do it privately by email if you don't want to release the code publicly.

MainMemory commented 3 years ago

I was just trying to integrate my music/DAC tool into the flamedriver disassembly, and I noticed that the first set of DACs has 30 drums defined in _smps2asm_inc.asm, but only 27 in DAC_Master_Table in Flamedriver.asm.

MainMemory commented 3 years ago

Also I haven't touched anything in the disassembly at all and it's building an 894KB ROM that doesn't run.

flamewing commented 3 years ago

Hm, github was trying to set it to fixed before I even got any feedback. Anyway, I committed a change by Clownacy that maybe fixes this. Can you guys let me know? I won't be able to test in the next several days.

MainMemory commented 3 years ago

This commit is where the ROM first breaks in the reference disassembly. I'll see if I can figure out anything more.

MainMemory commented 3 years ago

By copying over the Flamedriver.asm file from the commits here, I was able to narrow down the issue with the ROM being broken to the commit that adds DAC SFX. Oddly enough, Cursed S3K has the same DAC SFX support, but doesn't have this issue.

MainMemory commented 3 years ago

I tried building the example disassembly, and it was broken there. It first said that the size was too big and it built a broken rom, then, I changed the size to 1200 in the size guess, it built and some music and sfx sounded broken. I switched to a version from around the 29th of june of last year. Thanks for responding.

Turns out this is exactly the problem I'm having. Once I fixed the size equate, the ROM builds perfectly, and the DACs are correct now.

AngelKOR64 commented 3 years ago

I was finally able to try out the latest version and the DACs work fine now, however, some sound effects don't sound normal for some reason (and had to change the size of the sound driver guess to $117A). It's pretty noticeable with the elemental shields and tails' flight.

Clownacy commented 3 years ago

That bug seems to have been introduced in commit fc969638dc77a78c898ccbe7fca1636286e6c80c

Clownacy commented 3 years ago

This jump in zSendFMInstrument appears to be the cause:

        ld  a, (hl)                         ; Get current feedback/algorithm
        ld  (ix+zTrack.FeedbackAlgo), a     ; Save current feedback/algorithm
        jp  m, .gotssgeg                    ; Branch if yes

The 'Branch if yes' comment makes me wonder if that line's inclusion was accidental, since it doesn't make any sense. The previous version of the driver never detects SSGEG this way, and only seems to rely on HaveSSGEGFlag, which gets checked in the next few lines anyway.

Also, zTrack.FeedbackAlgo is never used after this, so this entire block of code can be removed.

flamewing commented 3 years ago

I have no idea where that branch came from; it doesn't even make sense as there is nothing near it to correctly set the m flag, making it essentially random.

And nice catch on FeedbackAlgo being essentially unused.

Clownacy commented 3 years ago

Oh yeah, you're right. I guess I've gotten rusty at Z80 ASM.

Clownacy commented 3 years ago

@AngelKOR64 Are you able to confirm whether this has fixed your problem or not?

flamewing commented 3 years ago

Marking it as closed.