From ec06811bae7ed4fc6bd80c3154fd473028ee8e13 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 1 Jun 2025 01:39:04 +0200 Subject: Pass Ratio around as an object rather than a pointer. --- src/lib/create_cli.cc | 4 +-- src/lib/create_cli.h | 4 +-- src/lib/encode_cli.cc | 2 +- src/lib/film.cc | 19 +++++--------- src/lib/film.h | 8 +++--- src/lib/hints.cc | 8 +++--- src/lib/make_dcp.cc | 4 --- src/lib/ratio.cc | 54 ++++++++++++++++++++++++++-------------- src/lib/ratio.h | 15 +++++++---- src/lib/video_content.cc | 13 ++++++---- src/lib/writer.cc | 4 +-- src/tools/dcpomatic_player.cc | 2 +- src/wx/dcp_panel.cc | 6 ++--- src/wx/film_viewer.cc | 2 +- test/create_cli_test.cc | 2 +- test/film_metadata_test.cc | 6 ++--- test/ratio_test.cc | 47 ++++++++++++++-------------------- test/video_content_scale_test.cc | 2 +- 18 files changed, 104 insertions(+), 98 deletions(-) diff --git a/src/lib/create_cli.cc b/src/lib/create_cli.cc index 5f3d672bd..51518ae82 100644 --- a/src/lib/create_cli.cc +++ b/src/lib/create_cli.cc @@ -374,7 +374,7 @@ CreateCLI::CreateCLI(int argc, char* argv[]) } if (!container_ratio_string.empty()) { - _container_ratio = Ratio::from_id(container_ratio_string); + _container_ratio = Ratio::from_id_if_exists(container_ratio_string); if (!_container_ratio) { error = String::compose("%1: unrecognised container ratio %2", argv[0], container_ratio_string); return; @@ -464,7 +464,7 @@ CreateCLI::make_film(function error) const film->set_name(_name); if (_container_ratio) { - film->set_container(_container_ratio); + film->set_container(*_container_ratio); } if (_dcp_content_type) { film->set_dcp_content_type(_dcp_content_type); diff --git a/src/lib/create_cli.h b/src/lib/create_cli.h index 4767b1aa7..00abf85e5 100644 --- a/src/lib/create_cli.h +++ b/src/lib/create_cli.h @@ -19,6 +19,7 @@ */ +#include "ratio.h" #include "video_frame_type.h" #include #include @@ -28,7 +29,6 @@ class DCPContentType; class Film; -class Ratio; struct create_cli_test; @@ -67,7 +67,7 @@ private: boost::optional _template_name; std::string _name; - Ratio const* _container_ratio = nullptr; + boost::optional _container_ratio; bool _no_encrypt = false; bool _encrypt = false; bool _twod = false; diff --git a/src/lib/encode_cli.cc b/src/lib/encode_cli.cc index fa304b6e0..88fc36ac5 100644 --- a/src/lib/encode_cli.cc +++ b/src/lib/encode_cli.cc @@ -111,7 +111,7 @@ static void print_dump(function out, shared_ptr film) { out(fmt::format("{}\n", film->dcp_name(true))); - out(fmt::format("{} at {}\n", film->container()->container_nickname(), film->resolution() == Resolution::TWO_K ? "2K" : "4K")); + out(fmt::format("{} at {}\n", film->container().container_nickname(), film->resolution() == Resolution::TWO_K ? "2K" : "4K")); out(fmt::format("{}Mbit/s\n", film->video_bit_rate(film->video_encoding()) / 1000000)); out(fmt::format("Duration {}\n", film->length().timecode(film->video_frame_rate()))); out(fmt::format("Output {}fps {} {}kHz\n", film->video_frame_rate(), film->three_d() ? "3D" : "2D", film->audio_frame_rate() / 1000)); diff --git a/src/lib/film.cc b/src/lib/film.cc index 9897a01bf..cdf29766d 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -245,9 +245,7 @@ Film::~Film() string Film::video_identifier() const { - DCPOMATIC_ASSERT(container()); - - string s = container()->id() + string s = container().id() + "_" + resolution_to_string(_resolution) + "_" + _playlist->video_identifier() + "_" + fmt::to_string(_video_frame_rate) @@ -402,10 +400,7 @@ Film::metadata(bool with_content_paths) const cxml::add_text_child(root, "DCPContentType", _dcp_content_type->isdcf_name()); } - if (_container) { - cxml::add_text_child(root, "Container", _container->id()); - } - + cxml::add_text_child(root, "Container", _container.id()); cxml::add_text_child(root, "Resolution", resolution_to_string(_resolution)); cxml::add_text_child(root, "J2KVideoBitRate", fmt::to_string(_video_bit_rate[VideoEncoding::JPEG2000])); cxml::add_text_child(root, "MPEG2VideoBitRate", fmt::to_string(_video_bit_rate[VideoEncoding::MPEG2])); @@ -978,9 +973,7 @@ Film::isdcf_name(bool if_created_now) const isdcf_name += "-" + fmt::to_string(video_frame_rate()); } - if (container()) { - isdcf_name += "_" + container()->isdcf_name(); - } + isdcf_name += "_" + container().isdcf_name(); auto content_list = content(); @@ -992,7 +985,7 @@ Film::isdcf_name(bool if_created_now) const if (first_video != content_list.end()) { if (auto scaled_size = (*first_video)->video->scaled_size(frame_size())) { auto first_ratio = lrintf(scaled_size->ratio() * 100); - auto container_ratio = lrintf(container()->ratio() * 100); + auto container_ratio = lrintf(container().ratio() * 100); if (first_ratio != container_ratio) { isdcf_name += "-" + fmt::to_string(first_ratio); } @@ -1166,7 +1159,7 @@ Film::set_dcp_content_type(DCPContentType const * t) * DCP-o-matic is guessing the best container to use. */ void -Film::set_container(Ratio const * c, bool explicit_user) +Film::set_container(Ratio c, bool explicit_user) { FilmChangeSignaller ch(this, FilmProperty::CONTAINER); _container = c; @@ -1783,7 +1776,7 @@ Film::full_frame() const dcp::Size Film::frame_size() const { - return fit_ratio_within(container()->ratio(), full_frame()); + return fit_ratio_within(container().ratio(), full_frame()); } diff --git a/src/lib/film.h b/src/lib/film.h index c14b0f4fa..1af21849e 100644 --- a/src/lib/film.h +++ b/src/lib/film.h @@ -36,6 +36,7 @@ #include "film_property.h" #include "frame_rate_change.h" #include "named_channel.h" +#include "ratio.h" #include "remembered_asset.h" #include "resolution.h" #include "signaller.h" @@ -75,7 +76,6 @@ class Film; class Job; class Log; class Playlist; -class Ratio; struct atmos_encrypted_passthrough_test; struct isdcf_name_test; struct isdcf_name_with_atmos; @@ -216,7 +216,7 @@ public: return _dcp_content_type; } - Ratio const * container() const { + Ratio const& container() const { return _container; } @@ -384,7 +384,7 @@ public: void move_content_earlier(std::shared_ptr); void move_content_later(std::shared_ptr); void set_dcp_content_type(DCPContentType const *); - void set_container(Ratio const *, bool user_explicit = true); + void set_container(Ratio c, bool user_explicit = true); void set_resolution(Resolution, bool user_explicit = true); void set_encrypted(bool); void set_video_bit_rate(VideoEncoding encoding, int64_t); @@ -503,7 +503,7 @@ private: /** The type of content that this Film represents (feature, trailer etc.) */ DCPContentType const * _dcp_content_type; /** The container to put this Film in (flat, scope, etc.) */ - Ratio const * _container; + Ratio _container; /** DCP resolution (2K or 4K) */ Resolution _resolution; bool _encrypted; diff --git a/src/lib/hints.cc b/src/lib/hints.cc index 17aa16d47..4cd6ad506 100644 --- a/src/lib/hints.cc +++ b/src/lib/hints.cc @@ -140,15 +140,15 @@ Hints::check_incorrect_container() for (auto i: film()->content()) { if (i->video && i->video->size()) { auto const r = Ratio::nearest_from_ratio(i->video->scaled_size(film()->frame_size())->ratio()); - if (r && r->id() == "239") { + if (r.id() == "239") { ++scope; - } else if (r && r->id() != "239" && r->id() != "235" && r->id() != "190") { + } else if (r.id() != "239" && r.id() != "235" && r.id() != "190") { ++narrower_than_scope; } } } - string const film_container = film()->container()->id(); + auto const film_container = film()->container().id(); if (scope && !narrower_than_scope && film_container == "185") { hint(_("All of your content is in Scope (2.39:1) but your DCP's container is Flat (1.85:1). This will letter-box your content inside a Flat (1.85:1) frame. You may prefer to set your DCP's container to Scope (2.39:1) in the \"DCP\" tab.")); @@ -163,7 +163,7 @@ Hints::check_incorrect_container() void Hints::check_unusual_container() { - auto const film_container = film()->container()->id(); + auto const film_container = film()->container().id(); if (film()->video_encoding() != VideoEncoding::MPEG2 && film_container != "185" && film_container != "239") { hint(_("Your DCP uses an unusual container ratio. This may cause problems on some projectors. If possible, use Flat or Scope for the DCP container ratio.")); } diff --git a/src/lib/make_dcp.cc b/src/lib/make_dcp.cc index 5fb93a618..c40f707ee 100644 --- a/src/lib/make_dcp.cc +++ b/src/lib/make_dcp.cc @@ -48,10 +48,6 @@ make_dcp (shared_ptr film, TranscodeJob::ChangedBehaviour behaviour) throw BadSettingError (_("name"), _("Cannot contain slashes")); } - if (film->container() == nullptr) { - throw MissingSettingError (_("container")); - } - if (film->content().empty()) { throw runtime_error (_("You must add some content to the DCP before creating it")); } diff --git a/src/lib/ratio.cc b/src/lib/ratio.cc index 717c32458..e2baeac63 100644 --- a/src/lib/ratio.cc +++ b/src/lib/ratio.cc @@ -39,7 +39,6 @@ vector Ratio::_ratios; void Ratio::setup_ratios() { - /* This must only be called once as we rely on the addresses of objects in _ratios staying the same */ DCPOMATIC_ASSERT(_ratios.empty()); _ratios.push_back(Ratio(float(1290) / 1080, "119", _("1.19"), {}, "119")); @@ -59,19 +58,15 @@ Ratio::setup_ratios() } -vector +vector Ratio::all() { - vector pointers; - for (Ratio& ratio: _ratios) { - pointers.push_back(&ratio); - } - return pointers; + return _ratios; } -Ratio const * -Ratio::from_id(string i) +optional +Ratio::from_id_if_exists(string i) { /* We removed the ratio with id 137; replace it with 138 */ if (i == "137") { @@ -84,15 +79,24 @@ Ratio::from_id(string i) } if (j == _ratios.end()) { - return nullptr; + return {}; } - return &(*j); + return *j; } -/** @return Ratio corresponding to a given fractional ratio (+/- 0.01), or 0 */ -Ratio const * +Ratio +Ratio::from_id(string id) +{ + auto ratio = Ratio::from_id_if_exists(id); + DCPOMATIC_ASSERT(ratio); + return *ratio; +} + + +/** @return Ratio corresponding to a given fractional ratio (+/- 0.01), or empty */ +optional Ratio::from_ratio(float r) { auto j = _ratios.begin(); @@ -101,14 +105,14 @@ Ratio::from_ratio(float r) } if (j == _ratios.end()) { - return nullptr; + return {}; } - return &(*j); + return *j; } -Ratio const * +Ratio Ratio::nearest_from_ratio(float r) { vector::const_iterator nearest = _ratios.end(); @@ -124,10 +128,10 @@ Ratio::nearest_from_ratio(float r) DCPOMATIC_ASSERT(nearest != _ratios.end()); - return &(*nearest); + return *nearest; } -vector +vector Ratio::containers() { if (Config::instance()->allow_any_container()) { @@ -150,3 +154,17 @@ Ratio::container_nickname() const return *_container_nickname; } + + +bool +operator==(Ratio const& a, Ratio const& b) +{ + return a.id() == b.id(); +} + + +bool +operator!=(Ratio const& a, Ratio const& b) +{ + return a.id() != b.id(); +} diff --git a/src/lib/ratio.h b/src/lib/ratio.h index d4580827b..e6b939eb4 100644 --- a/src/lib/ratio.h +++ b/src/lib/ratio.h @@ -65,13 +65,14 @@ public: } static void setup_ratios(); - static Ratio const * from_id(std::string i); - static Ratio const * from_ratio(float r); - static Ratio const * nearest_from_ratio(float r); + static Ratio from_id(std::string i); + static boost::optional from_id_if_exists(std::string i); + static boost::optional from_ratio(float r); + static Ratio nearest_from_ratio(float r); - static std::vector all(); + static std::vector all(); - static std::vector containers(); + static std::vector containers(); private: float _ratio; @@ -87,4 +88,8 @@ private: }; +bool operator==(Ratio const& a, Ratio const& b); +bool operator!=(Ratio const& a, Ratio const& b); + + #endif diff --git a/src/lib/video_content.cc b/src/lib/video_content.cc index d0c6c850a..3f2819082 100644 --- a/src/lib/video_content.cc +++ b/src/lib/video_content.cc @@ -123,13 +123,16 @@ VideoContent::VideoContent(Content* parent, cxml::ConstNodePtr node, int version _crop.bottom = node->number_child("BottomCrop"); if (version <= 7) { - _legacy_ratio = Ratio::from_id(r.get())->ratio(); if (auto r = node->optional_string_child("Ratio")) { + if (auto ratio = Ratio::from_id_if_exists(r.get())) { + _legacy_ratio = ratio->ratio(); + } } } else if (version <= 37) { - auto ratio = node->node_child("Scale")->optional_string_child("Ratio"); - if (ratio) { - _legacy_ratio = Ratio::from_id(ratio.get())->ratio(); + if (auto id = node->node_child("Scale")->optional_string_child("Ratio")) { + if (auto ratio = Ratio::from_id_if_exists(*id)) { + _legacy_ratio = ratio->ratio(); + } } if (auto scale = node->node_child("Scale")->optional_bool_child("Scale")) { if (*scale) { @@ -497,7 +500,7 @@ VideoContent::processing_description(shared_ptr film) if (scaled && *scaled != container_size) { d += String::compose( _("\nPadded with black to fit container %1 (%2x%3)"), - film->container()->container_nickname(), + film->container().container_nickname(), container_size.width, container_size.height ); diff --git a/src/lib/writer.cc b/src/lib/writer.cc index ce5d40c75..72a6eaf8b 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -731,8 +731,8 @@ Writer::write_cover_sheet() if (!cpls.empty()) { boost::algorithm::replace_all (text, "$CPL_FILENAME", cpls[0].cpl_file.filename().string()); } - boost::algorithm::replace_all (text, "$TYPE", film()->dcp_content_type()->pretty_name()); - boost::algorithm::replace_all (text, "$CONTAINER", film()->container()->container_nickname()); + boost::algorithm::replace_all(text, "$TYPE", film()->dcp_content_type()->pretty_name()); + boost::algorithm::replace_all(text, "$CONTAINER", film()->container().container_nickname()); auto audio_language = film()->audio_language(); if (audio_language) { diff --git a/src/tools/dcpomatic_player.cc b/src/tools/dcpomatic_player.cc index b74124e2c..ffb4f5dbd 100644 --- a/src/tools/dcpomatic_player.cc +++ b/src/tools/dcpomatic_player.cc @@ -507,7 +507,7 @@ public: if (i->video && i->video->size()) { auto const r = Ratio::nearest_from_ratio(i->video->size()->ratio()); - if (r->id() == "239") { + if (r.id() == "239") { /* Any scope content means we use scope */ _film->set_container(r); } diff --git a/src/wx/dcp_panel.cc b/src/wx/dcp_panel.cc index 9d6eafe13..7582ba0fa 100644 --- a/src/wx/dcp_panel.cc +++ b/src/wx/dcp_panel.cc @@ -549,16 +549,16 @@ DCPPanel::setup_container() wxArrayString new_ratios; for (auto ratio: ratios) { - new_ratios.Add(std_to_wx(ratio->container_nickname())); + new_ratios.Add(std_to_wx(ratio.container_nickname())); } _container->set_entries(new_ratios); - auto iter = std::find_if(ratios.begin(), ratios.end(), [this](Ratio const* ratio) { return ratio == _film->container(); }); + auto iter = std::find_if(ratios.begin(), ratios.end(), [this](Ratio const& ratio) { return ratio == _film->container(); }); DCPOMATIC_ASSERT(iter != ratios.end()); checked_set(_container, iter - ratios.begin()); - auto const size = fit_ratio_within(_film->container()->ratio(), _film->full_frame()); + auto const size = fit_ratio_within(_film->container().ratio(), _film->full_frame()); checked_set(_container_size, wxString::Format(char_to_wx("%dx%d"), size.width, size.height)); setup_dcp_name(); diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index b97ef7bfb..7b22999b0 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -309,7 +309,7 @@ FilmViewer::calculate_sizes() int const video_view_height = std::round(_video_view->get()->GetSize().y * scale); auto const view_ratio = float(video_view_width) / video_view_height; - auto const film_ratio = container ? container->ratio() : 1.78; + auto const film_ratio = container.ratio(); dcp::Size out_size; if (view_ratio < film_ratio) { diff --git a/test/create_cli_test.cc b/test/create_cli_test.cc index 60d708eef..c71031fbc 100644 --- a/test/create_cli_test.cc +++ b/test/create_cli_test.cc @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE (create_cli_test) cc = run ("dcpomatic2_create x --container-ratio 185"); BOOST_CHECK (!cc.error); - BOOST_CHECK_EQUAL(cc._container_ratio, Ratio::from_id("185")); + BOOST_CHECK(cc._container_ratio == Ratio::from_id("185")); cc = run ("dcpomatic2_create x --container-ratio XXX"); BOOST_CHECK (cc.error); diff --git a/test/film_metadata_test.cc b/test/film_metadata_test.cc index b99d1a572..600dbf56d 100644 --- a/test/film_metadata_test.cc +++ b/test/film_metadata_test.cc @@ -72,9 +72,9 @@ BOOST_AUTO_TEST_CASE (film_metadata_test) auto g = make_shared(dir); g->read_metadata (); - BOOST_CHECK_EQUAL (g->name(), "fred"); - BOOST_CHECK_EQUAL (g->dcp_content_type(), DCPContentType::from_isdcf_name ("SHR")); - BOOST_CHECK_EQUAL (g->container(), Ratio::from_id ("185")); + BOOST_CHECK_EQUAL(g->name(), "fred"); + BOOST_CHECK_EQUAL(g->dcp_content_type(), DCPContentType::from_isdcf_name ("SHR")); + BOOST_CHECK(g->container() == Ratio::from_id("185")); g->write_metadata (); check_xml ("test/data/metadata.xml.ref", dir.string() + "/metadata.xml", ignore); diff --git a/test/ratio_test.cc b/test/ratio_test.cc index a21859049..af3d9430a 100644 --- a/test/ratio_test.cc +++ b/test/ratio_test.cc @@ -32,44 +32,35 @@ using std::ostream; -BOOST_AUTO_TEST_CASE (ratio_test) +BOOST_AUTO_TEST_CASE(ratio_test) { - Ratio const * r = Ratio::from_id ("119"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (1290, 1080)); + auto r = Ratio::from_id("119"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(1290, 1080)); - r = Ratio::from_id ("133"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (1440, 1080)); + r = Ratio::from_id("133"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(1440, 1080)); - r = Ratio::from_id ("138"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (1485, 1080)); + r = Ratio::from_id("138"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(1485, 1080)); - r = Ratio::from_id ("166"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (1800, 1080)); + r = Ratio::from_id("166"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(1800, 1080)); - r = Ratio::from_id ("178"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (1920, 1080)); + r = Ratio::from_id("178"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(1920, 1080)); - r = Ratio::from_id ("185"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (1998, 1080)); + r = Ratio::from_id("185"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(1998, 1080)); - r = Ratio::from_id ("239"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (2048, 858)); + r = Ratio::from_id("239"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(2048, 858)); - r = Ratio::from_id ("190"); - BOOST_CHECK (r); - BOOST_CHECK_EQUAL (fit_ratio_within (r->ratio(), dcp::Size (2048, 1080)), dcp::Size (2048, 1080)); + r = Ratio::from_id("190"); + BOOST_CHECK(fit_ratio_within(r.ratio(), dcp::Size(2048, 1080)) == dcp::Size(2048, 1080)); } -BOOST_AUTO_TEST_CASE (ratios_use_same_pointers_test) +BOOST_AUTO_TEST_CASE(ratio_equivalence_test) { - auto const test = Ratio::from_id ("119"); - BOOST_CHECK_EQUAL (test, Ratio::from_id("119")); + BOOST_CHECK(Ratio::from_id("119") == Ratio::from_id("119")); } diff --git a/test/video_content_scale_test.cc b/test/video_content_scale_test.cc index c82392f81..31e50bb76 100644 --- a/test/video_content_scale_test.cc +++ b/test/video_content_scale_test.cc @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE (scaled_size_legacy_test) /* 640x480 content that the user had asked to be stretched to 1.85:1 */ VideoContent vc (0); vc._size = dcp::Size(640, 480); - vc._legacy_ratio = Ratio::from_id("185")->ratio(); + vc._legacy_ratio = Ratio::from_id("185").ratio(); BOOST_CHECK_EQUAL(*vc.scaled_size(FLAT), FLAT); } -- cgit v1.2.3