Fix various bugs in subtitle/ccap verification.
authorCarl Hetherington <cth@carlh.net>
Mon, 15 Mar 2021 00:36:51 +0000 (01:36 +0100)
committerCarl Hetherington <cth@carlh.net>
Mon, 15 Mar 2021 00:36:51 +0000 (01:36 +0100)
Check that subtitles don't overlap reel boundaries, and fix a few
tests that trip this check.

Fix confusion when calculating subtitle timings during verification
where the picture asset frame rate was being used rather than the
subtitle asset's edit rate.

Do the subtitle timing verification for Interop as well as SMPTE
subtitles.

Take <StartTime> tags into account when checking subtitles, even
though Bv2.1 says they should be set to 0.

Rename Time::as_editable_units to Time::as_editable_units_ceil
and add a _floor variant, then use that to round down when checking
reel boundary overlaps.

src/dcp_time.cc
src/dcp_time.h
src/reel_markers_asset.cc
src/smpte_subtitle_asset.cc
src/subtitle_asset_internal.cc
src/verify.cc
src/verify.h
test/verify_test.cc

index 2895cd37760659b72e331e6c8b2159d0226f4c0f..0bd0a4096c96d47f5e0d74421ceeeabf1f31dbc0 100644 (file)
@@ -340,7 +340,14 @@ Time::as_string (Standard standard) const
 
 
 int64_t
 
 
 int64_t
-Time::as_editable_units (int tcr_) const
+Time::as_editable_units_floor (int tcr_) const
+{
+       return floor(int64_t(e) * double(tcr_) / tcr) + int64_t(s) * tcr_ + int64_t(m) * 60 * tcr_ + int64_t(h) * 60 * 60 * tcr_;
+}
+
+
+int64_t
+Time::as_editable_units_ceil (int tcr_) const
 {
        return ceil(int64_t(e) * double(tcr_) / tcr) + int64_t(s) * tcr_ + int64_t(m) * 60 * tcr_ + int64_t(h) * 60 * 60 * tcr_;
 }
 {
        return ceil(int64_t(e) * double(tcr_) / tcr) + int64_t(s) * tcr_ + int64_t(m) * 60 * tcr_ + int64_t(h) * 60 * 60 * tcr_;
 }
index dcbadf31294137036654a9c06478a3494bb3da09..dbfdb7f0c060c0e3dd4309091137be61f268e182 100644 (file)
@@ -126,11 +126,17 @@ public:
        /** @return the total number of seconds that this time consists of */
        double as_seconds () const;
 
        /** @return the total number of seconds that this time consists of */
        double as_seconds () const;
 
+       /** @param tcr_ Timecode rate with which the return value should be counted
+        *  @return the total number of editable units that this time consists of at the specified timecode rate, rounded down
+        *  to the nearest editable unit. For example, as_editable_units_floor(24) returns the total time in frames at 24fps.
+        */
+       int64_t as_editable_units_floor (int tcr_) const;
+
        /** @param tcr_ Timecode rate with which the return value should be counted
         *  @return the total number of editable units that this time consists of at the specified timecode rate, rounded up
        /** @param tcr_ Timecode rate with which the return value should be counted
         *  @return the total number of editable units that this time consists of at the specified timecode rate, rounded up
-        *  to the nearest editable unit. For example, as_editable_units (24) returns the total time in frames at 24fps.
+        *  to the nearest editable unit. For example, as_editable_units_ceil(24) returns the total time in frames at 24fps.
         */
         */
