From b8d07b8be22a83291db50f41190ed47cbacaaf6c Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Mon, 16 Jan 2023 03:27:10 +0100 Subject: [PATCH] Export CmdPipeWriter: add option to use tmpfile Investigate issues with mp3 export (#9193) --- libs/ardour/export_graph_builder.cc | 25 +++++- .../audiographer/general/cmdpipe_writer.h | 76 +++++++++---------- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/libs/ardour/export_graph_builder.cc b/libs/ardour/export_graph_builder.cc index b160e501c3..f0483fe31f 100644 --- a/libs/ardour/export_graph_builder.cc +++ b/libs/ardour/export_graph_builder.cc @@ -24,6 +24,9 @@ #include +#include +#include "pbd/gstdio_compat.h" + #include #include #include @@ -376,6 +379,20 @@ ExportGraphBuilder::Encoder::init_writer (boost::shared_ptrcodec_quality (); + gchar* tmpfile_name = NULL; +#if 0 + gint fd = -1; +#else // write to tmp-file, do not pipe to ffmpeg + gint fd = g_file_open_tmp ("ardour-export.XXXXXX", &tmpfile_name, NULL); + if (fd < 0) { + tmpfile_name = NULL; + } +#endif + + /* Note mp3 encoding adds silence at start and end + * https://lame.sourceforge.io/tech-FAQ.txt + */ + int a=0; char **argp = (char**) calloc (100, sizeof(char*)); char tmp[64]; @@ -397,7 +414,11 @@ ExportGraphBuilder::Encoder::init_writer (boost::shared_ptrsample_rate()); argp[a++] = strdup (tmp); argp[a++] = strdup ("-i"); - argp[a++] = strdup ("pipe:0"); + if (fd >= 0) { + argp[a++] = strdup (tmpfile_name); + } else { + argp[a++] = strdup ("pipe:0"); + } argp[a++] = strdup ("-f"); argp[a++] = strdup ("mp3"); @@ -445,7 +466,7 @@ ExportGraphBuilder::Encoder::init_writer (boost::shared_ptrto_s () << "}" << endmsg; - writer.reset (new AudioGrapher::CmdPipeWriter (exec, writer_filename, false)); + writer.reset (new AudioGrapher::CmdPipeWriter (exec, writer_filename, fd, tmpfile_name)); 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 5afc7a0ffc..63ccefb3bd 100644 --- a/libs/audiographer/audiographer/general/cmdpipe_writer.h +++ b/libs/audiographer/audiographer/general/cmdpipe_writer.h @@ -29,37 +29,32 @@ class CmdPipeWriter , public FlagDebuggable<> { public: - CmdPipeWriter (ARDOUR::SystemExec* proc, std::string const& path, bool pipe1) + CmdPipeWriter (ARDOUR::SystemExec* proc, std::string const& path, int tmp_fd = -1, gchar* tmp_file = 0) : samples_written (0) , _proc (proc) , _path (path) - , encoder_file (0) + , _tmp_fd (tmp_fd) + , _tmp_file (tmp_file) { add_supported_flag (ProcessContext::EndOfInput); - 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"); - - if (!encoder_file) { - throw ARDOUR::ExportFailed ("Output file cannot be written to."); - } - } - - if (proc->start (ARDOUR::SystemExec::IgnoreAndClose)) { - if (encoder_file) { - fclose (encoder_file); - encoder_file = 0; - g_unlink (path.c_str()); - } + if (tmp_fd >= 0) { + ; + } else if (proc->start (ARDOUR::SystemExec::ShareWithParent)) { throw ARDOUR::ExportFailed ("External encoder (ffmpeg) cannot be started."); } + proc->Terminated.connect_same_thread (exec_connections, boost::bind (&CmdPipeWriter::encode_complete, this)); } virtual ~CmdPipeWriter () { delete _proc; + if (_tmp_fd >= 0) { + ::close (_tmp_fd); + } + if (_tmp_file) { + g_unlink (_tmp_file); + g_free (_tmp_file); + } } samplecnt_t get_samples_written() const { return samples_written; } @@ -74,13 +69,19 @@ public: { check_flags (*this, c); - if (!_proc || !_proc->is_running()) { + if (_tmp_fd < 0 && (!_proc || !_proc->is_running())) { throw Exception (*this, boost::str (boost::format ("Target encoder process is not running"))); } const size_t bytes_per_sample = sizeof (T); - samplecnt_t const written = _proc->write_to_stdin ((const void*) c.data(), c.samples() * bytes_per_sample) / bytes_per_sample; + samplecnt_t written; + if (_tmp_fd >= 0) { + written = write (_tmp_fd, (const void*) c.data(), c.samples() * bytes_per_sample) / bytes_per_sample; + } else { + written = _proc->write_to_stdin ((const void*) c.data(), c.samples() * bytes_per_sample) / bytes_per_sample; + } + samples_written += written; if (throw_level (ThrowProcess) && written != c.samples()) { @@ -89,17 +90,16 @@ public: } if (c.has_flag(ProcessContext::EndOfInput)) { - if (encoder_file) { - _proc->close_stdin (); - _proc->wait (); - int timeout = 500; - while (encoder_file && --timeout) { - Glib::usleep(10000); + if (_tmp_fd >= 0) { + ::close (_tmp_fd); + _tmp_fd = -1; + if (_proc->start (ARDOUR::SystemExec::ShareWithParent)) { + throw ARDOUR::ExportFailed ("External encoder (ffmpeg) cannot be started."); } } else { _proc->close_stdin (); - FileWritten (_path); } + _proc->wait (); } } @@ -111,18 +111,18 @@ private: CmdPipeWriter (CmdPipeWriter const & other) {} samplecnt_t samples_written; - PBD::SystemExec* _proc; + ARDOUR::SystemExec* _proc; std::string _path; + std::vector _tmpfile_path_buf; + int _tmp_fd; + gchar* _tmp_file; - FILE* encoder_file; - - void write_ffile (std::string d, size_t s) { - fwrite (d.c_str(), sizeof(char), s, encoder_file); - } - - void close_ffile () { - fclose (encoder_file); - encoder_file = 0; + void encode_complete () { + if (_tmp_file) { + g_unlink (_tmp_file); + g_free (_tmp_file); + _tmp_file = NULL; + } FileWritten (_path); }