shithub: ft²

Download patch

ref: 459e13975d774605e2ac841ed5791296346d8318
parent: 7d116f5046059d1dfb3811be767b183327b858a8
author: Olav Sørensen <olav.sorensen@live.no>
date: Tue Apr 9 15:31:47 EDT 2024

MIDI init/close refactor (+ disable on WinXP, buggy)

--- a/src/ft2_config.c
+++ b/src/ft2_config.c
@@ -290,7 +290,7 @@
 	audio.currInputDevice = getAudioInputDeviceFromConfig();
 
 #ifdef HAS_MIDI
-	if (midi.initThreadDone)
+	if (midi.supported && midi.initThreadDone)
 	{
 		setMidiInputDeviceFromConfig();
 		if (ui.configScreenShown && editor.currConfigScreen == CONFIG_SCREEN_MIDI_INPUT)
@@ -390,7 +390,8 @@
 
 	saveAudioDevicesToConfig(audio.currOutputDevice, audio.currInputDevice);
 #ifdef HAS_MIDI
-	saveMidiInputDeviceToConfig();
+	if (midi.supported)
+		saveMidiInputDeviceToConfig();
 #endif
 
 	FILE *f = UNICHAR_FOPEN(editor.configFileLocationU, "wb");
@@ -673,7 +674,9 @@
 	strcat(editor.configFileLocationU, "/FT2.CFG");
 #endif
 
-	editor.midiConfigFileLocationU = getFullMidiDevConfigPathU();
+	if (midi.supported)
+		editor.midiConfigFileLocationU = getFullMidiDevConfigPathU();
+
 	editor.audioDevConfigFileLocationU = getFullAudDevConfigPathU();
 }
 
@@ -781,6 +784,8 @@
 {
 	uint16_t tmpID;
 
+
+
 	uncheckRadioButtonGroup(RB_GROUP_CONFIG_SELECT);
 	switch (editor.currConfigScreen)
 	{
@@ -795,6 +800,14 @@
 	radioButtons[tmpID].state = RADIOBUTTON_CHECKED;
 
 	showRadioButtonGroup(RB_GROUP_CONFIG_SELECT);
+
+	// hide MIDI radio button if MIDI is not supported (hackish)
+	if (!midi.supported)
+	{
+		radioButton_t *t = &radioButtons[RB_CONFIG_MIDI_INPUT];
+		hideRadioButton(RB_CONFIG_MIDI_INPUT);
+		fillRect(t->x, t->y, RADIOBUTTON_W, RADIOBUTTON_H, PAL_DESKTOP);
+	}
 }
 
 void setConfigAudioRadioButtonStates(void) // accessed by other .c files
@@ -1018,7 +1031,10 @@
 	checkBoxes[CB_CONF_CHANGE_PATTLEN_INS_DEL].checked = config.recTrueInsert;
 	checkBoxes[CB_CONF_MIDI_ALLOW_PC].checked = config.recMIDIAllowPC;
 #ifdef HAS_MIDI
-	checkBoxes[CB_CONF_MIDI_ENABLE].checked = midi.enable;
+	if (midi.supported)
+		checkBoxes[CB_CONF_MIDI_ENABLE].checked = midi.enable;
+	else
+		checkBoxes[CB_CONF_MIDI_ENABLE].checked = false;
 #else
 	checkBoxes[CB_CONF_MIDI_ENABLE].checked = false;
 #endif
@@ -1109,7 +1125,8 @@
 	textOutShadow(21, 35, PAL_FORGRND, PAL_DSKTOP2, "Layout");
 	textOutShadow(21, 51, PAL_FORGRND, PAL_DSKTOP2, "Miscellaneous");
 #ifdef HAS_MIDI
-	textOutShadow(21, 67, PAL_FORGRND, PAL_DSKTOP2, "MIDI input");
+	if (midi.supported)
+		textOutShadow(21, 67, PAL_FORGRND, PAL_DSKTOP2, "MIDI input");
 #endif
 	textOutShadow(20, 93, PAL_FORGRND, PAL_DSKTOP2, "Auto save");
 
@@ -1377,6 +1394,7 @@
 		}
 		break;
 
