From 37a485b143da0d096aa617026cf88d32afb59a34 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 31 Oct 2019 21:45:41 +0100 Subject: [PATCH] Make separate reels for parts of the timeline with no video when we are in REEL_TYPE_BY_VIDEO_CONTENT mode. This fixes VF creation with gaps. Also the implementation of Film::reels() is cleaner now. Backport of 122bea7f0e08e07dcdaccd51751a9c83504f4c04 from master. --- src/lib/film.cc | 32 ++++++++++++++++---------------- src/lib/reel_writer.cc | 2 ++ test/reels_test.cc | 41 +++++++++++++++++++++++++++++++++++++++++ test/vf_test.cc | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 16 deletions(-) diff --git a/src/lib/film.cc b/src/lib/film.cc index d311c76cb..9f909633a 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -1563,29 +1563,29 @@ Film::reels () const break; case REELTYPE_BY_VIDEO_CONTENT: { - DCPTime last_split; - shared_ptr last_video; - BOOST_FOREACH (shared_ptr c, content ()) { + /* Collect all reel boundaries */ + list split_points; + split_points.push_back (DCPTime()); + split_points.push_back (len); + BOOST_FOREACH (shared_ptr c, content()) { if (c->video) { BOOST_FOREACH (DCPTime t, c->reel_split_points(shared_from_this())) { - if (last_split != t) { - p.push_back (DCPTimePeriod(last_split, t)); - last_split = t; - } + split_points.push_back (t); } - last_video = c; + split_points.push_back (c->end(shared_from_this())); } } - DCPTime video_end = last_video ? last_video->end(shared_from_this()) : DCPTime(0); - if (last_split != video_end) { - /* Definitely go from the last split to the end of the video content */ - p.push_back (DCPTimePeriod(last_split, video_end)); - } + split_points.sort (); + split_points.unique (); - if (video_end < len) { - /* And maybe go after that as well if there is any non-video hanging over the end */ - p.push_back (DCPTimePeriod (video_end, len)); + /* Make them into periods */ + optional last; + BOOST_FOREACH (DCPTime t, split_points) { + if (last) { + p.push_back (DCPTimePeriod(*last, t)); + } + last = t; } break; } diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc index 67606cee2..8044973ec 100644 --- a/src/lib/reel_writer.cc +++ b/src/lib/reel_writer.cc @@ -439,6 +439,8 @@ maybe_add_text ( shared_ptr ReelWriter::create_reel (list const & refs, list > const & fonts) { + LOG_GENERAL ("create_reel for %1-%2; %3 of %4", _period.from.get(), _period.to.get(), _reel_index, _reel_count); + shared_ptr reel (new dcp::Reel ()); shared_ptr reel_picture_asset; diff --git a/test/reels_test.cc b/test/reels_test.cc index e26dea5b8..45c87654d 100644 --- a/test/reels_test.cc +++ b/test/reels_test.cc @@ -440,3 +440,44 @@ BOOST_AUTO_TEST_CASE (reels_test11) BOOST_CHECK_EQUAL (r.back().from.get(), DCPTime::from_seconds(1).get()); BOOST_CHECK_EQUAL (r.back().to.get(), DCPTime::from_seconds(1 + 10).get()); } + +/** For VFs to work right we have to make separate reels for empty bits between + * video content. + */ +BOOST_AUTO_TEST_CASE (reels_test12) +{ + shared_ptr film = new_test_film2 ("reels_test12"); + film->set_video_frame_rate (24); + film->set_reel_type (REELTYPE_BY_VIDEO_CONTENT); + film->set_sequence (false); + + shared_ptr A(new FFmpegContent("test/data/flat_red.png")); + film->examine_and_add_content (A); + BOOST_REQUIRE (!wait_for_jobs()); + A->video->set_length (240); + A->set_video_frame_rate (24); + A->set_position (film, DCPTime::from_seconds(1)); + + shared_ptr B(new FFmpegContent("test/data/flat_red.png")); + film->examine_and_add_content (B); + BOOST_REQUIRE (!wait_for_jobs()); + B->video->set_length (120); + B->set_video_frame_rate (24); + B->set_position (film, DCPTime::from_seconds(14)); + + list r = film->reels (); + BOOST_REQUIRE_EQUAL (r.size(), 4); + list::const_iterator i = r.begin (); + + BOOST_CHECK_EQUAL (i->from.get(), 0); + BOOST_CHECK_EQUAL (i->to.get(), DCPTime::from_seconds(1).get()); + ++i; + BOOST_CHECK_EQUAL (i->from.get(), DCPTime::from_seconds(1).get()); + BOOST_CHECK_EQUAL (i->to.get(), DCPTime::from_seconds(11).get()); + ++i; + BOOST_CHECK_EQUAL (i->from.get(), DCPTime::from_seconds(11).get()); + BOOST_CHECK_EQUAL (i->to.get(), DCPTime::from_seconds(14).get()); + ++i; + BOOST_CHECK_EQUAL (i->from.get(), DCPTime::from_seconds(14).get()); + BOOST_CHECK_EQUAL (i->to.get(), DCPTime::from_seconds(19).get()); +} diff --git a/test/vf_test.cc b/test/vf_test.cc index b82c3c889..d1879f4e3 100644 --- a/test/vf_test.cc +++ b/test/vf_test.cc @@ -336,3 +336,39 @@ BOOST_AUTO_TEST_CASE (vf_test6) vf->make_dcp (); BOOST_REQUIRE (!wait_for_jobs()); } + +/** Test bug #1643 (the second part; referring fails if there are gaps) */ +BOOST_AUTO_TEST_CASE (vf_test7) +{ + /* First OV */ + shared_ptr ov1 = new_test_film2 ("vf_test7_ov1"); + ov1->set_video_frame_rate (24); + ov1->examine_and_add_content (content_factory("test/data/flat_red.png").front()); + BOOST_REQUIRE (!wait_for_jobs()); + ov1->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); + + /* Second OV */ + shared_ptr ov2 = new_test_film2 ("vf_test7_ov2"); + ov2->set_video_frame_rate (24); + ov2->examine_and_add_content (content_factory("test/data/flat_red.png").front()); + BOOST_REQUIRE (!wait_for_jobs()); + ov2->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); + + /* VF */ + shared_ptr vf = new_test_film2 ("vf_test7_vf"); + shared_ptr ov1_dcp (new DCPContent(ov1->dir(ov1->dcp_name()))); + vf->examine_and_add_content (ov1_dcp); + shared_ptr ov2_dcp (new DCPContent(ov1->dir(ov1->dcp_name()))); + vf->examine_and_add_content (ov2_dcp); + BOOST_REQUIRE (!wait_for_jobs()); + vf->set_reel_type (REELTYPE_BY_VIDEO_CONTENT); + ov1_dcp->set_reference_video (true); + ov2_dcp->set_reference_video (true); + ov1_dcp->set_position (vf, DCPTime::from_seconds(1)); + ov2_dcp->set_position (vf, DCPTime::from_seconds(20)); + vf->write_metadata (); + vf->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); +} -- 2.30.2