Back to the Vavoom Forum Archives


Forum

Patch for fixing a crash with MIDI music

Mon, 21 Dec 2009 22:59:25

Firebrand

I've found a small problem when MIDI music is being used, apparently, the logic called MidiDevice->Stop twice, and sometimes this would produce a crash, I just removed the calling of Stop method in the Play method and made a more proper check for music playing. This seems to fix the crash and fix the music playing in my system.
Index: source/snd_main.cpp
===================================================================
--- source/snd_main.cpp	(revision 4119)
+++ source/snd_main.cpp	(working copy)
@@ -1177,7 +1177,7 @@
 
 	if (StreamPlaying)
 		StreamMusicPlayer->Stop();
-	else if (MidiDevice)
+	else if (MidiDevice->IsPlaying())
 		MidiDevice->Stop();
 	StreamPlaying = false;
 
Index: source/snd_win32music.cpp
===================================================================
--- source/snd_win32music.cpp	(revision 4119)
+++ source/snd_win32music.cpp	(working copy)
@@ -315,9 +315,6 @@
 void VMMSystemMidiDevice::Play(void* Data, int len, const char* song, bool loop)
 {
 	guard(VMMSystemMidiDevice::Play);
-	//	Stop, close, etc. if we're still playing
-	Stop();
-
 	//	Open new file
 	SmfFlags = 0;
 	PendingUserEvent = 0;
Thu, 24 Dec 2009 10:52:24

Janis Legzdinsh

No, that's not a solution. You should be able to call Stop as many times as you want.
Sun, 03 Jan 2010 02:15:13

Firebrand

OK, here's another patch for trying to fix this problem:
Index: snd_win32music.cpp
===================================================================
--- snd_win32music.cpp	(revision 4120)
+++ snd_win32music.cpp	(working copy)
@@ -417,41 +422,41 @@
 	guard(VMMSystemMidiDevice::Stop);
 	if (MidiImage)
 	{
-		if (MusicPaused)
+		if (hMidi)
 		{
-			Resume();
-		}
-		if (State != STATE_Playing && State != STATE_Paused)
-		{
-			SeqFlags &= ~SEQF_Waiting;
-		}
-		else
-		{
-			State = STATE_Stopping;
-			SeqFlags |= SEQF_Waiting;
-
-			if (MMSYSERR_NOERROR != midiStreamStop((HMIDISTRM)hMidi))
+			if (MusicPaused)
 			{
+				Resume();
+			}
+			if (State != STATE_Playing && State != STATE_Paused)
+			{
 				SeqFlags &= ~SEQF_Waiting;
-				return;
 			}
+			else
+			{
+				State = STATE_Stopping;
+				SeqFlags |= SEQF_Waiting;
 
-			while (BuffersInMMSYSTEM)
-				Sleep(0);
-		}
+				if (MMSYSERR_NOERROR != midiStreamStop((HMIDISTRM)hMidi))
+				{
+					SeqFlags &= ~SEQF_Waiting;
+					return;
+				}
 
-		//	Close file.
-		if (State == STATE_Opened)
-		{
-			//	If we were prerolled, need to clean up -- have an open MIDI
-			// handle and buffers in the ready queue
-			for (LPMIDIHDR lpmh = FreeBuffers; lpmh; lpmh = lpmh->lpNext)
-				midiOutUnprepareHeader(hMidi, lpmh, sizeof(*lpmh));
-		}
+				while (BuffersInMMSYSTEM)
+					Sleep(0);
+			}
 
-		//	Close midi stream.	
-		if (hMidi)
-		{
+			//	Close file.
+			if (State == STATE_Opened)
+			{
+				//	If we were prerolled, need to clean up -- have an open MIDI
+				// handle and buffers in the ready queue
+				for (LPMIDIHDR lpmh = FreeBuffers; lpmh; lpmh = lpmh->lpNext)
+					midiOutUnprepareHeader(hMidi, lpmh, sizeof(*lpmh));
+			}
+
+			//	Close midi stream.	
 			midiStreamClose((HMIDISTRM)hMidi);
 			hMidi = NULL;
 		}
I've been investigating the problem with setting midi music volume not working, it seems that some hardware synths don't support setting the master volume level, the only way to set the volume with this devices is to do it using midi controller messages for setting the volume, but I'm not sure if Vavoom can actually do this, do you have an idea about how to fix this?
Mon, 04 Jan 2010 08:02:46

Janis Legzdinsh

OK, here's another patch for trying to fix this problem:
Looks good.
I've been investigating the problem with setting midi music volume not working, it seems that some hardware synths don't support setting the master volume level, the only way to set the volume with this devices is to do it using midi controller messages for setting the volume, but I'm not sure if Vavoom can actually do this, do you have an idea about how to fix this?
Will have to investigate.
Mon, 04 Jan 2010 19:56:11

Firebrand

Hmm, looks like this patch wasn't working to fix the crash, I had to build Vavoom in debug mode to be able to corner the problem better, I'm posting a new patch that fixed this and some other bugs that the debug build found.
Index: msvc/basepak_doom1.vcproj
===================================================================
--- msvc/basepak_doom1.vcproj	(revision 4120)
+++ msvc/basepak_doom1.vcproj	(working copy)
@@ -20,72 +20,22 @@
 			Name="Debug|Win32"
 			OutputDirectory="$(SolutionDir)$(ConfigurationName)"
 			IntermediateDirectory="$(ConfigurationName)"
-			ConfigurationType="1"
+			ConfigurationType="0"
 			CharacterSet="1"
 			>
 			<Tool
-				Name="VCPreBuildEventTool"
+				Name="VCNMakeTool"
+				BuildCommandLine="$(SolutionDir)$(ConfigurationName)\vlumpy.exe ..\basev\doom1\basepak.ls&#x0D;&#x0A;if exist $(OutDir) goto end&#x0D;&#x0A;goto no &#x0D;&#x0A;:no &#x0D;&#x0A;md $(ProjectDir)$(InputName)_$(ConfigurationName)&#x0D;&#x0A;goto end &#x0D;&#x0A;:end &#x0D;&#x0A;move basepak.pk3 $(OutDir)&#x0D;&#x0A;"
+				ReBuildCommandLine="$(SolutionDir)$(ConfigurationName)\vlumpy.exe ..\basev\doom1\basepak.ls&#x0D;&#x0A;if exist $(OutDir) goto end&#x0D;&#x0A;goto no &#x0D;&#x0A;:no &#x0D;&#x0A;md $(ProjectDir)$(InputName)_$(ConfigurationName)&#x0D;&#x0A;goto end &#x0D;&#x0A;:end &#x0D;&#x0A;move basepak.pk3 $(OutDir)&#x0D;&#x0A;"
+				CleanCommandLine=""
+				Output=""
+				PreprocessorDefinitions=""
+				IncludeSearchPath=""
+				ForcedIncludes=""
+				AssemblySearchPath=""
+				ForcedUsingAssemblies=""
+				CompileAsManaged=""
 			/>
-			<Tool
-				Name="VCCustomBuildTool"
-			/>
-			<Tool
-				Name="VCXMLDataGeneratorTool"
-			/>
-			<Tool
-				Name="VCWebServiceProxyGeneratorTool"
-			/>
-			<Tool
-				Name="VCMIDLTool"
-			/>
-			<Tool
-				Name="VCCLCompilerTool"
-				Optimization="0"
-				PreprocessorDefinitions="WIN32;_DEBUG;_WINDOWS"
-				MinimalRebuild="true"
-				BasicRuntimeChecks="3"
-				RuntimeLibrary="3"
-				UsePrecompiledHeader="0"
-				WarningLevel="3"
-				DebugInformationFormat="4"
-			/>
-			<Tool
-				Name="VCManagedResourceCompilerTool"
-			/>
-			<Tool
-				Name="VCResourceCompilerTool"
-			/>
-			<Tool
-				Name="VCPreLinkEventTool"
-			/>
-			<Tool
-				Name="VCLinkerTool"
-				LinkIncremental="2"
-				GenerateDebugInformation="true"
-				SubSystem="2"
-				TargetMachine="1"
-			/>
-			<Tool
-				Name="VCALinkTool"
-			/>
-			<Tool
-				Name="VCManifestTool"
-			/>
-			<Tool
-				Name="VCXDCMakeTool"
-			/>
-			<Tool
-				Name="VCBscMakeTool"
-			/>
-			<Tool
-				Name="VCFxCopTool"
-			/>
-			<Tool
-				Name="VCAppVerifierTool"
-			/>
-			<Tool
-				Name="VCPostBuildEventTool"
-			/>
 		</Configuration>
 		<Configuration
 			Name="Release|Win32"
@@ -119,54 +69,106 @@
 			<File
 				RelativePath="..\progs\doom\cgame\classes.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\ClientGame.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\IntermissionScreen.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenControls2.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenGameplayOptions.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenHelp.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenMain.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenMultiplayer.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenNewNetGame.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenOptions.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\MenuScreenPlayerSetup.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\StatusBarScreen.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\cgame\TitleScreen.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 		</Filter>
 		<Filter
@@ -175,22 +177,42 @@
 			<File
 				RelativePath="..\progs\doom\game\BotPlayer.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\game\classes.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\game\DoomLevelInfo.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\game\MainGameInfo.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 			<File
 				RelativePath="..\progs\doom\game\Player.vc"
 				>
+				<FileConfiguration
+					Name="Debug|Win32"
+					>
+				</FileConfiguration>
 			</File>
 		</Filter>
 	</Files>
Index: msvc/vavoom.vcproj
===================================================================
--- msvc/vavoom.vcproj	(revision 4120)
+++ msvc/vavoom.vcproj	(working copy)
@@ -160,14 +160,17 @@
 			<Tool
 				Name="VCCLCompilerTool"
 				Optimization="0"
-				PreprocessorDefinitions="_DEBUG;WIN32;_WINDOWS;FLAC__NO_DLL;_CRT_SECURE_NO_DEPRECATE;USE_ASM_I386=1"
+				PreprocessorDefinitions="_DEBUG;WIN32;_WINDOWS;_WIN32;_CRTDBG_MAP_ALLOC;FLAC__NO_DLL;_CRT_SECURE_NO_DEPRECATE;USE_ASM_I386=1"
 				MinimalRebuild="true"
-				BasicRuntimeChecks="3"
+				BasicRuntimeChecks="0"
 				RuntimeLibrary="1"
-				BrowseInformation="1"
-				WarningLevel="4"
+				EnableFunctionLevelLinking="true"
+				BrowseInformation="0"
+				WarningLevel="3"
 				SuppressStartupBanner="true"
-				DebugInformationFormat="4"
+				Detect64BitPortabilityProblems="true"
+				DebugInformationFormat="3"
+				CompileAs="0"
 			/>
 			<Tool
 				Name="VCManagedResourceCompilerTool"
@@ -175,14 +178,14 @@
 			<Tool
 				Name="VCResourceCompilerTool"
 				PreprocessorDefinitions="_DEBUG"
-				Culture="1062"
+				Culture="0"
 			/>
 			<Tool
 				Name="VCPreLinkEventTool"
 			/>
 			<Tool
 				Name="VCLinkerTool"
-				AdditionalDependencies="user32.lib ole32.lib gdi32.lib winmm.lib opengl32.lib openal32.lib wsock32.lib libpngd.lib libjpeg_d.lib zlibd.lib libmad_d.lib vorbis_static_d.lib ogg_static_d.lib mikmod_d.lib libFLAC_static_d.lib libFLAC++_static_d.lib"
+				AdditionalDependencies="user32.lib ole32.lib gdi32.lib winmm.lib opengl32.lib openal32.lib wsock32.lib libpngd.lib libjpeg_d.lib zlibd.lib libmad_d.lib libvorbis_static_d.lib libogg_static_d.lib libmikmod_d.lib libFLAC_static_d.lib libFLAC++_static_d.lib"
 				OutputFile="$(OutDir)/vavoom.exe"
 				LinkIncremental="2"
 				SuppressStartupBanner="true"
@@ -190,7 +193,7 @@
 				SubSystem="2"
 				RandomizedBaseAddress="1"
 				DataExecutionPrevention="0"
-				TargetMachine="1"
+				TargetMachine="0"
 			/>
 			<Tool
 				Name="VCALinkTool"
Index: source/d3d_main.cpp
===================================================================
--- source/d3d_main.cpp	(revision 4120)
+++ source/d3d_main.cpp	(working copy)
@@ -152,9 +152,12 @@
 
 	//	Shut down current mode
 	ReleaseTextures();
-	SAFE_RELEASE(DXBlockSurface[0]);
-	SAFE_RELEASE(DXBlockSurface[1]);
-	SAFE_RELEASE(RenderDevice)
+	if (RenderDevice)
+	{
+		SAFE_RELEASE(DXBlockSurface[0]);
+		SAFE_RELEASE(DXBlockSurface[1]);
+		SAFE_RELEASE(RenderDevice)
+	}
 
 	D3DPRESENT_PARAMETERS d3dpp;
 
@@ -642,7 +645,6 @@
 	D3DSURFACE_DESC desc;
 	surf->GetDesc(&desc);
 
-
 	//	Decode pixel format
 	int scr_rbits;
 	int scr_rshift;
@@ -689,7 +691,7 @@
 		}
 		default:
 		{
-			GCon->Log(NAME_Init, "Invalid pixel format");
+			GCon->Log(NAME_Init, "ReadScreen: Invalid pixel format");
 			Z_Free(dst);
 			return NULL;
 		}
@@ -785,7 +787,7 @@
 		}
 		default:
 		{
-			GCon->Log(NAME_Init, "Invalid pixel format");
+			GCon->Log(NAME_Init, "ReadBackScreen: Invalid pixel format");
 			return;
 		}
 	}
