Use critical sections for WildMidi locking

...because when you're trying to be thread-safe, it's generally a good
idea to use mechanisms that work across multiple processor cores.

src/CMakeLists.txt

@@ -1138,7 +1138,6 @@ add_executable( zdoom WIN32 MACOSX_BUNDLE
 	timidity/timidity.cpp
 	wildmidi/file_io.cpp
 	wildmidi/gus_pat.cpp
-	wildmidi/lock.cpp
 	wildmidi/reverb.cpp
 	wildmidi/wildmidi_lib.cpp
 	wildmidi/wm_error.cpp

src/wildmidi/lock.cpp

@@ -1,86 +0,0 @@
-/*
- lock.c - data locking code for lib
-
- Copyright (C) Chris Ison  2001-2011
- Copyright (C) Bret Curtis 2013-2014
-
- This file is part of WildMIDI.
-
- WildMIDI is free software: you can redistribute and/or modify the player
- under the terms of the GNU General Public License and you can redistribute
- and/or modify the library under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation, either version 3 of
- the licenses, or(at your option) any later version.
-
- WildMIDI is distributed in the hope that it will be useful, but WITHOUT
- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License and
- the GNU Lesser General Public License for more details.
-
- You should have received a copy of the GNU General Public License and the
- GNU Lesser General Public License along with WildMIDI.  If not,  see
- <http://www.gnu.org/licenses/>.
- */
-
-//#include "config.h"
-
-#ifndef __DJGPP__
-
-#ifdef _WIN32
-#include <windows.h>
-#else
-#include <unistd.h>
-#endif
-
-#include "lock.h"
-#include "common.h"
-
-/*
- _WM_Lock(wmlock)
-
- wmlock = a pointer to a value
-
- returns nothing
-
- Attempts to set a lock on the MDI tree so that
- only 1 library command may access it at any time.
- If lock fails the process retries until successful.
- */
-void _WM_Lock(int * wmlock) {
-	LOCK_START:
-	/* Check if lock is clear, if so set it */
-	if (*wmlock == 0) {
-		(*wmlock)++;
-		/* Now that the lock is set, make sure we
-		 * don't have a race condition.  If so,
-		 * decrement the lock by one and retry.  */
-		if (*wmlock == 1) {
-			return; /* Lock cleanly set */
-		}
-		(*wmlock)--;
-	}
-#ifdef _WIN32
-	Sleep(10);
-#else
-	usleep(500);
-#endif
-	goto LOCK_START;
-}
-
-/*
- _WM_Unlock(wmlock)
-
- wmlock = a pointer to a value
-
- returns nothing
-
- Removes a lock previously placed on the MDI tree.
- */
-void _WM_Unlock(int *wmlock) {
-	/* We don't want a -1 lock, so just to make sure */
-	if ((*wmlock) != 0) {
-		(*wmlock)--;
-	}
-}
-
-#endif /* __DJGPP__ */

src/wildmidi/lock.h

@@ -1,36 +0,0 @@
-/*
-    lock.h - data locking code for lib
-
-    Copyright (C) Chris Ison 2001-2011
-    Copyright (C) Bret Curtis 2013-2014
-
-    This file is part of WildMIDI.
-
-    WildMIDI is free software: you can redistribute and/or modify the player
-    under the terms of the GNU General Public License and you can redistribute
-    and/or modify the library under the terms of the GNU Lesser General Public
-    License as published by the Free Software Foundation, either version 3 of
-    the licenses, or(at your option) any later version.
-
-    WildMIDI is distributed in the hope that it will be useful, but WITHOUT
-    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-    FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License and
-    the GNU Lesser General Public License for more details.
-
-    You should have received a copy of the GNU General Public License and the
-    GNU Lesser General Public License along with WildMIDI.  If not,  see
-    <http://www.gnu.org/licenses/>.
-*/
-
-#ifndef __LOCK_H
-#define __LOCK_H
-
-extern void _WM_Lock (int * wmlock);
-extern void _WM_Unlock (int *wmlock);
-
-#ifdef __DJGPP__
-#define _WM_Lock(p) do {} while (0)
-#define _WM_Unlock(p) do {} while (0)
-#endif
-
-#endif /* __LOCK_H */

src/wildmidi/wildmidi_lib.cpp

@@ -46,10 +46,10 @@
 #include "common.h"
 #include "wm_error.h"
 #include "file_io.h"
-#include "lock.h"
 #include "reverb.h"
 #include "gus_pat.h"
 #include "wildmidi_lib.h"
+#include "critsec.h"
 
 #define IS_DIR_SEPARATOR(c)	((c) == '/' || (c) == '\\')
 #ifdef _WIN32
@@ -88,7 +88,7 @@ static int fix_release = 0;
 static int auto_amp = 0;
 static int auto_amp_with_amp = 0;
 
-static int patch_lock;
+static FCriticalSection patch_lock;
 
 struct _channel {
 	unsigned char bank;
@@ -149,7 +149,24 @@ struct _event_data {
 };
 
 struct _mdi {
-	int lock;
+	_mdi()
+	{
+		samples_to_mix = 0;
+		midi_master_vol = 0;
+		memset(&info, 0, sizeof(info));
+		tmp_info = NULL;
+		memset(&channel, 0, sizeof(channel));
+		note = NULL;
+		memset(note_table, 0, sizeof(note_table));
+		patches = NULL;
+		patch_count = 0;
+		amp = 0;
+		mix_buffer = NULL;
+		mix_buffer_size = NULL;
+		reverb = NULL;
+	}
+
+	FCriticalSection lock;
 	unsigned long int samples_to_mix;
 
 	unsigned short midi_master_vol;
@@ -177,7 +194,7 @@ static double newt_coeffs[58][58];	/* for start/end of samples */
 #define MAX_GAUSS_ORDER 34		/* 34 is as high as we can go before errors crop up */
 static double *gauss_table = NULL;	/* *gauss_table[1<<FPBITS] */
 static int gauss_n = MAX_GAUSS_ORDER;
-static int gauss_lock;
+static FCriticalSection gauss_lock;
 
 static void init_gauss(void) {
 	/* init gauss table */
@@ -190,9 +207,9 @@ static void init_gauss(void) {
 	double z[35];
 	double *gptr, *t;
 
-	_WM_Lock(&gauss_lock);
+	gauss_lock.Enter();
 	if (gauss_table) {
-		_WM_Unlock(&gauss_lock);
+		gauss_lock.Leave();
 		return;
 	}
 
@@ -239,14 +256,14 @@ static void init_gauss(void) {
 	}
 
 	gauss_table = t;
-	_WM_Unlock(&gauss_lock);
+	gauss_lock.Leave();
 }
 
 static void free_gauss(void) {
-	_WM_Lock(&gauss_lock);
+	gauss_lock.Enter();
 	free(gauss_table);
 	gauss_table = NULL;
-	_WM_Unlock(&gauss_lock);
+	gauss_lock.Leave();
 }
 
 struct _hndl {
@@ -520,7 +537,7 @@ static void WM_FreePatches(void) {
 	struct _patch * tmp_patch;
 	struct _sample * tmp_sample;
 
-	_WM_Lock(&patch_lock);
+	patch_lock.Enter();
 	for (i = 0; i < 128; i++) {
 		while (patch[i]) {
 			while (patch[i]->first_sample) {
@@ -535,7 +552,7 @@ static void WM_FreePatches(void) {
 			patch[i] = tmp_patch;
 		}
 	}
-	_WM_Unlock(&patch_lock);
+	patch_lock.Leave();
 }
 
 /* wm_strdup -- adds extra space for appending up to 4 chars */
@@ -1273,27 +1290,27 @@ static struct _patch *
 get_patch_data(unsigned short patchid) {
 	struct _patch *search_patch;
 
-	_WM_Lock(&patch_lock);
+	patch_lock.Enter();
 
 	search_patch = patch[patchid & 0x007F];
 
 	if (search_patch == NULL) {
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		return NULL;
 	}
 
 	while (search_patch) {
 		if (search_patch->patchid == patchid) {
-			_WM_Unlock(&patch_lock);
+			patch_lock.Leave();
 			return search_patch;
 		}
 		search_patch = search_patch->next;
 	}
 	if ((patchid >> 8) != 0) {
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		return (get_patch_data(patchid & 0x00FF));
 	}
-	_WM_Unlock(&patch_lock);
+	patch_lock.Leave();
 	return NULL;
 }
 
@@ -1312,16 +1329,16 @@ static void load_patch(struct _mdi *mdi, unsigned short patchid) {
 		return;
 	}
 
-	_WM_Lock(&patch_lock);
+	patch_lock.Enter();
 	if (!tmp_patch->loaded) {
 		if (load_sample(tmp_patch) == -1) {
-			_WM_Unlock(&patch_lock);
+			patch_lock.Leave();
 			return;
 		}
 	}
 
 	if (tmp_patch->first_sample == NULL) {
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		return;
 	}
 
@@ -1330,7 +1347,7 @@ static void load_patch(struct _mdi *mdi, unsigned short patchid) {
 			(sizeof(struct _patch*) * mdi->patch_count));
 	mdi->patches[mdi->patch_count - 1] = tmp_patch;
 	tmp_patch->inuse_count++;
-	_WM_Unlock(&patch_lock);
+	patch_lock.Leave();
 }
 
 static struct _sample *
@@ -1338,17 +1355,17 @@ get_sample_data(struct _patch *sample_patch, unsigned long int freq) {
 	struct _sample *last_sample = NULL;
 	struct _sample *return_sample = NULL;
 
-	_WM_Lock(&patch_lock);
+	patch_lock.Enter();
 	if (sample_patch == NULL) {
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		return NULL;
 	}
 	if (sample_patch->first_sample == NULL) {
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		return NULL;
 	}
 	if (freq == 0) {
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		return sample_patch->first_sample;
 	}
 
@@ -1357,7 +1374,7 @@ get_sample_data(struct _patch *sample_patch, unsigned long int freq) {
 	while (last_sample) {
 		if (freq > last_sample->freq_low) {
 			if (freq < last_sample->freq_high) {
-				_WM_Unlock(&patch_lock);
+				patch_lock.Leave();
 				return last_sample;
 			} else {
 				return_sample = last_sample;
@@ -1365,7 +1382,7 @@ get_sample_data(struct _patch *sample_patch, unsigned long int freq) {
 		}
 		last_sample = last_sample->next;
 	}
-	_WM_Unlock(&patch_lock);
+	patch_lock.Leave();
 	return return_sample;
 }
 
@@ -2021,8 +2038,7 @@ static struct _mdi *
 Init_MDI(void) {
 	struct _mdi *mdi;
 
-	mdi = (struct _mdi*)malloc(sizeof(struct _mdi));
-	memset(mdi, 0, (sizeof(struct _mdi)));
+	mdi = new _mdi;
 
 	mdi->info.copyright = NULL;
 	mdi->info.mixer_options = WM_MixerOptions;
@@ -2044,7 +2060,7 @@ static void freeMDI(struct _mdi *mdi) {
 	unsigned long int i;
 
 	if (mdi->patch_count != 0) {
-		_WM_Lock(&patch_lock);
+		patch_lock.Enter();
 		for (i = 0; i < mdi->patch_count; i++) {
 			mdi->patches[i]->inuse_count--;
 			if (mdi->patches[i]->inuse_count == 0) {
@@ -2058,7 +2074,7 @@ static void freeMDI(struct _mdi *mdi) {
 				mdi->patches[i]->loaded = 0;
 			}
 		}
-		_WM_Unlock(&patch_lock);
+		patch_lock.Leave();
 		free(mdi->patches);
 	}
 
@@ -2554,7 +2570,6 @@ int *WM_Mix(midi *handle, int *buffer, unsigned long count)
 	}
 }
 
-
 /*
  * =========================
  * External Functions
@@ -2602,9 +2617,6 @@ WM_SYMBOL int WildMidi_Init(const char * config_file, unsigned short int rate,
 		return -1;
 	}
 	_WM_SampleRate = rate;
-
-	gauss_lock = 0;
-	patch_lock = 0;
 	WM_Initialized = 1;
 
 	return 0;
@@ -2663,7 +2675,7 @@ WM_SYMBOL int WildMidi_Close(midi * handle) {
 				0);
 		return -1;
 	}
-	_WM_Lock(&mdi->lock);
+	mdi->lock.Enter();
 	if (first_handle->handle == handle) {
 		tmp_handle = first_handle->next;
 		free(first_handle);
@@ -2725,17 +2737,17 @@ WM_SYMBOL int WildMidi_SetOption(midi * handle, unsigned short int options,
 	}
 
 	mdi = (struct _mdi *) handle;
-	_WM_Lock(&mdi->lock);
+	mdi->lock.Enter();
 	if ((!(options & 0x0007)) || (options & 0xFFF8)) {
 		_WM_ERROR(__FUNCTION__, __LINE__, WM_ERR_INVALID_ARG, "(invalid option)",
 				0);
-		_WM_Unlock(&mdi->lock);
+		mdi->lock.Leave();
 		return -1;
 	}
 	if (setting & 0xFFF8) {
 		_WM_ERROR(__FUNCTION__, __LINE__, WM_ERR_INVALID_ARG,
 				"(invalid setting)", 0);
-		_WM_Unlock(&mdi->lock);
+		mdi->lock.Leave();
 		return -1;
 	}
 
@@ -2763,7 +2775,7 @@ WM_SYMBOL int WildMidi_SetOption(midi * handle, unsigned short int options,
 		_WM_reset_reverb(mdi->reverb);
 	}
 
-	_WM_Unlock(&mdi->lock);
+	mdi->lock.Leave();
 	return 0;
 }
 
@@ -2779,12 +2791,12 @@ WildMidi_GetInfo(midi * handle) {
 				0);
 		return NULL;
 	}
-	_WM_Lock(&mdi->lock);
+	mdi->lock.Enter();
 	if (mdi->tmp_info == NULL) {
 		mdi->tmp_info = (struct _WM_Info*)malloc(sizeof(struct _WM_Info));
 		if (mdi->tmp_info == NULL) {
 			_WM_ERROR(__FUNCTION__, __LINE__, WM_ERR_MEM, "to set info", 0);
-			_WM_Unlock(&mdi->lock);
+			mdi->lock.Leave();
 			return NULL;
 		}
 		mdi->tmp_info->copyright = NULL;
@@ -2799,7 +2811,7 @@ WildMidi_GetInfo(midi * handle) {
 	} else {
 		mdi->tmp_info->copyright = NULL;
 	}
-	_WM_Unlock(&mdi->lock);
+	mdi->lock.Leave();
 	return mdi->tmp_info;
 }
 

zdoom.vcproj

@@ -2819,10 +2819,6 @@
 						RelativePath=".\src\wildmidi\gus_pat.h"
 						>
 					</File>
-					<File
-						RelativePath=".\src\wildmidi\lock.h"
-						>
-					</File>
 					<File
 						RelativePath=".\src\wildmidi\reverb.h"
 						>
@@ -2847,10 +2843,6 @@
 						RelativePath=".\src\wildmidi\gus_pat.cpp"
 						>
 					</File>
-					<File
-						RelativePath=".\src\wildmidi\lock.cpp"
-						>
-					</File>
 					<File
 						RelativePath=".\src\wildmidi\reverb.cpp"
 						>