With Clownacy posting lots of fixes for the Sonic 2 driver recently, I remembered that I wanted to post some fixes for the Sonic 1 sound driver. Actually I planned to post these fixes a lot earlier (the text file with the original notes dates back to 30 March 2013), but kept forgetting about it. I ran into most of these bugs when working on the S2 Clone Driver for Sonic 2 Recreation. For some of the bugs there are example files that demonstrate them. There are MP3s and VGMs, the latter ones contain slightly longer segments of the used songs. Note: Most of the bugs were fixed in the Sonic 2 driver, sometimes in a slightly different way. Those that aren't should be easily portable to the S2 driver though. A few general notes about the code style: The code excerpts are based on the sound driver I used in S2R, so it's a mix between the 2005 disassembly (actual numbers for offsets) and the SVN/Git disassembly (comments/labels). Labels from the 2005 disassembly are listed in the comments, either directly after the label or in [square brackets] for branches. The locret_xxx labels are called @locret in the SVN/Git disassembly. Fix modulation during rests (fixed in Sonic 2) You won't notice this bug unless you have an FM instrument with a long release rate and enable modulation in areas with rests in your song. But in these cases, you'll likely be confronted with "rumbling" in your songs. What causes this? Sonic 1 applies the modulation effect on rests where the frequency is set to 0, so it sort of corrupts the frequency. (It will apply the modulation effect to frequency 0, so it will send frequencies like 0, 2, 0, 7FE, 0 to the FM chip. Also, the "rest frequency" is 0 for FM only. It is -1 for PSG channels, but the bug has no audible effect on them.) Code: DoModulation: ;sub_71DC6 ; XREF: FMUpdateTrack; PSGUpdateTrack addq.w #4,sp ; Do not return to caller (but see below) btst #3,(a5) ; Is modulation active? beq.s locret_71E16 ; Return if not The fix is quite easy and actually a backport from the Sonic 2 sound driver. Just paste these 2 lines right below the addq.w: Code: btst #1,(a5) ; Is note playing? bne.s locret_71E16 ; no - return Fix PSG fading bug (1-up bug, part 1) (fixed in Sonic 2) After the 1-up ends, when the music fades in, you may randomly hear some short and loud PSG notes. Or even worse, a long PSG tone with rising pitch. (Example MP3/VGM) The tone ends as soon as another notes plays on the respective PSG channels. It can still be pretty annoying though. The problem is, that some parts of the sound driver call the SetPSGVolume subroutine without checking, if the PSG volume is between $0 and $F. And if it isn't, the calculation will overflow and it will write to the next channel's frequency register instead. (The problematic code is not in DoFadeIn, btw. It's in PSGDoVolFX. It is possible to easily reproduce the bug by setting the PSG channel volume to $10+ and its instrument to $00.) To fix this, we could either add an additional check at every point where SetPSGVolume is called. Or we could add it one time, in PSGSendVolume, and catch all cases - this is what we'll do. This is the code that sends the PSG volume: Code: PSGSendVolume: ;loc_7297C or.b 1(a5),d6 ; Add in track selector bits addi.b #$10,d6 ; Mark it as a volume command move.b d6,($C00011).l We now need to limit the volume, which is stored in d6, to [$0, $F] before sending it to the PSG chip. Just paste these 4 lines directly under the PSGSendVolume-label: Code: cmpi.b #$10, d6 ; Is volume $10 or higher? blo.s L_psgsendvol ; Branch if not moveq #$F, d6 ; Limit to silence and fall through L_psgsendvol: No, we're not finished yet. Now the sound driver does the volume check 2 times at a few places. This is useless work and costs valuable CPU cycles, since we're within an interrupt routine. Remove useless PSG volume checks - step 1: Code: PSGDoVolFX: ;loc_7292E ; XREF: PSGUpdateTrack ; ... L_gotflutter: ;loc_72960 add.w d0,d6 ; Add flutter to volume cmpi.b #$10,d6 ; Is volume $10 or higher? bcs.s SetPSGVolume ; Branch if not [sub_7296A] moveq #$F,d6 ; Limit to silence and fall through Those last 3 lines look familiar, don't they? PSGSendVolume is right below, so delete the ones here or comment them out. Remove useless PSG volume checks - step 2: Code: DoFadeIn: ;sub_7267C: ; XREF: UpdateMusic ; ... L1_psgloop: ;loc_726B4 tst.b (a5) ; Is track playing? bpl.s L1_nextpsg ; Branch if not [loc_726CC] subq.b #1,9(a5) ; Reduce volume attenuation move.b 9(a5),d6 ; Get value cmpi.b #$10,d6 ; Is it is < $10? bcs.s L1_sendpsgvol ; Branch if yes moveq #$F,d6 ; Limit to $F (maximum attenuation) L1_sendpsgvol: ;loc_726C8 jsr SetPSGVolume(pc) Like above, you can remove the 3 lines from cmpi.b to moveq. You can also remove the sendpsgvol-label, because it isn't used anymore. Fix PSG noise bug (1-up bug, part 2) You probably never used the PSG's periodic noise, but it makes a nice sawtooth-like bass sound. Sonic 1 has a very annoying bug that occurs when fading back to a song that uses it: It doesn't set the noise mode after fading from the 1-up, so it plays white noise instead of a bass sound. (Example MP3/VGM) The solution is to restore the noise type in the cfFadeInToPrevious routine. (This is already done when an SFX ends, btw.) Code: cfFadeInToPrevious: ;loc_72B14 ; XREF: coordflagLookup ; ... moveq #2,d7 L2_psgloop: ;loc_72B66 btst #7,(a5) ; Is track playing? beq.s L2_nextpsg ; Branch if not bset #1,(a5) ; Set track at rest bit jsr PSGNoteOff(pc) ; [sub_729A0] add.b d6,9(a5) ; Apply current volume fade-in L2_nextpsg: ;loc_72B78 adda.w #$30,a5 dbf d7,L2_psgloop Directly above the L2_nextpsg label, paste these 3 lines: Code: cmpi.b #$E0, 1(a5) ; is this the Noise Channel? bne.s L2_nextpsg ; no - skip move.b $1F(a5), ($C00011).l ; restore Noise setting Done - enjoy using the periodic noise. Fix for 0 FM/DAC channels (fixed in Sonic 2) Most of you will probably never make SMPS files that use only the 3 PSG channels, but if you like to experiment with SMPS music, you might encounter this bug. In general, the song might or might not load. The behaviour can be random, because it uses uninitialized registers. Let's look at some of the code in the Sound_PlayBGM routine: Code: L_nospeedshoes: ;loc_72068 move.b d0,2(a6) move.b d0,1(a6) moveq #0,d1 movea.l a4,a3 addq.w #6,a4 ; Point past header moveq #0,d7 move.b 2(a3),d7 ; load number of FM+DAC channels beq.w L_bgm_fmdone ; branch if zero [loc_72114] subq.b #1,d7 move.b #$C0,d1 ; Default AMS+FMS+Panning move.b 4(a3),d4 ; [!] load tempo dividing timing moveq #$30,d6 ; [!] load F8 gosub coord. flag pointer move.b #1,d5 ; [!] Note duration for first "note" lea $40(a6),a1 You see the 3 lines marked with [!] - these load a few default values that are important for the driver setup. If there are 0 FM/DAC tracks, the beq.w L_bgm_fmdone skips them and the PSG tracks will be set up with garbage values. (Since you're lucky, an unmodded Sonic 1 driver just won't play anything at all. But it can also play the first note and hang then.) The fix is quite simple, just move these 3 lines a bit up somewhere above of the beq.w so that they get processed even if there are no FM/DAC channels. I recommend to move them between addq.w #6,a4 and moveq #0,d7. Fix the DAC 1-up bug (fixed in Sonic 2) You know how getting a 1-up in the Special Stage in Sonic 1 mutes the 6th FM channel when fading in? The reason is, that the 1-up enables the YM2612's DAC mode (replaces the 6th FM channel with 8-bit PCM), but when fading in it doesn't disable the DAC mode. The solution is really simple - just write value $00 to register $2B of YM2612's port 0 to disable the DAC and enable the 6th FM channel again. It doesn't do any harm to songs that use the DAC, because the Z80 DAC driver enables the DAC mode before playing any sound.* (Sonic 1 only disables the DAC when starting to play a song with exactly 7 FM/DAC channels, btw.) * Custom DAC drivers might behave differently. Now let's look at some code: Code: cfFadeInToPrevious: ;loc_72B14 ; XREF: coordflagLookup movea.l a6,a0 lea $3A0(a6),a1 move.w #$87,d0 ; $220 bytes to restore L_restoreramloop: ;loc_72B1E move.l (a1)+,(a0)+ dbf d0,L_restoreramloop bset #2,$40(a6) ; Set SFX overriding bit movea.l a5,a3 move.b #$28,d6 sub.b $26(a6),d6 ; If fade already in progress, this adjusts track volume accordingly All you need to do is pasting the following 3 lines between the dbf and the bset commands. (You can actually paste them at other places, too, but I recommend this one.) Code: move.b #$2B, d0 ; Register: DAC mode (bit 7 = enable) moveq #$00, d1 ; Value: DAC mode disable jsr WriteFMI(pc) ; Write to YM2612 Port 0 [sub_7272E] Note: There is also an alternative solution that isn't as simple, but would allow dynamic switching between DAC mode and 6 FM channels during a song: Keep track of the current DAC mode (driver init should set it to $00) by using a byte in sound RAM that is always the same as the last value written to YM2612 register $02B. Set the DAC mode to $80 when playing a DAC sound (at the end of @gotsampleduration/loc_71C88) and to $00 when playing a note on FM6. The latter requires an additional check in FMNoteOn to prevent it from disabling DAC mode when a note is played on FM 1-5. Fix the SEGA sound panning bug Does your Game Over/Continue song use panned DAC drums? If it does, you may've run into a bug where the SEGA chant is panned to the left or right speaker. The cause is, that the song's panning doesn't get reset when the SEGA chant is played, so the effect remains. (It should be noted that starting any song with 1-6 FM/DAC channels resets the panning.) Anyway, the solution is simple: We reset the panning bits before playing the SEGA chant. This is done using these 3 lines (the DAC channel uses the FM6 stereo mask): Code: move.b #$B6, d0 ; Register: FM3/6 Panning move.b #$C0, d1 ; Value: Enable both channels jsr WriteFMII(pc) ; Write to YM2612 Port 1 (for FM6) [sub_72764] This should be very the first thing you do in PlaySega/Sound_E1.
Since this bug has been brought up in one of the recent topics and two or more people had issues with it in the past years, I'd like to cross-post the fix I posted earlier. There's a bug in Sonic 1's SMPS driver that causes incorrect voice data to be loaded when updating the TL operators of the SFX channel. Surprisingly, the bug is unlikely to show up unless certain rare conditions are met (these likely depend on the voices used by currently playing BGM or recently played special SFX, they manage to mask the bug most of the time). Fixing the bug is simple. Go to the SendVoiceTL (or sub_72CB4 in case you're using Hivebrain's disassembly), find and replace this: Code: movea.l $20(a6),a1 with this Code: movea.l $20(a5),a1 The bug only affects SFX that use flag E6 to alter channel volume, most notably, spin SFX. The misspelled register caused SMPS to load the wrong voice, hence wrong TL values will be read, which sometimes make spin SFX volume overflow.
No offense to vladikcomper, or anything, but the Git disasm details quite a few bugs, including the one mentioned above. Just search for the word 'DANGER'.
I know this is now a bit of an old post, but thanks for this, ValleyBell. Fixed a lot of stuff that sounded off in my build that I can't catch outside of the ROM.
Ready for another PSG fading bug? If you try fading a song that uses PSG volume envelopes, you may hear oddities like this. This bug actually comes in two parts, easier to hear in a small test song: bug1 Popping can be heard during fading while the actual envelope is playing. This one's fairly hard to hear. bug2 A tone can be heard during fading after the envelope terminates. So what's going on? Well, basically, the fading routine fails to distinguish between PSG tracks that play a plain tone, and tracks that are using envelopes. You see, with envelopes, (in the case of S1's driver, anyway) the channel's volume is updated every frame. This is not the case with channels that are only playing tones: instead, their volume is only updated when the volume is actually changed. Or, at least, they should be. cfChangePSGVolume doesn't, for some reason (maybe a bug for another time). Because PSG volume envelopes work by directly modifying the channel's volume, you should never manually update the volume when an envelope is active: the envelope already handles this, and doing so will only interfere with it. This is the cause of that 'popping' in bug1: something is updating the volume manually on an envelope-using track. The culprit? The fading subroutines. Code: ; loc_72558: @sendpsgvol: move.b zTrackVolume(a5),d6 ; Store new volume attenuation jsr SetPSGVolume(pc) ; loc_72560: @nextpsg: Notice how there's no check for if an envelope is playing? Code: tst.b zTrackVoiceIndex(a5) bne.s @nextpsg move.b zTrackVolume(a5),d6 ; Store new volume attenuation jsr SetPSGVolume(pc) ; loc_72560: @nextpsg: That should do it. Apply this to both the fade-in and fade-out subroutines. So now, the popping's gone, and the tone's vanished. We're all good now, right? Nope. bug2.1 Turns out we've only masked bug2. In case you can't hear it, the channel isn't actually fading anymore. The moment the envelope terminates, the volume stops updating. In this example, the envelope terminated partway through the fade, so the last note is slightly quieter. This shows that the bug only manifests when the envelope ends. So what's the problem now? Well, remember what I said about envelopes updating the volume every frame? That does not apply when the envelope ends. When the envelope terminates, it effectively puts the track back in tone mode: the volume no longer updates unless it has to (which is never. Again, that thing with cfChangePSGVolume is weird, I'll have to look into it). This bug in particular was already fixed in a later version of SMPS (68k Type 2), by changing this: Code: ; loc_7299A: FlutterDone: subq.b #1,zTrackVolFlutter(a5) ; Decrement flutter index rts ...into this: Code: ; loc_7299A: FlutterDone: subq.b #2,zTrackVolFlutter(a5) ; Decrement flutter index rts So what exactly's going on with this code? When the envelope ends, it simply repeats its last value over and over again. The last value being the terminator byte. This is why the volume never updates: a new volume byte is never read. So, instead, we rewind the envelope by two bytes, one for the terminator byte, and another for the last volume byte. However, there is one shortcoming with this method: see that rts? That shouldn't be there. Rather, the code should loop back, and process the new volume byte (like VolEnv_Jump2Idx and VolEnv_Reset do in other drivers). Without doing this, the volume instead only updates on every other frame (30 times per second, instead of 60). To fix this, replace that 'rts' with 'bra.s PSGDoVolFX_Loop', and insert a label called 'PSGDoVolFX_Loop' after the 'movea.l (a0,d0.w),a0' instruction in PSGDoVolFX. And with that, the bug's fixed.