+#ifdef HAS_MIDI
 		case CONFIG_SCREEN_MIDI_INPUT:
 		{
 			drawFramework(110, 0, 394, 173, FRAMEWORK_TYPE1);
@@ -1387,15 +1405,14 @@
 
 			blitFast(517, 51, bmp.midiLogo, 103, 55);
 
-#ifdef HAS_MIDI
 			showPushButton(PB_CONFIG_MIDI_INPUT_DOWN);
 			showPushButton(PB_CONFIG_MIDI_INPUT_UP);
 			rescanMidiInputDevices();
 			drawMidiInputList();
 			showScrollBar(SB_MIDI_INPUT_SCROLL);
-#endif
 		}
 		break;
+#endif
 	}
 }
 
@@ -2072,7 +2089,17 @@
 void cbMIDIEnable(void)
 {
 #ifdef HAS_MIDI
-	midi.enable ^= 1;
+	if (midi.supported)
+	{
+		midi.enable ^= 1;
+	}
+	else
+	{
+		checkBoxes[CB_CONF_MIDI_ENABLE].checked = false;
+		drawCheckBox(CB_CONF_MIDI_ENABLE);
+
+		okBox(0, "System message", "MIDI support is disabled for Windows XP as it is buggy!", NULL);
+	}
 #else
 	checkBoxes[CB_CONF_MIDI_ENABLE].checked = false;
 	drawCheckBox(CB_CONF_MIDI_ENABLE);
--- a/src/ft2_config.h
+++ b/src/ft2_config.h
@@ -13,7 +13,9 @@
 	CONFIG_SCREEN_AUDIO,
 	CONFIG_SCREEN_LAYOUT,
 	CONFIG_SCREEN_MISCELLANEOUS,
+#ifdef HAS_MIDI
 	CONFIG_SCREEN_MIDI_INPUT,
+#endif
 
 	CONFIG_HIDE_ERRORS = 0,
 	CONFIG_SHOW_ERRORS = 1,
--- a/src/ft2_keyboard.c
+++ b/src/ft2_keyboard.c
@@ -24,6 +24,7 @@
 #include "ft2_audio.h"
 #include "ft2_trim.h"
 #include "ft2_sample_ed_features.h"
+#include "ft2_midi.h"
 #include "ft2_structs.h"
 
 keyb_t keyb; // globalized
@@ -1280,16 +1281,18 @@
 
 				return true;
 			}
-#ifdef HAS_MIDI
 			else if (keyb.leftCtrlPressed)
 			{
-				editor.currConfigScreen = 3;
-				showConfigScreen();
-				checkRadioButton(RB_CONFIG_MIDI_INPUT);
-
+#ifdef HAS_MIDI
+				if (midi.supported)
+				{
+					editor.currConfigScreen = 3;
+					showConfigScreen();
+					checkRadioButton(RB_CONFIG_MIDI_INPUT);
+				}
+#endif
 				return true;
 			}
-#endif
 		}
 		break;
 
--- a/src/ft2_main.c
+++ b/src/ft2_main.c
@@ -9,6 +9,7 @@
 #ifdef _WIN32
 #define WIN32_MEAN_AND_LEAN
 #include <windows.h>
+#include <versionhelpers.h>
 #include <SDL2/SDL_syswm.h>
 #else
 #include <unistd.h> // chdir()
@@ -35,10 +36,6 @@
 #include "ft2_structs.h"
 #include "ft2_hpc.h"
 
-#ifdef HAS_MIDI
-static SDL_Thread *initMidiThread;
-#endif
-
 static void initializeVars(void);
 static void cleanUpAndExit(void); // never call this inside the main loop
 #ifdef __APPLE__
@@ -99,6 +96,10 @@
 #endif
 
 #ifdef _WIN32
+	// disable MIDI support if using Windows XP, as it is unstable
+	if (!IsWindowsVistaOrGreater())
+		midi.supported = false;
+
 #ifndef _MSC_VER
 	SetProcessDPIAware();
 #endif
@@ -231,14 +232,16 @@
 
 #ifdef HAS_MIDI
 	// set up MIDI input (in a thread because it can take quite a while on f.ex. macOS)