-       int64_t as_editable_units (int tcr_) const;
+       int64_t as_editable_units_ceil (int tcr_) const;
 
        /** @param tcr_ New timecode rate
         *  @return A new Time which is this time at the spcified new timecode rate
 
        /** @param tcr_ New timecode rate
         *  @return A new Time which is this time at the spcified new timecode rate
index 360a855aac331a11020777dd59aaa7a84e30a068..c6339df5dd6969dae923b920cd3861ce697b02bb 100644 (file)
@@ -110,7 +110,7 @@ ReelMarkersAsset::write_to_cpl (xmlpp::Node* node, Standard standard) const
        for (auto const& i: _markers) {
                auto m = ml->add_child("Marker");
                m->add_child("Label")->add_child_text(marker_to_string(i.first));
        for (auto const& i: _markers) {
                auto m = ml->add_child("Marker");
                m->add_child("Label")->add_child_text(marker_to_string(i.first));
-               m->add_child("Offset")->add_child_text(raw_convert<string>(i.second.as_editable_units(tcr)));
+               m->add_child("Offset")->add_child_text(raw_convert<string>(i.second.as_editable_units_ceil(tcr)));
        }
 
        return asset;
        }
 
        return asset;
index 15a366ba153f7598bd61d86939ddd545de6d1676..a55b91ae8477e7242d9248666d59bac10500fc20 100644 (file)
@@ -195,7 +195,7 @@ SMPTESubtitleAsset::parse_xml (shared_ptr<cxml::Document> xml)
        }
 
        /* Guess intrinsic duration */
        }
 
        /* Guess intrinsic duration */
-       _intrinsic_duration = latest_subtitle_out().as_editable_units (_edit_rate.numerator / _edit_rate.denominator);
+       _intrinsic_duration = latest_subtitle_out().as_editable_units_ceil(_edit_rate.numerator / _edit_rate.denominator);
 }
 
 
 }
 
 
@@ -547,5 +547,5 @@ void
 SMPTESubtitleAsset::add (shared_ptr<Subtitle> s)
 {
        SubtitleAsset::add (s);
 SMPTESubtitleAsset::add (shared_ptr<Subtitle> s)
 {
        SubtitleAsset::add (s);
-       _intrinsic_duration = latest_subtitle_out().as_editable_units (_edit_rate.numerator / _edit_rate.denominator);
+       _intrinsic_duration = latest_subtitle_out().as_editable_units_ceil(_edit_rate.numerator / _edit_rate.denominator);
 }
 }
index f1674789650742f5b85da7b33b149b91fec337f5..80e861f117a9b55cae26299138197a8276bf038e 100644 (file)
@@ -233,8 +233,8 @@ order::Subtitle::as_xml (xmlpp::Element* parent, Context& context) const
                e->set_attribute ("FadeUpTime", _fade_up.rebase(context.time_code_rate).as_string(context.standard));
                e->set_attribute ("FadeDownTime", _fade_down.rebase(context.time_code_rate).as_string(context.standard));
        } else {
                e->set_attribute ("FadeUpTime", _fade_up.rebase(context.time_code_rate).as_string(context.standard));
                e->set_attribute ("FadeDownTime", _fade_down.rebase(context.time_code_rate).as_string(context.standard));
        } else {
-               e->set_attribute ("FadeUpTime", raw_convert<string> (_fade_up.as_editable_units(context.time_code_rate)));
-               e->set_attribute ("FadeDownTime", raw_convert<string> (_fade_down.as_editable_units(context.time_code_rate)));
+               e->set_attribute ("FadeUpTime", raw_convert<string> (_fade_up.as_editable_units_ceil(context.time_code_rate)));
+               e->set_attribute ("FadeDownTime", raw_convert<string> (_fade_down.as_editable_units_ceil(context.time_code_rate)));
        }
        return e;
 }
        }
        return e;
 }