@@ -793,7 +795,7 @@
 	D3DLOCKED_RECT lrect;
 	if (FAILED(surf->LockRect(&lrect, NULL, D3DLOCK_READONLY)))
 	{
-		Sys_Error("ReadScreen: Failed to lock screen");
+		Sys_Error("ReadBackScreen: Failed to lock screen");
 	}
 
 	rgba_t *pdst = Dest;
Index: source/snd_main.cpp
===================================================================
--- source/snd_main.cpp	(revision 4120)
+++ source/snd_main.cpp	(working copy)
@@ -1176,9 +1176,13 @@
 	}
 
 	if (StreamPlaying)
+	{
 		StreamMusicPlayer->Stop();
+	}
 	else if (MidiDevice)
+	{
 		MidiDevice->Stop();
+	}
 	StreamPlaying = false;
 
 	//	Get music volume for this song.
@@ -1206,12 +1210,18 @@
 			for (VXmlNode* N = Doc->Root.FirstChild; N; N = N->NextSibling)
 			{
 				if (N->Name != "song")
+				{
 					continue;
+				}
 				if (N->GetAttribute("name") != Song)
+				{
 					continue;
+				}
 				Lump = W_CheckNumForFileName(N->GetAttribute("file"));
 				if (Lump >= 0)
+				{
 					break;
+				}
 			}
 			delete Doc;
 		}
