Fix incorrect coalesce() output when one input range is wholly

covered by another.

diff --git a/src/lib/dcpomatic_time_coalesce.h b/src/lib/dcpomatic_time_coalesce.h
index e6e16641ef7e65091993dbe1d4e1b892fac0eeae..56f82bcb680932b1bb8ff86a263dddf594093892 100644 (file)
--- a/src/lib/dcpomatic_time_coalesce.h
+++ b/src/lib/dcpomatic_time_coalesce.h
@@ -32,11 +32,11 @@ std::list<TimePeriod<T> > coalesce (std::list<TimePeriod<T> > periods)
        do {
                coalesced.clear ();
                did_something = false;
        do {
                coalesced.clear ();
                did_something = false;

-               for (typename std::list<TimePeriod<T> >::const_iterator i = periods.begin(); i != periods.end(); ++i) {
-                       typename std::list<TimePeriod<T> >::const_iterator j = i;
+               for (auto i = periods.begin(); i != periods.end(); ++i) {
+                       auto j = i;

                        ++j;
                        if (j != periods.end() && (i->overlap(*j) || i->to == j->from)) {
                        ++j;
                        if (j != periods.end() && (i->overlap(*j) || i->to == j->from)) {

-                               coalesced.push_back (TimePeriod<T> (i->from, j->to));
+                               coalesced.push_back(TimePeriod<T>(std::min(i->from, j->from), std::max(i->to, j->to)));

                                did_something = true;
                                ++i;
                        } else {
                                did_something = true;
                                ++i;
                        } else {

diff --git a/src/lib/empty.h b/src/lib/empty.h
index 2a975562a4b2a8a30490e1d1f89fa9f3762bffd3..443d3322b8290360117c4ab98f48c62e14d0265b 100644 (file)
--- a/src/lib/empty.h
+++ b/src/lib/empty.h
@@ -29,6 +29,7 @@
 struct empty_test1;
 struct empty_test2;
 struct empty_test3;
 struct empty_test1;
 struct empty_test2;
 struct empty_test3;

+struct empty_test_with_overlapping_content;

 struct player_subframe_test;
 
 class Empty
 struct player_subframe_test;
 
 class Empty


@@ -51,6 +52,7 @@ private:
        friend struct ::empty_test1;
        friend struct ::empty_test2;
        friend struct ::empty_test3;
        friend struct ::empty_test1;
        friend struct ::empty_test2;
        friend struct ::empty_test3;

+       friend struct ::empty_test_with_overlapping_content;

        friend struct ::player_subframe_test;
 
        std::list<dcpomatic::DCPTimePeriod> _periods;
        friend struct ::player_subframe_test;
 
        std::list<dcpomatic::DCPTimePeriod> _periods;

diff --git a/test/dcpomatic_time_test.cc b/test/dcpomatic_time_test.cc
index e4383b5396ab8fde9f9955ce3a86be58823f81ff..5d23f247808135eeadfc94ad17451ffb7b42db45 100644 (file)
--- a/test/dcpomatic_time_test.cc
+++ b/test/dcpomatic_time_test.cc
@@ -302,6 +302,18 @@ BOOST_AUTO_TEST_CASE (dcpomatic_time_period_coalesce_test5)
        BOOST_CHECK (q.back()  == DCPTimePeriod(DCPTime(100), DCPTime(106)));
 }
 
        BOOST_CHECK (q.back()  == DCPTimePeriod(DCPTime(100), DCPTime(106)));
 }
 

+BOOST_AUTO_TEST_CASE (test_coalesce_with_overlapping_periods)
+{
+       DCPTimePeriod A (DCPTime(0), DCPTime(10));
+       DCPTimePeriod B (DCPTime(2), DCPTime(8));
+       list<DCPTimePeriod> p;
+       p.push_back (A);
+       p.push_back (B);
+       auto q = coalesce(p);
+       BOOST_REQUIRE_EQUAL (q.size(), 1U);
+       BOOST_CHECK (q.front() == DCPTimePeriod(DCPTime(0), DCPTime(10)));
+}
+

 /* Straightforward test of DCPTime::ceil */
 BOOST_AUTO_TEST_CASE (dcpomatic_time_ceil_test)
 {
 /* Straightforward test of DCPTime::ceil */
 BOOST_AUTO_TEST_CASE (dcpomatic_time_ceil_test)
 {

diff --git a/test/empty_test.cc b/test/empty_test.cc
index 9f55499da58e8f2db138edecee46dccca1200ab3..8a42bd6b8d3e840f73be13dd809d0af59edebd85 100644 (file)
--- a/test/empty_test.cc
+++ b/test/empty_test.cc
@@ -35,6 +35,7 @@
 #include <boost/test/unit_test.hpp>
 
 using std::list;
 #include <boost/test/unit_test.hpp>
 
 using std::list;

+using std::make_shared;

 using std::shared_ptr;
 #if BOOST_VERSION >= 106100
 using namespace boost::placeholders;
 using std::shared_ptr;
 #if BOOST_VERSION >= 106100
 using namespace boost::placeholders;


@@ -149,3 +150,26 @@ BOOST_AUTO_TEST_CASE (empty_test3)
        BOOST_CHECK (black.position() == DCPTime::from_frames(0, vfr));
 }
 
        BOOST_CHECK (black.position() == DCPTime::from_frames(0, vfr));
 }
 

+BOOST_AUTO_TEST_CASE (empty_test_with_overlapping_content)
+{
+       auto film = new_test_film2 ("empty_test_with_overlapping_content");
+       film->set_sequence (false);
+       auto contentA = make_shared<ImageContent>("test/data/simple_testcard_640x480.png");
+       auto contentB = make_shared<ImageContent>("test/data/simple_testcard_640x480.png");
+
+       film->examine_and_add_content (contentA);
+       film->examine_and_add_content (contentB);
+       BOOST_REQUIRE (!wait_for_jobs());
+
+       int const vfr = film->video_frame_rate ();
+
+       contentA->video->set_length (vfr * 3);
+       contentA->set_position (film, DCPTime());
+       contentB->video->set_length (vfr * 1);
+       contentB->set_position (film, DCPTime::from_seconds(1));
+
+       Empty black(film, film->playlist(), bind(&has_video, _1), film->playlist()->length(film));
+
+       BOOST_REQUIRE (black._periods.empty());
+}
+