Allow SMPTE/interop setting from template to work.
authorCarl Hetherington <cth@carlh.net>
Sat, 30 Sep 2023 10:28:05 +0000 (12:28 +0200)
committerCarl Hetherington <cth@carlh.net>
Sat, 30 Sep 2023 10:28:05 +0000 (12:28 +0200)
src/lib/create_cli.cc
src/lib/create_cli.h
test/create_cli_test.cc
test/data

index b0f7a843ebf4a3d2b9fa05274a0fa32755a8493f..41dd44d1a2dc1eae678a2e9a9fd90c27c466ffa4 100644 (file)
@@ -129,7 +129,7 @@ CreateCLI::CreateCLI (int argc, char* argv[])
 {
        string dcp_content_type_string = "TST";
        string container_ratio_string;
-       string standard_string = "SMPTE";
+       optional<string> standard_string;
        int dcp_frame_rate_int = 0;
        string template_name_string;
        int j2k_bandwidth_int = 0;
@@ -287,13 +287,15 @@ CreateCLI::CreateCLI (int argc, char* argv[])
                }
        }
 
-       if (standard_string != "SMPTE" && standard_string != "interop") {
-               error = String::compose("%1: standard must be SMPTE or interop", argv[0]);
-               return;
-       }
-
-       if (standard_string == "interop") {
-               _standard = dcp::Standard::INTEROP;
+       if (standard_string) {
+               if (*standard_string == "interop") {
+                       _standard = dcp::Standard::INTEROP;
+               } else if (*standard_string == "SMPTE") {
+                       _standard = dcp::Standard::SMPTE;
+               } else {
+                       error = String::compose("%1: standard must be SMPTE or interop", argv[0]);
+                       return;
+               }
        }
 
        if (_twod && _threed) {
@@ -328,6 +330,13 @@ CreateCLI::make_film() const
        dcpomatic_log->set_types(Config::instance()->log_types());
        if (_template_name) {
                film->use_template(_template_name.get());
+       } else {
+               /* No template: apply our own CLI tool defaults to override the ones in Config.
+                * Maybe one day there will be no defaults in Config any more (as they'll be in
+                * a default template) and we can decide whether to use the default template
+                * or not.
+                */
+               film->set_interop(false);
        }
        film->set_name(_name);
 
@@ -335,7 +344,9 @@ CreateCLI::make_film() const
                film->set_container(_container_ratio);
        }
        film->set_dcp_content_type(_dcp_content_type);
-       film->set_interop(_standard == dcp::Standard::INTEROP);
+       if (_standard) {
+               film->set_interop(*_standard == dcp::Standard::INTEROP);
+       }
        film->set_use_isdcf_name(!_no_use_isdcf_name);
        if (_no_encrypt) {
                film->set_encrypted(false);
index ee15fb0b8565139c42d00a419ccb99cc31935103..782aaf97478d10e01063c0373229657af1fd0583 100644 (file)
@@ -68,7 +68,7 @@ private:
        bool _twod = false;
        bool _threed = false;
        DCPContentType const* _dcp_content_type = nullptr;
-       dcp::Standard _standard = dcp::Standard::SMPTE;
+       boost::optional<dcp::Standard> _standard;
        bool _no_use_isdcf_name = false;
        bool _twok = false;
        bool _fourk = false;
index 3ebdcb81b7b88f41243d946a501dd59fff11f454..0359b2f358fb9183982b8c5279dde0d80d3433cd 100644 (file)
@@ -102,11 +102,13 @@ BOOST_AUTO_TEST_CASE (create_cli_test)
 
        cc = run ("dcpomatic2_create x --standard SMPTE");
        BOOST_CHECK (!cc.error);
-       BOOST_CHECK_EQUAL(cc._standard, dcp::Standard::SMPTE);
+       BOOST_REQUIRE(cc._standard);
+       BOOST_CHECK_EQUAL(*cc._standard, dcp::Standard::SMPTE);
 
        cc = run ("dcpomatic2_create x --standard interop");
        BOOST_CHECK (!cc.error);
-       BOOST_CHECK_EQUAL(cc._standard, dcp::Standard::INTEROP);
+       BOOST_REQUIRE(cc._standard);
+       BOOST_CHECK_EQUAL(*cc._standard, dcp::Standard::INTEROP);
 
        cc = run ("dcpomatic2_create x --standard SMPTEX");
        BOOST_CHECK (cc.error);
@@ -265,5 +267,42 @@ BOOST_AUTO_TEST_CASE(create_cli_template_test)
        cc = run("dcpomatic2_create test/data/flat_red.png --template encrypted --no-encrypt");
        film = cc.make_film();
        BOOST_CHECK(!film->encrypted());
+
+       cc = run("dcpomatic2_create test/data/flat_red.png");
+       film = cc.make_film();
+       BOOST_CHECK(!film->interop());
+
+       cc = run("dcpomatic2_create test/data/flat_red.png --template interop");
+       film = cc.make_film();
+       BOOST_CHECK(film->interop());
+
+       cc = run("dcpomatic2_create test/data/flat_red.png --template interop --standard SMPTE");
+       film = cc.make_film();
+       BOOST_CHECK(!film->interop());
+
+       cc = run("dcpomatic2_create test/data/flat_red.png --template smpte");
+       film = cc.make_film();
+       BOOST_CHECK(!film->interop());
+
+       cc = run("dcpomatic2_create test/data/flat_red.png --template smpte --standard interop");
+       film = cc.make_film();
+       BOOST_CHECK(film->interop());
+}
+
+
+BOOST_AUTO_TEST_CASE(create_cli_defaults_test)
+{
+       ConfigRestorer cr;
+
+       /* I think on balance dcpomatic2_create should not use the defaults from Config;
+        * it seems a bit surprising that settings from a GUI tool can change the behaviour of
+        * a CLI tool, and at some point we're probably going to remove all the default config
+        * options from the main DoM anyway (in favour of a default template).
+        */
+       Config::instance()->set_default_interop(true);
+       auto cc = run("dcpomatic2_create test/data/flat_red.png");
+       auto film = cc.make_film();
+       BOOST_CHECK(!film->interop());
+
 }
 
index 31f3a1e8d0217e065b9a2e6efbebe194f4dbae78..a8cdef5f99fcf9fc6506d0f2e6471ab064ad205c 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit 31f3a1e8d0217e065b9a2e6efbebe194f4dbae78
+Subproject commit a8cdef5f99fcf9fc6506d0f2e6471ab064ad205c