index bddf1683cc1c8b926fcfdf216da2d212f21933ae..97758e6192e632aae3bf4c498c167a90120818ad 100644 (file)
@@ -763,7 +763,7 @@ static
 void
 verify_text_timing (
        vector<shared_ptr<Reel>> reels,
 void
 verify_text_timing (
        vector<shared_ptr<Reel>> reels,
-       optional<int> picture_frame_rate,
+       int edit_rate,
        vector<VerificationNote>& notes,
        std::function<bool (shared_ptr<Reel>)> check,
        std::function<string (shared_ptr<Reel>)> xml,
        vector<VerificationNote>& notes,
        std::function<bool (shared_ptr<Reel>)> check,
        std::function<string (shared_ptr<Reel>)> xml,
@@ -775,32 +775,39 @@ verify_text_timing (
        auto too_short = false;
        auto too_close = false;
        auto too_early = false;
        auto too_short = false;
        auto too_close = false;
        auto too_early = false;
+       auto reel_overlap = false;
        /* current reel start time (in editable units) */
        int64_t reel_offset = 0;
 
        /* current reel start time (in editable units) */
        int64_t reel_offset = 0;
 
-       std::function<void (cxml::ConstNodePtr, int, int, bool)> parse;
-       parse = [&parse, &last_out, &too_short, &too_close, &too_early, &reel_offset](cxml::ConstNodePtr node, int tcr, int pfr, bool first_reel) {
+       std::function<void (cxml::ConstNodePtr, optional<int>, optional<Time>, int, bool)> parse;
+       parse = [&parse, &last_out, &too_short, &too_close, &too_early, &reel_offset](cxml::ConstNodePtr node, optional<int> tcr, optional<Time> start_time, int er, bool first_reel) {
                if (node->name() == "Subtitle") {
                        Time in (node->string_attribute("TimeIn"), tcr);
                if (node->name() == "Subtitle") {
                        Time in (node->string_attribute("TimeIn"), tcr);
+                       if (start_time) {
+                               in -= *start_time;
+                       }
                        Time out (node->string_attribute("TimeOut"), tcr);
                        Time out (node->string_attribute("TimeOut"), tcr);
-                       if (first_reel && in < Time(0, 0, 4, 0, tcr)) {
+                       if (start_time) {
+                               out -= *start_time;
+                       }
+                       if (first_reel && tcr && in < Time(0, 0, 4, 0, *tcr)) {
                                too_early = true;
                        }
                        auto length = out - in;
                                too_early = true;
                        }
                        auto length = out - in;
-                       if (length.as_editable_units(pfr) < 15) {
+                       if (length.as_editable_units_ceil(er) < 15) {
                                too_short = true;
                        }
                        if (last_out) {
                                /* XXX: this feels dubious - is it really what Bv2.1 means? */
                                too_short = true;
                        }
                        if (last_out) {
                                /* XXX: this feels dubious - is it really what Bv2.1 means? */
-                               auto distance = reel_offset + in.as_editable_units(pfr) - *last_out;
+                               auto distance = reel_offset + in.as_editable_units_ceil(er) - *last_out;
                                if (distance >= 0 && distance < 2) {
                                        too_close = true;
                                }
                        }
                                if (distance >= 0 && distance < 2) {
                                        too_close = true;
                                }
                        }
-                       last_out = reel_offset + out.as_editable_units(pfr);
+                       last_out = reel_offset + out.as_editable_units_floor(er);
                } else {
                        for (auto i: node->node_children()) {
                } else {
                        for (auto i: node->node_children()) {
-                               parse(i, tcr, pfr, first_reel);
+                               parse(i, tcr, start_time, er, first_reel);
                        }
                }
        };
                        }
                }
        };
@@ -814,11 +821,31 @@ verify_text_timing (
                 * read in by libdcp's parser.
                 */
 
                 * read in by libdcp's parser.
                 */
 
-               auto doc = make_shared<cxml::Document>("SubtitleReel");
-               doc->read_string (xml(reels[i]));
-               auto const tcr = doc->number_child<int>("TimeCodeRate");
-               parse (doc, tcr, picture_frame_rate.get_value_or(24), i == 0);
-               reel_offset += duration(reels[i]);
+               shared_ptr<cxml::Document> doc;
+               optional<int> tcr;
+               optional<Time> start_time;
+               try {
+                       doc = make_shared<cxml::Document>("SubtitleReel");
+                       doc->read_string (xml(reels[i]));
+                       tcr = doc->number_child<int>("TimeCodeRate");
+                       auto start_time_string = doc->optional_string_child("StartTime");
+                       if (start_time_string) {
+                               start_time = Time(*start_time_string, tcr);
+                       }
+               } catch (...) {
+                       doc = make_shared<cxml::Document>("DCSubtitle");
+                       doc->read_string (xml(reels[i]));
+               }
+               parse (doc, tcr, start_time, edit_rate, i == 0);
+               auto end = reel_offset + duration(reels[i]);
+               if (last_out && *last_out > end) {
+                       reel_overlap = true;
+               }
+               reel_offset = end;
+       }
+
+       if (last_out && *last_out > reel_offset) {
+               reel_overlap = true;
        }
 
        if (too_early) {
        }
 
        if (too_early) {
@@ -838,6 +865,12 @@ verify_text_timing (
                        VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_SPACING
                });
        }
                        VerificationNote::Type::WARNING, VerificationNote::Code::INVALID_SUBTITLE_SPACING
                });
        }