@@ -1332,9 +1342,13 @@
 	if (command == "off")
 	{
 		if (MidiDevice)
+		{
 			MidiDevice->Stop();
+		}
 		if (StreamMusicPlayer)
+		{
 			StreamMusicPlayer->Stop();
+		}
 		MusicEnabled = false;
 		return;
 	}
@@ -1369,27 +1383,39 @@
 	if (command == "pause")
 	{
 		if (StreamPlaying)
+		{
 			StreamMusicPlayer->Pause();
+		}
 		else if (MidiDevice)
+		{
 			MidiDevice->Pause();
+		}
 		return;
 	}
 
 	if (command == "resume")
 	{
 		if (StreamPlaying)
+		{
 			StreamMusicPlayer->Resume();
+		}
 		else if (MidiDevice)
+		{
 			MidiDevice->Resume();
+		}
 		return;
 	}
 
 	if (command == "stop")
 	{
 		if (StreamPlaying)
+		{
 			StreamMusicPlayer->Stop();
+		}
 		else if (MidiDevice)
+		{
 			MidiDevice->Stop();
+		}
 		return;
 	}
 
@@ -1440,7 +1466,9 @@
 	if (command == "off")
 	{
 		if (CDAudioDevice->Playing)
+		{
 			CDAudioDevice->Stop();
+		}
 		CDAudioDevice->Enabled = false;
 		return;
 	}
