From 2ed3dadd6287859551fcbeaf85e09b0b3f1e8ff5 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 24 Sep 2023 00:34:15 +0200 Subject: Rearrange encoder threading. Soon we'll add a new encoder type, and the existing structure was already creaking a bit at the seams while handling local and remote encodes. Here we split out an encoder thread and introduce the concept of a "sync" thread (which blocks while the encoding is happening). Later we'll have another type which submits the encode request to a GPU and receives the reply back later. --- src/lib/grok_j2k_encoder_thread.cc | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 src/lib/grok_j2k_encoder_thread.cc (limited to 'src/lib/grok_j2k_encoder_thread.cc') diff --git a/src/lib/grok_j2k_encoder_thread.cc b/src/lib/grok_j2k_encoder_thread.cc new file mode 100644 index 000000000..8d0dee5a9 --- /dev/null +++ b/src/lib/grok_j2k_encoder_thread.cc @@ -0,0 +1,46 @@ +#include "cross.h" +#include "dcpomatic_log.h" +#include "dcp_video.h" +#include "grok_j2k_encoder_thread.h" +#include "j2k_encoder.h" +#include "scope_guard.h" +#include "util.h" + +#include "i18n.h" + + +using std::make_shared; +using std::shared_ptr; + + +GrokJ2KEncoderThread::GrokJ2KEncoderThread(J2KEncoder& encoder, grk_plugin::GrokContext* context) + : J2KEncoderThread(encoder) + , _context(context) +{ + +} + + +void +GrokJ2KEncoderThread::run() +try +{ + while (true) + { + LOG_TIMING("encoder-sleep thread=%1", thread_id()); + auto frame = _encoder.pop(); + + ScopeGuard frame_guard([this, &frame]() { + _encoder.retry(frame); + }); + + LOG_TIMING("encoder-pop thread=%1 frame=%2 eyes=%3", thread_id(), frame.index(), static_cast(frame.eyes())); + + if (_context->launch(frame, Config::instance()->selected_gpu()) && _context->scheduleCompress(frame)) { + frame_guard.cancel(); + } + } +} catch (boost::thread_interrupted& e) { +} catch (...) { + store_current(); +} -- cgit v1.2.3 From f0a03cacb2f2214b7452e42f2dec465888422e6f Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 26 Sep 2023 13:32:00 +0200 Subject: Forward-declare grk_plugin stuff. --- src/lib/grok_j2k_encoder_thread.cc | 2 ++ src/lib/j2k_encoder.cc | 1 + src/lib/j2k_encoder.h | 8 +++++--- 3 files changed, 8 insertions(+), 3 deletions(-) (limited to 'src/lib/grok_j2k_encoder_thread.cc') diff --git a/src/lib/grok_j2k_encoder_thread.cc b/src/lib/grok_j2k_encoder_thread.cc index 8d0dee5a9..d5256619d 100644 --- a/src/lib/grok_j2k_encoder_thread.cc +++ b/src/lib/grok_j2k_encoder_thread.cc @@ -1,6 +1,8 @@ +#include "config.h" #include "cross.h" #include "dcpomatic_log.h" #include "dcp_video.h" +#include "grok/context.h" #include "grok_j2k_encoder_thread.h" #include "j2k_encoder.h" #include "scope_guard.h" diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc index fe63deacd..ad04ac0fa 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -34,6 +34,7 @@ #include "film.h" #include "cpu_j2k_encoder_thread.h" #ifdef DCPOMATIC_GROK +#include "grok/context.h" #include "grok_j2k_encoder_thread.h" #endif #include "remote_j2k_encoder_thread.h" diff --git a/src/lib/j2k_encoder.h b/src/lib/j2k_encoder.h index 913beb5a9..0dbe654a4 100644 --- a/src/lib/j2k_encoder.h +++ b/src/lib/j2k_encoder.h @@ -32,9 +32,6 @@ #include "enum_indexed_vector.h" #include "event_history.h" #include "exception_store.h" -#ifdef DCPOMATIC_GROK -#include "grok/context.h" -#endif #include "j2k_encoder_thread.h" #include "writer.h" #include @@ -52,6 +49,11 @@ class Film; class Job; class PlayerVideo; +namespace grk_plugin { + struct DcpomaticContext; + struct GrokContext; +} + struct local_threads_created_and_destroyed; struct remote_threads_created_and_destroyed; struct frames_not_lost_when_threads_disappear; -- cgit v1.2.3 From ed89aa948cb7426d066213f53ab69ec9d9f5b0ce Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 4 Oct 2023 15:36:55 +0200 Subject: Log failure to schedule a frame with grok. --- src/lib/grok_j2k_encoder_thread.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'src/lib/grok_j2k_encoder_thread.cc') diff --git a/src/lib/grok_j2k_encoder_thread.cc b/src/lib/grok_j2k_encoder_thread.cc index d5256619d..79b80f694 100644 --- a/src/lib/grok_j2k_encoder_thread.cc +++ b/src/lib/grok_j2k_encoder_thread.cc @@ -33,6 +33,7 @@ try auto frame = _encoder.pop(); ScopeGuard frame_guard([this, &frame]() { + LOG_ERROR("Failed to schedule encode of %1 using grok", frame.index()); _encoder.retry(frame); }); -- cgit v1.2.3 From 617b8cd303b4a96621a207724c55bed1d749b10c Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 6 Oct 2023 22:42:44 +0200 Subject: Clean up grok's presence in the config file and make sure it's optional. It should be allowed to not have any grok stuff in the config file, and we should generally call it grok rather than GPU in case other non-grok GPU stuff arrives in the future. --- src/lib/config.cc | 66 ++++++++++++++++++++++++---------- src/lib/config.h | 74 +++++++++++++------------------------- src/lib/grok/context.h | 11 +++--- src/lib/grok_j2k_encoder_thread.cc | 4 ++- src/lib/j2k_encoder.cc | 31 ++++++++++------ src/lib/j2k_encoder.h | 4 +-- src/wx/grok/gpu_config_panel.h | 69 ++++++++++++++++++++--------------- 7 files changed, 145 insertions(+), 114 deletions(-) (limited to 'src/lib/grok_j2k_encoder_thread.cc') diff --git a/src/lib/config.cc b/src/lib/config.cc index 3a7442c39..938be090c 100644 --- a/src/lib/config.cc +++ b/src/lib/config.cc @@ -212,12 +212,9 @@ Config::set_defaults () set_notification_email_to_default (); set_cover_sheet_to_default (); - _gpu_binary_location = ""; - _enable_gpu = false; - _selected_gpu = 0; - _gpu_license_server = ""; - _gpu_license_port = 5000; - _gpu_license = ""; +#ifdef DCPOMATIC_GROK + _grok = boost::none; +#endif _main_divider_sash_position = {}; _main_content_divider_sash_position = {}; @@ -640,12 +637,11 @@ try _allow_smpte_bv20 = f.optional_bool_child("AllowSMPTEBv20").get_value_or(false); _isdcf_name_part_length = f.optional_number_child("ISDCFNamePartLength").get_value_or(14); - _enable_gpu = f.optional_bool_child("EnableGPU").get_value_or(false); - _gpu_binary_location = f.string_child("GPUBinaryLocation"); - _selected_gpu = f.number_child("SelectedGPU"); - _gpu_license_server = f.string_child ("GPULicenseServer"); - _gpu_license_port = f.number_child ("GPULicensePort"); - _gpu_license = f.string_child("GPULicense"); +#ifdef DCPOMATIC_GROK + if (auto grok = f.optional_node_child("Grok")) { + _grok = Grok(grok); + } +#endif _export.read(f.optional_node_child("Export")); } @@ -1133,12 +1129,11 @@ Config::write_config () const /* [XML] ISDCFNamePartLength Maximum length of the "name" part of an ISDCF name, which should be 14 according to the standard */ root->add_child("ISDCFNamePartLength")->add_child_text(raw_convert(_isdcf_name_part_length)); - root->add_child("GPUBinaryLocation")->add_child_text (_gpu_binary_location.string()); - root->add_child("EnableGPU")->add_child_text ((_enable_gpu ? "1" : "0")); - root->add_child("SelectedGPU")->add_child_text (raw_convert (_selected_gpu)); - root->add_child("GPULicenseServer")->add_child_text (_gpu_license_server); - root->add_child("GPULicensePort")->add_child_text (raw_convert (_gpu_license_port)); - root->add_child("GPULicense")->add_child_text (_gpu_license); +#ifdef DCPOMATIC_GROK + if (_grok) { + _grok->as_xml(root->add_child("Grok")); + } +#endif _export.write(root->add_child("Export")); @@ -1660,3 +1655,38 @@ Config::initial_path(string id) const return iter->second; } + +#ifdef DCPOMATIC_GROK + +Config::Grok::Grok(cxml::ConstNodePtr node) + : enable(node->bool_child("Enable")) + , binary_location(node->string_child("BinaryLocation")) + , selected(node->number_child("Selected")) + , licence_server(node->string_child("LicenceServer")) + , licence_port(node->number_child("LicencePort")) + , licence(node->string_child("Licence")) +{ + +} + + +void +Config::Grok::as_xml(xmlpp::Element* node) const +{ + node->add_child("BinaryLocation")->add_child_text(binary_location.string()); + node->add_child("Enable")->add_child_text((enable ? "1" : "0")); + node->add_child("Selected")->add_child_text(raw_convert(selected)); + node->add_child("LicenceServer")->add_child_text(licence_server); + node->add_child("LicencePort")->add_child_text(raw_convert(licence_port)); + node->add_child("Licence")->add_child_text(licence); +} + + +void +Config::set_grok(Grok const& grok) +{ + _grok = grok; + changed(OTHER); +} + +#endif diff --git a/src/lib/config.h b/src/lib/config.h index 9aba9e9d2..eaf85451d 100644 --- a/src/lib/config.h +++ b/src/lib/config.h @@ -618,27 +618,27 @@ public: return _allow_smpte_bv20; } - boost::filesystem::path gpu_binary_location() const { - return _gpu_binary_location; - } - - bool enable_gpu () const { - return _enable_gpu; - } - - int selected_gpu () const { - return _selected_gpu; - } - std::string gpu_license_server () const { - return _gpu_license_server; - } +#ifdef DCPOMATIC_GROK + class Grok + { + public: + Grok() = default; + Grok(cxml::ConstNodePtr node); + + void as_xml(xmlpp::Element* node) const; + + bool enable = false; + boost::filesystem::path binary_location; + int selected = 0; + std::string licence_server; + int licence_port = 5000; + std::string licence; + }; - int gpu_license_port () const { - return _gpu_license_port; - } - std::string gpu_license () const { - return _gpu_license; + boost::optional grok() const { + return _grok; } +#endif int isdcf_name_part_length() const { return _isdcf_name_part_length; @@ -1221,29 +1221,9 @@ public: maybe_set(_allow_smpte_bv20, allow, ALLOW_SMPTE_BV20); } - void set_gpu_binary_location(boost::filesystem::path location) { - maybe_set (_gpu_binary_location, location); - } - - void set_enable_gpu (bool enable) { - maybe_set (_enable_gpu, enable); - } - - void set_selected_gpu (int selected) { - maybe_set (_selected_gpu, selected); - } - - void set_gpu_license_server (std::string s) { - maybe_set (_gpu_license_server, s); - } - - void set_gpu_license_port (int p) { - maybe_set (_gpu_license_port, p); - } - - void set_gpu_license (std::string p) { - maybe_set (_gpu_license, p); - } +#ifdef DCPOMATIC_GROK + void set_grok(Grok const& grok); +#endif void set_isdcf_name_part_length(int length) { maybe_set(_isdcf_name_part_length, length, ISDCF_NAME_PART_LENGTH); @@ -1490,13 +1470,9 @@ private: bool _allow_smpte_bv20; int _isdcf_name_part_length; - /* GPU */ - bool _enable_gpu; - boost::filesystem::path _gpu_binary_location; - int _selected_gpu; - std::string _gpu_license_server; - int _gpu_license_port; - std::string _gpu_license; +#ifdef DCPOMATIC_GROK + boost::optional _grok; +#endif ExportConfig _export; diff --git a/src/lib/grok/context.h b/src/lib/grok/context.h index d27caa5f6..521faae8d 100644 --- a/src/lib/grok/context.h +++ b/src/lib/grok/context.h @@ -112,7 +112,8 @@ public: explicit GrokContext(DcpomaticContext* dcpomatic_context) : _dcpomatic_context(dcpomatic_context) { - if (!Config::instance()->enable_gpu()) { + auto grok = Config::instance()->grok().get_value_or({}); + if (!grok.enable) { return; } @@ -214,7 +215,7 @@ public: auto s = dcpv.get_size(); _dcpomatic_context->set_dimensions(s.width, s.height); - auto config = Config::instance(); + auto grok = Config::instance()->grok().get_value_or({}); if (!_messenger->launchGrok( _dcpomatic_context->location, _dcpomatic_context->width, @@ -226,9 +227,9 @@ public: _dcpomatic_context->film->resolution() == Resolution::FOUR_K, _dcpomatic_context->film->video_frame_rate(), _dcpomatic_context->film->j2k_bandwidth(), - config->gpu_license_server(), - config->gpu_license_port(), - config->gpu_license())) { + grok.licence_server, + grok.licence_port, + grok.licence)) { _launch_failed = true; return false; } diff --git a/src/lib/grok_j2k_encoder_thread.cc b/src/lib/grok_j2k_encoder_thread.cc index 79b80f694..79fb1bbae 100644 --- a/src/lib/grok_j2k_encoder_thread.cc +++ b/src/lib/grok_j2k_encoder_thread.cc @@ -39,7 +39,9 @@ try LOG_TIMING("encoder-pop thread=%1 frame=%2 eyes=%3", thread_id(), frame.index(), static_cast(frame.eyes())); - if (_context->launch(frame, Config::instance()->selected_gpu()) && _context->scheduleCompress(frame)) { + auto grok = Config::instance()->grok().get_value_or({}); + + if (_context->launch(frame, grok.selected) && _context->scheduleCompress(frame)) { frame_guard.cancel(); } } diff --git a/src/lib/j2k_encoder.cc b/src/lib/j2k_encoder.cc index 3bb7ccaed..e68402483 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -95,11 +95,14 @@ J2KEncoder::J2KEncoder(shared_ptr film, Writer& writer) : _film (film) , _history (200) , _writer (writer) +{ #ifdef DCPOMATIC_GROK - , _dcpomatic_context(new grk_plugin::DcpomaticContext(film, writer, _history, Config::instance()->gpu_binary_location())) - , _context(Config::instance()->enable_gpu() ? new grk_plugin::GrokContext(_dcpomatic_context) : nullptr) + auto grok = Config::instance()->grok().get_value_or({}); + _dcpomatic_context = new grk_plugin::DcpomaticContext(film, writer, _history, grok.binary_location); + if (grok.enable) { + _context = new grk_plugin::GrokContext(_dcpomatic_context); + } #endif -{ servers_list_changed (); } @@ -121,9 +124,14 @@ void J2KEncoder::servers_list_changed() { auto config = Config::instance(); +#ifdef DCPOMATIC_GROK + auto const grok_enable = config->grok().get_value_or({}).enable; +#else + auto const grok_enable = false; +#endif - auto const cpu = (config->enable_gpu() || config->only_servers_encode()) ? 0 : config->master_encoding_threads(); - auto const gpu = config->enable_gpu() ? config->master_encoding_threads() : 0; + auto const cpu = (grok_enable || config->only_servers_encode()) ? 0 : config->master_encoding_threads(); + auto const gpu = grok_enable ? config->master_encoding_threads() : 0; remake_threads(cpu, gpu, EncodeServerFinder::instance()->servers()); } @@ -141,16 +149,17 @@ J2KEncoder::begin () void J2KEncoder::pause() { - if (!Config::instance()->enable_gpu()) { +#ifdef DCPOMATIC_GROK + if (!Config::instance()->grok().get_value_or({}).enable) { return; } + return; terminate_threads (); /* Something might have been thrown during terminate_threads */ rethrow (); -#ifdef DCPOMATIC_GROK delete _context; _context = nullptr; #endif @@ -159,14 +168,14 @@ J2KEncoder::pause() void J2KEncoder::resume() { - if (!Config::instance()->enable_gpu()) { +#ifdef DCPOMATIC_GROK + if (!Config::instance()->grok().get_value_or({}).enable) { return; } -#ifdef DCPOMATIC_GROK _context = new grk_plugin::GrokContext(_dcpomatic_context); -#endif servers_list_changed(); +#endif } @@ -203,7 +212,7 @@ J2KEncoder::end() */ for (auto & i: _queue) { #ifdef DCPOMATIC_GROK - if (Config::instance()->enable_gpu ()) { + if (Config::instance()->grok().get_value_or({}).enable) { if (!_context->scheduleCompress(i)){ LOG_GENERAL (N_("[%1] J2KEncoder thread pushes frame %2 back onto queue after failure"), thread_id(), i.index()); // handle error diff --git a/src/lib/j2k_encoder.h b/src/lib/j2k_encoder.h index 0dbe654a4..6bfbaea49 100644 --- a/src/lib/j2k_encoder.h +++ b/src/lib/j2k_encoder.h @@ -127,8 +127,8 @@ private: boost::signals2::scoped_connection _server_found_connection; #ifdef DCPOMATIC_GROK - grk_plugin::DcpomaticContext* _dcpomatic_context; - grk_plugin::GrokContext *_context; + grk_plugin::DcpomaticContext* _dcpomatic_context = nullptr; + grk_plugin::GrokContext *_context = nullptr; #endif bool _ending = false; diff --git a/src/wx/grok/gpu_config_panel.h b/src/wx/grok/gpu_config_panel.h index 40e999181..cbf037592 100644 --- a/src/wx/grok/gpu_config_panel.h +++ b/src/wx/grok/gpu_config_panel.h @@ -59,9 +59,9 @@ public: void update() { - auto cfg = Config::instance(); - auto lister_binary = cfg->gpu_binary_location() / "gpu_lister"; - auto lister_file = cfg->gpu_binary_location () / "gpus.txt"; + auto grok = Config::instance()->grok().get_value_or({}); + auto lister_binary = grok.binary_location / "gpu_lister"; + auto lister_file = grok.binary_location / "gpus.txt"; if (boost::filesystem::exists(lister_binary)) { auto gpu_names = get_gpu_names(lister_binary, lister_file); @@ -84,7 +84,9 @@ private: { auto selection = _combo_box->GetSelection(); if (selection != wxNOT_FOUND) { - Config::instance()->set_selected_gpu(selection); + auto grok = Config::instance()->grok().get_value_or({}); + grok.selected = selection; + Config::instance()->set_grok(grok); } } @@ -141,68 +143,79 @@ private: table->Add(_port); add_label_to_sizer(table, _panel, _("License"), true, 0, wxLEFT | wxRIGHT | wxALIGN_CENTRE_VERTICAL); - _license = new PasswordEntry(_panel); - table->Add(_license->get_panel(), 1, wxEXPAND | wxALL); + _licence = new PasswordEntry(_panel); + table->Add(_licence->get_panel(), 1, wxEXPAND | wxALL); _enable_gpu->bind(&GPUPage::enable_gpu_changed, this); _binary_location->Bind(wxEVT_DIRPICKER_CHANGED, boost::bind (&GPUPage::binary_location_changed, this)); _server->Bind(wxEVT_TEXT, boost::bind(&GPUPage::server_changed, this)); _port->Bind(wxEVT_SPINCTRL, boost::bind(&GPUPage::port_changed, this)); - _license->Changed.connect(boost::bind(&GPUPage::license_changed, this)); + _licence->Changed.connect(boost::bind(&GPUPage::licence_changed, this)); setup_sensitivity(); } void setup_sensitivity() { - auto config = Config::instance(); + auto grok = Config::instance()->grok().get_value_or({}); - _binary_location->Enable(config->enable_gpu()); - _gpu_list_control->Enable(config->enable_gpu()); - _server->Enable(config->enable_gpu()); - _port->Enable(config->enable_gpu()); - _license->get_panel()->Enable(config->enable_gpu()); + _binary_location->Enable(grok.enable); + _gpu_list_control->Enable(grok.enable); + _server->Enable(grok.enable); + _port->Enable(grok.enable); + _licence->get_panel()->Enable(grok.enable); } void config_changed() override { - auto config = Config::instance(); + auto grok = Config::instance()->grok().get_value_or({}); - checked_set(_enable_gpu, config->enable_gpu()); - _binary_location->SetPath(std_to_wx(config->gpu_binary_location().string())); + checked_set(_enable_gpu, grok.enable); + _binary_location->SetPath(std_to_wx(grok.binary_location.string())); _gpu_list_control->update(); - _gpu_list_control->set_selection(config->selected_gpu()); - checked_set(_server, config->gpu_license_server()); - checked_set(_port, config->gpu_license_port()); - checked_set(_license, config->gpu_license()); + _gpu_list_control->set_selection(grok.selected); + checked_set(_server, grok.licence_server); + checked_set(_port, grok.licence_port); + checked_set(_licence, grok.licence); } void enable_gpu_changed() { - auto config = Config::instance(); - config->set_enable_gpu(_enable_gpu->GetValue()); + auto grok = Config::instance()->grok().get_value_or({}); + grok.enable = _enable_gpu->GetValue(); + Config::instance()->set_grok(grok); + setup_sensitivity(); } void binary_location_changed() { - Config::instance()->set_gpu_binary_location(wx_to_std(_binary_location->GetPath())); + auto grok = Config::instance()->grok().get_value_or({}); + grok.binary_location = wx_to_std(_binary_location->GetPath()); + Config::instance()->set_grok(grok); + _gpu_list_control->update(); } void server_changed() { - Config::instance()->set_gpu_license_server(wx_to_std(_server->GetValue())); + auto grok = Config::instance()->grok().get_value_or({}); + grok.licence_server = wx_to_std(_server->GetValue()); + Config::instance()->set_grok(grok); } void port_changed() { - Config::instance()->set_gpu_license_port(_port->GetValue()); + auto grok = Config::instance()->grok().get_value_or({}); + grok.licence_port = _port->GetValue(); + Config::instance()->set_grok(grok); } - void license_changed() + void licence_changed() { - Config::instance()->set_gpu_license(_license->get()); + auto grok = Config::instance()->grok().get_value_or({}); + grok.licence = wx_to_std(_licence->get()); + Config::instance()->set_grok(grok); } CheckBox* _enable_gpu = nullptr; @@ -210,5 +223,5 @@ private: GpuList* _gpu_list_control = nullptr; wxTextCtrl* _server = nullptr; wxSpinCtrl* _port = nullptr; - PasswordEntry* _license = nullptr; + PasswordEntry* _licence = nullptr; }; -- cgit v1.2.3