don't click here

Various bugfixes for the Sonic 1 sound driver

Discussion in 'Engineering & Reverse Engineering' started by ValleyBell, Mar 24, 2015.

  1. ValleyBell

    ValleyBell

    Tech Member
    246
    25
    28
    researching PC-98/X68000 sound drivers
    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 (ASM):
    1. DoModulation:   ;sub_71DC6      ; XREF: FMUpdateTrack; PSGUpdateTrack
    2.         addq.w  #4,sp       ; Do not return to caller (but see below)
    3.         btst    #3,(a5)     ; Is modulation active?
    4.         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 (ASM):
    1.         btst    #1,(a5)     ; Is note playing?
    2.         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 (ASM):
    1. PSGSendVolume:  ;loc_7297C
    2.         or.b    1(a5),d6    ; Add in track selector bits
    3.         addi.b  #$10,d6     ; Mark it as a volume command
    4.         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 (ASM):
    1.         cmpi.b  #$10, d6    ; Is volume $10 or higher?
    2.         blo.s   L_psgsendvol    ; Branch if not
    3.         moveq   #$F, d6     ; Limit to silence and fall through
    4. 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 (ASM):
    1. PSGDoVolFX: ;loc_7292E      ; XREF: PSGUpdateTrack
    2.     ; ...
    3. L_gotflutter:   ;loc_72960
    4.         add.w   d0,d6       ; Add flutter to volume
    5.         cmpi.b  #$10,d6     ; Is volume $10 or higher?
    6.         bcs.s   SetPSGVolume    ; Branch if not [sub_7296A]
    7.         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 (ASM):
    1. DoFadeIn:   ;sub_7267C:     ; XREF: UpdateMusic
    2.     ; ...
    3. L1_psgloop: ;loc_726B4
    4.         tst.b   (a5)        ; Is track playing?
    5.         bpl.s   L1_nextpsg  ; Branch if not [loc_726CC]
    6.         subq.b  #1,9(a5)    ; Reduce volume attenuation
    7.         move.b  9(a5),d6    ; Get value
    8.         cmpi.b  #$10,d6     ; Is it is < $10?
    9.         bcs.s   L1_sendpsgvol   ; Branch if yes
    10.         moveq   #$F,d6      ; Limit to $F (maximum attenuation)
    11.  
    12. L1_sendpsgvol:  ;loc_726C8
    13.         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 (ASM):
    1. cfFadeInToPrevious: ;loc_72B14  ; XREF: coordflagLookup
    2.         ; ...
    3.         moveq   #2,d7
    4.  
    5. L2_psgloop: ;loc_72B66
    6.         btst    #7,(a5)     ; Is track playing?
    7.         beq.s   L2_nextpsg  ; Branch if not
    8.         bset    #1,(a5)     ; Set track at rest bit
    9.         jsr PSGNoteOff(pc)  ; [sub_729A0]
    10.         add.b   d6,9(a5)    ; Apply current volume fade-in
    11.  
    12. L2_nextpsg: ;loc_72B78
    13.         adda.w  #$30,a5
    14.         dbf d7,L2_psgloop
    Directly above the L2_nextpsg label, paste these 3 lines:
    Code (ASM):
    1.         cmpi.b  #$E0, 1(a5) ; is this the Noise Channel?
    2.         bne.s   L2_nextpsg  ; no - skip
    3.         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 (ASM):
    1. L_nospeedshoes: ;loc_72068
    2.         move.b  d0,2(a6)
    3.         move.b  d0,1(a6)
    4.         moveq   #0,d1
    5.         movea.l a4,a3
    6.         addq.w  #6,a4       ; Point past header
    7.         moveq   #0,d7
    8.         move.b  2(a3),d7    ; load number of FM+DAC channels
    9.         beq.w   L_bgm_fmdone    ; branch if zero [loc_72114]
    10.         subq.b  #1,d7
    11.         move.b  #$C0,d1     ; Default AMS+FMS+Panning
    12.         move.b  4(a3),d4    ; [!] load tempo dividing timing
    13.         moveq   #$30,d6     ; [!] load F8 gosub coord. flag pointer
    14.         move.b  #1,d5       ; [!] Note duration for first "note"
    15.         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 (ASM):
    1. cfFadeInToPrevious: ;loc_72B14  ; XREF: coordflagLookup
    2.         movea.l a6,a0
    3.         lea $3A0(a6),a1
    4.         move.w  #$87,d0     ; $220 bytes to restore
    5.  
    6. L_restoreramloop:   ;loc_72B1E
    7.         move.l  (a1)+,(a0)+
    8.         dbf d0,L_restoreramloop
    9.  
    10.         bset    #2,$40(a6)  ; Set SFX overriding bit
    11.         movea.l a5,a3
    12.         move.b  #$28,d6
    13.         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 (ASM):
    1.         move.b  #$2B, d0    ; Register: DAC mode (bit 7 = enable)
    2.         moveq   #$00, d1    ; Value: DAC mode disable
    3.         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 (ASM):
    1.         move.b  #$B6, d0    ; Register: FM3/6 Panning
    2.         move.b  #$C0, d1    ; Value: Enable both channels
    3.         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.
     
  2. Very nice! The S1 driver has a lot of problems. I've ran into a few of these bugs myself and they're quite annoying, Glad to see these fixes implemented.
     
  3. Clownacy

    Clownacy

    Tech Member
    1,093
    666
    93
    [23:36] (+vladiklaptop) So, I found a fatal programming flow in SMPS 68k.
    [23:36] (+vladiklaptop) The reason why they couldn't get it properly working with Horizontal Interrupts in Sonic 1, the reason they had to go with that dirty and unreliable trick of cancelling SMPS call if interrupt is about to happen above line #96 and moving the call to HBlank routine itself.
    [23:36] (+vladiklaptop) Due to a program error, SMPS 68k enters an infinite loop if an interrupt happens during DoModulation routine.
    [23:36] (+vladiklaptop) They couldn't figure the issue in Sonic 1, thus, they had to ensure nothing will interrupt SMPS. On the other side, if interrupts were disabled during SMPS processing, the game won't receive HBlank at time and LZ water will glitch out. So they came up with the dirty solution described above.
    [23:38] (+vladiklaptop) The bug is really intersting and hideous here
    [23:38] (+vladiklaptop) What may cause the processor to hang after an interrupt?
    [23:38] (+vladiklaptop) Almost there
    [23:38] (+vladiklaptop) Hold on
    [23:39] (+vladiklaptop) [02:14:30] <vladiklaptop> There's a programming error in DoMudilation routine that only shows itself when interrupts occur
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> It does this:
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> DoMudulation:
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> ___addq.w #4,sp ; don't want to return to caller =(
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> ___<Some code>
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> ___<Some condition>
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> ___beq.s @locret
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> ___subq.w #4,sp ; nah, changed my mind. return me to the caller, dear M68K!
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> @locret:
    [23:39] (+vladiklaptop) [02:16:21] <vladiklaptop> ___rts
    [23:39] (+vladiklaptop) [02:17:08] <vladiklaptop> So, when interrupt happens between the shown lines, the stack contents are being rewritten, so when it attempts to change the mind and return to the caller...
    [23:39] (+vladiklaptop) [02:17:23] <vladiklaptop> ... it will return to itself
    [23:39] (+vladiklaptop) [02:17:35] <vladiklaptop> ...causing an infinite loop
    [23:39] (+vladiklaptop) [02:18:40] <vladiklaptop> because that stack was overwritten once interrupt happen -- the processor has pushed the address of the opcode where interrupt happenned, overwritting the "caller" it wanted to return to

    This issue was seemingly resolved in SMPS 68k Type 2, where no such stack meddling takes place. Rather, it seems the subroutine that the addq.w was to avoid performs a check to decide if it should run or not. This fix seems to rely on Type 2's FM modulation, so, for Sonic 1's Type 1-based driver, I'll provide a more basic fix.

    This is DoModulation in its original state:
    Code (ASM):
    1. ; sub_71DC6:
    2. DoModulation:
    3.         addq.w  #4,sp               ; Do not return to caller (but see below)
    4.         btst    #3,(a5)             ; Is modulation active? (zTrackPlaybackControl)
    5.         beq.s   @locret             ; Return if not
    6.         tst.b   zTrackModulationWait(a5)    ; Has modulation wait expired?
    7.         beq.s   @waitdone           ; If yes, branch
    8.         subq.b  #1,zTrackModulationWait(a5) ; Update wait timeout
    9.         rts
    10. ; ===========================================================================
    11. ; loc_71DDA:
    12. @waitdone:
    13.         subq.b  #1,zTrackModulationSpeed(a5)    ; Update speed
    14.         beq.s   @updatemodulation       ; If it expired, want to update modulation
    15.         rts
    16. ; ===========================================================================
    17. ; loc_71DE2:
    18. @updatemodulation:
    19.         movea.l zTrackModulationPtr(a5),a0  ; Get modulation data
    20.         move.b  1(a0),zTrackModulationSpeed(a5) ; Restore modulation speed
    21.         tst.b   zTrackModulationSteps(a5)   ; Check number of steps
    22.         bne.s   @calcfreq           ; If nonzero, branch
    23.         move.b  3(a0),zTrackModulationSteps(a5) ; Restore from modulation data
    24.         neg.b   zTrackModulationDelta(a5)   ; Negate modulation delta
    25.         rts
    26. ; ===========================================================================
    27. ; loc_71DFE:
    28. @calcfreq:
    29.         subq.b  #1,zTrackModulationSteps(a5)    ; Update modulation steps
    30.         move.b  zTrackModulationDelta(a5),d6    ; Get modulation delta
    31.         ext.w   d6
    32.         add.w   zTrackModulationVal(a5),d6  ; Add cumulative modulation change
    33.         move.w  d6,zTrackModulationVal(a5)  ; Store it
    34.         add.w   zTrackFreq(a5),d6       ; Add note frequency to it
    35.         subq.w  #4,sp       ; In this case, we want to return to caller after all
    36. ; locret_71E16:
    37. @locret:
    38.         rts
    39. ; End of function DoModulation
    All 'rts's here skip the subroutine's caller, safe for the one under @calcfreq, which is sometimes used without skipping. The most obvious fix would be to remove the original 'addq.w #4,sp' and 'subq.w #4,sp', and insert 'addq.w #4,sp' before all 'rts's but the one under @calcfreq. There is a slight complication with this, however: as said before, DoModulation uses @locret to skip the caller, but the 'rts' under @locret is used by @calcfreq without skipping. The ideal thing to do would be to move the @locret label to before the 'rts' (and 'addq.w #4,sp') above @waitdone.

    By doing this, you can remove UpdateMusic from H-Int: remove the VBla_Exit label and the sole branch to it, along with its 'addq.w #4,sp', under VBla_08. Then remove the jsr to UpdateMusic in loc_119E.
     
  4. vladikcomper

    vladikcomper

    Tech Member
    211
    170
    43
    Sonic Warped
    There's one important thing you forget.

    Upon calling UpdateMusic, any interrupts are still masked, because the status registers (SR) is set to 26xx when processor accepts level 6 interrupt, that is vertical blanking interrupt (VBlank). Since interrupt priority mask is set to 6, all interrupts of this level or less will be inhibited. Since horizontal blanking interrupts (HBlank) are level 4 interrupts, the processor won't any interrupts unless the main VBlank routine finishes, which doesn't happen unless UpdateMusic finishes. That brings us the point of why they actually moved UpdateMusic to VBlank, besides the bug described above.

    The SMPS 68K driver itself can be a real monster sometimes. When it comes to initializing the BGM playback, or loading many instruments at once, -- it can take up to 25% of CPU time. It happens rarely, it happens one frame worth, but still, it happens. And when it does, the SMPS takes so long to execute, that the M68K still cannot leave the VBlank when the VDP starts drawing a new TV-frame already. Here comes the big problem: when VDP reaches a scanline where the horizontal interrupt should happen, but the processor still hasn't finished the vertical interrupt, the first one be received in time. This will lead to a lot of graphical glitches with water in LZ, which may only pop up for a couple of frames, but still rather annoying.

    So, let me describe the fix step by step in detail. It's really nothing bug and it's almost identical to yours, except for a few important additions.



    Fixing the interrupt crash bug in SMPS and optimizing the horizontal interrupts

    It's really more of a design choice -- you may completely ignore this fix. Everything won't get more buggy or less stable as Sonic Team did implement a solution for this major bug. It's a dirty, unwise and straight-forward solution, but it works -- and it works rather well.

    However, if you're looking into more flexibility, less code hacks and slightly more polished and organized interrupt routines, you may consider the steps I suggest below. Doing these little steps will allow for proper interrupt handling in the game, the way it should be done. This in turn, will allow you to perform fairly more interrupt-intensive tricks, like having per-line horizontal interrupts, something that wasn't possible with the original setup.

    First of all, open s1.sounddriver.asm, go to the DoModulation routine and replace all its code with this:

    Code (ASM):
    1.  
    2. ; ||||||||||||||| S U B R O U T I N E |||||||||||||||||||||||||||||||||||||||
    3.  
    4. ; Vladikcomper: Fixed a programming error with stack usage
    5.  
    6. DoModulation:
    7.         btst    #3,(a5)     ; Is modulation active?
    8.         beq.s   @dontreturn ; Return if not
    9.         tst.b   $18(a5)     ; Has modulation wait expired?
    10.         beq.s   @waitdone   ; If yes, branch
    11.         subq.b  #1,$18(a5)  ; Update wait timeout
    12.        
    13. @dontreturn:
    14.         addq.w  #4,sp       ; ++ Do not return to caller (but see below)
    15.         rts
    16. ; ===========================================================================
    17.  
    18. @waitdone:
    19.         subq.b  #1,$19(a5)  ; Update speed
    20.         beq.s   @updatemodulation   ; If it expired, want to update modulation
    21.         addq.w  #4,sp       ; ++ Do not return to caller (but see below)
    22.         rts
    23. ; ===========================================================================
    24.  
    25. @updatemodulation:
    26.         movea.l $14(a5),a0  ; Get modulation data
    27.         move.b  1(a0),$19(a5)   ; Restore modulation speed
    28.         tst.b   $1B(a5)     ; Check number of steps
    29.         bne.s   @calcfreq   ; If nonzero, branch
    30.         move.b  3(a0),$1B(a5)   ; Restore from modulation data
    31.         neg.b   $1A(a5)     ; Negate modulation delta
    32.         addq.w  #4,sp       ; ++ Do not return to caller (but see below)
    33.         rts
    34. ; ===========================================================================
    35.  
    36. @calcfreq:
    37.         subq.b  #1,$1B(a5)  ; Update modulation steps
    38.         move.b  $1A(a5),d6  ; Get modulation delta
    39.         ext.w   d6
    40.         add.w   $1C(a5),d6  ; Add cumulative modulation change
    41.         move.w  d6,$1C(a5)  ; Store it
    42.         add.w   $10(a5),d6  ; Add note frequency to it
    43.  
    44. @locret:
    45.         rts
    46. ; End of function DoModulation
    47.  
    With this, you've already fixed the bug. See the post above and read the beginning of this post for explanation.

    Now, open sonic.asm, go to VBla_08. At the end of it, find and completely delete these lines:

    Code (ASM):
    1.         cmpi.b  #96,(v_hbla_line).w
    2.         bhs.s   Demo_Time
    3.         move.b  #1,($FFFFF64F).w
    4.         addq.l  #4,sp
    5.         bra.w   VBla_Exit
    6.  
    That was the mentioned dirty workaround. If the water was to appear above scanline #96, they knew SMPS can slow down VBlank to longer than that, so they didn't call SMPS then, they set a special flag ($FFFFF64F), so SMPS will be called later in HBlank routine itself. However, this solution isn't exactly perfect: if SMPS happens to lag for more than entire frame, VBlank may be missed or delayed, moreover, future horizontal interrupts may be delayed or have their timming messed up. But that's an exotic case.


    Now, it's time to clean up HBlank routine from the coding garbage. Find it, find this lines near the end:

    Code (ASM):
    1.         tst.b   ($FFFFF64F).w
    2.         bne.s   loc_119E
    Delete them. You may also delete this code below as it becomes a dead code:

    Code (ASM):
    1. loc_119E:
    2.         clr.b   ($FFFFF64F).w
    3.         movem.l d0-a6,-(sp)
    4.         bsr.w   Demo_Time
    5.         jsr UpdateMusic
    6.         movem.l (sp)+,d0-a6
    7.         rte
    Now, HBlank routine looks way better, no dirty hacks and workarounds included.

    Now it's time for the final move, handling SMPS execution correctly.
    Find this:

    Code (ASM):
    1. VBla_Music:             ; XREF: VBla_00
    2.         jsr UpdateMusic
    Replace it with this:

    Code (ASM):
    1. VBla_Music:
    2.                 move    #$2300,sr       ; enable interrupts (we can accept horizontal interrupts from now on)
    3.                 bset    #0,($FFFFF64F).w    ; set "SMPS running flag"
    4.                 bne.s   VBla_Exit       ; if it was set already, don't call another instance of SMPS
    5.         jsr UpdateMusic     ; run SMPS
    6.                 clr.b   ($FFFFF64F).w       ; reset "SMPS running flag"
    7.  
    Since we've enabled interrupts within VBlank, there appears a chance, although almost impossible, that SMPS will lag so hard so it takes about a TV-frame to process, when the next VBlank will be received. In this case, we don't want another instance of SMPS to overlay the existing one, otherwise, in this almost impossible case, the sound system may crash completely. The SMPS/Treasure implements the said fix.

    Now, there appears the following race condition: when SMPS has lagged badly and the next VBlank occurred, there will be a conflict on taking Z80 bus. The original SMPS stops the Z80 for the whole time and starts it by the end, however, if VBlank will overlay its execution, various routines will attempt to stop Z80 again, then start it. So, by the end of VBlank, the Z80 will be running and when processor returns to the SMPS, it'll suffer from not being able to write to the YM2612 chip properly.

    In order to fix the last issue, go to VBla_08, then VBla_00 and remove every single stopZ80, waitZ80 and startZ80 macro from there. This way, the Z80 will remain stopped if VBlank interrupts SMPS. You may remove Z80 stops from the other VBlank routines as well.
     
  5. AURORA☆FIELDS

    AURORA☆FIELDS

    The cute one here Tech Member
    216
    24
    18
    Finland
    AMPS
    Fix Modulation Frequency bug on note-on

    So, it turns out SMPS is terrible! I know, its super shocking... Anyway, this is a fairly obscure bug probably nobody would ever notice, except me. But I did, and I am gonna help you fix it, its fairly easy too. Simply, all the bug is, is that SMPS forgets to include modulation frequency when updating frequency just after reading the tracker. Have a listen. So then, lets do this! In Hivebrain disassembly, Sonic1.asm, go to label sub_71E18. You should see following:
    Code (Text):
    1. sub_71E18:              ; XREF: sub_71CCA
    2.         btst    #1,(a5)
    3.         bne.s   locret_71E48
    4.         move.w  $10(a5),d6
    5.         beq.s   loc_71E4A
    This piece of code handles updating FM volume to YM2612. Change the above code to this:
    Code (Text):
    1. sub_71E18:              ; XREF: sub_71CCA
    2.         btst    #1,(a5)     ; check for resting flag
    3.         bne.s   locret_71E48    ; branch if is
    4.         move.w  $10(a5),d6  ; get frequency to d6
    5.         beq.s   loc_71E4A   ; if 0, then set rest flag
    6.         btst    #3,(a5)     ; check if modulation is active
    7.         beq.s   loc_71E24   ; if not, branch
    8.         add.w   $1C(a5),d6  ; add modulation frequency to d6
    Next up, PSG. Go to sub_728DC. You should see the following:
    Code (Text):
    1. sub_728DC:              ; XREF: sub_72850
    2.         move.w  $10(a5),d6 
    3.         bmi.s   loc_72920
    Simply change it to
    Code (Text):
    1. sub_728DC:              ; XREF: sub_72850
    2.         move.w  $10(a5),d6  ; get frequency to d6
    3.         bmi.s   loc_72920   ; if -1, set rest flag
    4.         btst    #3,(a5)     ; check if modulation is active
    5.         beq.s   sub_728E2   ; if not, branch
    6.         add.w   $1C(a5),d6  ; add modulation frequency to d6
    And that is it, you are now done!
     
  6. Clownacy

    Clownacy

    Tech Member
    1,093
    666
    93
    Are you sure this is a thorough fix? Looking at the code, the issue is that the modulation isn't being updated on the frame a 'no attack' note starts. The modulation not being applied to the frequency seems like a side-effect of it. S2 fixed both of those at the same time, as overkill as its method was.

    (For anyone who doesn't know S2's fix)
    Some code from my Clone Driver v2:
    Code (ASM):
    1. ; sub_71CCA:
    2. FMUpdateTrack:
    3.     subq.b  #1,SMPS_Track.DurationTimeout(a5)   ; Update duration timeout
    4.     bne.s   .notegoing          ; Branch if it hasn't expired
    5.     bclr    #4,SMPS_Track.PlaybackControl(a5)   ; Clear 'do not attack next note' bit
    6.     bsr.s   FMDoNext
    7.     bsr.w   FMPrepareNote
    8.     bsr.w   FMNoteOn
    9.     ; Clownacy | Sonic 2 adds these two branches
    10.     bsr.w   DoModulation
    11.     bra.w   FMUpdateFreq
    12. ; ===========================================================================
    13. ; loc_71CE0:
    14. .notegoing:
    15.     bsr.w   NoteFillUpdate
    16.     bsr.w   DoModulation
    17.     bra.w   FMUpdateFreq
    18. ; End of function FMUpdateTrack
    19.  

    EDIT: Now that I look at it, the Note Fill doesn't seem to be updated either.
     
  7. AURORA☆FIELDS

    AURORA☆FIELDS

    The cute one here Tech Member
    216
    24
    18
    Finland
    AMPS
    It is correct modulation is not updated on the frame. However, this goes for all notes. I do not know if it was intentional or not, so I did not fix it as if it was unintentional.