From 2f93574e2ddef467bd879d559340f7967642615d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 7 Feb 2026 07:06:43 +0100 Subject: Make "why_not" reasons an optional return. --- src/lib/dcp_content.cc | 96 ++++++++++++++++++++++++++-------------- src/lib/dcp_content.h | 8 ++-- src/lib/film.cc | 9 ++-- src/wx/dcp_referencing_dialog.cc | 10 ++--- test/required_disk_space_test.cc | 3 +- test/text_entry_point_test.cc | 2 +- test/vf_test.cc | 34 +++++++------- 7 files changed, 92 insertions(+), 70 deletions(-) diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc index e37fbc9cc..ace0c0589 100644 --- a/src/lib/dcp_content.cc +++ b/src/lib/dcp_content.cc @@ -684,25 +684,31 @@ DCPContent::reel_split_points(shared_ptr film) const bool -DCPContent::can_reuse_anything(shared_ptr film, string& why_not) const +DCPContent::can_reuse_anything(shared_ptr film, string* why_not) const { /* We must be using the same standard as the film */ if (_standard) { if (_standard.get() == dcp::Standard::INTEROP && !film->interop()) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it is Interop and the film is set to SMPTE."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it is Interop and the film is set to SMPTE."); + } return false; } else if (_standard.get() == dcp::Standard::SMPTE && film->interop()) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it is SMPTE and the film is set to Interop."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it is SMPTE and the film is set to Interop."); + } return false; } } /* And the same frame rate */ if (!video_frame_rate() || (lrint(video_frame_rate().get()) != film->video_frame_rate())) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it has a different frame rate to the film."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it has a different frame rate to the film."); + } return false; } @@ -724,8 +730,10 @@ DCPContent::can_reuse_anything(shared_ptr film, string& why_not) con */ for (auto i: reel_list) { if (find(fr.begin(), fr.end(), i) == fr.end()) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("its reel lengths differ from those in the film; set the reel mode to 'split by video content'."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("its reel lengths differ from those in the film; set the reel mode to 'split by video content'."); + } return false; } } @@ -742,25 +750,31 @@ DCPContent::overlaps(shared_ptr film, function film, string& why_not) const +DCPContent::can_reuse_video(shared_ptr film, string* why_not) const { if (!video) { - why_not = _("There is no video in this DCP"); + if (why_not) { + *why_not = _("There is no video in this DCP"); + } return false; } if (film->resolution() != resolution()) { - if (resolution() == Resolution::FOUR_K) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it is 4K and the film is 2K."); - } else { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it is 2K and the film is 4K."); + if (why_not) { + if (resolution() == Resolution::FOUR_K) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it is 4K and the film is 2K."); + } else { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it is 2K and the film is 4K."); + } } return false; } else if (film->frame_size() != video->size()) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("its video frame size differs from the film's."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("its video frame size differs from the film's."); + } return false; } @@ -769,8 +783,10 @@ DCPContent::can_reuse_video(shared_ptr film, string& why_not) const }; if (overlaps(film, part)) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it overlaps other video content."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it overlaps other video content."); + } return false; } @@ -779,12 +795,14 @@ DCPContent::can_reuse_video(shared_ptr film, string& why_not) const bool -DCPContent::can_reuse_audio(shared_ptr film, string& why_not) const +DCPContent::can_reuse_audio(shared_ptr film, string* why_not) const { if (audio && audio->stream()) { auto const channels = audio->stream()->channels(); if (channels != film->audio_channels()) { - why_not = fmt::format(_("it has a different number of audio channels than the project; set the project to have {} channels."), channels); + if (why_not) { + *why_not = fmt::format(_("it has a different number of audio channels than the project; set the project to have {} channels."), channels); + } return false; } } @@ -794,8 +812,10 @@ DCPContent::can_reuse_audio(shared_ptr film, string& why_not) const }; if (overlaps(film, part)) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it overlaps other audio content."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it overlaps other audio content."); + } return false; } @@ -804,23 +824,29 @@ DCPContent::can_reuse_audio(shared_ptr film, string& why_not) const bool -DCPContent::can_reuse_text(shared_ptr film, TextType type, string& why_not) const +DCPContent::can_reuse_text(shared_ptr film, TextType type, string* why_not) const { if (_has_non_zero_entry_point[TextType::OPEN_SUBTITLE]) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("one of its subtitle reels has a non-zero entry point so it must be re-written."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("one of its subtitle reels has a non-zero entry point so it must be re-written."); + } return false; } if (_has_non_zero_entry_point[TextType::CLOSED_CAPTION]) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("one of its closed caption has a non-zero entry point so it must be re-written."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("one of its closed caption has a non-zero entry point so it must be re-written."); + } return false; } if (trim_start() != dcpomatic::ContentTime()) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it has a start trim so its subtitles or closed captions must be re-written."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it has a start trim so its subtitles or closed captions must be re-written."); + } return false; } @@ -829,8 +855,10 @@ DCPContent::can_reuse_text(shared_ptr film, TextType type, string& w }; if (overlaps(film, part)) { - /// TRANSLATORS: this string will follow "Cannot reference this DCP: " - why_not = _("it overlaps other text content."); + if (why_not) { + /// TRANSLATORS: this string will follow "Cannot reference this DCP: " + *why_not = _("it overlaps other text content."); + } return false; } diff --git a/src/lib/dcp_content.h b/src/lib/dcp_content.h index 1dd00b015..bc60dadb7 100644 --- a/src/lib/dcp_content.h +++ b/src/lib/dcp_content.h @@ -130,7 +130,7 @@ public: bool needs_kdm() const; bool needs_assets() const; - bool can_reuse_anything(std::shared_ptr film, std::string& why_not) const; + bool can_reuse_anything(std::shared_ptr film, std::string* why_not = nullptr) const; void set_reference_video(bool r); @@ -139,7 +139,7 @@ public: return _reference_video; } - bool can_reuse_video(std::shared_ptr film, std::string &) const; + bool can_reuse_video(std::shared_ptr film, std::string* why_not = nullptr) const; void set_reference_audio(bool r); @@ -148,7 +148,7 @@ public: return _reference_audio; } - bool can_reuse_audio(std::shared_ptr film, std::string &) const; + bool can_reuse_audio(std::shared_ptr film, std::string* why_not = nullptr) const; void set_reference_text(TextType type, bool r); @@ -160,7 +160,7 @@ public: return _reference_text[type]; } - bool can_reuse_text(std::shared_ptr film, TextType type, std::string &) const; + bool can_reuse_text(std::shared_ptr film, TextType type, std::string* why_not = nullptr) const; bool reference_anything() const; diff --git a/src/lib/film.cc b/src/lib/film.cc index 1a9185afa..16a38e21b 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -1810,20 +1810,19 @@ Film::check_settings_consistency() continue; } - string why_not; - if (d->reference_video() && !d->can_reuse_video(shared_from_this(), why_not)) { + if (d->reference_video() && !d->can_reuse_video(shared_from_this())) { d->set_reference_video(false); change_made = true; } - if (d->reference_audio() && !d->can_reuse_audio(shared_from_this(), why_not)) { + if (d->reference_audio() && !d->can_reuse_audio(shared_from_this())) { d->set_reference_audio(false); change_made = true; } - if (d->reference_text(TextType::OPEN_SUBTITLE) && !d->can_reuse_text(shared_from_this(), TextType::OPEN_SUBTITLE, why_not)) { + if (d->reference_text(TextType::OPEN_SUBTITLE) && !d->can_reuse_text(shared_from_this(), TextType::OPEN_SUBTITLE)) { d->set_reference_text(TextType::OPEN_SUBTITLE, false); change_made = true; } - if (d->reference_text(TextType::CLOSED_CAPTION) && !d->can_reuse_text(shared_from_this(), TextType::CLOSED_CAPTION, why_not)) { + if (d->reference_text(TextType::CLOSED_CAPTION) && !d->can_reuse_text(shared_from_this(), TextType::CLOSED_CAPTION)) { d->set_reference_text(TextType::CLOSED_CAPTION, false); change_made = true; } diff --git a/src/wx/dcp_referencing_dialog.cc b/src/wx/dcp_referencing_dialog.cc index 6d9a76481..337724092 100644 --- a/src/wx/dcp_referencing_dialog.cc +++ b/src/wx/dcp_referencing_dialog.cc @@ -168,34 +168,34 @@ DCPReferencingDialog::setup() string why_not; - if (!dcp_content->can_reuse_anything(_film, why_not)) { + if (!dcp_content->can_reuse_anything(_film, &why_not)) { for (auto const& part: all_parts) { record.check_box[part]->Enable(false); } add_problem(_("Cannot reference this DCP"), why_not); } else { - if (!dcp_content->can_reuse_video(_film, why_not)) { + if (!dcp_content->can_reuse_video(_film, &why_not)) { record.check_box[Part::VIDEO]->Enable(false); if (dcp_content->video) { add_problem(_("Cannot reference this DCP's video"), why_not); } } - if (!dcp_content->can_reuse_audio(_film, why_not)) { + if (!dcp_content->can_reuse_audio(_film, &why_not)) { record.check_box[Part::AUDIO]->Enable(false); if (dcp_content->audio) { add_problem(_("Cannot reference this DCP's audio"), why_not); } } - if (!dcp_content->can_reuse_text(_film, TextType::OPEN_SUBTITLE, why_not)) { + if (!dcp_content->can_reuse_text(_film, TextType::OPEN_SUBTITLE, &why_not)) { record.check_box[Part::SUBTITLES]->Enable(false); if (dcp_content->text_of_original_type(TextType::OPEN_SUBTITLE)) { add_problem(_("Cannot reference this DCP's subtitles"), why_not); } } - if (!dcp_content->can_reuse_text(_film, TextType::CLOSED_CAPTION, why_not)) { + if (!dcp_content->can_reuse_text(_film, TextType::CLOSED_CAPTION, &why_not)) { record.check_box[Part::CLOSED_CAPTIONS]->Enable(false); if (dcp_content->text_of_original_type(TextType::CLOSED_CAPTION)) { add_problem(_("Cannot reference this DCP's closed captions"), why_not); diff --git a/test/required_disk_space_test.cc b/test/required_disk_space_test.cc index 47ecbfba8..7f5a17101 100644 --- a/test/required_disk_space_test.cc +++ b/test/required_disk_space_test.cc @@ -69,8 +69,7 @@ BOOST_AUTO_TEST_CASE (required_disk_space_test) 16 ); - std::string why_not; - BOOST_CHECK(content_b->can_reuse_audio(film, why_not)); + BOOST_CHECK(content_b->can_reuse_audio(film)); content_b->set_reference_audio (true); check_within_n ( diff --git a/test/text_entry_point_test.cc b/test/text_entry_point_test.cc index 995b6db49..a79fb72b3 100644 --- a/test/text_entry_point_test.cc +++ b/test/text_entry_point_test.cc @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(test_text_entry_point) film2->read_metadata(); string why_not; - BOOST_CHECK(!dcp_content->can_reuse_text(film2, TextType::OPEN_SUBTITLE, why_not)); + BOOST_CHECK(!dcp_content->can_reuse_text(film2, TextType::OPEN_SUBTITLE, &why_not)); BOOST_CHECK_EQUAL(why_not, "one of its subtitle reels has a non-zero entry point so it must be re-written."); } diff --git a/test/vf_test.cc b/test/vf_test.cc index b39529c16..bf7527003 100644 --- a/test/vf_test.cc +++ b/test/vf_test.cc @@ -72,19 +72,18 @@ BOOST_AUTO_TEST_CASE(vf_test1) /* Multi-reel DCP can't be referenced if we are using a single reel for the project */ film->set_reel_type(ReelType::SINGLE); film->set_audio_channels(16); - string why_not; - BOOST_CHECK(!dcp->can_reuse_video(film, why_not)); - BOOST_CHECK(!dcp->can_reuse_audio(film, why_not)); - BOOST_CHECK(!dcp->can_reuse_text(film, TextType::OPEN_SUBTITLE, why_not)); - BOOST_CHECK(!dcp->can_reuse_text(film, TextType::CLOSED_CAPTION, why_not)); + BOOST_CHECK(!dcp->can_reuse_video(film)); + BOOST_CHECK(!dcp->can_reuse_audio(film)); + BOOST_CHECK(!dcp->can_reuse_text(film, TextType::OPEN_SUBTITLE)); + BOOST_CHECK(!dcp->can_reuse_text(film, TextType::CLOSED_CAPTION)); /* Multi-reel DCP can be referenced if we are using by-video-content */ film->set_reel_type(ReelType::BY_VIDEO_CONTENT); - BOOST_CHECK(dcp->can_reuse_video(film, why_not)); - BOOST_CHECK(dcp->can_reuse_audio(film, why_not)); + BOOST_CHECK(dcp->can_reuse_video(film)); + BOOST_CHECK(dcp->can_reuse_audio(film)); /* (but reels_test2 has no texts to reference) */ - BOOST_CHECK(!dcp->can_reuse_text(film, TextType::OPEN_SUBTITLE, why_not)); - BOOST_CHECK(!dcp->can_reuse_text(film, TextType::CLOSED_CAPTION, why_not)); + BOOST_CHECK(!dcp->can_reuse_text(film, TextType::OPEN_SUBTITLE)); + BOOST_CHECK(!dcp->can_reuse_text(film, TextType::CLOSED_CAPTION)); auto other = make_shared("test/data/test.mp4"); film->examine_and_add_content({other}); @@ -93,15 +92,15 @@ BOOST_AUTO_TEST_CASE(vf_test1) /* Not possible if there is overlap; we only check video here as that's all test.mp4 has */ other->set_position(film, DCPTime()); - BOOST_CHECK(!dcp->can_reuse_video(film, why_not)); + BOOST_CHECK(!dcp->can_reuse_video(film)); /* This should not be considered an overlap */ other->set_position(film, dcp->end(film)); - BOOST_CHECK(dcp->can_reuse_video(film, why_not)); - BOOST_CHECK(dcp->can_reuse_audio(film, why_not)); + BOOST_CHECK(dcp->can_reuse_video(film)); + BOOST_CHECK(dcp->can_reuse_audio(film)); /* (reels_test2 has no texts to reference) */ - BOOST_CHECK(!dcp->can_reuse_text(film, TextType::OPEN_SUBTITLE, why_not)); - BOOST_CHECK(!dcp->can_reuse_text(film, TextType::CLOSED_CAPTION, why_not)); + BOOST_CHECK(!dcp->can_reuse_text(film, TextType::OPEN_SUBTITLE)); + BOOST_CHECK(!dcp->can_reuse_text(film, TextType::CLOSED_CAPTION)); } @@ -389,9 +388,7 @@ BOOST_AUTO_TEST_CASE(test_referencing_ov_with_subs_when_adding_ccaps) ccaps->text[0]->set_type(TextType::CLOSED_CAPTION); ccaps->set_position(vf, dcpomatic::DCPTime()); - string why_not; - BOOST_CHECK(ov_dcp->can_reuse_text(vf, TextType::OPEN_SUBTITLE, why_not)); - std::cout << why_not << "\n"; + BOOST_CHECK(ov_dcp->can_reuse_text(vf, TextType::OPEN_SUBTITLE)); } @@ -416,8 +413,7 @@ BOOST_AUTO_TEST_CASE(test_duplicate_font_id_in_vf) ov_dcp->text[0]->set_use(true); ccaps->text[0]->set_type(TextType::CLOSED_CAPTION); ccaps->set_position(vf, dcpomatic::DCPTime()); - string why_not; - BOOST_CHECK_MESSAGE(ov_dcp->can_reuse_text(vf, TextType::OPEN_SUBTITLE, why_not), why_not); + BOOST_CHECK(ov_dcp->can_reuse_text(vf, TextType::OPEN_SUBTITLE)); ov_dcp->set_reference_text(TextType::OPEN_SUBTITLE, true); vf->write_metadata(); make_dcp(vf, TranscodeJob::ChangedBehaviour::IGNORE); -- cgit v1.2.3