In general the player assumes that it won't receive out of order video.

This clearly can happen with separate L/R sources.  A pass in L might
emit two frames which means the arrivals can't possibly be in order.

This commit fixes this by introducing a Shuffler which all alternate-3D
sources send their video to.  The Shuffler re-orders things before they
arrive at the player.

It also fixes the code which inserts video frames before one that arrives
after a gap.  This didn't cope with 3D right before.

The audio code solves a similar (perhaps the same?) problem with the
AudioMerger; perhaps we should have a similar thing for video and make
the player emit complete 3D frames.

Should help with #976.

diff --git a/src/lib/piece.h b/src/lib/piece.h
index 711e292eed451bd7961b8d11d3fa5111a83bdb40..440beecf4bd6af079bd5b552fae0fddd3ead7a44 100644 (file)
--- a/src/lib/piece.h
+++ b/src/lib/piece.h
@@ -22,6 +22,7 @@
 #define DCPOMATIC_PIECE_H
 
 #include "types.h"
+#include "frame_rate_change.h"
 
 class Content;
 class Decoder;

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 5539217264353bf8cbdcb14465938cd334aeb117..522b8304f533aa6674fd02fb1ccef6da3fb43bdb 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2017 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -48,6 +48,7 @@
 #include "dcp_decoder.h"
 #include "image_decoder.h"
 #include "compose.hpp"
+#include "shuffler.h"
 #include <dcp/reel.h>
 #include <dcp/reel_sound_asset.h>
 #include <dcp/reel_subtitle_asset.h>
@@ -87,6 +88,7 @@ Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist
        , _fast (false)
        , _play_referenced (false)
        , _audio_merger (_film->audio_frame_rate())
+       , _shuffler (0)
 {
        _film_changed_connection = _film->Changed.connect (bind (&Player::film_changed, this, _1));
        _playlist_changed_connection = _playlist->Changed.connect (bind (&Player::playlist_changed, this));
@@ -98,11 +100,20 @@ Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist
        seek (DCPTime (), true);
 }
 
