From f190ce5af38c87b2586cff0b49d6be0bb38f5b0d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 00:07:32 +0100 Subject: Remove unnecessary new line. --- src/lib/encode_cli.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index a682fa4e3..6209d4c01 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -489,9 +489,9 @@ encode_cli(int argc, char* argv[], function out, functionname())); + out(fmt::format("Exporting {}\n", film->name())); } else { - out(fmt::format("\nMaking DCP for {}\n", film->name())); + out(fmt::format("Making DCP for {}\n", film->name())); } } -- cgit v1.2.3 From 3be10e6264ee8a817c4b6ed744a2a34cf99cecea Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 00:09:01 +0100 Subject: Add a basic test of the encode CLI. --- src/lib/encode_cli.cc | 3 ++ test/encode_cli_test.cc | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ test/wscript | 1 + 3 files changed, 92 insertions(+) create mode 100644 test/encode_cli_test.cc diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index 6209d4c01..d7c4541cb 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -271,6 +271,9 @@ encode_cli(int argc, char* argv[], function out, function export_filename; bool hints = false; + /* This makes it possible to call getopt several times in the same executable, for tests */ + optind = 0; + int option_index = 0; while (true) { static struct option long_options[] = { diff --git a/test/encode_cli_test.cc b/test/encode_cli_test.cc new file mode 100644 index 000000000..fc70c02d5 --- /dev/null +++ b/test/encode_cli_test.cc @@ -0,0 +1,88 @@ +/* + Copyright (C) 2025 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/content_factory.h" +#include "lib/encode_cli.h" +#include "lib/film.h" +#include "test.h" +#include +#include +#include +#include +#include + + +using std::cout; +using std::string; +using std::vector; +using boost::optional; + + +static +optional +run(vector const& args, vector& output) +{ + vector argv(args.size() + 1); + for (auto i = 0U; i < args.size(); ++i) { + argv[i] = const_cast(args[i].c_str()); + } + argv[args.size()] = nullptr; + + auto error = encode_cli(args.size(), argv.data(), [&output](string s) { output.push_back(s); }, []() { }); + if (error) { + std::cout << *error << "\n"; + } + + return error; +} + + +static +bool +find_in_order(vector const& output, vector const& check) +{ + BOOST_REQUIRE(!check.empty()); + + auto next = check.begin(); + for (auto line: output) { + if (line.find(*next) != string::npos) { + ++next; + if (next == check.end()) { + return true; + } + } + } + + return false; +} + + +BOOST_AUTO_TEST_CASE(basic_encode_cli_test) +{ + auto content = content_factory("test/data/flat_red.png"); + auto film = new_test_film("basic_encode_cli_test", content); + film->write_metadata(); + + vector output; + run({ "cli", "build/test/basic_encode_cli_test" }, output); + + BOOST_CHECK(find_in_order(output, { "Making DCP for", "Examining content", "OK", "Transcoding DCP", "OK" })); +} diff --git a/test/wscript b/test/wscript index 85bfccf56..714c237a4 100644 --- a/test/wscript +++ b/test/wscript @@ -85,6 +85,7 @@ def build(bld): email_test.cc empty_caption_test.cc empty_test.cc + encode_cli_test.cc encryption_test.cc file_extension_test.cc ffmpeg_audio_only_test.cc -- cgit v1.2.3 From 09471c55b24d5e69359675fe669397f031aa62d3 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 00:17:27 +0100 Subject: Preparation for offering command in the encode CLI. --- src/lib/encode_cli.cc | 10 +++++++++- test/encode_cli_test.cc | 13 +++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index d7c4541cb..c321d5014 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -66,7 +66,9 @@ using boost::optional; static void help(function out) { - out(fmt::format("Syntax: {} [OPTION] []\n", program_name)); + out(fmt::format("Syntax: {} [OPTION] [COMMAND] []\n", program_name)); + out("Commands:\n"); + out("make-dcp make DCP from the given film; default if no other command is specified\n"); out(variant::insert_dcpomatic(" -v, --version show %1 version\n")); out(" -h, --help show this help\n"); out(" -f, --flags show flags passed to C++ compiler on build\n"); @@ -270,6 +272,7 @@ encode_cli(int argc, char* argv[], function out, function export_format; optional export_filename; bool hints = false; + string command = "make-dcp"; /* This makes it possible to call getopt several times in the same executable, for tests */ optind = 0; @@ -360,6 +363,11 @@ encode_cli(int argc, char* argv[], function out, functionwrite_metadata(); + + vector output; + run({ "cli", "make-dcp", "build/test/basic_encode_cli_test" }, output); + + BOOST_CHECK(find_in_order(output, { "Making DCP for", "Examining content", "OK", "Transcoding DCP", "OK" })); +} -- cgit v1.2.3 From 9325b94009dfe3ed02559303e6803e235299e151 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 00:24:01 +0100 Subject: Make --list-servers into a command. --- src/lib/encode_cli.cc | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index c321d5014..fb307e019 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -68,7 +68,8 @@ help(function out) { out(fmt::format("Syntax: {} [OPTION] [COMMAND] []\n", program_name)); out("Commands:\n"); - out("make-dcp make DCP from the given film; default if no other command is specified\n"); + out("make-dcp make DCP from the given film; default if no other command is specified\n"); + out(variant::insert_dcpomatic("list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); out(variant::insert_dcpomatic(" -v, --version show %1 version\n")); out(" -h, --help show this help\n"); out(" -f, --flags show flags passed to C++ compiler on build\n"); @@ -79,6 +80,7 @@ help(function out) out(" -k, --keep-going keep running even when the job is complete\n"); out(" -s, --servers specify servers to use in a text file\n"); out(variant::insert_dcpomatic(" -l, --list-servers just display a list of encoding servers that %1 is configured to use; don't encode\n")); + out(" (deprecated - use the list-servers command instead)\n"); out(" -d, --dcp-path echo DCP's path to stdout on successful completion (implies -n)\n"); out(" -c, --config directory containing config.xml and cinemas.xml\n"); out(" --dump just dump a summary of the film's settings; don't encode\n"); @@ -363,9 +365,19 @@ encode_cli(int argc, char* argv[], function out, function commands = { + "make-dcp", + "list-servers" + }; + if (optind < argc - 1) { /* Command with a film specified afterwards */ command = argv[optind++]; + } else if (optind < argc) { + /* Look for a valid command, hoping that it's not the name of a film */ + if (std::find(commands.begin(), commands.end(), argv[optind]) != commands.end()) { + command = argv[optind++]; + } } if (config) { @@ -387,7 +399,7 @@ encode_cli(int argc, char* argv[], function out, functionset_servers(servers); } - if (list_servers_) { + if (command == "list-servers" || list_servers_) { list_servers(out); return {}; } -- cgit v1.2.3 From e662e5369a0aec59dbbd5763e999132c3fece565 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 00:28:29 +0100 Subject: Make --dump into a command. --- src/lib/encode_cli.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index fb307e019..4fd2feac5 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -70,6 +70,7 @@ help(function out) out("Commands:\n"); out("make-dcp make DCP from the given film; default if no other command is specified\n"); out(variant::insert_dcpomatic("list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); + out("dump show a summary of the film's settings\n"); out(variant::insert_dcpomatic(" -v, --version show %1 version\n")); out(" -h, --help show this help\n"); out(" -f, --flags show flags passed to C++ compiler on build\n"); @@ -84,6 +85,7 @@ help(function out) out(" -d, --dcp-path echo DCP's path to stdout on successful completion (implies -n)\n"); out(" -c, --config directory containing config.xml and cinemas.xml\n"); out(" --dump just dump a summary of the film's settings; don't encode\n"); + out(" (deprecated - use the dump command instead)\n"); out(" --no-check don't check project's content files for changes before making the DCP\n"); out(" --export-format export project to a file, rather than making a DCP: specify mov or mp4\n"); out(" --export-filename filename to export to with --export-format\n"); @@ -367,7 +369,8 @@ encode_cli(int argc, char* argv[], function out, function commands = { "make-dcp", - "list-servers" + "list-servers", + "dump" }; if (optind < argc - 1) { @@ -443,7 +446,7 @@ encode_cli(int argc, char* argv[], function out, function Date: Mon, 3 Mar 2025 00:42:15 +0100 Subject: Tweak help to suggest that the film is really a command parameter. --- src/lib/encode_cli.cc | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index 4fd2feac5..77970ea2b 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -66,12 +66,14 @@ using boost::optional; static void help(function out) { - out(fmt::format("Syntax: {} [OPTION] [COMMAND] []\n", program_name)); - out("Commands:\n"); - out("make-dcp make DCP from the given film; default if no other command is specified\n"); - out(variant::insert_dcpomatic("list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); - out("dump show a summary of the film's settings\n"); - out(variant::insert_dcpomatic(" -v, --version show %1 version\n")); + out(fmt::format("Syntax: {} [OPTION] [COMMAND] []\n", program_name)); + + out("\nCommands:\n\n"); + out(" make-dcp make DCP from the given film; default if no other command is specified\n"); + out(variant::insert_dcpomatic(" list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); + out(" dump show a summary of the film's settings\n"); + + out(variant::insert_dcpomatic("\n -v, --version show %1 version\n")); out(" -h, --help show this help\n"); out(" -f, --flags show flags passed to C++ compiler on build\n"); out(" -n, --no-progress do not print progress to stdout\n"); @@ -91,7 +93,6 @@ help(function out) out(" --export-filename filename to export to with --export-format\n"); out(" --hints analyze film for hints before encoding and abort if any are found\n"); out("\n"); - out(" is the film directory.\n"); } -- cgit v1.2.3 From f180d199dcb0217ea9750ab0a788217689733c41 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 01:11:03 +0100 Subject: Further help clarification. --- src/lib/encode_cli.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index 77970ea2b..67c82c141 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -69,11 +69,12 @@ help(function out) out(fmt::format("Syntax: {} [OPTION] [COMMAND] []\n", program_name)); out("\nCommands:\n\n"); - out(" make-dcp make DCP from the given film; default if no other command is specified\n"); - out(variant::insert_dcpomatic(" list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); - out(" dump show a summary of the film's settings\n"); + out(" make-dcp make DCP from the given film; default if no other command is specified\n"); + out(variant::insert_dcpomatic(" list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); + out(" dump show a summary of the film's settings\n"); - out(variant::insert_dcpomatic("\n -v, --version show %1 version\n")); + out("\nOptions:\n\n"); + out(variant::insert_dcpomatic(" -v, --version show %1 version\n")); out(" -h, --help show this help\n"); out(" -f, --flags show flags passed to C++ compiler on build\n"); out(" -n, --no-progress do not print progress to stdout\n"); @@ -92,6 +93,8 @@ help(function out) out(" --export-format export project to a file, rather than making a DCP: specify mov or mp4\n"); out(" --export-filename filename to export to with --export-format\n"); out(" --hints analyze film for hints before encoding and abort if any are found\n"); + out("\ne.g.\n"); + out(fmt::format("\n {} -t 4 make-dcp my_great_movie\n", program_name)); out("\n"); } -- cgit v1.2.3 From 3f2675aab119e55f958563e2fe6949192a2b976d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 20:51:48 +0100 Subject: Don't use an optional for the Grok config object. I'm not sure why I did it like this in the first place - perhaps so that if the API endpoint changes there aren't so many old values stuck in config files all over the place? Anyway, it seems cleaner to do it like this, as it's how we handle the other config. --- src/lib/config.cc | 6 ++---- src/lib/config.h | 4 ++-- src/lib/grok/context.h | 4 ++-- src/lib/grok_j2k_encoder_thread.cc | 2 +- src/lib/j2k_encoder.cc | 10 +++++----- src/lib/util.cc | 4 ++-- src/wx/grok/gpu_config_panel.h | 18 +++++++++--------- test/config_test.cc | 9 +++++++-- test/data | 2 +- 9 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/lib/config.cc b/src/lib/config.cc index 3227ea433..49c64e5b6 100644 --- a/src/lib/config.cc +++ b/src/lib/config.cc @@ -238,7 +238,7 @@ Config::set_defaults() set_cover_sheet_to_default(); #ifdef DCPOMATIC_GROK - _grok = boost::none; + _grok = {}; #endif _main_divider_sash_position = {}; @@ -1151,9 +1151,7 @@ Config::write_config() const cxml::add_text_child(root, "LayoutForShortScreen", _layout_for_short_screen ? "1" : "0"); #ifdef DCPOMATIC_GROK - if (_grok) { - _grok->as_xml(cxml::add_child(root, "Grok")); - } + _grok.as_xml(cxml::add_child(root, "Grok")); #endif _export.write(cxml::add_child(root, "Export")); diff --git a/src/lib/config.h b/src/lib/config.h index d9a95ebfd..b2a979ffa 100644 --- a/src/lib/config.h +++ b/src/lib/config.h @@ -666,7 +666,7 @@ public: std::string licence; }; - boost::optional grok() const { + Grok grok() const { return _grok; } #endif @@ -1495,7 +1495,7 @@ private: bool _layout_for_short_screen; #ifdef DCPOMATIC_GROK - boost::optional _grok; + Grok _grok; #endif ExportConfig _export; diff --git a/src/lib/grok/context.h b/src/lib/grok/context.h index 602c8b13f..b31867cf6 100644 --- a/src/lib/grok/context.h +++ b/src/lib/grok/context.h @@ -99,7 +99,7 @@ public: explicit GrokContext(DcpomaticContext* dcpomatic_context) : _dcpomatic_context(dcpomatic_context) { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); if (!grok.enable) { return; } @@ -216,7 +216,7 @@ public: auto s = dcpv.get_size(); _dcpomatic_context->set_dimensions(s.width, s.height); - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); if (!_messenger->launch_grok( _dcpomatic_context->location, _dcpomatic_context->width, diff --git a/src/lib/grok_j2k_encoder_thread.cc b/src/lib/grok_j2k_encoder_thread.cc index e6c256f11..d6825113c 100644 --- a/src/lib/grok_j2k_encoder_thread.cc +++ b/src/lib/grok_j2k_encoder_thread.cc @@ -62,7 +62,7 @@ try LOG_TIMING("encoder-pop thread=%1 frame=%2 eyes=%3", thread_id(), frame.index(), static_cast(frame.eyes())); - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); 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 309fce0b3..50452fbad 100644 --- a/src/lib/j2k_encoder.cc +++ b/src/lib/j2k_encoder.cc @@ -98,7 +98,7 @@ J2KEncoder::J2KEncoder(shared_ptr film, Writer& writer) #endif { #ifdef DCPOMATIC_GROK - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); _dcpomatic_context = new grk_plugin::DcpomaticContext(film, writer, _history, grok.binary_location); if (grok.enable) { _context = new grk_plugin::GrokContext(_dcpomatic_context); @@ -135,7 +135,7 @@ J2KEncoder::servers_list_changed() { auto config = Config::instance(); #ifdef DCPOMATIC_GROK - auto const grok_enable = config->grok().get_value_or({}).enable; + auto const grok_enable = config->grok().enable; #else auto const grok_enable = false; #endif @@ -162,7 +162,7 @@ void J2KEncoder::pause() { #ifdef DCPOMATIC_GROK - if (!Config::instance()->grok().get_value_or({}).enable) { + if (!Config::instance()->grok().enable) { return; } return; @@ -183,7 +183,7 @@ J2KEncoder::pause() void J2KEncoder::resume() { #ifdef DCPOMATIC_GROK - if (!Config::instance()->grok().get_value_or({}).enable) { + if (!Config::instance()->grok().enable) { return; } @@ -226,7 +226,7 @@ J2KEncoder::end() */ for (auto & i: _queue) { #ifdef DCPOMATIC_GROK - if (Config::instance()->grok().get_value_or({}).enable) { + if (Config::instance()->grok().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/util.cc b/src/lib/util.cc index df15e1abb..1e2f7d61f 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -1146,7 +1146,7 @@ setup_grok_library_path() } } auto const grok = Config::instance()->grok(); - if (!grok || grok->binary_location.empty()) { + if (grok.binary_location.empty()) { setenv("LD_LIRARY_PATH", old_path.c_str(), 1); return; } @@ -1155,7 +1155,7 @@ setup_grok_library_path() if (!new_path.empty()) { new_path += ":"; } - new_path += grok->binary_location.string(); + new_path += grok.binary_location.string(); setenv("LD_LIBRARY_PATH", new_path.c_str(), 1); } diff --git a/src/wx/grok/gpu_config_panel.h b/src/wx/grok/gpu_config_panel.h index df38f9373..3f4d0312c 100644 --- a/src/wx/grok/gpu_config_panel.h +++ b/src/wx/grok/gpu_config_panel.h @@ -63,7 +63,7 @@ public: void update() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); auto lister_binary = grok.binary_location / "gpu_lister"; auto lister_file = grok.binary_location / "gpus.txt"; if (boost::filesystem::exists(lister_binary)) { @@ -88,7 +88,7 @@ private: { auto selection = _combo_box->GetSelection(); if (selection != wxNOT_FOUND) { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); grok.selected = selection; Config::instance()->set_grok(grok); } @@ -155,7 +155,7 @@ private: void setup_sensitivity() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); _binary_location->Enable(grok.enable); _gpu_list_control->Enable(grok.enable); @@ -165,7 +165,7 @@ private: void config_changed() override { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); checked_set(_enable_gpu, grok.enable); _binary_location->SetPath(std_to_wx(grok.binary_location.string())); @@ -177,7 +177,7 @@ private: void enable_gpu_changed() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); grok.enable = _enable_gpu->GetValue(); Config::instance()->set_grok(grok); @@ -186,7 +186,7 @@ private: void binary_location_changed() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); grok.binary_location = wx_to_std(_binary_location->GetPath()); Config::instance()->set_grok(grok); @@ -195,20 +195,20 @@ private: void server_changed() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); grok.licence_server = wx_to_std(_server->GetValue()); Config::instance()->set_grok(grok); } void port_changed() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); Config::instance()->set_grok(grok); } void licence_changed() { - auto grok = Config::instance()->grok().get_value_or({}); + auto grok = Config::instance()->grok(); grok.licence = _licence->get(); Config::instance()->set_grok(grok); } diff --git a/test/config_test.cc b/test/config_test.cc index a9b95bedf..8fd19b693 100644 --- a/test/config_test.cc +++ b/test/config_test.cc @@ -182,9 +182,11 @@ BOOST_AUTO_TEST_CASE (config_upgrade_test1) check_xml (dir / "config.xml", "test/data/2.14.config.xml", {}); check_xml (dir / "cinemas.xml", "test/data/2.14.cinemas.xml", {}); -#ifdef DCPOMATIC_WINDOWS +#if defined(DCPOMATIC_WINDOWS) /* This file has the windows path for dkdm_recipients.xml (with backslashes) */ check_xml(dir / "2.18" / "config.xml", "test/data/2.18.config.windows.sqlite.xml", {}); +#elif defined(DCPOMATIC_GROK) + check_xml(dir / "2.18" / "config.xml", "test/data/2.18.config.sqlite.grok.xml", {}); #else check_xml(dir / "2.18" / "config.xml", "test/data/2.18.config.sqlite.xml", {}); #endif @@ -215,10 +217,13 @@ BOOST_AUTO_TEST_CASE (config_upgrade_test2) Config::instance()->write(); check_xml(dir / "cinemas.xml", "test/data/2.14.cinemas.xml", {}); -#ifdef DCPOMATIC_WINDOWS +#if defined(DCPOMATIC_WINDOWS) /* This file has the windows path for dkdm_recipients.xml (with backslashes) */ check_xml(dir / "2.18" / "config.xml", "test/data/2.18.config.windows.xml", {}); check_xml(dir / "config.xml", "test/data/2.16.config.windows.xml", {}); +#elif defined(DCPOMATIC_GROK) + check_xml(dir / "2.18" / "config.xml", "test/data/2.18.config.grok.xml", {}); + check_xml(dir / "config.xml", "test/data/2.16.config.xml", {}); #else check_xml(dir / "2.18" / "config.xml", "test/data/2.18.config.xml", {}); check_xml(dir / "config.xml", "test/data/2.16.config.xml", {}); diff --git a/test/data b/test/data index df601f158..7d2ed165d 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit df601f1580d852de043c2eca6acc4cfc9a2446b5 +Subproject commit 7d2ed165d8e65d92a02a20e22c1caedd15db82b3 -- cgit v1.2.3 From 27f9ce96db8b800b301ad3b21fb81355e982941a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 20:52:47 +0100 Subject: Allow configuration of Grok licence via dcpomatic2_cli (#2981). --- src/lib/encode_cli.cc | 38 ++++++++++++++++++++++++++++++++++++++ test/encode_cli_test.cc | 26 +++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index 67c82c141..7e9c25a6a 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -72,6 +72,10 @@ help(function out) out(" make-dcp make DCP from the given film; default if no other command is specified\n"); out(variant::insert_dcpomatic(" list-servers display a list of encoding servers that %1 can use (until Ctrl-C)\n")); out(" dump show a summary of the film's settings\n"); +#ifdef DCPOMATIC_GROK + out(" config-params list the parameters that can be set with `config`\n"); + out(" config set a DCP-o-matic configuration value\n"); +#endif out("\nOptions:\n\n"); out(variant::insert_dcpomatic(" -v, --version show %1 version\n")); @@ -95,6 +99,7 @@ help(function out) out(" --hints analyze film for hints before encoding and abort if any are found\n"); out("\ne.g.\n"); out(fmt::format("\n {} -t 4 make-dcp my_great_movie\n", program_name)); + out(fmt::format("\n {} config grok-licence 12345ABCD\n", program_name)); out("\n"); } @@ -374,7 +379,13 @@ encode_cli(int argc, char* argv[], function out, function commands = { "make-dcp", "list-servers", +#ifdef DCPOMATIC_GROK + "dump", + "config-params", + "config" +#else "dump" +#endif }; if (optind < argc - 1) { @@ -387,6 +398,33 @@ encode_cli(int argc, char* argv[], function out, functiongrok(); + if (parameter == "grok-licence") { + grok.licence = value; + Config::instance()->set_grok(grok); + Config::instance()->write(); + } else { + return fmt::format("Unrecognised configuration parameter `{}'", parameter); + } + } else { + return fmt::format("Missing configuration parameter: use {} config ", program_name); + } + return {}; + } +#endif + if (config) { State::override_path = *config; } diff --git a/test/encode_cli_test.cc b/test/encode_cli_test.cc index d857476a6..8ee656277 100644 --- a/test/encode_cli_test.cc +++ b/test/encode_cli_test.cc @@ -47,8 +47,11 @@ run(vector const& args, vector& output) argv[args.size()] = nullptr; auto error = encode_cli(args.size(), argv.data(), [&output](string s) { output.push_back(s); }, []() { }); + for (auto i: output) { + std::cout << "O:" << i; + } if (error) { - std::cout << *error << "\n"; + std::cout << "E:" << *error << "\n"; } return error; @@ -99,3 +102,24 @@ BOOST_AUTO_TEST_CASE(encode_cli_with_explicit_encode_command_test) BOOST_CHECK(find_in_order(output, { "Making DCP for", "Examining content", "OK", "Transcoding DCP", "OK" })); } + + +#ifdef DCPOMATIC_GROK +BOOST_AUTO_TEST_CASE(encode_cli_set_grok_licence) +{ + boost::filesystem::path config = "build/encode_cli_set_grok_licence"; + boost::filesystem::remove_all(config); + boost::filesystem::create_directories(config); + ConfigRestorer cr(config); + + vector output; + auto error = run({ "cli", "config", "grok-licence", "12345678ABC" }, output); + BOOST_CHECK(output.empty()); + BOOST_CHECK(!error); + + cxml::Document check("Config"); + check.read_file(config / "2.18" / "config.xml"); + BOOST_CHECK_EQUAL(check.node_child("Grok")->string_child("Licence"), "12345678ABC"); +} +#endif + -- cgit v1.2.3 From 641f5f2f61b77d79cb1e8c737cda0766b1ee2de8 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 22:10:56 +0100 Subject: Support some more grok configuration in the dcpomatic2_cli. --- src/lib/encode_cli.cc | 18 +++++++++++++++--- test/encode_cli_test.cc | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index 7e9c25a6a..f28b93421 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -402,7 +402,9 @@ encode_cli(int argc, char* argv[], function out, function out, functiongrok(); if (parameter == "grok-licence") { grok.licence = value; - Config::instance()->set_grok(grok); - Config::instance()->write(); + } else if (parameter == "grok-enable") { + if (value == "1") { + grok.enable = true; + } else if (value == "0") { + grok.enable = false; + } else { + return fmt::format("Invalid value {} for grok-enable (use 1 to enable, 0 to disable)", value); + } + } else if (parameter == "grok-binary-location") { + grok.binary_location = value; } else { return fmt::format("Unrecognised configuration parameter `{}'", parameter); } + Config::instance()->set_grok(grok); + Config::instance()->write(); } else { return fmt::format("Missing configuration parameter: use {} config ", program_name); } diff --git a/test/encode_cli_test.cc b/test/encode_cli_test.cc index 8ee656277..0a0a17e3a 100644 --- a/test/encode_cli_test.cc +++ b/test/encode_cli_test.cc @@ -121,5 +121,43 @@ BOOST_AUTO_TEST_CASE(encode_cli_set_grok_licence) check.read_file(config / "2.18" / "config.xml"); BOOST_CHECK_EQUAL(check.node_child("Grok")->string_child("Licence"), "12345678ABC"); } + + +BOOST_AUTO_TEST_CASE(encode_cli_enable_grok) +{ + boost::filesystem::path config = "build/encode_cli_enable_grok"; + boost::filesystem::remove_all(config); + boost::filesystem::create_directories(config); + ConfigRestorer cr(config); + + for (auto value: vector{ "1", "0"}) { + vector output; + auto error = run({ "cli", "config", "grok-enable", value }, output); + BOOST_CHECK(output.empty()); + BOOST_CHECK(!error); + + cxml::Document check("Config"); + check.read_file(config / "2.18" / "config.xml"); + BOOST_CHECK_EQUAL(check.node_child("Grok")->string_child("Enable"), value); + } +} + + +BOOST_AUTO_TEST_CASE(encode_cli_set_grok_binary_location) +{ + boost::filesystem::path config = "build/encode_cli_set_grok_binary_location"; + boost::filesystem::remove_all(config); + boost::filesystem::create_directories(config); + ConfigRestorer cr(config); + + vector output; + auto error = run({ "cli", "config", "grok-binary-location", "foo/bar/baz" }, output); + BOOST_CHECK(output.empty()); + BOOST_CHECK(!error); + + cxml::Document check("Config"); + check.read_file(config / "2.18" / "config.xml"); + BOOST_CHECK_EQUAL(check.node_child("Grok")->string_child("BinaryLocation"), "foo/bar/baz"); +} #endif -- cgit v1.2.3 From 0f2ed1b51af625767b9e0c41eb59651f2ceb1d88 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 22:40:13 +0100 Subject: Move get_gpu_names() to its own file. --- src/lib/grok/util.cc | 50 ++++++++++++++++++++++++++++++++++++++++++ src/lib/grok/util.h | 27 +++++++++++++++++++++++ src/lib/wscript | 2 +- src/wx/grok/gpu_config_panel.h | 22 +------------------ 4 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 src/lib/grok/util.cc create mode 100644 src/lib/grok/util.h diff --git a/src/lib/grok/util.cc b/src/lib/grok/util.cc new file mode 100644 index 000000000..e9b535df8 --- /dev/null +++ b/src/lib/grok/util.cc @@ -0,0 +1,50 @@ +/* + Copyright (C) 2023 Grok Image Compression Inc. + + 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 "util.h" + + +using std::string; +using std::vector; + + +vector +get_gpu_names(boost::filesystem::path binary, boost::filesystem::path filename) +{ + // Execute the GPU listing program and redirect its output to a file + if (std::system((binary.string() + " > " + filename.string()).c_str()) < 0) { + return {}; + } + + std::vector gpu_names; + std::ifstream file(filename.c_str()); + if (file.is_open()) + { + std::string line; + while (std::getline(file, line)) + gpu_names.push_back(line); + file.close(); + } + + return gpu_names; +} + + diff --git a/src/lib/grok/util.h b/src/lib/grok/util.h new file mode 100644 index 000000000..069a6cc08 --- /dev/null +++ b/src/lib/grok/util.h @@ -0,0 +1,27 @@ +/* + Copyright (C) 2023 Grok Image Compression Inc. + + 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 +#include +#include + + +extern std::vector get_gpu_names(boost::filesystem::path binary, boost::filesystem::path filename); diff --git a/src/lib/wscript b/src/lib/wscript index 79f0d563e..cae78fc4a 100644 --- a/src/lib/wscript +++ b/src/lib/wscript @@ -267,7 +267,7 @@ def build(bld): obj.uselib += ' POLKIT' if bld.env.ENABLE_GROK: - obj.source += ' grok_j2k_encoder_thread.cc' + obj.source += ' grok_j2k_encoder_thread.cc grok/util.cc' if bld.env.TARGET_WINDOWS_64 or bld.env.TARGET_WINDOWS_32: obj.uselib += ' WINSOCK2 DBGHELP SHLWAPI MSWSOCK BOOST_LOCALE SETUPAPI OLE32 UUID' diff --git a/src/wx/grok/gpu_config_panel.h b/src/wx/grok/gpu_config_panel.h index 3f4d0312c..6b8a4f150 100644 --- a/src/wx/grok/gpu_config_panel.h +++ b/src/wx/grok/gpu_config_panel.h @@ -22,30 +22,10 @@ #pragma once +#include "lib/grok/util.h" #include -static std::vector get_gpu_names(boost::filesystem::path binary, boost::filesystem::path filename) -{ - // Execute the GPU listing program and redirect its output to a file - if (std::system((binary.string() + " > " + filename.string()).c_str()) < 0) { - return {}; - } - - std::vector gpu_names; - std::ifstream file(filename.c_str()); - if (file.is_open()) - { - std::string line; - while (std::getline(file, line)) - gpu_names.push_back(line); - file.close(); - } - - return gpu_names; -} - - class GpuList : public wxPanel { public: -- cgit v1.2.3 From 2b3a495f31d3f9de57e6bbe2605843e6f1efbe3a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 22:47:42 +0100 Subject: Add a basic test for get_gpu_names(). --- test/gpu_lister | 6 ++++++ test/grok_util_test.cc | 36 ++++++++++++++++++++++++++++++++++++ test/wscript | 1 + 3 files changed, 43 insertions(+) create mode 100755 test/gpu_lister create mode 100644 test/grok_util_test.cc diff --git a/test/gpu_lister b/test/gpu_lister new file mode 100755 index 000000000..6dcb750de --- /dev/null +++ b/test/gpu_lister @@ -0,0 +1,6 @@ +#!/bin/bash + +echo "Foo bar baz" +echo "Spondoolix Mega Kompute 2000" +echo "Energy Sink-o-matic" + diff --git a/test/grok_util_test.cc b/test/grok_util_test.cc new file mode 100644 index 000000000..355e67219 --- /dev/null +++ b/test/grok_util_test.cc @@ -0,0 +1,36 @@ +/* + Copyright (C) 2025 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/gpu_util.h" +#include "test.h" +#include + + +#ifdef DCPOMATIC_GROK +BOOST_AUTO_TEST_CASE(get_gpu_names_test) +{ + auto names = get_gpu_names("test/gpu_lister", "build/test/gpus.txt"); + BOOST_REQUIRE_EQUAL(names.size(), 3U); + BOOST_CHECK_EQUAL(names[0], "Foo bar baz"); + BOOST_CHECK_EQUAL(names[1], "Spondoolix Mega Kompute 2000"); + BOOST_CHECK_EQUAL(names[2], "Energy Sink-o-matic"); +} +#endif diff --git a/test/wscript b/test/wscript index 714c237a4..7e53dfb02 100644 --- a/test/wscript +++ b/test/wscript @@ -110,6 +110,7 @@ def build(bld): font_id_allocator_test.cc frame_interval_checker_test.cc frame_rate_test.cc + grok_util_test.cc guess_crop_test.cc hints_test.cc image_content_fade_test.cc -- cgit v1.2.3 From eef48c36012180d20001def874864607e15968b8 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 23:16:09 +0100 Subject: Avoid temporary file for listing GPUs. --- src/lib/grok/util.cc | 34 +++++++++++++++++----------------- src/lib/grok/util.h | 2 +- src/wx/grok/gpu_config_panel.h | 3 +-- test/grok_util_test.cc | 4 ++-- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/lib/grok/util.cc b/src/lib/grok/util.cc index e9b535df8..8a6d2c4f9 100644 --- a/src/lib/grok/util.cc +++ b/src/lib/grok/util.cc @@ -20,6 +20,8 @@ #include "util.h" +#include +#include using std::string; @@ -27,24 +29,22 @@ using std::vector; vector -get_gpu_names(boost::filesystem::path binary, boost::filesystem::path filename) +get_gpu_names(boost::filesystem::path binary) { - // Execute the GPU listing program and redirect its output to a file - if (std::system((binary.string() + " > " + filename.string()).c_str()) < 0) { - return {}; - } - - std::vector gpu_names; - std::ifstream file(filename.c_str()); - if (file.is_open()) - { - std::string line; - while (std::getline(file, line)) - gpu_names.push_back(line); - file.close(); - } - - return gpu_names; + namespace bp = boost::process; + + bp::ipstream stream; + bp::child child(binary, bp::std_out > stream); + + string line; + vector gpu_names; + while (child.running() && std::getline(stream, line) && !line.empty()) { + gpu_names.push_back(line); + } + + child.wait(); + + return gpu_names; } diff --git a/src/lib/grok/util.h b/src/lib/grok/util.h index 069a6cc08..9996fa0e9 100644 --- a/src/lib/grok/util.h +++ b/src/lib/grok/util.h @@ -24,4 +24,4 @@ #include -extern std::vector get_gpu_names(boost::filesystem::path binary, boost::filesystem::path filename); +extern std::vector get_gpu_names(boost::filesystem::path binary); diff --git a/src/wx/grok/gpu_config_panel.h b/src/wx/grok/gpu_config_panel.h index 6b8a4f150..b61bccbde 100644 --- a/src/wx/grok/gpu_config_panel.h +++ b/src/wx/grok/gpu_config_panel.h @@ -45,9 +45,8 @@ public: { auto grok = Config::instance()->grok(); 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); + auto gpu_names = get_gpu_names(lister_binary); _combo_box->Clear(); for (auto const& name: gpu_names) { diff --git a/test/grok_util_test.cc b/test/grok_util_test.cc index 355e67219..8fba372a9 100644 --- a/test/grok_util_test.cc +++ b/test/grok_util_test.cc @@ -19,7 +19,7 @@ */ -#include "lib/gpu_util.h" +#include "lib/grok/util.h" #include "test.h" #include @@ -27,7 +27,7 @@ #ifdef DCPOMATIC_GROK BOOST_AUTO_TEST_CASE(get_gpu_names_test) { - auto names = get_gpu_names("test/gpu_lister", "build/test/gpus.txt"); + auto names = get_gpu_names("test/gpu_lister"); BOOST_REQUIRE_EQUAL(names.size(), 3U); BOOST_CHECK_EQUAL(names[0], "Foo bar baz"); BOOST_CHECK_EQUAL(names[1], "Spondoolix Mega Kompute 2000"); -- cgit v1.2.3 From 7d88bebfeb9931a39c6adc1e9dc9d6e1c0e3ea71 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 23:22:31 +0100 Subject: Get gpu_lister path from config. --- src/lib/grok/util.cc | 5 ++++- src/lib/grok/util.h | 2 +- src/wx/grok/gpu_config_panel.h | 12 +++--------- test/grok_util_test.cc | 9 ++++++++- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/lib/grok/util.cc b/src/lib/grok/util.cc index 8a6d2c4f9..3cbc55678 100644 --- a/src/lib/grok/util.cc +++ b/src/lib/grok/util.cc @@ -20,6 +20,7 @@ #include "util.h" +#include "../config.h" #include #include @@ -29,10 +30,12 @@ using std::vector; vector -get_gpu_names(boost::filesystem::path binary) +get_gpu_names() { namespace bp = boost::process; + auto binary = Config::instance()->grok().binary_location / "gpu_lister"; + bp::ipstream stream; bp::child child(binary, bp::std_out > stream); diff --git a/src/lib/grok/util.h b/src/lib/grok/util.h index 9996fa0e9..a78ecabca 100644 --- a/src/lib/grok/util.h +++ b/src/lib/grok/util.h @@ -24,4 +24,4 @@ #include -extern std::vector get_gpu_names(boost::filesystem::path binary); +extern std::vector get_gpu_names(); diff --git a/src/wx/grok/gpu_config_panel.h b/src/wx/grok/gpu_config_panel.h index b61bccbde..34bf38f12 100644 --- a/src/wx/grok/gpu_config_panel.h +++ b/src/wx/grok/gpu_config_panel.h @@ -43,15 +43,9 @@ public: void update() { - auto grok = Config::instance()->grok(); - auto lister_binary = grok.binary_location / "gpu_lister"; - if (boost::filesystem::exists(lister_binary)) { - auto gpu_names = get_gpu_names(lister_binary); - - _combo_box->Clear(); - for (auto const& name: gpu_names) { - _combo_box->Append(std_to_wx(name)); - } + _combo_box->Clear(); + for (auto const& name: get_gpu_names()) { + _combo_box->Append(std_to_wx(name)); } } diff --git a/test/grok_util_test.cc b/test/grok_util_test.cc index 8fba372a9..2a84fe2a4 100644 --- a/test/grok_util_test.cc +++ b/test/grok_util_test.cc @@ -19,6 +19,7 @@ */ +#include "lib/config.h" #include "lib/grok/util.h" #include "test.h" #include @@ -27,7 +28,13 @@ #ifdef DCPOMATIC_GROK BOOST_AUTO_TEST_CASE(get_gpu_names_test) { - auto names = get_gpu_names("test/gpu_lister"); + ConfigRestorer cr; + + Config::Grok grok; + grok.binary_location = "test"; + Config::instance()->set_grok(grok); + + auto names = get_gpu_names(); BOOST_REQUIRE_EQUAL(names.size(), 3U); BOOST_CHECK_EQUAL(names[0], "Foo bar baz"); BOOST_CHECK_EQUAL(names[1], "Spondoolix Mega Kompute 2000"); -- cgit v1.2.3 From 7ff15e15e08d0d8603633980bb128f447861c947 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 23:29:06 +0100 Subject: Add list-gpus command to dcpomatic2_cli. --- src/lib/encode_cli.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index f28b93421..7681252b2 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -30,6 +30,7 @@ #include "filter.h" #ifdef DCPOMATIC_GROK #include "grok/context.h" +#include "grok/util.h" #endif #include "hints.h" #include "job_manager.h" @@ -75,6 +76,7 @@ help(function out) #ifdef DCPOMATIC_GROK out(" config-params list the parameters that can be set with `config`\n"); out(" config set a DCP-o-matic configuration value\n"); + out(" list-gpus list available GPUs\n"); #endif out("\nOptions:\n\n"); @@ -382,7 +384,8 @@ encode_cli(int argc, char* argv[], function out, function out, function ", program_name); } return {}; + } else if (command == "list-gpus") { + int N = 0; + for (auto gpu: get_gpu_names()) { + out(fmt::format("{}: {}\n", N++, gpu)); + } + return {}; } #endif -- cgit v1.2.3 From 42b92cbe4518a170b217ab54a26b51f56246a50f Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 3 Mar 2025 23:40:33 +0100 Subject: Allow configuration of Grok GPU index. --- src/lib/encode_cli.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index 7681252b2..fa304b6e0 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -44,6 +44,7 @@ #include "version.h" #include "video_content.h" #include +#include #include #include #include @@ -408,6 +409,7 @@ encode_cli(int argc, char* argv[], function out, function out, function(value); } else { return fmt::format("Unrecognised configuration parameter `{}'", parameter); } -- cgit v1.2.3