-	initMidiThread = SDL_CreateThread(initMidiFunc, NULL, NULL);
-	if (initMidiThread == NULL)
+	if (midi.supported)
 	{
-		showErrorMsgBox("Couldn't create MIDI initialization thread!");
-		cleanUpAndExit();
-		return 1;
+		midi.initMidiThread = SDL_CreateThread(initMidiFunc, NULL, NULL);
+		if (midi.initMidiThread == NULL)
+		{
+			showErrorMsgBox("Couldn't create MIDI initialization thread!");
+			cleanUpAndExit();
+			return 1;
+		}
 	}
-	SDL_DetachThread(initMidiThread); // don't wait for this thread, let it clean up when done
 #endif
 
 	hpc_ResetCounters(&video.vblankHpc); // quirk: this is needed for potential okBox() calls in handleModuleLoadFromArg()
@@ -271,6 +274,10 @@
 	cpu.hasSSE2 = SDL_HasSSE2();
 
 	// clear common structs
+#ifdef HAS_MIDI
+	memset(&midi, 0, sizeof (midi));
+	midi.supported = true;
+#endif
 	memset(&video, 0, sizeof (video));
 	memset(&keyb, 0, sizeof (keyb));
 	memset(&mouse, 0, sizeof (mouse));
@@ -289,7 +296,7 @@
 
 	// now set data that must be initialized to non-zero values...
 
-	audio.locked = true;
+	audio.locked = true; // XXX: Why..?
 	audio.rescanAudioDevicesSupported = true;
 
 	// set non-zero values
@@ -305,7 +312,7 @@
 	editor.srcInstr = 1;
 	editor.curInstr = 1;
 	editor.curOctave = 4;
-	editor.smpEd_NoteNr = 48 + 1; // middle-C
+	editor.smpEd_NoteNr = 1+NOTE_C4;
 
 	editor.ptnJumpPos[0] = 0x00;
 	editor.ptnJumpPos[1] = 0x10;
@@ -316,15 +323,15 @@
 	memset(editor.copyMask, 1, sizeof (editor.copyMask));
 	memset(editor.pasteMask, 1, sizeof (editor.pasteMask));
 
-#ifdef HAS_MIDI
-	midi.enable = true;
-#endif
-
 	editor.diskOpReadOnOpen = true;
 
 	audio.linearPeriodsFlag = true;
 	calcReplayerLogTab();
 
+#ifdef HAS_MIDI
+	midi.enable = true;
+#endif
+
 	editor.programRunning = true;
 }
 