+Player::~Player ()
+{
+       delete _shuffler;
+}
+
 void
 Player::setup_pieces ()
 {
        _pieces.clear ();
 
+       delete _shuffler;
+       _shuffler = new Shuffler();
+       _shuffler->Video.connect(bind(&Player::video, this, _1, _2));
+
        BOOST_FOREACH (shared_ptr<Content> i, _playlist->content ()) {
 
                if (!i->paths_valid ()) {
@@ -137,7 +148,11 @@ Player::setup_pieces ()
                _pieces.push_back (piece);
 
                if (decoder->video) {
-                       decoder->video->Data.connect (bind (&Player::video, this, weak_ptr<Piece> (piece), _1));
+                       if (i->video->frame_type() == VIDEO_FRAME_TYPE_3D_LEFT || i->video->frame_type() == VIDEO_FRAME_TYPE_3D_RIGHT) {
+                               decoder->video->Data.connect (bind (&Shuffler::video, _shuffler, weak_ptr<Piece>(piece), _1));
+                       } else {
+                               decoder->video->Data.connect (bind (&Player::video, this, weak_ptr<Piece>(piece), _1));
+                       }
                }
 
                if (decoder->audio) {
@@ -164,6 +179,7 @@ Player::setup_pieces ()
        _silent = Empty (_film->content(), _film->length(), bind(&Content::audio, _1));
 
        _last_video_time = DCPTime ();
+       _last_video_eyes = EYES_BOTH;
        _last_audio_time = DCPTime ();
        _have_valid_pieces = true;
 }
@@ -309,7 +325,7 @@ Player::transform_image_subtitles (list<ImageSubtitle> subs) const
 }
 
 shared_ptr<PlayerVideo>
-Player::black_player_video_frame () const
+Player::black_player_video_frame (Eyes eyes) const
 {
        return shared_ptr<PlayerVideo> (
                new PlayerVideo (
@@ -318,7 +334,7 @@ Player::black_player_video_frame () const
                        optional<double> (),
                        _video_container_size,
                        _video_container_size,
-                       EYES_BOTH,
+                       eyes,
                        PART_WHOLE,
                        PresetColourConversion::all().front().conversion
                )
@@ -516,7 +532,7 @@ Player::pass ()
 
        if (_playlist->length() == DCPTime()) {
                /* Special case of an empty Film; just give one black frame */
-               emit_video (black_player_video_frame(), DCPTime());
+               emit_video (black_player_video_frame(EYES_BOTH), DCPTime());
                return true;
        }
 
@@ -534,6 +550,7 @@ Player::pass ()
                if (t > i->content->end()) {
                        i->done = true;
                } else {
+
                        /* Given two choices at the same time, pick the one with a subtitle so we see it before
                           the video.
                        */
@@ -572,7 +589,7 @@ Player::pass ()
                earliest_content->done = earliest_content->decoder->pass ();
                break;
        case BLACK:
-               emit_video (black_player_video_frame(), _black.position());
+               emit_video (black_player_video_frame(EYES_BOTH), _black.position());
                _black.set_position (_black.position() + one_video_frame());
                break;
        case SILENT:
@@ -622,6 +639,9 @@ Player::pass ()
                emit_audio (i->first, i->second);
        }
 
+       if (done) {
+               _shuffler->flush ();
+       }
        return done;
 }
 
@@ -677,14 +697,33 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
        /* Fill gaps that we discover now that we have some video which needs to be emitted */
 
        if (_last_video_time) {
-               /* XXX: this may not work for 3D */
                DCPTime fill_from = max (*_last_video_time, piece->content->position());
-               for (DCPTime j = fill_from; j < time; j += one_video_frame()) {
-                       LastVideoMap::const_iterator k = _last_video.find (wp);
-                       if (k != _last_video.end ()) {
-                               emit_video (k->second, j);
-                       } else {
-                               emit_video (black_player_video_frame(), j);
+               LastVideoMap::const_iterator last = _last_video.find (wp);
+               if (_film->three_d()) {
+                       DCPTime j = fill_from;
+                       Eyes eyes = _last_video_eyes.get_value_or(EYES_LEFT);
+                       if (eyes == EYES_BOTH) {
+                               eyes = EYES_LEFT;
+                       }
+                       while (j < time || eyes != video.eyes) {
+                               if (last != _last_video.end()) {
+                                       last->second->set_eyes (eyes);
+                                       emit_video (last->second, j);
+                               } else {
+                                       emit_video (black_player_video_frame(eyes), j);
+                               }
+                               if (eyes == EYES_RIGHT) {
+                                       j += one_video_frame();
+                               }
+                               eyes = increment_eyes (eyes);
+                       }
+               } else {
+                       for (DCPTime j = fill_from; j < time; j += one_video_frame()) {
+                               if (last != _last_video.end()) {
+                                       emit_video (last->second, j);
+                               } else {
+                                       emit_video (black_player_video_frame(EYES_BOTH), j);
+                               }
                        }
                }
        }
@@ -872,6 +911,10 @@ Player::seek (DCPTime time, bool accurate)
                setup_pieces ();
        }
 
+       if (_shuffler) {
+               _shuffler->clear ();
+       }
+
        if (_audio_processor) {
                _audio_processor->flush ();
        }
@@ -896,9 +939,11 @@ Player::seek (DCPTime time, bool accurate)
 
        if (accurate) {
                _last_video_time = time;
+               _last_video_eyes = EYES_LEFT;
                _last_audio_time = time;
        } else {
                _last_video_time = optional<DCPTime>();
+               _last_video_eyes = optional<Eyes>();
                _last_audio_time = optional<DCPTime>();
        }
 
@@ -922,6 +967,7 @@ Player::emit_video (shared_ptr<PlayerVideo> pv, DCPTime time)
                _last_video_time = time + one_video_frame();
                _active_subtitles.clear_before (time);
        }
+       _last_video_eyes = increment_eyes (pv->eyes());
 }
 
 void

diff --git a/src/lib/player.h b/src/lib/player.h
index 7cc3b0ef704ded47bd596fbb21cc3a30eb1746d6..0b8540c15e046c783ac995acd814352a4a3d999b 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -46,6 +46,7 @@ class Playlist;
 class Font;
 class AudioBuffers;
 class ReferencedReelAsset;
+class Shuffler;
 
 /** @class Player
  *  @brief A class which can `play' a Playlist.
@@ -54,6 +55,7 @@ class Player : public boost::enable_shared_from_this<Player>, public boost::nonc
 {
 public:
        Player (boost::shared_ptr<const Film>, boost::shared_ptr<const Playlist> playlist);
+       ~Player ();
 
        bool pass ();
        void seek (DCPTime time, bool accurate);
@@ -105,7 +107,7 @@ private:
        DCPTime resampled_audio_to_dcp (boost::shared_ptr<const Piece> piece, Frame f) const;
        ContentTime dcp_to_content_time (boost::shared_ptr<const Piece> piece, DCPTime t) const;
        DCPTime content_time_to_dcp (boost::shared_ptr<const Piece> piece, ContentTime t) const;
-       boost::shared_ptr<PlayerVideo> black_player_video_frame () const;
+       boost::shared_ptr<PlayerVideo> black_player_video_frame (Eyes eyes) const;
        void video (boost::weak_ptr<Piece>, ContentVideo);
        void audio (boost::weak_ptr<Piece>, AudioStreamPtr, ContentAudio);
        void image_subtitle_start (boost::weak_ptr<Piece>, ContentImageSubtitle);
@@ -146,6 +148,7 @@ private:
 
        /** Time just after the last video frame we emitted, or the time of the last accurate seek */
        boost::optional<DCPTime> _last_video_time;
+       boost::optional<Eyes> _last_video_eyes;
        /** Time just after the last audio frame we emitted, or the time of the last accurate seek */
        boost::optional<DCPTime> _last_audio_time;
 
@@ -155,6 +158,7 @@ private:
        LastVideoMap _last_video;
 
        AudioMerger _audio_merger;
+       Shuffler* _shuffler;
 
        class StreamState
        {

diff --git a/src/lib/shuffler.cc b/src/lib/shuffler.cc
new file mode 100644 (file)
index 0000000..997d91f
--- /dev/null
+++ b/src/lib/shuffler.cc
@@ -0,0 +1,93 @@
+/*
+    Copyright (C) 2018 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#include "shuffler.h"
+#include "content_video.h"
+#include "dcpomatic_assert.h"
+#include <boost/foreach.hpp>
+#include <iostream>
+
+using std::make_pair;
+using boost::weak_ptr;
+using boost::shared_ptr;
+using boost::optional;
+
+struct Comparator
+{
+       bool operator()(Shuffler::Store const & a, Shuffler::Store const & b) {
+               if (a.second.frame != b.second.frame) {
+                       return a.second.frame < b.second.frame;
+               }
+               return a.second.eyes < b.second.eyes;
+       }
+};
+
+void
+Shuffler::video (weak_ptr<Piece> weak_piece, ContentVideo video)
+{
+       std::cout << "shuffler gets " << video.frame << " " << video.eyes << "\n";
+
+       /* Something has gong wrong if our store gets too big */
+       DCPOMATIC_ASSERT (_store.size() != 8);
+       /* We should only ever see 3D_LEFT / 3D_RIGHT */
+       DCPOMATIC_ASSERT (video.eyes == EYES_LEFT || video.eyes == EYES_RIGHT);
+
+       shared_ptr<Piece> piece = weak_piece.lock ();
+       DCPOMATIC_ASSERT (piece);
+
+       if (!_last) {
+               /* We haven't seen anything since the last clear() so assume everything is OK */
+               Video (weak_piece, video);
+               _last = video;
+               return;
+       }
+
+       _store.push_back (make_pair (weak_piece, video));
+       _store.sort (Comparator());
+
+       while (
+               !_store.empty() &&
+               (
+                       (_store.front().second.frame == _last->frame && _store.front().second.eyes == EYES_RIGHT && _last->eyes == EYES_LEFT) ||
+                       (_store.front().second.frame == (_last->frame + 1) && _store.front().second.eyes == EYES_LEFT && _last->eyes == EYES_RIGHT) ||
+                       _store.size() > 8
+                       )
+               ) {
+
+               std::cout << "shuffler emits " << _store.front().second.frame << " " << _store.front().second.eyes << "\n";
+               Video (_store.front().first, _store.front().second);
+               _last = _store.front().second;
+               _store.pop_front ();
+       }
+}
+
+void
+Shuffler::clear ()
+{
+       _last = optional<ContentVideo>();
+}
+
+void
+Shuffler::flush ()
+{
+       BOOST_FOREACH (Store i, _store) {
+               Video (i.first, i.second);
+       }
+}

diff --git a/src/lib/shuffler.h b/src/lib/shuffler.h
new file mode 100644 (file)
index 0000000..54e13d4
--- /dev/null
+++ b/src/lib/shuffler.h
@@ -0,0 +1,42 @@
+/*
+    Copyright (C) 2018 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#include "types.h"
+#include "content_video.h"
+#include <boost/signals2.hpp>
+
+class Piece;
+
+class Shuffler
+{
+public:
+       void clear ();
+       void flush ();
+
+       void video (boost::weak_ptr<Piece>, ContentVideo video);
+       boost::signals2::signal<void (boost::weak_ptr<Piece>, ContentVideo)> Video;
+
+       typedef std::pair<boost::weak_ptr<Piece>, ContentVideo> Store;
+
+private:
+
+       boost::optional<ContentVideo> _last;
+       std::list<Store> _store;
+};

diff --git a/src/lib/util.cc b/src/lib/util.cc
index 637030e86da56d5cc5c8de343b7c9f949ad78db3..8ed87856127a2ba57689aa07fd24c82c953c39ca 100644 (file)
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -774,3 +774,13 @@ remap (shared_ptr<const AudioBuffers> input, int output_channels, AudioMapping m
 
        return mapped;
 }
+
+Eyes
+increment_eyes (Eyes e)
+{
+       if (e == EYES_LEFT) {
+               return EYES_RIGHT;
+       }
+
+       return EYES_LEFT;
+}

diff --git a/src/lib/util.h b/src/lib/util.h
index 1d4b013581870c97184eff2b11453766a205c97a..53a0a2d6674317cfbb1f4a61d0ec9a4e193fe864 100644 (file)
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -92,5 +92,6 @@ extern float relaxed_string_to_float (std::string);
 extern std::string careful_string_filter (std::string);
 extern std::pair<int, int> audio_channel_types (std::list<int> mapped, int channels);
 extern boost::shared_ptr<AudioBuffers> remap (boost::shared_ptr<const AudioBuffers> input, int output_channels, AudioMapping map);
+extern Eyes increment_eyes (Eyes e);
 
 #endif

diff --git a/src/lib/wscript b/src/lib/wscript
index 7fa32c7e7d7d9ebbac0788ba29edd30030168ac8..ae8ab30cba195e1ca4589a8e59f96629f61541e6 100644 (file)
--- a/src/lib/wscript
+++ b/src/lib/wscript
@@ -129,6 +129,7 @@ sources = """
           send_kdm_email_job.cc
           send_problem_report_job.cc
           server.cc
+          shuffler.cc
           string_log_entry.cc
           subtitle_content.cc
           subtitle_decoder.cc

diff --git a/test/shuffler_test.cc b/test/shuffler_test.cc
new file mode 100644 (file)
index 0000000..879f2e0
--- /dev/null
+++ b/test/shuffler_test.cc
@@ -0,0 +1,122 @@
+#include "lib/shuffler.h"
+#include "lib/piece.h"
+#include "lib/content_video.h"
+#include <boost/test/unit_test.hpp>
+
+using std::list;
+using boost::shared_ptr;
+using boost::weak_ptr;
+using boost::optional;
+
+static void
+push (Shuffler& s, int frame, Eyes eyes)
+{
+       shared_ptr<Piece> piece (new Piece (shared_ptr<Content>(), shared_ptr<Decoder>(), FrameRateChange(24, 24)));
+       ContentVideo cv;
+       cv.frame = frame;
+       cv.eyes = eyes;
+       s.video (piece, cv);
+}
+
+list<ContentVideo> pending_cv;
+
+static void
+receive (weak_ptr<Piece>, ContentVideo cv)
+{
+       pending_cv.push_back (cv);
+}
+
+static void
+check (int frame, Eyes eyes, int line)
+{
+       BOOST_REQUIRE_MESSAGE (!pending_cv.empty(), "Check at " << line << " failed.");
+       BOOST_CHECK_MESSAGE (pending_cv.front().frame == frame, "Check at " << line << " failed.");
+       BOOST_CHECK_MESSAGE (pending_cv.front().eyes == eyes, "Check at " << line << " failed.");
+       pending_cv.pop_front();
+}
+
+/** A perfect sequence */
+BOOST_AUTO_TEST_CASE (shuffler_test1)
+{
+       Shuffler s;
+       s.Video.connect (boost::bind (&receive, _1, _2));
+
+       for (int i = 0; i < 10; ++i) {
+               push (s, i, EYES_LEFT);
+               push (s, i, EYES_RIGHT);
+               check (i, EYES_LEFT, __LINE__);
+               check (i, EYES_RIGHT, __LINE__);
+       }
+}
+
+/** Everything present but some simple shuffling needed */
+BOOST_AUTO_TEST_CASE (shuffler_test2)
+{
+       Shuffler s;
+       s.Video.connect (boost::bind (&receive, _1, _2));
+
+       for (int i = 0; i < 10; i += 2) {
+               push (s, i, EYES_LEFT);
+               push (s, i + 1, EYES_LEFT);
+               push (s, i, EYES_RIGHT);
+               push (s, i + 1, EYES_RIGHT);
+               check (i, EYES_LEFT, __LINE__);
+               check (i, EYES_RIGHT, __LINE__);
+               check (i + 1, EYES_LEFT, __LINE__);
+               check (i + 1, EYES_RIGHT, __LINE__);
+       }
+}
+
+/** One missing left eye image */
+BOOST_AUTO_TEST_CASE (shuffler_test3)
+{
+       Shuffler s;
+       s.Video.connect (boost::bind (&receive, _1, _2));
+
+       push (s, 0, EYES_LEFT);
+       check (0, EYES_LEFT, __LINE__);
+       push (s, 0, EYES_RIGHT);
+       check (0, EYES_RIGHT, __LINE__);
+       push (s, 1, EYES_LEFT);
+       check (1, EYES_LEFT, __LINE__);
+       push (s, 1, EYES_RIGHT);
+       check (1, EYES_RIGHT, __LINE__);
+       push (s, 2, EYES_RIGHT);
+       push (s, 3, EYES_LEFT);
+       push (s, 3, EYES_RIGHT);
+       push (s, 4, EYES_LEFT);
+       push (s, 4, EYES_RIGHT);
+       s.flush ();
+       check (2, EYES_RIGHT, __LINE__);
+       check (3, EYES_LEFT, __LINE__);
+       check (3, EYES_RIGHT, __LINE__);
+       check (4, EYES_LEFT, __LINE__);
+       check (4, EYES_RIGHT, __LINE__);
+}
+
+/** One missing right eye image */
+BOOST_AUTO_TEST_CASE (shuffler_test4)
+{
+       Shuffler s;
+       s.Video.connect (boost::bind (&receive, _1, _2));
+
+       push (s, 0, EYES_LEFT);
+       check (0, EYES_LEFT, __LINE__);
+       push (s, 0, EYES_RIGHT);
+       check (0, EYES_RIGHT, __LINE__);
+       push (s, 1, EYES_LEFT);
+       check (1, EYES_LEFT, __LINE__);
+       push (s, 1, EYES_RIGHT);
+       check (1, EYES_RIGHT, __LINE__);
+       push (s, 2, EYES_LEFT);
+       push (s, 3, EYES_LEFT);
+       push (s, 3, EYES_RIGHT);
+       push (s, 4, EYES_LEFT);
+       push (s, 4, EYES_RIGHT);
+       s.flush ();
+       check (2, EYES_LEFT, __LINE__);
+       check (3, EYES_LEFT, __LINE__);
+       check (3, EYES_RIGHT, __LINE__);
+       check (4, EYES_LEFT, __LINE__);
+       check (4, EYES_RIGHT, __LINE__);
+}

diff --git a/test/wscript b/test/wscript
index f0533cfa16dd170e6b2ef51fac3fca1616b4d9d4..fef16dbed71ec280bd2daabc8c496cdfaf54e47a 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -93,6 +93,7 @@ def build(bld):
                  render_subtitles_test.cc
                  scaling_test.cc
                  silence_padding_test.cc
+                 shuffler_test.cc
                  skip_frame_test.cc
                  srt_subtitle_test.cc
                  ssa_subtitle_test.cc