Fix Empty/Player behaviour when using a playlist that is not the same as the Film's.

Previously Empty would use the length of the film for its end point.
Now it takes a Playlist (rather than a list of Pieces) and uses the length
of that playlist for its end point.

This fixes #1543, in which single-content audio analysis jobs would run for the whole
length of the film, rather than the length of the content, producing
strange graphs and incorrect progress reports.

diff --git a/src/lib/empty.cc b/src/lib/empty.cc
index 206acfdb0a67d62131bb11c688906e68f01b49e2..71bf3aa956072d27aa9f50bd40f14e6a971395d8 100644 (file)
--- a/src/lib/empty.cc
+++ b/src/lib/empty.cc
@@ -36,16 +36,16 @@ using boost::dynamic_pointer_cast;
 using boost::function;
 using namespace dcpomatic;
 
-Empty::Empty (shared_ptr<const Film> film, list<shared_ptr<Piece> > pieces, function<bool (shared_ptr<Piece>)> part)
+Empty::Empty (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist, function<bool (shared_ptr<const Content>)> part)
 {
        list<DCPTimePeriod> full;
-       BOOST_FOREACH (shared_ptr<Piece> i, pieces) {
+       BOOST_FOREACH (shared_ptr<Content> i, playlist->content()) {
                if (part(i)) {
-                       full.push_back (DCPTimePeriod (i->content->position(), i->content->end(film)));
+                       full.push_back (DCPTimePeriod (i->position(), i->end(film)));
                }
        }
 
-       _periods = subtract (DCPTimePeriod(DCPTime(), film->length()), coalesce(full));
+       _periods = subtract (DCPTimePeriod(DCPTime(), playlist->length(film)), coalesce(full));
 
        if (!_periods.empty ()) {
                _position = _periods.front().from;

diff --git a/src/lib/empty.h b/src/lib/empty.h
index bd295206e3b4cdea8a691e56230a872b3b87d1c3..abd6fdef003bb63a875c58465a241bda9d8ba822 100644 (file)
--- a/src/lib/empty.h
+++ b/src/lib/empty.h
@@ -28,14 +28,14 @@
 
 struct empty_test1;
 struct empty_test2;
+struct empty_test3;
 struct player_subframe_test;
-class Piece;
 
 class Empty
 {
 public:
        Empty () {}
-       Empty (boost::shared_ptr<const Film> film, std::list<boost::shared_ptr<Piece> > pieces, boost::function<bool (boost::shared_ptr<Piece>)> part);
+       Empty (boost::shared_ptr<const Film> film, boost::shared_ptr<const Playlist> playlist, boost::function<bool (boost::shared_ptr<const Content>)> part);
 
        dcpomatic::DCPTime position () const {
                return _position;
@@ -50,6 +50,7 @@ public:
 private:
        friend struct ::empty_test1;
        friend struct ::empty_test2;
+       friend struct ::empty_test3;
        friend struct ::player_subframe_test;
 
        std::list<dcpomatic::DCPTimePeriod> _periods;

diff --git a/src/lib/player.cc b/src/lib/player.cc
index d5a558184196100a6ebc94eb660e6b30e0b186f5..3202c1b85dab30e7b9a0721abf983211402a2aba 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -125,15 +125,15 @@ Player::setup_pieces ()
 }
 
 bool
-have_video (shared_ptr<Piece> piece)
+have_video (shared_ptr<const Content> content)
 {
-       return piece->decoder && piece->decoder->video;
+       return static_cast<bool>(content->video);
 }
 
 bool
-have_audio (shared_ptr<Piece> piece)
+have_audio (shared_ptr<const Content> content)
 {
-       return piece->decoder && piece->decoder->audio;
+       return static_cast<bool>(content->audio);
 }
 
 void
@@ -237,8 +237,8 @@ Player::setup_pieces_unlocked ()
                }
        }
 
-       _black = Empty (_film, _pieces, bind(&have_video, _1));
-       _silent = Empty (_film, _pieces, bind(&have_audio, _1));
+       _black = Empty (_film, _playlist, bind(&have_video, _1));
+       _silent = Empty (_film, _playlist, bind(&have_audio, _1));
 
        _last_video_time = DCPTime ();
        _last_video_eyes = EYES_BOTH;

diff --git a/src/wx/audio_dialog.h b/src/wx/audio_dialog.h
index e866aca3f83a8823d0a52e76edd8e1a07440782d..3a02fd87fdb2c0a8da0fa1d596bc6140c9475cf6 100644 (file)
--- a/src/wx/audio_dialog.h
+++ b/src/wx/audio_dialog.h
@@ -49,6 +49,7 @@ private:
 
        boost::shared_ptr<AudioAnalysis> _analysis;
        boost::weak_ptr<Film> _film;
+       /** content to analyse, or 0 to analyse all the film's content */
        boost::weak_ptr<Content> _content;
        int _channels;
        boost::shared_ptr<const Playlist> _playlist;

diff --git a/test/empty_test.cc b/test/empty_test.cc
index f0fa7138f46408a456e7db54df24fdbfdb29c1c8..1a4d03300bf5a81740f7d1f987bb917a053c40d8 100644 (file)
--- a/test/empty_test.cc
+++ b/test/empty_test.cc
@@ -34,17 +34,14 @@ using boost::shared_ptr;
 using namespace dcpomatic;
 
 bool
-has_video (shared_ptr<Piece> piece)
+has_video (shared_ptr<const Content> content)
 {
-        return piece->decoder && piece->decoder->video;
+        return static_cast<bool>(content->video);
 }
 
 BOOST_AUTO_TEST_CASE (empty_test1)
 {
-       shared_ptr<Film> film = new_test_film ("empty_test1");
-       film->set_dcp_content_type (DCPContentType::from_isdcf_name ("FTR"));
-       film->set_name ("empty_test1");
-       film->set_container (Ratio::from_id ("185"));
+       shared_ptr<Film> film = new_test_film2 ("empty_test1");
        film->set_sequence (false);
        shared_ptr<ImageContent> contentA (new ImageContent("test/data/simple_testcard_640x480.png"));
        shared_ptr<ImageContent> contentB (new ImageContent("test/data/simple_testcard_640x480.png"));
@@ -55,6 +52,9 @@ BOOST_AUTO_TEST_CASE (empty_test1)
 
        int const vfr = film->video_frame_rate ();
 
+       /* 0 1 2 3 4 5 6 7
+        *     A A A     B
+        */
        contentA->video->set_scale (VideoContentScale (Ratio::from_id ("185")));
        contentA->video->set_length (3);
        contentA->set_position (film, DCPTime::from_frames (2, vfr));
@@ -62,27 +62,20 @@ BOOST_AUTO_TEST_CASE (empty_test1)
        contentB->video->set_length (1);
        contentB->set_position (film, DCPTime::from_frames (7, vfr));
 
-       shared_ptr<Player> player (new Player(film, film->playlist()));
-       Empty black (film, player->_pieces, bind(&has_video, _1));
-       BOOST_REQUIRE_EQUAL (black._periods.size(), 3);
+       Empty black (film, film->playlist(), bind(&has_video, _1));
+       BOOST_REQUIRE_EQUAL (black._periods.size(), 2);
        list<dcpomatic::DCPTimePeriod>::const_iterator i = black._periods.begin();
-       BOOST_CHECK (i->from == DCPTime());
-       BOOST_CHECK (i->to == DCPTime::from_frames(2, vfr));
+       BOOST_CHECK (i->from == DCPTime::from_frames(0, vfr));
+       BOOST_CHECK (i->to ==   DCPTime::from_frames(2, vfr));
        ++i;
        BOOST_CHECK (i->from == DCPTime::from_frames(5, vfr));
-       BOOST_CHECK (i->to == DCPTime::from_frames(7, vfr));
-       ++i;
-       BOOST_CHECK (i->from == DCPTime::from_frames(8, vfr));
-       BOOST_CHECK (i->to == DCPTime::from_frames(24, vfr));
+       BOOST_CHECK (i->to ==   DCPTime::from_frames(7, vfr));
 }
 
 /** Some tests where the first empty period is not at time 0 */
 BOOST_AUTO_TEST_CASE (empty_test2)
 {
-       shared_ptr<Film> film = new_test_film ("empty_test1");
-       film->set_dcp_content_type (DCPContentType::from_isdcf_name ("FTR"));
-       film->set_name ("empty_test1");
-       film->set_container (Ratio::from_id ("185"));
+       shared_ptr<Film> film = new_test_film2 ("empty_test2");
        film->set_sequence (false);
        shared_ptr<ImageContent> contentA (new ImageContent("test/data/simple_testcard_640x480.png"));
        shared_ptr<ImageContent> contentB (new ImageContent("test/data/simple_testcard_640x480.png"));
@@ -93,6 +86,9 @@ BOOST_AUTO_TEST_CASE (empty_test2)
 
        int const vfr = film->video_frame_rate ();
 
+       /* 0 1 2 3 4 5 6 7
+        * A A A         B
+        */
        contentA->video->set_scale (VideoContentScale (Ratio::from_id ("185")));
        contentA->video->set_length (3);
        contentA->set_position (film, DCPTime(0));
@@ -100,9 +96,8 @@ BOOST_AUTO_TEST_CASE (empty_test2)
        contentB->video->set_length (1);
        contentB->set_position (film, DCPTime::from_frames(7, vfr));
 
-       shared_ptr<Player> player (new Player(film, film->playlist()));
-       Empty black (film, player->_pieces, bind(&has_video, _1));
-       BOOST_REQUIRE_EQUAL (black._periods.size(), 2);
+       Empty black (film, film->playlist(), bind(&has_video, _1));
+       BOOST_REQUIRE_EQUAL (black._periods.size(), 1);
        BOOST_CHECK (black._periods.front().from == DCPTime::from_frames(3, vfr));
        BOOST_CHECK (black._periods.front().to == DCPTime::from_frames(7, vfr));
 
@@ -114,7 +109,41 @@ BOOST_AUTO_TEST_CASE (empty_test2)
        black.set_position (DCPTime::from_frames (4, vfr));
        BOOST_CHECK (!black.done ());
        black.set_position (DCPTime::from_frames (7, vfr));
-       BOOST_CHECK (!black.done ());
-       black.set_position (DCPTime::from_frames (24, vfr));
        BOOST_CHECK (black.done ());
 }
+
+/** Test for when the film's playlist is not the same as the one passed into Empty */
+BOOST_AUTO_TEST_CASE (empty_test3)
+{
+       shared_ptr<Film> film = new_test_film2 ("empty_test3");
+       film->set_sequence (false);
+       shared_ptr<ImageContent> contentA (new ImageContent("test/data/simple_testcard_640x480.png"));
+       shared_ptr<ImageContent> contentB (new 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 ();
+
+       /* 0 1 2 3 4 5 6 7
+        * A A A         B
+        */
+       contentA->video->set_scale (VideoContentScale (Ratio::from_id ("185")));
+       contentA->video->set_length (3);
+       contentA->set_position (film, DCPTime(0));
+       contentB->video->set_scale (VideoContentScale (Ratio::from_id ("185")));
+       contentB->video->set_length (1);
+       contentB->set_position (film, DCPTime::from_frames(7, vfr));
+
+       shared_ptr<Playlist> playlist (new Playlist);
+       playlist->add (film, contentB);
+       Empty black (film, playlist, bind(&has_video, _1));
+       BOOST_REQUIRE_EQUAL (black._periods.size(), 1);
+       BOOST_CHECK (black._periods.front().from == DCPTime::from_frames(0, vfr));
+       BOOST_CHECK (black._periods.front().to == DCPTime::from_frames(7, vfr));
+
+       /* position should initially be the start of the first empty period */
+       BOOST_CHECK (black.position() == DCPTime::from_frames(0, vfr));
+}
+