From 93bb51e24c2e2baf1b7b1e7a9b9eae4b56d22d8e Mon Sep 17 00:00:00 2001 From: Shamus Hammons Date: Tue, 24 Feb 2009 15:32:01 +0000 Subject: [PATCH] Fixes to threaded programming model (prevent lockups on exit) --- src/apple2.cpp | 91 ++++++++++++++--- src/sound.cpp | 258 ++++++++++++++----------------------------------- 2 files changed, 150 insertions(+), 199 deletions(-) diff --git a/src/apple2.cpp b/src/apple2.cpp index 31c043b..fa25cb4 100755 --- a/src/apple2.cpp +++ b/src/apple2.cpp @@ -57,9 +57,10 @@ // Debug and misc. defines #define THREADED_65C02 -//#define CPU_THREAD_OVERFLOW_COMPENSATION +#define CPU_THREAD_OVERFLOW_COMPENSATION //#define DEBUG_LC #define CPU_CLOCK_CHECKING +#define THREAD_DEBUGGING // Global variables @@ -106,6 +107,7 @@ static void BlinkTimer(void); static SDL_Thread * cpuThread = NULL; //static SDL_mutex * cpuMutex = NULL; static SDL_cond * cpuCond = NULL; +static SDL_sem * mainSem = NULL; static bool cpuFinished = false; static bool cpuSleep = false; @@ -120,6 +122,8 @@ int CPUThreadFunc(void * data) // Also, must be created in the thread that uses it... SDL_mutex * cpuMutex = SDL_CreateMutex(); +// decrement mainSem... +//SDL_SemWait(mainSem); #ifdef CPU_THREAD_OVERFLOW_COMPENSATION float overflow = 0.0; #endif @@ -129,6 +133,12 @@ int CPUThreadFunc(void * data) if (cpuSleep) SDL_CondWait(cpuCond, cpuMutex); +// decrement mainSem... +#ifdef THREAD_DEBUGGING +WriteLog("CPU: SDL_SemWait(mainSem);\n"); +#endif +SDL_SemWait(mainSem); + uint32 cycles = 17066; #ifdef CPU_THREAD_OVERFLOW_COMPENSATION // ODD! It's closer *without* this overflow compensation. ??? WHY ??? @@ -141,11 +151,22 @@ int CPUThreadFunc(void * data) } #endif +#ifdef THREAD_DEBUGGING +WriteLog("CPU: Execute65C02(&mainCPU, cycles);\n"); +#endif Execute65C02(&mainCPU, cycles); // how much? 1 frame (after 1 s, off by 40 cycles) not any more--it's off by as much as 240 now! - SDL_mutexP(cpuMutex); + // Adjust the sound routine's last cycle toggled time base // Also, since we're finished executing, .clock is now valid +#ifdef THREAD_DEBUGGING +WriteLog("CPU: AdjustLastToggleCycles(mainCPU.clock);\n"); +#endif AdjustLastToggleCycles(mainCPU.clock); + +#ifdef THREAD_DEBUGGING +WriteLog("CPU: SDL_mutexP(cpuMutex);\n"); +#endif + SDL_mutexP(cpuMutex); #if 0 if (SDL_CondWait(cpuCond, cpuMutex) != 0) { @@ -153,12 +174,28 @@ int CPUThreadFunc(void * data) exit(-1); } #else +// increment mainSem... +#ifdef THREAD_DEBUGGING +WriteLog("CPU: SDL_SemPost(mainSem);\n"); +#endif +SDL_SemPost(mainSem); +// SDL_CondSignal(mainCond); // In case something is waiting on us... + +#ifdef THREAD_DEBUGGING +WriteLog("CPU: SDL_CondWait(cpuCond, cpuMutex);\n"); +#endif SDL_CondWait(cpuCond, cpuMutex); + +#endif +#ifdef THREAD_DEBUGGING +WriteLog("CPU: SDL_mutexV(cpuMutex);\n"); #endif SDL_mutexV(cpuMutex); } while (!cpuFinished); + SDL_DestroyMutex(cpuMutex); + return 0; } #endif @@ -945,8 +982,11 @@ memcpy(ram + 0xD000, ram + 0xC000, 0x1000); startTicks = SDL_GetTicks(); #ifdef THREADED_65C02 -cpuCond = SDL_CreateCond(); -cpuThread = SDL_CreateThread(CPUThreadFunc, NULL); + cpuCond = SDL_CreateCond(); + cpuThread = SDL_CreateThread(CPUThreadFunc, NULL); +//Hmm... CPU does POST (+1), wait, then WAIT (-1) + mainSem = SDL_CreateSemaphore(1); +// SDL_sem * mainMutex = SDL_CreateMutex(); #endif WriteLog("Entering main loop...\n"); @@ -972,11 +1012,31 @@ totalCPU += USEC_TO_M6502_CYCLES(timeToNextEvent); } #ifdef THREADED_65C02 +WriteLog("Main: cpuFinished = true;\n"); cpuFinished = true; +//#warning "If sound thread is behind, CPU thread will never wake up... !!! FIX !!!" [DONE] +//What to do? How do you know when the CPU is sleeping??? +//USE A CONDITIONAL!!! OF COURSE!!!!!!11!11!11!!!1! +#if 0 +SDL_mutexP(mainMutex); +SDL_CondWait(mainCond, mainMutex); // Wait for CPU thread to get to signal point... +SDL_mutexV(mainMutex); +#else +//Nope, use a semaphore... +WriteLog("Main: SDL_SemWait(mainSem);\n"); +SDL_SemWait(mainSem);//should lock until CPU thread is waiting... +#endif + +WriteLog("Main: SDL_CondSignal(cpuCond);//thread is probably asleep, wake it up\n"); SDL_CondSignal(cpuCond);//thread is probably asleep, wake it up +WriteLog("Main: SDL_WaitThread(cpuThread, NULL);\n"); SDL_WaitThread(cpuThread, NULL); //nowok:SDL_WaitThread(CPUThreadFunc, NULL); +WriteLog("Main: SDL_DestroyCond(cpuCond);\n"); SDL_DestroyCond(cpuCond); + +//SDL_DestroyMutex(mainMutex); +SDL_DestroySemaphore(mainSem); #endif if (settings.autoStateSaving) @@ -1119,15 +1179,19 @@ else if (event.key.keysym.sym == SDLK_F10) if (event.key.keysym.sym == SDLK_F5) { VolumeDown(); - char volStr[10] = "*********"; - volStr[GetVolume()] = 0; + char volStr[19] = "[****************]"; +// volStr[GetVolume()] = 0; + for(int i=GetVolume(); i<16; i++) + volStr[1 + i] = '-'; SpawnMessage("Volume: %s", volStr); } else if (event.key.keysym.sym == SDLK_F6) { VolumeUp(); - char volStr[10] = "*********"; - volStr[GetVolume()] = 0; + char volStr[19] = "[****************]"; +// volStr[GetVolume()] = 0; + for(int i=GetVolume(); i<16; i++) + volStr[1 + i] = '-'; SpawnMessage("Volume: %s", volStr); } @@ -1142,14 +1206,15 @@ else if (event.key.keysym.sym == SDLK_F10) #ifdef CPU_CLOCK_CHECKING //We know it's stopped, so we can get away with this... -uint64 clock = GetCurrentV65C02Clock(); -totalCPU += (uint32)(clock - lastClock); -lastClock = clock; counter++; if (counter == 60) { - printf("Executed %u cycles...\n", totalCPU); - totalCPU = 0; + uint64 clock = GetCurrentV65C02Clock(); +//totalCPU += (uint32)(clock - lastClock); + + printf("Executed %u cycles...\n", (uint32)(clock - lastClock)); + lastClock = clock; +// totalCPU = 0; counter = 0; } #endif diff --git a/src/sound.cpp b/src/sound.cpp index 73e8dda..202eaae 100755 --- a/src/sound.cpp +++ b/src/sound.cpp @@ -29,18 +29,23 @@ // Useful defines //#define DEBUG +//#define WRITE_OUT_WAVE -//#define SAMPLE_RATE (44100.0) -#define SAMPLE_RATE (48000.0) +#define SAMPLE_RATE (44100.0) +//#define SAMPLE_RATE (48000.0) #define SAMPLES_PER_FRAME (SAMPLE_RATE / 60.0) +// This works for AppleWin but not here... ??? WHY ??? // ~ 21 -//#define CYCLES_PER_SAMPLE (1024000.0 / SAMPLE_RATE) +// #define CYCLES_PER_SAMPLE (1024000.0 / SAMPLE_RATE) // ~ 17 (lower pitched than above...!) // Makes sense, as this is the divisor for # of cycles passed -#define CYCLES_PER_SAMPLE (800000.0 / SAMPLE_RATE) +//#define CYCLES_PER_SAMPLE (800000.0 / SAMPLE_RATE) +// This seems about right, compared to AppleWin--but AW runs @ 1.024 MHz +// 23 (1.024) vs. 20 (0.900) +#define CYCLES_PER_SAMPLE (900000.0 / SAMPLE_RATE) //nope, too high #define CYCLES_PER_SAMPLE (960000.0 / SAMPLE_RATE) //#define SOUND_BUFFER_SIZE (8192) -#define SOUND_BUFFER_SIZE (16384) +#define SOUND_BUFFER_SIZE (32768) // Global variables @@ -50,18 +55,19 @@ static SDL_AudioSpec desired; static bool soundInitialized = false; static bool speakerState = false; -static uint8 soundBuffer[SOUND_BUFFER_SIZE]; +static int16 soundBuffer[SOUND_BUFFER_SIZE]; static uint32 soundBufferPos; -static uint32 sampleBase; static uint64 lastToggleCycles; -static uint64 samplePosition; static SDL_cond * conditional = NULL; static SDL_mutex * mutex = NULL; static SDL_mutex * mutex2 = NULL; -static int8 sample; -static uint8 ampPtr = 5; // Start with -16 - +16 -static uint16 amplitude[17] = { 0, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, - 4096, 8192, 16384, 32768 }; +static int16 sample; +static uint8 ampPtr = 14; // Start with -16 - +16 +static int16 amplitude[17] = { 0, 1, 2, 3, 7, 15, 31, 63, 127, 255, 511, 1023, 2047, + 4095, 8191, 16383, 32767 }; +#ifdef WRITE_OUT_WAVE +static FILE * fp = NULL; +#endif // Private function prototypes @@ -78,8 +84,8 @@ return; #endif desired.freq = SAMPLE_RATE; // SDL will do conversion on the fly, if it can't get the exact rate. Nice! - desired.format = AUDIO_S8; // This uses the native endian (for portability)... -// desired.format = AUDIO_S16SYS; // This uses the native endian (for portability)... +// desired.format = AUDIO_S8; // This uses the native endian (for portability)... + desired.format = AUDIO_S16SYS; // This uses the native endian (for portability)... desired.channels = 1; // desired.samples = 4096; // Let's try a 4K buffer (can always go lower) // desired.samples = 2048; // Let's try a 2K buffer (can always go lower) @@ -95,16 +101,17 @@ return; conditional = SDL_CreateCond(); mutex = SDL_CreateMutex(); mutex2 = SDL_CreateMutex();// Let's try real signalling... -// SDL_mutexP(mutex); // Must lock the mutex for the cond to work properly... soundBufferPos = 0; - sampleBase = 0; lastToggleCycles = 0; - samplePosition = 0; sample = desired.silence; // ? wilwok ? yes SDL_PauseAudio(false); // Start playback! soundInitialized = true; WriteLog("Sound: Successfully initialized.\n"); + +#ifdef WRITE_OUT_WAVE + fp = fopen("./apple2.wav", "wb"); +#endif } // @@ -120,13 +127,17 @@ void SoundDone(void) SDL_DestroyMutex(mutex); SDL_DestroyMutex(mutex2); WriteLog("Sound: Done.\n"); + +#ifdef WRITE_OUT_WAVE + fclose(fp); +#endif } } // // Sound card callback handler // -static void SDLSoundCallback(void * userdata, Uint8 * buffer, int length) +static void SDLSoundCallback(void * userdata, Uint8 * buffer8, int length8) { // The sound buffer should only starve when starting which will cause it to // lag behind the emulation at most by around 1 frame... @@ -136,38 +147,37 @@ static void SDLSoundCallback(void * userdata, Uint8 * buffer, int length) // (Should NOT starve now, now that we properly handle frame edges...) // Let's try using a mutex for shared resource consumption... +//Actually, I think Lock/UnlockAudio() does this already... SDL_mutexP(mutex2); - if (soundBufferPos < (uint32)length) // The sound buffer is starved... + // Recast this as a 16-bit type... + int16 * buffer = (int16 *)buffer8; + uint32 length = (uint32)length8 / 2; + + if (soundBufferPos < length) // The sound buffer is starved... { -//printf("Sound buffer starved!\n"); -//fflush(stdout); for(uint32 i=0; i 95085)//(time & 0x80000000) -{ - WriteLog("ToggleSpeaker() given bad time value: %08X (%u)!\n", time, time); -// fflush(stdout); -} -#endif - - // 1.024 MHz / 60 = 17066.6... cycles (23.2199 cycles per sample) - // Need the last frame position in order to calculate correctly... - // (or do we?) - -// SDL_LockAudio(); - SDL_mutexP(mutex2); -// uint32 currentPos = sampleBase + (uint32)((double)elapsedCycles / CYCLES_PER_SAMPLE); + // Step 2: Calculate new buffer position uint32 currentPos = (uint32)((double)deltaCycles / CYCLES_PER_SAMPLE); -/* -The problem: - - ______ | ______________ | ______ -____| | | |_______ - -Speaker is toggled, then not toggled for a while. How to find buffer position in the -last frame? - -IRQ buffer len is 1024. - -Could check current CPU clock, take delta. If delta > 1024, then ... - -Could add # of cycles in IRQ to lastToggleCycles, then currentPos will be guaranteed -to fall within acceptable limits. - -This *should* work, but if the IRQ isn't scheduled & etc, could screw timing up. -Need to have a way to suspend IRQ thread as well as CPU thread when in the GUI, -for example - -Another method would be to add to lastToggleCycles on every timeslice of the CPU, -just like we used to. - -Or, run the CPU for CYCLES_PER_SAMPLE and take a sample, then copy the buffer over -at the end of the timeslice. That way, we could just fill the buffer and let the -IRQ handle draining it. No muss, no fuss. -*/ - - - if ((soundBufferPos + currentPos) > (SOUND_BUFFER_SIZE - 1)) + // Step 3: Make sure there's room for it + // We need to lock since we touch both soundBuffer and soundBufferPos + SDL_mutexP(mutex2); + while ((soundBufferPos + currentPos) > (SOUND_BUFFER_SIZE - 1)) { -#if 0 -WriteLog("ToggleSpeaker() about to go into spinlock at time: %08X (%u) (sampleBase=%u)!\n", time, time, sampleBase); -#endif -// Still hanging on this spinlock... -// That could be because the "time" value is too high and so the buffer will NEVER be -// empty enough... -// Now that we're using a conditional, it seems to be working OK--though not perfectly... -/* -ToggleSpeaker() about to go into spinlock at time: 00004011 (16401) (sampleBase=3504)! -16401 -> 706 samples, 3504 + 706 = 4210 - -And it still thrashed the sound even though it didn't run into a spinlock... - -Seems like it's OK now that I've fixed the buffer-less-than-length bug... -*/ -// SDL_UnlockAudio(); -// SDL_CondWait(conditional, mutex); -// SDL_LockAudio(); -// Hm. -// This might not empty the buffer enough, causing hash and trash. !!! FIX !!! -SDL_mutexV(mutex2);//Release it so sound thread can get it, - SDL_mutexP(mutex); // Must lock the mutex for the cond to work properly... -SDL_CondWait(conditional, mutex);//Sleep/wait for the sound thread - SDL_mutexV(mutex); // Must unlock the mutex for the cond to work properly... -SDL_mutexP(mutex2);//Re-lock it until we're done with it... - -// currentPos = sampleBase + (uint32)((double)deltaCycles / CYCLES_PER_SAMPLE); - currentPos = (uint32)((double)deltaCycles / CYCLES_PER_SAMPLE); -#if 0 -WriteLog("--> after spinlock (sampleBase=%u)...\n", sampleBase); -#endif + SDL_mutexV(mutex2); // Release it so sound thread can get it, + SDL_mutexP(mutex); // Must lock the mutex for the cond to work properly... + SDL_CondWait(conditional, mutex); // Sleep/wait for the sound thread + SDL_mutexV(mutex); // Must unlock the mutex for the cond to work properly... + SDL_mutexP(mutex2); // Re-lock it until we're done with it... } - sample = (speakerState ? amplitude[ampPtr] : -amplitude[ampPtr]); - + // Step 4: Backfill and adjust lastToggleCycles // currentPos is position from "zero" or soundBufferPos... currentPos += soundBufferPos; +#ifdef WRITE_OUT_WAVE + uint32 sbpSave = soundBufferPos; +#endif + // Backfill with current toggle state while (soundBufferPos < currentPos) - soundBuffer[soundBufferPos++] = (uint8)sample; + soundBuffer[soundBufferPos++] = (uint16)sample; + +#ifdef WRITE_OUT_WAVE + fwrite(&soundBuffer[sbpSave], sizeof(int16), currentPos - sbpSave, fp); +#endif - // This is done *after* in case the buffer had a long dead spot (I think...) - speakerState = !speakerState; - sample = (speakerState ? amplitude[ampPtr] : -amplitude[ampPtr]); - lastToggleCycles = elapsedCycles; SDL_mutexV(mutex2); -// SDL_UnlockAudio(); + lastToggleCycles = elapsedCycles; } -void AddToSoundTimeBase(uint32 cycles) +void ToggleSpeaker(uint64 elapsedCycles) { if (!soundInitialized) return; -// SDL_LockAudio(); - SDL_mutexP(mutex2); - sampleBase += (uint32)((double)cycles / CYCLES_PER_SAMPLE); - SDL_mutexV(mutex2); -// SDL_UnlockAudio(); + HandleBuffer(elapsedCycles); + speakerState = !speakerState; + sample = (speakerState ? amplitude[ampPtr] : -amplitude[ampPtr]); } void AdjustLastToggleCycles(uint64 elapsedCycles) { -#if 0 if (!soundInitialized) return; - - SDL_mutexP(mutex2); - lastToggleCycles += elapsedCycles; - SDL_mutexV(mutex2); - -// We should also fill the buffer here as well, even if the speaker -// didn't toggle... !!! FIX !!! -#else /* BOOKKEEPING @@ -344,56 +278,14 @@ every frame--this way we don't have to worry about keeping current time and crap like that. So, we have to move the "zero" the right amount, just like in ToggleSpeaker(), and backfill only without toggling. */ -#warning "This is VERY similar to ToggleSpeaker(); merge into common function. !!! FIX !!!" - if (!soundInitialized) - return; - -#ifdef DEBUG -printf("SOUND: AdjustLastToggleCycles() start...\n"); -#endif - // Step 1: Calculate delta time - uint64 deltaCycles = elapsedCycles - lastToggleCycles; - - // Step 2: Calculate new buffer position - uint32 currentPos = (uint32)((double)deltaCycles / CYCLES_PER_SAMPLE); - - // Step 3: Make sure there's room for it - // We need to lock since we touch both soundBuffer and soundBufferPos - SDL_mutexP(mutex2); - while ((soundBufferPos + currentPos) > (SOUND_BUFFER_SIZE - 1)) - { - // Hm. - // This might not empty the buffer enough, causing hash and trash. !!! FIX !!! [DONE] - SDL_mutexV(mutex2);//Release it so sound thread can get it, - SDL_mutexP(mutex); // Must lock the mutex for the cond to work properly... - SDL_CondWait(conditional, mutex);//Sleep/wait for the sound thread - SDL_mutexV(mutex); // Must unlock the mutex for the cond to work properly... - SDL_mutexP(mutex2);//Re-lock it until we're done with it... - -//HMM, this doesn't need to lock or recalculate this value -// currentPos = (uint32)((double)deltaCycles / CYCLES_PER_SAMPLE); - } - - // Step 4: Backfill and adjust lastToggleCycles - // currentPos is position from "zero" or soundBufferPos... - currentPos += soundBufferPos; - - // Backfill with current toggle state - while (soundBufferPos < currentPos) - soundBuffer[soundBufferPos++] = (uint8)sample; - - SDL_mutexV(mutex2); - lastToggleCycles = elapsedCycles; -#ifdef DEBUG -printf("SOUND: AdjustLastToggleCycles() end...\n"); -#endif -#endif + HandleBuffer(elapsedCycles); } void VolumeUp(void) { // Currently set for 8-bit samples - if (ampPtr < 8) + // Now 16 + if (ampPtr < 16) ampPtr++; } @@ -471,9 +363,3 @@ Still getting random clicks when running... (This may be due to the lock/unlock sound happening in ToggleSpeaker()...) */ - - - - - - -- 2.37.2