+
+       if (reel_overlap) {
+               notes.push_back ({
+                       VerificationNote::Type::ERROR, VerificationNote::Code::SUBTITLE_OVERLAPS_REEL_BOUNDARY
+               });
+       }
 }
 
 
 }
 
 
@@ -948,13 +981,8 @@ verify_text_timing (vector<shared_ptr<Reel>> reels, vector<VerificationNote>& no
                return;
        }
 
                return;
        }
 
-       optional<int> picture_frame_rate;
-       if (reels[0]->main_picture()) {
-               picture_frame_rate = reels[0]->main_picture()->frame_rate().numerator;
-       }
-
        if (reels[0]->main_subtitle()) {
        if (reels[0]->main_subtitle()) {
-               verify_text_timing (reels, picture_frame_rate, notes,
+               verify_text_timing (reels, reels[0]->main_subtitle()->edit_rate().numerator, notes,
                        [](shared_ptr<Reel> reel) {
                                return static_cast<bool>(reel->main_subtitle());
                        },
                        [](shared_ptr<Reel> reel) {
                                return static_cast<bool>(reel->main_subtitle());
                        },
@@ -968,7 +996,7 @@ verify_text_timing (vector<shared_ptr<Reel>> reels, vector<VerificationNote>& no
        }
 
        for (auto i = 0U; i < reels[0]->closed_captions().size(); ++i) {
        }
 
        for (auto i = 0U; i < reels[0]->closed_captions().size(); ++i) {
-               verify_text_timing (reels, picture_frame_rate, notes,
+               verify_text_timing (reels, reels[0]->closed_captions()[i]->edit_rate().numerator, notes,
                        [i](shared_ptr<Reel> reel) {
                                return i < reel->closed_captions().size();
                        },
                        [i](shared_ptr<Reel> reel) {
                                return i < reel->closed_captions().size();
                        },
@@ -1262,6 +1290,8 @@ dcp::verify (
                                most_closed_captions = std::max (most_closed_captions, reel->closed_captions().size());
                        }
 
                                most_closed_captions = std::max (most_closed_captions, reel->closed_captions().size());
                        }
 
+                       verify_text_timing (cpl->reels(), notes);
+
                        if (dcp->standard() == Standard::SMPTE) {
 
                                if (have_main_subtitle && have_no_main_subtitle) {
                        if (dcp->standard() == Standard::SMPTE) {
 
                                if (have_main_subtitle && have_no_main_subtitle) {
@@ -1292,14 +1322,12 @@ dcp::verify (
                                if (lfoc == markers_seen.end()) {
                                        notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSING_LFOC});
                                } else {
                                if (lfoc == markers_seen.end()) {
                                        notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::MISSING_LFOC});
                                } else {
-                                       auto lfoc_time = lfoc->second.as_editable_units(lfoc->second.tcr);
+                                       auto lfoc_time = lfoc->second.as_editable_units_ceil(lfoc->second.tcr);
                                        if (lfoc_time != (cpl->reels().back()->duration() - 1)) {
                                                notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INCORRECT_LFOC, raw_convert<string>(lfoc_time)});
                                        }
                                }
 
                                        if (lfoc_time != (cpl->reels().back()->duration() - 1)) {
                                                notes.push_back ({VerificationNote::Type::WARNING, VerificationNote::Code::INCORRECT_LFOC, raw_convert<string>(lfoc_time)});
                                        }
                                }
 
-                               verify_text_timing (cpl->reels(), notes);
-
                                LinesCharactersResult result;
                                for (auto reel: cpl->reels()) {
                                        if (reel->main_subtitle() && reel->main_subtitle()->asset()) {
                                LinesCharactersResult result;
                                for (auto reel: cpl->reels()) {
                                        if (reel->main_subtitle() && reel->main_subtitle()->asset()) {
@@ -1458,6 +1486,8 @@ dcp::note_to_string (VerificationNote note)
                return "At least one subtitle lasts less than 15 frames.";
        case VerificationNote::Code::INVALID_SUBTITLE_SPACING:
                return "At least one pair of subtitles is separated by less than 2 frames.";
                return "At least one subtitle lasts less than 15 frames.";
        case VerificationNote::Code::INVALID_SUBTITLE_SPACING:
                return "At least one pair of subtitles is separated by less than 2 frames.";
+       case VerificationNote::Code::SUBTITLE_OVERLAPS_REEL_BOUNDARY:
+               return "At least one subtitle extends outside of its reel.";
        case VerificationNote::Code::INVALID_SUBTITLE_LINE_COUNT:
                return "There are more than 3 subtitle lines in at least one place in the DCP.";
        case VerificationNote::Code::NEARLY_INVALID_SUBTITLE_LINE_LENGTH:
        case VerificationNote::Code::INVALID_SUBTITLE_LINE_COUNT:
                return "There are more than 3 subtitle lines in at least one place in the DCP.";
        case VerificationNote::Code::NEARLY_INVALID_SUBTITLE_LINE_LENGTH:
index f96fd4d244693abbed5d28ece25d841ca8f35c0c..16700127ae49e4f1ffc28c32029a275139ced221 100644 (file)
@@ -214,6 +214,8 @@ public:
                INVALID_SUBTITLE_DURATION,
                /** At least one pair of subtitles are separated by less than the the minimum of 2 frames suggested by [Bv2.1_7.2.5] */
                INVALID_SUBTITLE_SPACING,
                INVALID_SUBTITLE_DURATION,
                /** At least one pair of subtitles are separated by less than the the minimum of 2 frames suggested by [Bv2.1_7.2.5] */
                INVALID_SUBTITLE_SPACING,
+               /** A subtitle lasts for longer than the reel which contains it */
+               SUBTITLE_OVERLAPS_REEL_BOUNDARY,
                /** There are more than 3 subtitle lines in at least one place [Bv2.1_7.2.7] */
                INVALID_SUBTITLE_LINE_COUNT,
                /** There are more than 52 characters in at least one subtitle line [Bv2.1_7.2.7] */
                /** There are more than 3 subtitle lines in at least one place [Bv2.1_7.2.7] */
                INVALID_SUBTITLE_LINE_COUNT,
                /** There are more than 52 characters in at least one subtitle line [Bv2.1_7.2.7] */
index 4312194126452a2a0a632a3e15d8f55d142b7ee6..80e985e59b03bf93a9635413848ce231abe4bd0c 100644 (file)
@@ -202,6 +202,7 @@ void
 check_verify_result (vector<path> dir, vector<dcp::VerificationNote> test_notes)
 {
        auto notes = dcp::verify ({dir}, &stage, &progress, xsd_test);
 check_verify_result (vector<path> dir, vector<dcp::VerificationNote> test_notes)
 {
        auto notes = dcp::verify ({dir}, &stage, &progress, xsd_test);
+       dump_notes (notes);
        BOOST_REQUIRE_EQUAL (notes.size(), test_notes.size());
        for (auto i = 0U; i < notes.size(); ++i) {
                BOOST_REQUIRE_EQUAL (notes[i], test_notes[i]);
        BOOST_REQUIRE_EQUAL (notes.size(), test_notes.size());
        for (auto i = 0U; i < notes.size(); ++i) {
                BOOST_REQUIRE_EQUAL (notes[i], test_notes[i]);
@@ -677,7 +678,7 @@ BOOST_AUTO_TEST_CASE (verify_valid_smpte_subtitles)
        prepare_directory (dir);
        copy_file ("test/data/subs.mxf", dir / "subs.mxf");
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
        prepare_directory (dir);
        copy_file ("test/data/subs.mxf", dir / "subs.mxf");
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
-       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(24, 1), 16 * 24, 0);
+       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(25, 1), 300 * 24, 0);
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        check_verify_result ({dir}, {{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }});
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        check_verify_result ({dir}, {{ dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }});
@@ -692,7 +693,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_smpte_subtitles)
        prepare_directory (dir);
        copy_file ("test/data/broken_smpte.mxf", dir / "subs.mxf");
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
        prepare_directory (dir);
        copy_file ("test/data/broken_smpte.mxf", dir / "subs.mxf");
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
-       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(24, 1), 16 * 24, 0);
+       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(24, 1), 300 * 24, 0);
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        check_verify_result (
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        check_verify_result (
@@ -890,7 +891,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_language1)
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
        asset->_language = "wrong-andbad";
        asset->write (dir / "subs.mxf");
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
        asset->_language = "wrong-andbad";
        asset->write (dir / "subs.mxf");
-       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(24, 1), 16 * 24, 0);
+       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(24, 1), 300 * 24, 0);
        reel_asset->_language = "badlang";
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        reel_asset->_language = "badlang";
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
@@ -913,7 +914,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_language2)
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
        asset->_language = "wrong-andbad";
        asset->write (dir / "subs.mxf");
        auto asset = make_shared<dcp::SMPTESubtitleAsset>(dir / "subs.mxf");
        asset->_language = "wrong-andbad";
        asset->write (dir / "subs.mxf");
-       auto reel_asset = make_shared<dcp::ReelClosedCaptionAsset>(asset, dcp::Fraction(24, 1), 16 * 24, 0);
+       auto reel_asset = make_shared<dcp::ReelClosedCaptionAsset>(asset, dcp::Fraction(24, 1), 300 * 24, 0);
        reel_asset->_language = "badlang";
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        reel_asset->_language = "badlang";
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
@@ -1168,7 +1169,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_closed_caption_xml_size_in_bytes)
        }
        asset->set_language (dcp::LanguageTag("de-DE"));
        asset->write (dir / "subs.mxf");
        }
        asset->set_language (dcp::LanguageTag("de-DE"));
        asset->write (dir / "subs.mxf");
-       auto reel_asset = make_shared<dcp::ReelClosedCaptionAsset>(asset, dcp::Fraction(24, 1), 16 * 24, 0);
+       auto reel_asset = make_shared<dcp::ReelClosedCaptionAsset>(asset, dcp::Fraction(24, 1), 2049 * 24, 0);
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        check_verify_result (
        auto cpl = write_dcp_with_single_asset (dir, reel_asset);
 
        check_verify_result (
@@ -1637,6 +1638,29 @@ BOOST_AUTO_TEST_CASE (verify_valid_subtitle_duration)
 }
 
 
 }
 
 
+BOOST_AUTO_TEST_CASE (verify_subtitle_overlapping_reel_boundary)
+{
+       auto const dir = path("build/test/verify_subtitle_overlapping_reel_boundary");
+       prepare_directory (dir);
+       auto asset = make_shared<dcp::SMPTESubtitleAsset>();
+       asset->set_start_time (dcp::Time());
+       add_test_subtitle (asset, 0, 4 * 24);
+       asset->set_language (dcp::LanguageTag("de-DE"));
+       asset->write (dir / "subs.mxf");
+
+       auto reel_asset = make_shared<dcp::ReelSubtitleAsset>(asset, dcp::Fraction(24, 1), 3 * 24, 0);
+       auto cpl = write_dcp_with_single_asset (dir, reel_asset);
+       check_verify_result (
+               {dir},
+               {
+                       { dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME },
+                       { dcp::VerificationNote::Type::ERROR, dcp::VerificationNote::Code::SUBTITLE_OVERLAPS_REEL_BOUNDARY },
+                       { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_CPL_METADATA, cpl->id(), cpl->file().get() }
+               });
+
+}
+
+
 BOOST_AUTO_TEST_CASE (verify_invalid_subtitle_line_count1)
 {
        auto const dir = path ("build/test/invalid_subtitle_line_count1");
 BOOST_AUTO_TEST_CASE (verify_invalid_subtitle_line_count1)
 {
        auto const dir = path ("build/test/invalid_subtitle_line_count1");