Merge remote-tracking branch 'origin/main' into v2.17.x

10 files changed, 125 insertions, 171 deletions

diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc
index 094e104ef..0b50bcd5a 100644
--- a/src/lib/j2k_encoder.cc
+++ b/src/lib/j2k_encoder.cc
@@ -108,6 +108,16 @@ J2KEncoder::~J2KEncoder ()
 {
 	_server_found_connection.disconnect();
 
+	/* One of our encoder threads may be waiting on Writer::write() to return, if that method
+	 * is blocked with the writer queue full waiting for _full_condition.  In that case, the
+	 * attempt to terminate the encoder threads below (in terminate_threads()) will fail because
+	 * the encoder thread waiting for ::write() will have interruption disabled.
+	 *
+	 * To work around that, make the writer into a zombie to unblock any pending write()s and
+	 * not block on any future ones.
+	 */
+	_writer.zombify();
+
 	terminate_threads();
 
 #ifdef DCPOMATIC_GROK
@@ -153,6 +163,8 @@ J2KEncoder::pause()
 	}
 	return;
 
+	/* XXX; the same problem may occur here as in the destructor, perhaps? */
+
 	terminate_threads ();
 
 	/* Something might have been thrown during terminate_threads */
diff --git a/src/lib/writer.cc b/src/lib/writer.cc
index 3dd22718f..c0b083ff0 100644
--- a/src/lib/writer.cc
+++ b/src/lib/writer.cc
@@ -146,6 +146,10 @@ Writer::write (shared_ptr<const Data> encoded, Frame frame, Eyes eyes)
 {
 	boost::mutex::scoped_lock lock (_state_mutex);
 
+	if (_zombie) {
+		return;
+	}
+
 	while (_queued_full_in_memory > _maximum_frames_in_memory) {
 		/* There are too many full frames in memory; wake the main writer thread and
 		   wait until it sorts everything out */
@@ -382,6 +386,9 @@ try
 	while (true)
 	{
 		boost::mutex::scoped_lock lock (_state_mutex);
+		if (_zombie) {
+			return;
+		}
 
 		while (true) {
 
@@ -1047,3 +1054,17 @@ Writer::write_hanging_text (ReelWriter& reel)
 	}
 	_hanging_texts = new_hanging_texts;
 }
+
+
+/** Set the writer so that it has no queue and drops any pending or future requests to write images */
+void
+Writer::zombify()
+{
+	boost::mutex::scoped_lock lock(_state_mutex);
+
+	_queue.clear();
+	_queued_full_in_memory = 0;
+	_zombie = true;
+	_full_condition.notify_all();
+}
+
diff --git a/src/lib/writer.h b/src/lib/writer.h
index 1dd88d8a9..f9ec0b88c 100644
--- a/src/lib/writer.h
+++ b/src/lib/writer.h
@@ -35,6 +35,7 @@
 #include "font_id_map.h"
 #include "player_text.h"
 #include "text_type.h"
+#include "types.h"
 #include "weak_film.h"
 #include <dcp/atmos_frame.h>
 #include <dcp/frame_info.h>
@@ -129,6 +130,8 @@ public:
 
 	void set_encoder_threads (int threads);
 
+	void zombify();
+
 private:
 	friend struct ::writer_disambiguate_font_ids1;
 	friend struct ::writer_disambiguate_font_ids2;
@@ -231,6 +234,8 @@ private:
 	};
 
 	std::vector<HangingText> _hanging_texts;
+
+	bool _zombie = false;
 };
 
 
diff --git a/src/tools/dcpomatic.cc b/src/tools/dcpomatic.cc
index 85fc96c3e..8f65fa83d 100644
--- a/src/tools/dcpomatic.cc
+++ b/src/tools/dcpomatic.cc
@@ -49,7 +49,6 @@
 #include "wx/report_problem_dialog.h"
 #include "wx/save_template_dialog.h"
 #include "wx/self_dkdm_dialog.h"
-#include "wx/send_i18n_dialog.h"
 #include "wx/servers_list_dialog.h"
 #include "wx/standard_controls.h"
 #include "wx/system_information_dialog.h"