@@ -1451,9 +1479,13 @@
 
 		CDAudioDevice->Enabled = true;
 		if (CDAudioDevice->Playing)
+		{
 			CDAudioDevice->Stop();
+		}
 		for (n = 0; n < 100; n++)
+		{
 			CDAudioDevice->Remap[n] = n;
+		}
 		CDAudioDevice->GetInfo();
 		return;
 	}
@@ -1467,12 +1499,18 @@
 		if (ret <= 0)
 		{
 			for (n = 1; n < 100; n++)
+			{
 				if (CDAudioDevice->Remap[n] != n)
+				{
 					GCon->Logf("%d -> %d", n, CDAudioDevice->Remap[n]);
+				}
+			}
 			return;
 		}
 		for (n = 1; n <= ret; n++)
+		{
 			CDAudioDevice->Remap[n] = atoi(*Args[n + 1]);
+		}
 		return;
 	}
 
@@ -1484,7 +1522,9 @@
 	if (command == "eject")
 	{
 		if (CDAudioDevice->Playing)
+		{
 			CDAudioDevice->Stop();
+		}
 		CDAudioDevice->OpenDoor();
 		CDAudioDevice->CDValid = false;
 		return;
Index: source/snd_win32music.cpp
===================================================================
--- source/snd_win32music.cpp	(revision 4121)
+++ source/snd_win32music.cpp	(working copy)
@@ -342,16 +342,19 @@
 	}
 
 	//	Play
