Locked
Posted: 1/22/2005 1:45 AM
(Edited: 1/22/2005 1:51 AM)
Post subject: [PATCH/unix] Sound desync bug fix proposal
Quote
Fabianx
Joined: 12/17/2004
Posts: 99
Location: Karlsruhe, Germany
Hi, did I say I wanted to look at this tomorrow again? Well I guess I could not sleep. First the bad news: It seems there is no generic approach for all architectures possible, so each one has to be fixed seperately. (else you would duplicate very much code) But here the good news: With just a very small change its possible to make at least the unix version completely deterministic (by adding a -detsound option, which will be needed for recording as for playing - I think its the best we can do at the moment) I don't have windows, so I can't implement it for the Win version. I propose the following patch, which might loose sound samples (once in a while, see the outcommented printf), but is completely deterministic. I have not heard any glitches though. 
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/cpuexec.cpp snes9x-1.43-dev-src/snes9x/cpuexec.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/cpuexec.cpp      2005-01-22 02:19:03.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/cpuexec.cpp      2005-01-22 02:27:07.000000000 +0100
@@ -104,6 +104,8 @@

 void S9xMainLoop (void)
 {
+    static int counter=1;
+
     for (;;)
     {
 #ifdef DEBUG_MAXCOUNT
@@ -201,7 +203,10 @@
        if (SA1.Executing)
            S9xSA1MainLoop ();
        DO_HBLANK_CHECK();
+       counter++;
     }
+    printf("Got %8d times through the main for-loop\n", counter);
+    counter=0;
     Registers.PC = CPU.PC - CPU.PCBase;
     S9xPackStatus ();
     APURegisters.PC = IAPU.PC - IAPU.RAM;
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/Makefile snes9x-1.43-dev-src/snes9x/Makefile
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/Makefile 2005-01-22 02:13:43.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/Makefile 2005-01-22 02:27:17.000000000 +0100
@@ -1,7 +1,7 @@
 # Generated automatically from Makefile.in by configure.
 ZSNESFX=1
 ZSNESC4=1
-ASMCPU=1
+#ASMCPU=1
 #SPC700ASM=1
 NETPLAY=1
 UNZIP=1
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.cpp snes9x-1.43-dev-src/snes9x/snes9x.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.cpp       2005-01-21 17:01:42.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/snes9x.cpp       2005-01-22 02:32:50.000000000 +0100
@@ -254,6 +254,12 @@
            {
                Settings.NextAPUEnabled = FALSE;
            }
+           else if (strcasecmp (argv [i], "-ds") == 0 ||
+                    strcasecmp (argv [i], "-detsound") == 0)
+           {
+               Settings.SoundDeterministic = TRUE;
+           }
+
            else if (strcasecmp (argv [i], "-soundskip") == 0 ||
                     strcasecmp (argv [i], "-sk") == 0)
            {
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.h snes9x-1.43-dev-src/snes9x/snes9x.h
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.h 2005-01-22 02:20:10.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/snes9x.h 2005-01-22 02:13:32.000000000 +0100
@@ -330,6 +330,7 @@
     bool8  DisableSampleCaching;
     bool8  DisableMasterVolume;
     bool8  SoundSync;
+    bool8  SoundDeterministic;
     bool8  InterpolatedSound;
     bool8  ThreadSound;
     bool8  Mute;
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/unix/unix.cpp snes9x-1.43-dev-src/snes9x/unix/unix.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/unix/unix.cpp    2005-01-22 02:22:15.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/unix/unix.cpp    2005-01-22 02:37:12.000000000 +0100
@@ -392,6 +392,9 @@

 #include "cheats.h"

+static void SoundTrigger();
+static void S9xPrepareMixSamples(void);
+
 int main (int argc, char **argv)
 {
     if (argc < S9xMinCommandLineArgs ())
@@ -641,7 +644,12 @@
            || (CPU.Flags & (DEBUG_MODE_FLAG | SINGLE_STEP_FLAG))
 #endif
            )
-           S9xMainLoop ();
+       {
+          S9xMainLoop ();
+          if (Settings.SoundDeterministic)
+                  S9xPrepareMixSamples();
+       }
+

        if (Settings.Paused
 #ifdef DEBUGGER
@@ -1928,6 +1936,29 @@
     }
 }

+static void S9xPrepareMixSamples()
+{
+       /* Where to put the samples to */
+       unsigned byte_offset = so.play_position +
+                     (so.sixteen_bit ? (so.samples_mixed_so_far << 1)
+                                     : so.samples_mixed_so_far);
+       /* Number of samples to generate now */
+        int sample_count = so.buffer_size;
+
+        if (so.sixteen_bit)
+        {
+           /* to prevent running out of buffer space,
+            * create less samples
+            */
+            sample_count >>= 1;
+        }
+
+       /* Mix the missing samples */
+       S9xMixSamplesO (Buf, sample_count,
+                         byte_offset & SOUND_BUFFER_SIZE_MASK);
+       so.samples_mixed_so_far += sample_count;
+}
+
 void *S9xProcessSound (void *)
 {
     /* Linux and Sun versions */
@@ -2013,8 +2044,9 @@
                      (so.sixteen_bit ? (so.samples_mixed_so_far << 1)
                                      : so.samples_mixed_so_far);
 //printf ("%d:", sample_count - so.samples_mixed_so_far); fflush (stdout);
-       if (Settings.SoundSync == 2)
+       if (Settings.SoundSync == 2 || Settings.SoundDeterministic)
        {
+           //printf("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Really big BUG!\n");
            /*memset (Buf + (byte_offset & SOUND_BUFFER_SIZE_MASK), 0,
                    sample_count - so.samples_mixed_so_far);*/
        }
Its against my patchset, so it might not apply completely clean, but you should easily get the idea. Also the first two files are for determining deterministic behaviour. I do run: snes9x -detsound [...] > loga1 snes9x -detsound [...] > loga2 And edit the files after I have loaded the movie. They should be exact the same until some offset, where one movie was stopped earlier than the other one. I would have implemented the same for a (blind) windows patch, but windows does not use a ring buffer, but a linear one it seems. TNSe, blip: Perhaps you want to try to implement such a S9xPrepareMixSamples function for windows and run it after S9xMainLoop? cu Fabian PS: I hope that this will finally fix those sound issues.
Posted: 1/22/2005 1:51 AM
Quote
Bisqwit
Editor, Active player (296)
Joined: 3/8/2004
Posts: 7469
Location: Arzareth
I already have a completely deterministic sound code in my recording patch (assuming threads are not used). http://tasvideos.org/ConvertingSMVToAVIInLinux.html
Posted: 1/22/2005 2:01 AM
Quote
Fabianx
Joined: 12/17/2004
Posts: 99
Location: Karlsruhe, Germany
Bisqwit wrote:
I already have a completely deterministic sound code in my recording patch (assuming threads are not used). http://tasvideos.org/ConvertingSMVToAVIInLinux.html
Yes, this is a _very nice_ solution and I think its even the best one (using NO_BLOCK in a _very_ nice and efficient way), although I think I would put it after PutFrame and not before. Sorry, for not appreciating your work at first bisqwit, but I was a bit shocked that you already had a very good solution for something on what I worked a whole day. Well, at least I learned a lot ;-). But I think we should try to get any of such option into the mainstream code soon, because then we can have timeattacks for some more great games and we even fix the netplay code! ;-) cu Fabian
Posted: 1/23/2005 2:31 AM
Post subject: Deterministc sound proposal
Quote
Fabianx
Joined: 12/17/2004
Posts: 99
Location: Karlsruhe, Germany
Hi, here is a deterministic sound proposal for all architectures: 
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/cpuexec.cpp snes9x-1.43-dev-src/snes9x/cpuexec.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/cpuexec.cpp      2005-01-22 02:19:03.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/cpuexec.cpp      2005-01-23 03:25:15.000000000 +0100
@@ -95,6 +95,7 @@
 #include "debug.h"
 #include "snapshot.h"
 #include "gfx.h"
