From 8f8c2ef2efd77367e707e93f0a67111d7a118044 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Fri, 1 Sep 2017 10:22:33 -0700 Subject: [PATCH] Build Timidity++ args separately on non-Windows Rather than building a command line that's going to be manually split into individual arguments passed to execvp, build the individual arguments directly. --- .../music_timiditypp_mididevice.cpp | 142 ++++++++---------- 1 file changed, 65 insertions(+), 77 deletions(-) diff --git a/src/sound/mididevices/music_timiditypp_mididevice.cpp b/src/sound/mididevices/music_timiditypp_mididevice.cpp index ee836efec..cc0c3684d 100644 --- a/src/sound/mididevices/music_timiditypp_mididevice.cpp +++ b/src/sound/mididevices/music_timiditypp_mididevice.cpp @@ -34,6 +34,8 @@ #include "i_midi_win32.h" +#include +#include #include "i_musicinterns.h" #include "c_cvars.h" @@ -46,6 +48,7 @@ #include #include +#include #include #include @@ -81,14 +84,16 @@ protected: HANDLE ReadWavePipe; HANDLE WriteWavePipe; HANDLE ChildProcess; + FString CommandLine; + size_t LoopPos; bool Validated; bool ValidateTimidity(); #else // _WIN32 int WavePipe[2]; pid_t ChildProcess; #endif - FString CommandLine; - size_t LoopPos; + FString ExeName; + bool Looping; static bool FillStream(SoundStream *stream, void *buff, int len, void *userdata); #ifdef _WIN32 @@ -162,20 +167,24 @@ TimidityPPMIDIDevice::TimidityPPMIDIDevice(const char *args) #ifdef _WIN32 ReadWavePipe(INVALID_HANDLE_VALUE), WriteWavePipe(INVALID_HANDLE_VALUE), ChildProcess(INVALID_HANDLE_VALUE), - Validated(false) + Validated(false), #else - ChildProcess(-1) + ChildProcess(-1), #endif + Looping(false) { #ifndef _WIN32 WavePipe[0] = WavePipe[1] = -1; #endif if (args == NULL || *args == 0) args = timidity_exe; + ExeName = args; +#ifdef _WIN32 CommandLine.Format("%s %s -EFchorus=%s -EFreverb=%s -s%d ", args, *timidity_extargs, *timidity_chorus, *timidity_reverb, *timidity_frequency); +#endif if (DiskName == NULL) { @@ -229,14 +238,17 @@ bool TimidityPPMIDIDevice::Preprocess(MIDIStreamer *song, bool looping) bool success; FILE *f; - if (CommandLine.IsEmpty()) + if (ExeName.IsEmpty()) { return false; } // Tell TiMidity++ whether it should loop or not +#ifdef _WIN32 CommandLine.LockBuffer()[LoopPos] = looping ? 'l' : ' '; CommandLine.UnlockBuffer(); +#endif + Looping = looping; // Write MIDI song to temporary file song->CreateSMF(midi, looping ? 0 : 1); @@ -327,12 +339,15 @@ int TimidityPPMIDIDevice::Open(MidiCallback callback, void *userdata) Printf(PRINT_BOLD, "If your soundcard cannot play more than one\n" "wave at a time, you will hear no music.\n"); } +#ifdef _WIN32 else { CommandLine += "-o - -Ors"; } +#endif } +#ifdef _WIN32 if (pipeSize == 0) { CommandLine += "-Od"; @@ -349,6 +364,7 @@ int TimidityPPMIDIDevice::Open(MidiCallback callback, void *userdata) CommandLine += " -idl "; CommandLine += DiskName.GetName(); +#endif return 0; } @@ -456,6 +472,7 @@ bool TimidityPPMIDIDevice::ValidateTimidity() bool TimidityPPMIDIDevice::LaunchTimidity () { +#ifdef _WIN32 if (CommandLine.IsEmpty()) { return false; @@ -463,7 +480,6 @@ bool TimidityPPMIDIDevice::LaunchTimidity () DPrintf (DMSG_NOTIFY, "cmd: \x1cG%s\n", CommandLine.GetChars()); -#ifdef _WIN32 STARTUPINFO startup = { sizeof(startup), }; PROCESS_INFORMATION procInfo; @@ -509,6 +525,11 @@ bool TimidityPPMIDIDevice::LaunchTimidity () } return false; #else + if (ExeName.IsEmpty()) + { + return false; + } + if (WavePipe[0] != -1 && WavePipe[1] == -1 && Stream != NULL) { // Timidity was previously launched, so the write end of the pipe @@ -523,78 +544,50 @@ bool TimidityPPMIDIDevice::LaunchTimidity () } int forkres; + wordexp_t words; glob_t glb; // Get timidity executable path - int spaceIdx = 0; - int spaceInExePathCount = -1; - FString TimidityExe; - do - { - spaceIdx = CommandLine.IndexOf(' ', spaceIdx); - TimidityExe = CommandLine.Left(spaceIdx); - glob(TimidityExe.GetChars(), 0, NULL, &glb); - spaceIdx += 1; - spaceInExePathCount += 1; - } while (spaceIdx != 0 && glb.gl_pathc == 0); - if (spaceIdx == 0) - { - TimidityExe = FString("timidity"); // Maybe it's in your PATH? - spaceInExePathCount = 0; - } - globfree(&glb); + const char *exename = "timidity"; // Fallback default + glob(ExeName.GetChars(), 0, NULL, &glb); + if(glb.gl_pathc != 0) + exename = glb.gl_pathv[0]; + // Get user-defined extra args + wordexp(timidity_extargs, &words, WRDE_NOCMD); - int strCount = 1; - for (spaceIdx = 0; spaceIdx < static_cast(CommandLine.Len()); spaceIdx++) - { - if (CommandLine[spaceIdx] == ' ') - { - ++strCount; - if (CommandLine[spaceIdx+1] == ' ') - { - --strCount; - } - } - } - strCount -= spaceInExePathCount; + std::string chorusarg = std::string("-EFchorus=") + *timidity_chorus; + std::string reverbarg = std::string("-EFreverb=") + *timidity_reverb; + std::string sratearg = std::string("-s") + std::to_string(*timidity_frequency); + std::string outfilearg = "-o"; // An extra "-" is added later + std::string outmodearg = "-Or"; + outmodearg += timidity_8bit ? "u8" : "s1"; + outmodearg += timidity_stereo ? "S" : "M"; + if(timidity_byteswap) outmodearg += "x"; + std::string ifacearg = "-id"; + if(Looping) ifacearg += "l"; - char** TimidityArgs = new char*[strCount + 1]; - TimidityArgs[strCount] = NULL; + std::vector arglist; + arglist.push_back(exename); + for(size_t i = 0;i < words.we_wordc;i++) + arglist.push_back(words.we_wordv[i]); + arglist.push_back(chorusarg.c_str()); + arglist.push_back(reverbarg.c_str()); + arglist.push_back(sratearg.c_str()); + arglist.push_back(outfilearg.c_str()); + arglist.push_back("-"); + arglist.push_back(outmodearg.c_str()); + arglist.push_back(ifacearg.c_str()); + arglist.push_back(DiskName.GetName()); - spaceIdx = CommandLine.IndexOf(' '); - int curSpace = spaceIdx, i = 1; - - TimidityArgs[0] = new char[TimidityExe.Len() + 1]; - TimidityArgs[0][TimidityExe.Len()] = 0; - strcpy(TimidityArgs[0], TimidityExe.GetChars()); - - int argLen; - while (curSpace != -1) - { - curSpace = CommandLine.IndexOf(' ', spaceIdx); - if (curSpace != spaceIdx) - { - argLen = curSpace - spaceIdx + 1; - if (argLen < 0) - { - argLen = CommandLine.Len() - curSpace; - } - TimidityArgs[i] = new char[argLen]; - TimidityArgs[i][argLen-1] = 0; - strcpy(TimidityArgs[i], CommandLine.Mid(spaceIdx, curSpace - spaceIdx).GetChars()); - i += 1; - } - spaceIdx = curSpace + 1; - } - - DPrintf(DMSG_NOTIFY, "Timidity EXE: \x1cG%s\n", TimidityExe.GetChars()); - for (i = 0; i < strCount; i++) - { - DPrintf(DMSG_NOTIFY, "arg %d: \x1cG%s\n", i, TimidityArgs[i]); - } + DPrintf(DMSG_NOTIFY, "Timidity EXE: \x1cG%s\n", exename); + int i = 1; + std::for_each(arglist.begin()+1, arglist.end(), + [&i](const char *arg) + { DPrintf(DMSG_NOTIFY, "arg %d: \x1cG%s\n", i++, arg); } + ); + arglist.push_back(nullptr); forkres = fork (); - if (forkres == 0) { close (WavePipe[0]); @@ -603,7 +596,7 @@ bool TimidityPPMIDIDevice::LaunchTimidity () // freopen ("/dev/null", "w", stderr); close (WavePipe[1]); - execvp (TimidityExe.GetChars(), TimidityArgs); + execvp (exename, const_cast(arglist.data())); fprintf(stderr,"execvp failed: %s\n", strerror(errno)); _exit (0); // if execvp succeeds, we never get here } @@ -624,12 +617,7 @@ bool TimidityPPMIDIDevice::LaunchTimidity () }*/ } - for (i = 0; i < strCount; i++) - { - delete [] TimidityArgs[i]; - } - - delete [] TimidityArgs; + wordfree(&words); globfree (&glb); return ChildProcess != -1; #endif // _WIN32