From f8a0f78aaeb113cbf952d48b8d61bffc8bd2ee6d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 4 May 2024 00:19:00 +0200 Subject: Remove in-place translations support. It feels like there have been too many failed attempts and not enough (maybe zero?) successes so it's just too confusing. --- src/tools/dcpomatic.cc | 33 ------------------- src/wx/send_i18n_dialog.cc | 82 ---------------------------------------------- src/wx/send_i18n_dialog.h | 50 ---------------------------- src/wx/wscript | 1 - 4 files changed, 166 deletions(-) delete mode 100644 src/wx/send_i18n_dialog.cc delete mode 100644 src/wx/send_i18n_dialog.h (limited to 'src') diff --git a/src/tools/dcpomatic.cc b/src/tools/dcpomatic.cc index 1b1ef0629..584ebd27d 100644 --- a/src/tools/dcpomatic.cc +++ b/src/tools/dcpomatic.cc @@ -47,7 +47,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" @@ -241,7 +240,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, @@ -353,7 +351,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); @@ -1107,35 +1104,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, _("You must enter a valid email address when sending translations, " - "otherwise the DCP-o-matic maintainers cannot credit you or contact you with questions.")); - } else { - Email email(dialog.email(), { "carl@dcpomatic.com" }, "DCP-o-matic 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); @@ -1399,7 +1367,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 - - 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 . - -*/ - - -#include "i18n_hook.h" -#include "send_i18n_dialog.h" -#include "wx_util.h" -#include -LIBDCP_DISABLE_WARNINGS -#include -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 - - 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 . - -*/ - - -#include "wx_util.h" -#include -LIBDCP_DISABLE_WARNINGS -#include -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/wscript b/src/wx/wscript index 9c6ea6b84..a6eefa69f 100644 --- a/src/wx/wscript +++ b/src/wx/wscript @@ -139,7 +139,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 -- cgit v1.2.3 From 4541d4d553b5903b63753f22254a6471c73337da Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 8 May 2024 00:33:13 +0200 Subject: Supporters update. --- src/wx/supporters.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') 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")); -- cgit v1.2.3 From f5e08d6f36161a980682dd3cb9b0678d44adadfd Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 5 May 2024 21:34:22 +0200 Subject: Add some missing includes. --- src/lib/writer.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/lib/writer.h b/src/lib/writer.h index efb6a17d8..6e7f5ca90 100644 --- a/src/lib/writer.h +++ b/src/lib/writer.h @@ -34,6 +34,8 @@ #include "exception_store.h" #include "font_id_map.h" #include "player_text.h" +#include "text_type.h" +#include "types.h" #include "weak_film.h" #include #include -- cgit v1.2.3 From 32d04ddb5c583938f470ed74bda8a50cc2ec9960 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 5 May 2024 21:34:29 +0200 Subject: Work around deadlock when destroying J2KEncoder with a full writer queue (#2784). This feels like a hack, but I can't think of a nicer way to do it. The interruption disable makes sense because when we destroy encoder threads during a DCP encode (because a remote server goes away, for example) we don't want any frames to be lost due to the encode thread being interrupted between taking the frame off the queue and sending it to the writer. When we're destroying the encoder we don't care about this, but I can't see how you'd differentiate. Maybe the encoder queue could have two lists: to-do and in-progress; the encoder thread atomically moves a frame from to-do to in-progress, but then how do you know when the in-progress ones are orphaned and need to be re-added to the main queue. You could make the writer return saying "no" if the queue is full (rather than blocking and waiting for the queue to empty) but that seems wasteful as then the frame would be re-encoded. --- src/lib/j2k_encoder.cc | 10 ++++++ src/lib/writer.cc | 21 ++++++++++++ src/lib/writer.h | 4 +++ test/j2k_encoder_test.cc | 83 ++++++++++++++++++++++++++++++++++++++++++++++++ test/wscript | 1 + 5 files changed, 119 insertions(+) create mode 100644 test/j2k_encoder_test.cc (limited to 'src') diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc index 7c9777c16..32d2fefc2 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -69,6 +69,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(); + boost::mutex::scoped_lock lm (_threads_mutex); terminate_threads (); } diff --git a/src/lib/writer.cc b/src/lib/writer.cc index fbe2d248d..7b9defd73 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -144,6 +144,10 @@ Writer::write (shared_ptr 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 */ @@ -377,6 +381,9 @@ try while (true) { boost::mutex::scoped_lock lock (_state_mutex); + if (_zombie) { + return; + } while (true) { @@ -1042,3 +1049,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 6e7f5ca90..f0f1fe69a 100644 --- a/src/lib/writer.h +++ b/src/lib/writer.h @@ -127,6 +127,8 @@ public: void set_encoder_threads (int threads); + void zombify(); + private: friend struct ::writer_disambiguate_font_ids1; friend struct ::writer_disambiguate_font_ids2; @@ -229,6 +231,8 @@ private: }; std::vector _hanging_texts; + + bool _zombie = false; }; 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 + + 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 . + +*/ + + +#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 +} +#include + + +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(AV_PIX_FMT_RGB24, dcp::Size(1998, 1080), Image::Alignment::PADDED); + auto image_proxy = make_shared(image); + encoder.encode( + std::make_shared( + image_proxy, + Crop(), + optional(), + dcp::Size(1998, 1080), + dcp::Size(1998, 1080), + Eyes::BOTH, + Part::WHOLE, + optional(), + VideoRange::VIDEO, + weak_ptr(), + optional(), + false + ), + {} + ); + } + + dcpomatic_sleep_seconds(10); +} + diff --git a/test/wscript b/test/wscript index bb66d1a5f..cd23badc0 100644 --- a/test/wscript +++ b/test/wscript @@ -112,6 +112,7 @@ def build(bld): interrupt_encoder_test.cc isdcf_name_test.cc j2k_bandwidth_test.cc + j2k_encoder_test.cc job_manager_test.cc kdm_cli_test.cc kdm_naming_test.cc -- cgit v1.2.3