-	State = STATE_Playing;
-	midiStreamRestart((HMIDISTRM)hMidi);
+	if (hMidi)
+	{
+		State = STATE_Playing;
+		midiStreamRestart((HMIDISTRM)hMidi);
 
-	//	Pause if needed.
-	if (!MusVolume || MusicPaused)
-	{
-		Pause();
+		//	Pause if needed.
+		if (!MusVolume || MusicPaused)
+		{
+			Pause();
+		}
+		CurrSong = VName(song, VName::AddLower8);
+		CurrLoop = loop;
 	}
-	CurrSong = VName(song, VName::AddLower8);
-	CurrLoop = loop;
 	unguard;
 }
 
@@ -367,9 +370,12 @@
 	if (State != STATE_Playing)
 		return;
 
-	State = STATE_Paused;
-	midiStreamPause((HMIDISTRM)hMidi);
-	MusicPaused = true;
+	if (hMidi)
+	{
+		State = STATE_Paused;
+		midiStreamPause((HMIDISTRM)hMidi);
+		MusicPaused = true;
+	}
 	unguard;
 }
 
@@ -385,9 +391,12 @@
 	if (State != STATE_Paused)
 		return;
 
-	State = STATE_Playing;
-	midiStreamRestart((HMIDISTRM)hMidi);
-	MusicPaused = false;
+	if (hMidi)
+	{
+		State = STATE_Playing;
+		midiStreamRestart((HMIDISTRM)hMidi);
+		MusicPaused = false;
+	}
 	unguard;
 }
 
@@ -417,7 +426,7 @@
 	guard(VMMSystemMidiDevice::Stop);
 	if (MidiImage)
 	{
-		if (hMidi)
+		if (hMidi != NULL)
 		{
 			if (MusicPaused)
 			{
@@ -432,7 +441,7 @@
 				State = STATE_Stopping;
 				SeqFlags |= SEQF_Waiting;
 
-				if (MMSYSERR_NOERROR != midiStreamStop((HMIDISTRM)hMidi))
+				if (!hMidi || MMSYSERR_NOERROR != midiStreamStop((HMIDISTRM)hMidi))
 				{
 					SeqFlags &= ~SEQF_Waiting;
 					return;
@@ -451,9 +460,12 @@
 					midiOutUnprepareHeader(hMidi, lpmh, sizeof(*lpmh));
 			}
 
-			//	Close midi stream.	
-			midiStreamClose((HMIDISTRM)hMidi);
-			hMidi = NULL;
+			//	Close midi stream.
+			if (hMidi)
+			{
+				midiStreamClose((HMIDISTRM)hMidi);
+				hMidi = NULL;
+			}
 		}
 		State = STATE_NoFile;
 		Z_Free((void*)MidiImage);
I also fixed the debug configuration of the project files, because it wasn't compiling properly. Let me know what you think of it.
Tue, 05 Jan 2010 10:26:33

Janis Legzdinsh

Looks good
Tue, 09 Mar 2010 21:57:38

Firebrand

I have been investigating more around the MIDI volume and it seems that the latest Windows versions change the MIDI system somehow... so the drivers for Vista (that's what I've tested) seem to report that MIDI devices other than the default one don't support setting volume, so the problem is not in the code, but in the drivers... I highly doubt that Creative is going to change that anytime soon thought... <!-- s:lol: --><img src="{SMILIES_PATH}/icon_lol.gif" alt=":lol:" title="Laughing" /><!-- s:lol: -->

Back to the Vavoom Forum Archives