From 27e136dcfcc945d19045de7a9dbcdf8aea6066a9 Mon Sep 17 00:00:00 2001
From: Robin Gareus <robin@gareus.org>
Date: Thu, 5 Jan 2023 03:32:40 +0100
Subject: [PATCH] Cont'd tweaks of ffmpeg export pipe

---
 libs/ardour/export_graph_builder.cc           | 15 +++++++-
 .../audiographer/general/cmdpipe_writer.h     | 34 ++++++++++++++-----
 libs/pbd/system_exec.cc                       |  8 +++--
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/libs/ardour/export_graph_builder.cc b/libs/ardour/export_graph_builder.cc
index d000c6ac82..35d982ad02 100644
--- a/libs/ardour/export_graph_builder.cc
+++ b/libs/ardour/export_graph_builder.cc
@@ -426,7 +426,20 @@ ExportGraphBuilder::Encoder::init_writer (boost::shared_ptr<AudioGrapher::CmdPip
 		argp[a++] = SystemExec::format_key_value_parameter (it->first.c_str(), it->second.c_str());
 	}
 
+#ifdef PLATFORM_WINDOWS
+	/* A pipe is used to work-around SystemExec::make_wargs
+	 * filename escape and encoding.
+	 *
+	 * A slight downside of using a pipe is that the file duration
+	 * of the produced .mp3 may not be reported accurately.
+	 */
+	bool pipe1 = true;
 	argp[a++] = strdup ("pipe:1");
+#else
+	bool pipe1 = false;
+	argp[a++] = strdup ("-y");
+	argp[a++] = strdup (writer_filename.c_str());
+#endif
 	argp[a] = (char *)0;
 
 	/* argp is free()d in ~SystemExec,
@@ -434,7 +447,7 @@ ExportGraphBuilder::Encoder::init_writer (boost::shared_ptr<AudioGrapher::CmdPip
 	ARDOUR::SystemExec* exec = new ARDOUR::SystemExec (ffmpeg_exe, argp, true);
 
 	PBD::info << "Encode command: { " << exec->to_s () << "}" << endmsg;
-	writer.reset (new AudioGrapher::CmdPipeWriter<T> (exec, writer_filename));
+	writer.reset (new AudioGrapher::CmdPipeWriter<T> (exec, writer_filename, pipe1));
 	writer->FileWritten.connect_same_thread (copy_files_connection, boost::bind (&ExportGraphBuilder::Encoder::copy_files, this, _1));
 }
 
diff --git a/libs/audiographer/audiographer/general/cmdpipe_writer.h b/libs/audiographer/audiographer/general/cmdpipe_writer.h
index d88eab2d60..5afc7a0ffc 100644
--- a/libs/audiographer/audiographer/general/cmdpipe_writer.h
+++ b/libs/audiographer/audiographer/general/cmdpipe_writer.h
@@ -29,31 +29,37 @@ class CmdPipeWriter
   , public FlagDebuggable<>
 {
 public:
-	CmdPipeWriter (ARDOUR::SystemExec* proc, std::string const& path)
+	CmdPipeWriter (ARDOUR::SystemExec* proc, std::string const& path, bool pipe1)
 		: samples_written (0)
 		, _proc (proc)
 		, _path (path)
+		, encoder_file (0)
 	{
 		add_supported_flag (ProcessContext<T>::EndOfInput);
 
-		proc->ReadStdout.connect_same_thread (exec_connections, boost::bind (&CmdPipeWriter::write_ffile, this, _1, _2));
-		proc->Terminated.connect_same_thread (exec_connections, boost::bind (&CmdPipeWriter::close_ffile, this));
+		if (pipe1) {
+			proc->ReadStdout.connect_same_thread (exec_connections, boost::bind (&CmdPipeWriter::write_ffile, this, _1, _2));
+			proc->Terminated.connect_same_thread (exec_connections, boost::bind (&CmdPipeWriter::close_ffile, this));
 
-		encoder_file = g_fopen (path.c_str(), "wb");
+			encoder_file = g_fopen (path.c_str(), "wb");
 
-		if (!encoder_file) {
-			throw ARDOUR::ExportFailed ("Output file cannot be written to.");
+			if (!encoder_file) {
+				throw ARDOUR::ExportFailed ("Output file cannot be written to.");
+			}
 		}
 
 		if (proc->start (ARDOUR::SystemExec::IgnoreAndClose)) {
-			fclose (encoder_file);
+			if (encoder_file) {
+				fclose (encoder_file);
+				encoder_file = 0;
+				g_unlink (path.c_str());
+			}
 			throw ARDOUR::ExportFailed ("External encoder (ffmpeg) cannot be started.");
 		}
 	}
 
 	virtual ~CmdPipeWriter () {
 		delete _proc;
-		assert (!encoder_file);
 	}
 
 	samplecnt_t get_samples_written() const { return samples_written; }
@@ -83,7 +89,17 @@ public:
 		}
 
 		if (c.has_flag(ProcessContext<T>::EndOfInput)) {
-			_proc->close_stdin ();
+			if (encoder_file) {
+				_proc->close_stdin ();
+				_proc->wait ();
+				int timeout = 500;
+				while (encoder_file && --timeout) {
+					Glib::usleep(10000);
+				}
+			} else {
+				_proc->close_stdin ();
+				FileWritten (_path);
+			}
 		}
 	}
 
diff --git a/libs/pbd/system_exec.cc b/libs/pbd/system_exec.cc
index ce6f608adb..fd8974c96e 100644
--- a/libs/pbd/system_exec.cc
+++ b/libs/pbd/system_exec.cc
@@ -422,6 +422,9 @@ SystemExec::terminate ()
 		delete pid;
 		pid=0;
 	}
+
+	if (thread_active) pthread_join(thread_id_tt, NULL);
+	thread_active = false;
 	::pthread_mutex_unlock(&write_lock);
 }
 
@@ -561,6 +564,8 @@ SystemExec::close_stdin()
 	if (stdinP[1] != INVALID_HANDLE_VALUE) FlushFileBuffers (stdinP[1]);
 	Sleep(200);
 	destroy_pipe (stdinP);
+	if (stdoutP[0] != INVALID_HANDLE_VALUE) FlushFileBuffers (stdoutP[0]);
+	if (stdoutP[1] != INVALID_HANDLE_VALUE) FlushFileBuffers (stdoutP[1]);
 }
 
 size_t
@@ -917,8 +922,7 @@ SystemExec::close_stdin()
 	}
 	close_fd (pin[0]);
 	close_fd (pin[1]);
-	close_fd (pout[0]);
-	close_fd (pout[1]);
+	fsync (pout[0]);
 }
 
 size_t