Fix incorrect reading of markers from multi-reel DCPs (#3105).

4 files changed, 47 insertions, 14 deletions

diff --git a/src/lib/dcp_content.cc b/src/lib/dcp_content.cc
index 513ebef54..6bcb89a41 100644
--- a/src/lib/dcp_content.cc
+++ b/src/lib/dcp_content.cc
@@ -323,9 +323,7 @@ DCPContent::examine(shared_ptr<const Film> film, shared_ptr<Job> job, bool toler
 		_content_kind = examiner->content_kind ();
 		_cpl = examiner->cpl ();
 		_reel_lengths = examiner->reel_lengths ();
-		for (auto const& i: examiner->markers()) {
-			_markers[i.first] = ContentTime(i.second.as_editable_units_ceil(DCPTime::HZ));
-		}
+		_markers = examiner->markers();
 		_ratings = examiner->ratings ();
 		_content_versions = examiner->content_versions ();
 		_has_non_zero_entry_point = examiner->has_non_zero_entry_point();
diff --git a/src/lib/dcp_examiner.cc b/src/lib/dcp_examiner.cc
index 8e9586c52..59bd47702 100644
--- a/src/lib/dcp_examiner.cc
+++ b/src/lib/dcp_examiner.cc
@@ -136,6 +136,8 @@ DCPExaminer::DCPExaminer(shared_ptr<const DCPContent> content, bool tolerant)
 	LOG_GENERAL("Looking at {} reels", selected_cpl->reels().size());
 
 	int reel_index = 0;
+	dcpomatic::ContentTime reel_time;
+
 	for (auto reel: selected_cpl->reels()) {
 		LOG_GENERAL("Reel {}", reel->id());
 
@@ -272,8 +274,10 @@ DCPExaminer::DCPExaminer(shared_ptr<const DCPContent> content, bool tolerant)
 		read_closed_text(reel->closed_captions(), TextType::CLOSED_CAPTION, "caption", _dcp_caption_tracks);
 
 		if (reel->main_markers ()) {
-			auto rm = reel->main_markers()->get();
-			_markers.insert(rm.begin(), rm.end());
+			auto edit_rate = reel->main_markers()->edit_rate().numerator;
+			for (auto const& marker: reel->main_markers()->get()) {
+				_markers[marker.first] = reel_time + dcpomatic::ContentTime::from_frames(marker.second.as_editable_units_floor(edit_rate), edit_rate);
+			}
 		}
 
 		if (reel->atmos()) {
@@ -285,22 +289,27 @@ DCPExaminer::DCPExaminer(shared_ptr<const DCPContent> content, bool tolerant)
 			_atmos_edit_rate = reel->atmos()->edit_rate();
 		}
 
+		shared_ptr<dcp::ReelAsset> asset_determining_length;
+
 		if (reel->main_picture()) {
-			_reel_lengths.push_back(reel->main_picture()->actual_duration());
+			asset_determining_length = reel->main_picture();
 		} else if (reel->main_sound()) {
-			_reel_lengths.push_back(reel->main_sound()->actual_duration());
+			asset_determining_length = reel->main_sound();
 		} else if (reel->main_subtitle()) {
-			_reel_lengths.push_back(reel->main_subtitle()->actual_duration());
+			asset_determining_length = reel->main_subtitle();
 		} else if (reel->main_caption()) {
-			_reel_lengths.push_back(reel->main_caption()->actual_duration());
+			asset_determining_length = reel->main_caption();
 		} else if (!reel->closed_subtitles().empty()) {
-			_reel_lengths.push_back(reel->closed_subtitles().front()->actual_duration());
+			asset_determining_length = reel->closed_subtitles().front();
 		} else if (!reel->closed_captions().empty()) {
-			_reel_lengths.push_back(reel->closed_captions().front()->actual_duration());
+			asset_determining_length = reel->closed_captions().front();
 		} else if (!reel->atmos()) {
-			_reel_lengths.push_back(reel->atmos()->actual_duration());
+			asset_determining_length = reel->atmos();
 		}
 
+		_reel_lengths.push_back(asset_determining_length->actual_duration());
+
+		reel_time += dcpomatic::ContentTime::from_frames(asset_determining_length->actual_duration(), asset_determining_length->edit_rate().numerator);
 		++reel_index;
 	}
 
diff --git a/src/lib/dcp_examiner.h b/src/lib/dcp_examiner.h
index ca3cae76e..0a6045ed0 100644
--- a/src/lib/dcp_examiner.h
+++ b/src/lib/dcp_examiner.h
@@ -163,7 +163,8 @@ public:
 		return _reel_lengths;
 	}
 
-	std::map<dcp::Marker, dcp::Time> markers() const {
+	/** @return DCP markers, with times offset from the start of the DCP */
+	std::map<dcp::Marker, dcpomatic::ContentTime> markers() const {
 		return _markers;
 	}
 
@@ -224,7 +225,7 @@ private:
 	boost::optional<dcp::ContentKind> _content_kind;
 	std::string _cpl;
 	std::list<int64_t> _reel_lengths;
-	std::map<dcp::Marker, dcp::Time> _markers;
+	std::map<dcp::Marker, dcpomatic::ContentTime> _markers;
 	std::vector<dcp::Rating> _ratings;
 	std::vector<std::string> _content_versions;
 	bool _has_atmos = false;
diff --git a/test/copy_dcp_details_to_film_test.cc b/test/copy_dcp_details_to_film_test.cc
index 6dd913aa0..a46a03d0c 100644
--- a/test/copy_dcp_details_to_film_test.cc
+++ b/test/copy_dcp_details_to_film_test.cc
@@ -29,6 +29,8 @@
 
 
 using std::make_shared;
+using std::shared_ptr;
+using std::vector;
 
 
 BOOST_AUTO_TEST_CASE(copy_audio_language_to_film)
@@ -50,3 +52,26 @@ BOOST_AUTO_TEST_CASE(copy_audio_language_to_film)
 	BOOST_CHECK_EQUAL(film2->audio_language()->as_string(), "de-DE");
 }
 
+
+BOOST_AUTO_TEST_CASE(test_copy_dcp_markers_to_film)
+{
+	auto video = vector<shared_ptr<Content>>{
+		content_factory("test/data/flat_red.png")[0],
+		content_factory("test/data/flat_red.png")[0],
+		content_factory("test/data/flat_red.png")[0]
+	};
+
+	auto film = new_test_film("test_copy_dcp_markers_to_film", video);
+	film->set_reel_type(ReelType::BY_VIDEO_CONTENT);
+	film->set_marker(dcp::Marker::FFEC, dcpomatic::DCPTime::from_seconds(22));
+	make_and_verify_dcp(film);
+
+	auto dcp = make_shared<DCPContent>(film->dir(film->dcp_name()));
+
+	auto film2 = new_test_film("test_copy_dcp_markers_to_film2", { dcp });
+	copy_dcp_markers_to_film(dcp, film2);
+
+	BOOST_CHECK(film2->marker(dcp::Marker::FFEC) == dcpomatic::DCPTime::from_seconds(22));
+}
+
+