+#include "soundux.h"
 #include "missing.h"
 #include "apu.h"
 #include "dma.h"
@@ -301,6 +302,8 @@
        {
            // Start of V-blank
            S9xEndScreenRefresh ();
+           if (Settings.SoundDeterministic)
+               S9xMixSamplesPrepare ();
            IPPU.HDMA = 0;
            // Bits 7 and 6 of $4212 are computed when read in S9xGetPPU.
            missing.dma_this_frame = 0;
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/i386/cpuexec.S snes9x-1.43-dev-src/snes9x/i386/cpuexec.S
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/i386/cpuexec.S   2005-01-21 14:55:18.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/i386/cpuexec.S   2005-01-23 03:19:25.000000000 +0100
@@ -391,6 +391,10 @@
        cmpl %eax,%edx
        jne .L165
        ccall S9xEndScreenRefresh
+       testb $0xff, SoundDeterministic
+       jz .nosounddet
+       ccall S9xMixSamplesPrepare
+.nosounddet:
        movb Brightness,%al
        xorl %ecx,%ecx
        movb %al,MaxBrightness
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/i386/offsets.h snes9x-1.43-dev-src/snes9x/i386/offsets.h
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/i386/offsets.h   2005-01-22 02:13:34.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/i386/offsets.h   2005-01-23 03:55:07.000000000 +0100
@@ -108,6 +108,7 @@
 #define Paused Settings + 17
 #define PAL Settings + 29
 #define SoundSync Settings + 108
+#define SoundDeterministic Settings + 109
 #define SA1Enabled Settings + 82
 #define SuperFXEnabled Settings + 80
 #define ApuP APURegisters + 0
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/offsets.cpp snes9x-1.43-dev-src/snes9x/offsets.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/offsets.cpp      2004-07-11 23:50:59.000000000 +0200
+++ snes9x-1.43-dev-src/snes9x/offsets.cpp      2005-01-23 03:55:06.000000000 +0100
@@ -243,6 +243,7 @@
     OFFSET7(Paused,Paused)
     OFFSET7(PAL,PAL)
     OFFSET7(SoundSync,SoundSync)
+    OFFSET7(SoundDeterministic,SoundDeterministic)
     OFFSET7(SA1Enabled,SA1)
     OFFSET7(SuperFXEnabled,SuperFX)

diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.cpp snes9x-1.43-dev-src/snes9x/snes9x.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.cpp       2005-01-21 17:01:42.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/snes9x.cpp       2005-01-22 02:32:50.000000000 +0100
@@ -254,6 +254,12 @@
            {
                Settings.NextAPUEnabled = FALSE;
            }
+           else if (strcasecmp (argv [i], "-ds") == 0 ||
+                    strcasecmp (argv [i], "-detsound") == 0)
+           {
+               Settings.SoundDeterministic = TRUE;
+           }
+
            else if (strcasecmp (argv [i], "-soundskip") == 0 ||
                     strcasecmp (argv [i], "-sk") == 0)
            {
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.h snes9x-1.43-dev-src/snes9x/snes9x.h
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/snes9x.h 2005-01-22 02:20:10.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/snes9x.h 2005-01-22 02:13:32.000000000 +0100
@@ -330,6 +330,7 @@
     bool8  DisableSampleCaching;
     bool8  DisableMasterVolume;
     bool8  SoundSync;
+    bool8  SoundDeterministic;
     bool8  InterpolatedSound;
     bool8  ThreadSound;
     bool8  Mute;
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/soundux.cpp snes9x-1.43-dev-src/snes9x/soundux.cpp
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/soundux.cpp      2005-01-22 01:48:29.000000000 +0100
+++ snes9x-1.43-dev-src/snes9x/soundux.cpp      2005-01-23 14:57:59.000000000 +0100
@@ -1442,6 +1442,102 @@
 extern uint8 int2ulaw (int);
 #endif

+
+uint8 SoundBuffer[SOUND_BUFFER_SIZE];
+int32 play_position=0;
+int32 samples_mixed_so_far=0;
+int32 turbo_on=0;
+
+void S9xMixSamplesPrepare ()
+{
+        static int carry=0;
+       if (!Settings.SoundDeterministic)
+               return;
+
+       // Go back to "sync with soundcard" after turbo mode
+       if (!Settings.TurboMode && turbo_on)
+       {
+        play_position=0;
+        samples_mixed_so_far=0;
+         turbo_on=0;
+       }
+
+       if (Settings.TurboMode)
+          turbo_on=1;
+       /* Where to put the samples to */
+       unsigned byte_offset = play_position +
+                     (so.sixteen_bit ? (samples_mixed_so_far << 1)
+                                     : samples_mixed_so_far);
+       byte_offset &= SOUND_BUFFER_SIZE_MASK; /* wrap */
+
+       /* Number of samples to generate now */
+       carry += so.playback_rate * Settings.FrameTime;
+       int sample_count = carry / 1000000, sample16=1;
+        carry -= sample_count * 1000000;
+       if(so.stereo)       sample_count *= 2;
+
+        if (so.sixteen_bit)
+           sample16=2;
+
+       /* Mix the samples */
+       samples_mixed_so_far += sample_count;
+       for(;;)
+       {
+
+        int I = sample_count;
+        if (byte_offset + I*sample16 > SOUND_BUFFER_SIZE)
+        {
+                I = (SOUND_BUFFER_SIZE - byte_offset) / sample16;
+        }
+        if(I == 0) break;
+
+        S9xMixSamplesR (SoundBuffer+byte_offset, I);
+
+        sample_count-= I;
+        byte_offset += I*sample16;
+        byte_offset &= SOUND_BUFFER_SIZE_MASK; /* wrap */
+    }
+}
+
+void S9xMixSamples (uint8 *buffer, int sample_count)
+{
+    if (samples_mixed_so_far<sample_count)
+           return;
+
+    if (!Settings.SoundDeterministic)
+    {
+        S9xMixSamplesR (buffer, sample_count);
+       return;
+    }
+    unsigned bytes_to_write = sample_count;
+    if(so.sixteen_bit) bytes_to_write <<= 1;
+
+    unsigned byte_offset = play_position, byte_offset_w=0;
+    play_position += bytes_to_write;
+    play_position &= SOUND_BUFFER_SIZE_MASK; /* wrap to beginning */
+
+    /* Copy the samples to the buffer until nothing is left */
+    for(;;)
+    {
+
+        int I = bytes_to_write;
+        if (byte_offset + I > SOUND_BUFFER_SIZE)
+        {
+                I = SOUND_BUFFER_SIZE - byte_offset;
+        }
+        if(I == 0) break;
+
+        memcpy ((char *) buffer + byte_offset_w, (char *) SoundBuffer + byte_offset, I);
+
+        bytes_to_write -= I;
+        byte_offset_w += I;
+        byte_offset += I;
+        byte_offset &= SOUND_BUFFER_SIZE_MASK; /* wrap */
+    }
+    /* All data copied. */
+    samples_mixed_so_far -= sample_count;
+}
+
 // For backwards compatibility with older port specific code
 void S9xMixSamplesO (uint8 *buffer, int sample_count, int byte_offset)
 {
@@ -1451,7 +1547,7 @@
 END_OF_FUNCTION(S9xMixSamplesO);
 #endif

-void S9xMixSamples (uint8 *buffer, int sample_count)
+void S9xMixSamplesR (uint8 *buffer, int sample_count)
 {
     int J;
     int I;
diff -ur snes9x-1.43-dev-src.fabian-patchset/snes9x/soundux.h snes9x-1.43-dev-src/snes9x/soundux.h
--- snes9x-1.43-dev-src.fabian-patchset/snes9x/soundux.h        2004-07-11 23:51:00.000000000 +0200
+++ snes9x-1.43-dev-src/snes9x/soundux.h        2005-01-23 03:36:37.000000000 +0100
@@ -239,7 +239,9 @@
 void S9xFixEnvelope (int channel, uint8 gain, uint8 adsr1, uint8 adsr2);
 void S9xStartSample (int channel);

+EXTERN_C void S9xMixSamplesPrepare (void);
 EXTERN_C void S9xMixSamples (uint8 *buffer, int sample_count);
+EXTERN_C void S9xMixSamplesR (uint8 *buffer, int sample_count);
 EXTERN_C void S9xMixSamplesO (uint8 *buffer, int sample_count, int byte_offset);
 bool8 S9xOpenSoundDevice (int, bool8, int);
 void S9xSetPlaybackRate (uint32 rate);
Tell me what you think of it. cu Fabian PS: And yes this was mostly copied from the unix ringbuffer-version. PPS: Revision 3
Posted: 2/6/2005 7:21 PM
Quote
nitsuja
Emulator Coder, Skilled player (1301)
Joined: 12/21/2004
Posts: 2687
Computer TASer of 2011 TAS of 2011 First edition TAS of 2011 Computer TAS of 2011 Sega TAS of 2010 Sega TAS of 2008 Sega TAS of 2007 Glitchy TAS of 2007
I tried applying these patches to the Windows version of Snes9x, but with no success. Apparently there are multiple things causing the problem on Windows: 1.) While sound is generated in sync, that sound gets processed from an OS-threaded timer 2.) Processing the sound also generates more sound in addition to playing it 3.) Processing the sound queries DirectSound how far it's gotten in playing the sound and uses that number to decide what to do (1) can be fixed by moving what the timer thread does into some regular-interval called function. (2) can be fixed easily but the problem is that the sound relies on this extra sound generation to not sound like absolute crap. Maybe fixable by decreasing the above interval time. (3) I don't see how this can be fixed; the value it returns seems to change randomly within a large range, so nothing deterministic can approximate it without totally destroying the sound quality The above patch looks like it doesn't even change or disable the sound code causing for these problems so I don't see how it can work.

Locked