Merge pull request #4206 from bollu/AudioArtefactFix

optional atomic audio locks

Core/Config.cpp

@@ -118,6 +118,8 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename)
 	cpu->Get("Jit", &bJit, true);
 #endif
 	cpu->Get("SeparateCPUThread", &bSeparateCPUThread, false);
+	cpu->Get("AtomicAudioLocks", &bAtomicAudioLocks, false);
+	
 #ifdef __SYMBIAN32__
 	cpu->Get("SeparateIOThread", &bSeparateIOThread, false);
 #else
@@ -314,6 +316,7 @@ void Config::Save() {
 		IniFile::Section *cpu = iniFile.GetOrCreateSection("CPU");
 		cpu->Set("Jit", bJit);
 		cpu->Set("SeparateCPUThread", bSeparateCPUThread);
+		cpu->Set("AtomicAudioLocks", bAtomicAudioLocks);	
 		cpu->Set("SeparateIOThread", bSeparateIOThread);
 		cpu->Set("FastMemory", bFastMemory);
 		cpu->Set("CPUSpeed", iLockedCPUSpeed);

Core/Config.h

@@ -52,6 +52,7 @@ public:
 	// Definitely cannot be changed while game is running.
 	bool bSeparateCPUThread;
 	bool bSeparateIOThread;
+	bool bAtomicAudioLocks;
 	int iLockedCPUSpeed;
 	bool bAutoSaveSymbolMap;
 	std::string sReportHost;

Core/HLE/__sceAudio.cpp

@@ -28,12 +28,16 @@
 #include "ChunkFile.h"
 #include "FixedSizeQueue.h"
 #include "Common/Thread.h"
+#include "Common/Atomics.h"
+#include "../../native/base/mutex.h"
 
 // Should be used to lock anything related to the outAudioQueue.
-recursive_mutex section;
+//atomic locks are used on the lock. TODO: make this lock-free
+atomic_flag atomicLock_;
+recursive_mutex mutex_;
 
 int eventAudioUpdate = -1;
-int eventHostAudioUpdate = -1;
+int eventHostAudioUpdate = -1; 
 int mixFrequency = 44100;
 
 const int hwSampleRate = 44100;
@@ -55,6 +59,11 @@ static int chanQueueMinSizeFactor;
 // is bad mojo.
 FixedSizeQueue<s16, 512 * 16> outAudioQueue;
 
+bool __gainAudioQueueLock();
+void __releaseAcquiredLock();
+void __blockForAudioQueueLock();
+
+
 static inline s16 clamp_s16(int i) {
 	if (i > 32767)
 		return 32767;
@@ -108,6 +117,8 @@ void __AudioInit() {
 
 	mixBuffer = new s32[hwBlockSize * 2];
 	memset(mixBuffer, 0, hwBlockSize * 2 * sizeof(s32));
+
+	
 }
 
 void __AudioDoState(PointerWrap &p) {
@@ -122,9 +133,16 @@ void __AudioDoState(PointerWrap &p) {
 
 	p.Do(mixFrequency);
 
-	{
-		lock_guard guard(section);
+	{	
+		//block until a lock is achieved. Not a good idea at all, but
+		//can't think of a better one...
+		__blockForAudioQueueLock();
+
 		outAudioQueue.DoState(p);
+
+		//release the atomic lock
+		__releaseAcquiredLock();
+		
 	}
 
 	int chanCount = ARRAY_SIZE(chans);
@@ -140,6 +158,7 @@ void __AudioDoState(PointerWrap &p) {
 
 void __AudioShutdown() {
 	delete [] mixBuffer;
+
 	mixBuffer = 0;
 	for (u32 i = 0; i < PSP_AUDIO_CHANNEL_MAX + 1; i++)
 		chans[i].clear();
@@ -322,11 +341,19 @@ void __AudioUpdate() {
 	}
 
 	if (g_Config.bEnableSound) {
-		lock_guard guard(section);
+
+		__blockForAudioQueueLock();
+		/*
+		if (!__gainAudioQueueLock()){
+			return;
+		}
+		*/
+
 		if (outAudioQueue.room() >= hwBlockSize * 2) {
 			s16 *buf1 = 0, *buf2 = 0;
 			size_t sz1, sz2;
 			outAudioQueue.pushPointers(hwBlockSize * 2, &buf1, &sz1, &buf2, &sz2);
+			
 			for (size_t s = 0; s < sz1; s++)
 				buf1[s] = clamp_s16(mixBuffer[s]);
 			if (buf2) {
@@ -337,6 +364,8 @@ void __AudioUpdate() {
 			// This happens quite a lot. There's still something slightly off
 			// about the amount of audio we produce.
 		}
+		//release the atomic lock
+		__releaseAcquiredLock();
 	}
 }
 
@@ -344,6 +373,7 @@ void __AudioUpdate() {
 // This is called from *outside* the emulator thread.
 int __AudioMix(short *outstereo, int numFrames)
 {
+
 	// TODO: if mixFrequency != the actual output frequency, resample!
 	int underrun = -1;
 	s16 sampleL = 0;
@@ -352,12 +382,22 @@ int __AudioMix(short *outstereo, int numFrames)
 	const s16 *buf1 = 0, *buf2 = 0;
 	size_t sz1, sz2;
 	{
-		lock_guard guard(section);
+		
+		//TODO: do rigorous testing to see whether just blind locking will improve speed.
+		if (!__gainAudioQueueLock()){
+			 memset(outstereo, 0, numFrames * 2 * sizeof(short)); 
+			 return 0;
+		}
+		
 		outAudioQueue.popPointers(numFrames * 2, &buf1, &sz1, &buf2, &sz2);
+
 		memcpy(outstereo, buf1, sz1 * sizeof(s16));
 		if (buf2) {
 			memcpy(outstereo + sz1, buf2, sz2 * sizeof(s16));
 		}
+
+		//release the atomic lock
+		__releaseAcquiredLock();
 	}
 
 	int remains = (int)(numFrames * 2 - sz1 - sz2);
@@ -370,3 +410,39 @@ int __AudioMix(short *outstereo, int numFrames)
 	}
 	return underrun >= 0 ? underrun : numFrames;
 }
+
+
+
+/*returns whether the lock was successfully gained or not.
+i.e - whether the lock belongs to you 
+*/
+inline bool __gainAudioQueueLock(){
+	if (g_Config.bAtomicAudioLocks){
+		/*if the previous state was 0, that means the lock was "unlocked". So,
+		we return !0, which is true thanks to C's int to bool conversion
+
+		One the other hand, if it was locked, then the lock would return 1.
+		so, !1 = 0 = false.
+		*/		
+		return atomicLock_.test_and_set() == 0;
+	} else {
+		mutex_.lock();
+		return true;
+	}
+};
+
+inline void __releaseAcquiredLock(){
+	if (g_Config.bAtomicAudioLocks){
+		atomicLock_.clear();
+	} else {
+		mutex_.unlock();
+	}
+}
+
+inline void __blockForAudioQueueLock(){
+	if (g_Config.bAtomicAudioLocks){
+		while ((atomicLock_.test_and_set() == 0)){ }
+	} else {
+		mutex_.lock();
+	}
+}

UI/GameSettingsScreen.cpp

@@ -246,6 +246,8 @@ void GameSettingsScreen::CreateViews() {
 #endif
 	systemSettings->Add(new PopupSliderChoice(&g_Config.iLockedCPUSpeed, 0, 1000, s->T("Change CPU Clock", "Change CPU Clock (0 = default)"), screenManager()));
 
+	systemSettings->Add(new CheckBox(&g_Config.bAtomicAudioLocks, s->T("Atomic Audio locks (experimental)")))->SetEnabled(!PSP_IsInited());
+
 	enableReports_ = Reporting::IsEnabled();
 //#ifndef ANDROID 
 	systemSettings->Add(new ItemHeader(s->T("Cheats", "Cheats (experimental, see forums)")));