@@ -251,7 +250,6 @@ enum {
 	ID_tools_encoding_servers,
 	ID_tools_manage_templates,
 	ID_tools_check_for_updates,
-	ID_tools_send_translations,
 	ID_tools_system_information,
 	ID_tools_restore_default_preferences,
 	ID_tools_export_preferences,
@@ -364,7 +362,6 @@ public:
 		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_encoding_servers, this),  ID_tools_encoding_servers);
 		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_manage_templates, this),  ID_tools_manage_templates);
 		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_check_for_updates, this), ID_tools_check_for_updates);
-		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_send_translations, this), ID_tools_send_translations);
 		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_system_information, this),ID_tools_system_information);
 		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_restore_default_preferences, this), ID_tools_restore_default_preferences);
 		Bind (wxEVT_MENU, boost::bind (&DOMFrame::tools_export_preferences, this), ID_tools_export_preferences);
@@ -1145,40 +1142,6 @@ private:
 		UpdateChecker::instance()->run();
 	}
 
-	void tools_send_translations ()
-	{
-		SendI18NDialog dialog(this);
-		if (dialog.ShowModal() != wxID_OK) {
-			return;
-		}
-
-		string body;
-		body += dialog.name() + "\n";
-		body += dialog.language() + "\n";
-		body += string(dcpomatic_version) + " " + string(dcpomatic_git_commit) + "\n";
-		body += "--\n";
-		auto translations = I18NHook::translations ();
-		for (auto i: translations) {
-			body += i.first + "\n" + i.second + "\n\n";
-		}
-		if (dialog.email().find("@") == string::npos) {
-			error_dialog(
-				this,
-				variant::wx::insert_dcpomatic(
-					_("You must enter a valid email address when sending translations, "
-					  "otherwise the %s maintainers cannot credit you or contact you with questions.")
-					)
-				);
-		} else {
-			Email email(dialog.email(), { "carl@dcpomatic.com" }, variant::insert_dcpomatic("%1 translations"), body);
-			try {
-				email.send("main.carlh.net", 2525, EmailProtocol::STARTTLS);
-			} catch (NetworkError& e) {
-				error_dialog (this, _("Could not send translations"), std_to_wx(e.what()));
-			}
-		}
-	}
-
 	void help_about ()
 	{
 		AboutDialog dialog(this);
@@ -1455,7 +1418,6 @@ private:
 		add_item (tools, _("Encoding servers..."), ID_tools_encoding_servers, 0);
 		add_item (tools, _("Manage templates..."), ID_tools_manage_templates, 0);
 		add_item (tools, _("Check for updates"), ID_tools_check_for_updates, 0);
-		add_item (tools, _("Send translations..."), ID_tools_send_translations, 0);
 		add_item (tools, _("System information..."), ID_tools_system_information, 0);
 		tools->AppendSeparator ();
 		add_item (tools, _("Restore default preferences"), ID_tools_restore_default_preferences, ALWAYS);
diff --git a/src/wx/send_i18n_dialog.cc b/src/wx/send_i18n_dialog.cc
deleted file mode 100644
index 6efcf993c..000000000
--- a/src/wx/send_i18n_dialog.cc
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
-    Copyright (C) 2018-2021 Carl Hetherington <cth@carlh.net>
-
-    This file is part of DCP-o-matic.
-
-    DCP-o-matic is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    DCP-o-matic is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
-
-*/
-
-
-#include "i18n_hook.h"
-#include "send_i18n_dialog.h"
-#include "wx_util.h"
-#include <dcp/warnings.h>
-LIBDCP_DISABLE_WARNINGS
-#include <wx/listctrl.h>
-LIBDCP_ENABLE_WARNINGS
-
-
-using std::string;
-using std::map;
-
-
-SendI18NDialog::SendI18NDialog (wxWindow* parent)
-	: wxDialog (parent, wxID_ANY, _("Send translations"))
-{
-	auto overall_sizer = new wxBoxSizer (wxVERTICAL);
-
-	auto table = new wxFlexGridSizer (2, DCPOMATIC_SIZER_X_GAP, DCPOMATIC_SIZER_Y_GAP);
-	table->AddGrowableCol (1, 1);
-
-	add_label_to_sizer (table, this, _("Your name"), true, 0, wxLEFT | wxRIGHT | wxALIGN_CENTRE_VERTICAL);
-	_name = new wxTextCtrl (this, wxID_ANY);
-	table->Add (_name, 0, wxEXPAND);
-
-	add_label_to_sizer (table, this, _("Your email"), true, 0, wxLEFT | wxRIGHT | wxALIGN_CENTRE_VERTICAL);
-	_email = new wxTextCtrl (this, wxID_ANY);
-	table->Add (_email, 0, wxEXPAND);
-
-	add_label_to_sizer (table, this, _("Language"), true, 0, wxLEFT | wxRIGHT | wxALIGN_CENTRE_VERTICAL);
-	_language = new wxTextCtrl (this, wxID_ANY);
-	table->Add (_language, 0, wxEXPAND);
-
-	auto list = new wxListCtrl (this, wxID_ANY, wxDefaultPosition, wxSize(800, -1), wxLC_REPORT | wxLC_NO_HEADER);
-	list->AppendColumn(wxT(""), wxLIST_FORMAT_LEFT, 400);
-	list->AppendColumn(wxT(""), wxLIST_FORMAT_LEFT, 400);
-
-	auto translations = I18NHook::translations ();
-	int N = 0;
-	for (auto const& i: translations) {
-		wxListItem it;
-		it.SetId(N);
-		it.SetColumn(0);
-		it.SetText(std_to_wx(i.first));
-		list->InsertItem(it);
-		it.SetColumn(1);
-		it.SetText(std_to_wx(i.second));
-		list->SetItem(it);
-		++N;
-	}
-
-	overall_sizer->Add (table, 0, wxEXPAND | wxALL, DCPOMATIC_SIZER_GAP);
-	overall_sizer->Add (list, 1, wxEXPAND | wxALL, DCPOMATIC_SIZER_GAP);
-
-	auto buttons = CreateSeparatedButtonSizer (wxOK | wxCANCEL);
-	if (buttons) {
-		overall_sizer->Add (buttons, wxSizerFlags().Expand().DoubleBorder());
-	}
-
-	SetSizerAndFit (overall_sizer);
-}
diff --git a/src/wx/send_i18n_dialog.h b/src/wx/send_i18n_dialog.h
deleted file mode 100644
index 4651fb811..000000000
--- a/src/wx/send_i18n_dialog.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
-    Copyright (C) 2018 Carl Hetherington <cth@carlh.net>
-
-    This file is part of DCP-o-matic.
-
-    DCP-o-matic is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    DCP-o-matic is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
-
-*/
-
-
-#include "wx_util.h"
-#include <dcp/warnings.h>
-LIBDCP_DISABLE_WARNINGS
-#include <wx/wx.h>
-LIBDCP_ENABLE_WARNINGS
-
-
-class SendI18NDialog : public wxDialog
-{
-public:
-	SendI18NDialog (wxWindow* parent);
-
-	std::string name () {
-		return wx_to_std (_name->GetValue());
-	}
-
-	std::string email () {
-		return wx_to_std (_email->GetValue());
-	}
-
-	std::string language () {
-		return wx_to_std (_language->GetValue());
-	}
-
-private:
-	wxTextCtrl* _name;
-	wxTextCtrl* _email;
-	wxTextCtrl* _language;
-};
diff --git a/src/wx/supporters.cc b/src/wx/supporters.cc
index b39dbd5dc..374beb656 100644
--- a/src/wx/supporters.cc
+++ b/src/wx/supporters.cc
@@ -687,6 +687,7 @@ supported_by.Add (wxT ("Daniel Martinez Lara"));
 supported_by.Add (wxT ("Gabriel Montagné Láscaris-Comneno"));
 supported_by.Add (wxT ("Marga Laube"));
 supported_by.Add (wxT ("James Lauchlan"));
+supported_by.Add (wxT ("Luc Lavergne"));
 supported_by.Add (wxT ("Nicholas Lavigne"));
 supported_by.Add (wxT ("Philip Lawrence"));
 supported_by.Add (wxT ("David Lawrence"));
@@ -961,6 +962,7 @@ supported_by.Add (wxT ("Jason Phelps"));
 supported_by.Add (wxT ("John Phillips"));
 supported_by.Add (wxT ("Nat Phong"));
 supported_by.Add (wxT ("Phonotone"));
+supported_by.Add (wxT ("Kari Layland Photography"));
 supported_by.Add (wxT ("MelRish Photos And Films"));
 supported_by.Add (wxT ("Paolo Piccioli"));
 supported_by.Add (wxT ("Peccadillo Pictures"));
diff --git a/src/wx/wscript b/src/wx/wscript
index a41e3827e..bdae9708e 100644
--- a/src/wx/wscript
+++ b/src/wx/wscript
@@ -151,7 +151,6 @@ sources = """
           screen_dialog.cc
           screens_panel.cc
           self_dkdm_dialog.cc
-          send_i18n_dialog.cc
           send_test_email_dialog.cc
           server_dialog.cc
           servers_list_dialog.cc
diff --git a/test/j2k_encoder_test.cc b/test/j2k_encoder_test.cc
new file mode 100644
index 000000000..bc3bd97b2
--- /dev/null
+++ b/test/j2k_encoder_test.cc
@@ -0,0 +1,83 @@
+/*
+    Copyright (C) 2024 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+#include "lib/config.h"
+#include "lib/cross.h"
+#include "lib/image.h"
+#include "lib/j2k_encoder.h"
+#include "lib/player_video.h"
+#include "lib/raw_image_proxy.h"
+#include "lib/writer.h"
+#include "test.h"
+extern "C" {
+#include <libavutil/pixfmt.h>
+}
+#include <boost/test/unit_test.hpp>
+
+
+using std::make_shared;
+using std::weak_ptr;
+using boost::optional;
+
+
+BOOST_AUTO_TEST_CASE(j2k_encoder_deadlock_test)
+{
+	ConfigRestorer cr;
+
+	auto film = new_test_film2("j2k_encoder_deadlock_test");
+
+	/* Don't call ::start() on this Writer, so it can never write anything */
+	Writer writer(film, {});
+	writer.set_encoder_threads(4);
+
+	/* We want to test the case where the writer queue fills, and this can't happen unless there
+	 * are enough encoding threads (each of which will end up waiting for the writer to empty,
+	 * which will never happen).
+	 */
+	Config::instance()->set_master_encoding_threads(4);
+	J2KEncoder encoder(film, writer);
+	encoder.begin();
+
+	for (int i = 0; i < 26; ++i) {
+		auto image = make_shared<Image>(AV_PIX_FMT_RGB24, dcp::Size(1998, 1080), Image::Alignment::PADDED);
+		auto image_proxy = make_shared<RawImageProxy>(image);
+		encoder.encode(
+			std::make_shared<PlayerVideo>(
+				image_proxy,
+				Crop(),
+				optional<double>(),
+				dcp::Size(1998, 1080),
+				dcp::Size(1998, 1080),
+				Eyes::BOTH,
+				Part::WHOLE,
+				optional<ColourConversion>(),
+				VideoRange::VIDEO,
+				weak_ptr<Content>(),
+				optional<Frame>(),
+				false
+				),
+			{}
+			);
+	}
+
+	dcpomatic_sleep_seconds(10);
+}
+
diff --git a/test/wscript b/test/wscript
index 7555f76db..52b909b52 100644
--- a/test/wscript
+++ b/test/wscript
@@ -113,7 +113,9 @@ def build(bld):
                  import_dcp_test.cc
                  interrupt_encoder_test.cc
                  isdcf_name_test.cc
+                 j2k_bandwidth_test.cc
                  j2k_encode_threading_test.cc
+                 j2k_encoder_test.cc
                  job_manager_test.cc
                  j2k_video_bit_rate_test.cc
                  kdm_cli_test.cc