@@ -331,10 +338,26 @@
 static void cleanUpAndExit(void) // never call this inside the main loop!
 {
 #ifdef HAS_MIDI
-	if (midi.closeMidiOnExit)
+	if (midi.supported)
 	{
+		if (midi.initMidiThread != NULL)
+		{
+			SDL_WaitThread(midi.initMidiThread, NULL);
+			midi.initMidiThread = NULL;
+		}
+
+		midi.enable = false; // stop MIDI callback from doing things
+		while (midi.callbackBusy) SDL_Delay(1); // wait for MIDI callback to finish
+
 		closeMidiInDevice();
 		freeMidiIn();
+		freeMidiInputDeviceList();
+
+		if (midi.inputDeviceName != NULL)
+		{
+			free(midi.inputDeviceName);
+			midi.inputDeviceName = NULL;
+		}
 	}
 #endif
 
@@ -345,22 +368,11 @@
 	freeDiskOp();
 	clearCopyBuffer();
 	freeAudioDeviceSelectorBuffers();
-#ifdef HAS_MIDI
-	freeMidiInputDeviceList();
-#endif
 	windUpFTHelp();
 	freeTextBoxes();
 	freeMouseCursors();
 	freeBMPs();
 
-#ifdef HAS_MIDI
-	if (midi.inputDeviceName != NULL)
-	{
-		free(midi.inputDeviceName);
-		midi.inputDeviceName = NULL;
-	}
-#endif
-
 	if (editor.audioDevConfigFileLocationU != NULL)
 	{
 		free(editor.audioDevConfigFileLocationU);
@@ -396,7 +408,7 @@
 static void osxSetDirToProgramDirFromArgs(char **argv)
 {
 	/* OS X/macOS: hackish way of setting the current working directory to the place where we double clicked
-	** on the icon (for protracker.ini loading)
+	** on the icon (for FT2.CFG loading)
 	*/
 
 	// if we launched from the terminal, argv[0][0] would be '.'
--- a/src/ft2_midi.c
+++ b/src/ft2_midi.c
@@ -1,3 +1,5 @@
+// this implements MIDI input only!
+
 #ifdef HAS_MIDI
 
 // for finding memory leaks in debug mode with Visual Studio
@@ -18,20 +20,16 @@
 #include "ft2_structs.h"
 #include "rtmidi/rtmidi_c.h"
 
-#define MAX_DEV_STR_LEN 256
-
 // hide POSIX warnings
 #ifdef _MSC_VER
 #pragma warning(disable: 4996)
 #endif
 
-// This implements MIDI input only!
-
 midi_t midi; // globalized
 
 static volatile bool midiDeviceOpened;
 static bool recMIDIValidChn = true;
-static RtMidiPtr midiDev;
+static RtMidiPtr midiInDev;
 
 static inline void midiInSetChannel(uint8_t status)
 {
@@ -97,7 +95,7 @@
 	}
 }
 
-static void midiInCallback(double dTimeStamp, const unsigned char *message, size_t messageSize, void *userData)
+static void midiInCallback(double timeStamp, const unsigned char *message, size_t messageSize, void *userData)
 {
 	uint8_t byte[3];
 
@@ -104,6 +102,8 @@
 	if (!midi.enable || messageSize < 2)
 		return;
 
+	midi.callbackBusy = true;
+
 	byte[0] = message[0];
 	if (byte[0] > 127 && byte[0] < 240)
 	{
@@ -122,25 +122,27 @@
 		else if (byte[0] >= 224 && byte[0] <= 224+15) midiInPitchBendChange(byte[1], byte[2]);
 	}
 
-	(void)dTimeStamp;
+	midi.callbackBusy = false;
+
+	(void)timeStamp;
 	(void)userData;
 }
 
 static uint32_t getNumMidiInDevices(void)
 {
-	if (midiDev == NULL)
+	if (midiInDev == NULL)
 		return 0;
 
-	return rtmidi_get_port_count(midiDev);
+	return rtmidi_get_port_count(midiInDev);
 }
 
 static char *getMidiInDeviceName(uint32_t deviceID)
 {
-	if (midiDev == NULL)
-		return NULL;
+	if (midiInDev == NULL)
+		return NULL; // MIDI not initialized
 
-	char *devStr = (char *)rtmidi_get_port_name(midiDev, deviceID);
-	if (!midiDev->ok)
+	char *devStr = (char *)rtmidi_get_port_name(midiInDev, deviceID);
+	if (devStr == NULL || !midiInDev->ok)
 		return NULL;
 
 	return devStr;
@@ -148,14 +150,12 @@
 
 void closeMidiInDevice(void)
 {
-	while (!midi.initThreadDone);
-
 	if (midiDeviceOpened)
 	{
-		if (midiDev != NULL)
+		if (midiInDev != NULL)
 		{
-			rtmidi_in_cancel_callback(midiDev);
-			rtmidi_close_port(midiDev);
+			rtmidi_in_cancel_callback(midiInDev);
+			rtmidi_close_port(midiInDev);
 		}
 
 		midiDeviceOpened = false;
@@ -164,51 +164,48 @@
 
 void freeMidiIn(void)
 {
-	while (!midi.initThreadDone);
-
-	if (midiDev != NULL)
+	if (midiInDev != NULL)
 	{
-		rtmidi_in_free(midiDev);
-		midiDev = NULL;
+		rtmidi_in_free(midiInDev);
+		midiInDev = NULL;
 	}
 }
 
 bool initMidiIn(void)
 {
-	if (midiDev != NULL)
-		return false; // already initialized
+	midiInDev = rtmidi_in_create_default();
+	if (midiInDev == NULL)
+		return false;
 
-	midiDev = rtmidi_in_create_default();
-	if (!midiDev->ok)
+	if (!midiInDev->ok)
 	{
-		midiDev = NULL;
+		rtmidi_in_free(midiInDev);
 		return false;
 	}
 
-	midiDeviceOpened = false;
 	return true;
 }
 
 bool openMidiInDevice(uint32_t deviceID)
 {
-	if (midiDev == NULL)
+	if (midiDeviceOpened)
 		return false;
 
-	if (getNumMidiInDevices() == 0)
+	if (midiInDev == NULL || getNumMidiInDevices() == 0)
 		return false;
 
-	rtmidi_open_port(midiDev, deviceID, "FT2 Clone MIDI Port");
-	if (!midiDev->ok)
+	rtmidi_open_port(midiInDev, deviceID, "FT2 Clone MIDI Port");
+	if (!midiInDev->ok)
 		return false;
 
-	rtmidi_in_set_callback(midiDev, midiInCallback, NULL);
-	if (!midiDev->ok)
+	rtmidi_in_set_callback(midiInDev, midiInCallback, NULL);
+	if (!midiInDev->ok)
 	{
-		rtmidi_close_port(midiDev);
+		rtmidi_close_port(midiInDev);
 		return false;
 	}
 
-	rtmidi_in_ignore_types(midiDev, true, true, true);
+	rtmidi_in_ignore_types(midiInDev, true, true, true);
 
 	midiDeviceOpened = true;
 	return true;
@@ -255,7 +252,7 @@
 
 bool saveMidiInputDeviceToConfig(void)
 {
-	if (!midi.initThreadDone || midiDev == NULL || !midiDeviceOpened)
+	if (!midi.initThreadDone || midiInDev == NULL || !midiDeviceOpened)
 		return false;
 
 	const uint32_t numDevices = getNumMidiInDevices();
@@ -284,9 +281,11 @@
 {
 	uint32_t i;
 
-	if (midi.inputDeviceName != NULL)
-		free(midi.inputDeviceName);
+	// XXX: Something in here is corrupting!
 
+	if (editor.midiConfigFileLocationU == NULL)
+		goto setDefMidiInputDev;
+
 	const uint32_t numDevices = getNumMidiInDevices();
 	if (numDevices == 0)
 		goto setDefMidiInputDev;
@@ -295,16 +294,15 @@
 	if (f == NULL)
 		goto setDefMidiInputDev;
 
-	char *devString = (char *)malloc((MAX_DEV_STR_LEN+4) * sizeof (char));
+	char *devString = (char *)malloc(1024+2);
 	if (devString == NULL)
 	{
 		fclose(f);
 		goto setDefMidiInputDev;
 	}
-
 	devString[0] = '\0';
 
-	if (fgets(devString, MAX_DEV_STR_LEN, f) == NULL)
+	if (fgets(devString, 1024, f) == NULL)
 	{
 		fclose(f);
 		free(devString);
@@ -334,6 +332,12 @@
 	if (i == numDevices)
 		goto setDefMidiInputDev;
 
+	if (midi.inputDeviceName != NULL)
+	{
+		free(midi.inputDeviceName);
+		midi.inputDeviceName = NULL;
+	}
+
 	midi.inputDevice = i;
 	midi.inputDeviceName = midiInStr;
 	midi.numInputDevices = numDevices;
@@ -342,9 +346,15 @@
 
 	// couldn't load device, set default
 setDefMidiInputDev:
+	if (midi.inputDeviceName != NULL)
+	{
+		free(midi.inputDeviceName);
+		midi.inputDeviceName = NULL;
+	}
+
 	midi.inputDevice = 0;
 	midi.inputDeviceName = strdup("RtMidi");
-	midi.numInputDevices = numDevices;
+	midi.numInputDevices = 1;
 
 	return false;
 }
@@ -371,7 +381,7 @@
 	if (midi.numInputDevices > MAX_MIDI_DEVICES)
 		midi.numInputDevices = MAX_MIDI_DEVICES;
 
-	for (int32_t i = 0; i < midi.numInputDevices; i++)
+	for (uint32_t i = 0; i < midi.numInputDevices; i++)
 	{
 		char *deviceName = getMidiInDeviceName(i);
 		if (deviceName == NULL)
@@ -393,7 +403,7 @@
 {
 	clearRect(114, 4, 365, 165);
 
-	if (!midi.initThreadDone || midiDev == NULL || midi.numInputDevices == 0)
+	if (!midi.initThreadDone || midiInDev == NULL || midi.numInputDevices == 0)
 	{
 		textOut(114, 4 + (0 * 11), PAL_FORGRND, "No MIDI input devices found!");
 		textOut(114, 4 + (1 * 11), PAL_FORGRND, "Either wait a few seconds for MIDI to initialize, or restart the");
@@ -403,7 +413,10 @@
 
 	for (uint16_t i = 0; i < 15; i++)
 	{
-		const int32_t deviceEntry = getScrollBarPos(SB_MIDI_INPUT_SCROLL) + i;
+		uint32_t deviceEntry = getScrollBarPos(SB_MIDI_INPUT_SCROLL) + i;
+		if (deviceEntry > MAX_MIDI_DEVICES)
+			deviceEntry = MAX_MIDI_DEVICES;
+
 		if (deviceEntry < midi.numInputDevices)
 		{
 			if (midi.inputDeviceNames[deviceEntry] == NULL)
@@ -448,19 +461,19 @@
 bool testMidiInputDeviceListMouseDown(void)
 {
 	if (!ui.configScreenShown || editor.currConfigScreen != CONFIG_SCREEN_MIDI_INPUT)
-		return false; // we didn't click the area
+		return false;
 
+	if (mouse.x < 114 || mouse.x > 479 || mouse.y < 4 || mouse.y > 166)
+		return false; // we didn't click inside the list area
+
 	if (!midi.initThreadDone)
 		return true;
 
-	const int32_t mx = mouse.x;
-	const int32_t my = mouse.y;
+	uint32_t deviceNum = (uint32_t)scrollBars[SB_MIDI_INPUT_SCROLL].pos + ((mouse.y - 4) / 11);
+	if (deviceNum > MAX_MIDI_DEVICES)
+		deviceNum = MAX_MIDI_DEVICES;
 
-	if (my < 4 || my > 166 || mx < 114 || mx > 479)
-		return false; // we didn't click the area
-
-	const int32_t deviceNum = (int32_t)scrollBars[SB_MIDI_INPUT_SCROLL].pos + ((my - 4) / 11);
-	if (midi.numInputDevices <= 0 || deviceNum >= midi.numInputDevices)
+	if (midi.numInputDevices == 0 || deviceNum >= midi.numInputDevices)
 		return true;
 
 	if (midi.inputDeviceName != NULL)
@@ -469,6 +482,7 @@
 			return true; // we clicked the currently selected device, do nothing
 
 		free(midi.inputDeviceName);
+		midi.inputDeviceName = NULL;
 	}
 
 	midi.inputDeviceName = strdup(midi.inputDeviceNames[deviceNum]);
@@ -485,16 +499,12 @@
 
 int32_t SDLCALL initMidiFunc(void *ptr)
 {
-	midi.closeMidiOnExit = true;
-
 	midi.initThreadDone = false;
 	initMidiIn();
 	setMidiInputDeviceFromConfig();
 	openMidiInDevice(midi.inputDevice);
-	midi.initThreadDone = true;
-
 	midi.rescanDevicesFlag = true;
-
+	midi.initThreadDone = true;
 	return true;
 
 	(void)ptr;
@@ -501,5 +511,5 @@
 }
 
 #else
-typedef int make_iso_compilers_happy; // kludge: prevent warning about empty .c file if HAS_MIDI is not defined
+typedef int prevent_compiler_warning; // kludge: prevent warning about empty .c file if HAS_MIDI is not defined
 #endif
--- a/src/ft2_midi.h
+++ b/src/ft2_midi.h
@@ -12,11 +12,11 @@
 typedef struct midi_t
 {
 	char *inputDeviceName, *inputDeviceNames[MAX_MIDI_DEVICES];
-	volatile bool closeMidiOnExit, initThreadDone;
-	uint32_t inputDevice;
-	bool enable, rescanDevicesFlag;
+	volatile bool supported, initThreadDone, callbackBusy, enable;
+	bool rescanDevicesFlag;
+	uint32_t inputDevice, numInputDevices;
 	int16_t currMIDIVibDepth, currMIDIPitch;
-	int32_t numInputDevices;
+	SDL_Thread *initMidiThread;
 } midi_t;
 
 extern midi_t midi; // ft2_midi.c
--