Add a 2-frame `delay' on content arriving at the player to give

subtitle content the chance to catch up.  Fixes problems observed
when overlaying a DCP subtitle onto an existing DCP and then seeking
into the first subtitle.  After the seek the decoder positions were:

DCP: 0.
subtitle: first subtitle time.

This causes the DCP decoder to be pass()ed first and so the subtitle
for the video frame has not arrived yet.

I hope this does not cause unpredicted side effects...

diff --git a/src/lib/content_video.h b/src/lib/content_video.h
index f59e99cfc65cd25cbbe57552a17648841c06527c..8b3d984df9f808db12448ad93e04fa7e1dafdb9d 100644 (file)
--- a/src/lib/content_video.h
+++ b/src/lib/content_video.h
@@ -21,6 +21,8 @@
 #ifndef DCPOMATIC_CONTENT_VIDEO_H
 #define DCPOMATIC_CONTENT_VIDEO_H
 
+#include "types.h"
+
 class ImageProxy;
 
 /** @class ContentVideo

diff --git a/src/lib/delay.cc b/src/lib/delay.cc
new file mode 100644 (file)
index 0000000..6f98b3b
--- /dev/null
+++ b/src/lib/delay.cc
@@ -0,0 +1,41 @@
+/*
+    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 "delay.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;
+
+void
+Delay::video (weak_ptr<Piece> weak_piece, ContentVideo video)
+{
+       _store.push_back (make_pair (weak_piece, video));
+       /* Delay by 2 frames */
+       while (_store.size() > 2) {
+               Video (_store.front().first, _store.front().second);
+               _store.pop_front ();
+       }
+}

diff --git a/src/lib/delay.h b/src/lib/delay.h
new file mode 100644 (file)
index 0000000..fa08b05
--- /dev/null
+++ b/src/lib/delay.h
@@ -0,0 +1,43 @@
+/*
+    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/>.
+
+*/
+
+#ifndef DCPOMATIC_DELAY_H
+#define DCPOMATIC_DELAY_H
+
+#include "types.h"
+#include "video_adjuster.h"
+#include "content_video.h"
+#include <boost/signals2.hpp>
+
+class Piece;
+
+/** A class which `delays' received video by 2 frames; i.e. when it receives
+ *  a video frame it emits the last-but-one it received.
+ */
+class Delay : public VideoAdjuster
+{
+public:
+       void video (boost::weak_ptr<Piece>, ContentVideo video);
+
+private:
+       boost::optional<ContentVideo> _last;
+};
+
+#endif

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 71b04e7b2851efc8e3ef61c97722031d67f6d973..a8a72a229a2b756e5e9e03d3fe3b6de228ef409b 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -49,6 +49,7 @@
 #include "image_decoder.h"
 #include "compose.hpp"
 #include "shuffler.h"
+#include "delay.h"
 #include <dcp/reel.h>
 #include <dcp/reel_sound_asset.h>
 #include <dcp/reel_subtitle_asset.h>
@@ -89,6 +90,7 @@ Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist
        , _play_referenced (false)
        , _audio_merger (_film->audio_frame_rate())
        , _shuffler (0)
+       , _delay (0)
 {
        _film_changed_connection = _film->Changed.connect (bind (&Player::film_changed, this, _1));
        _playlist_changed_connection = _playlist->Changed.connect (bind (&Player::playlist_changed, this));
@@ -103,6 +105,7 @@ Player::Player (shared_ptr<const Film> film, shared_ptr<const Playlist> playlist
 Player::~Player ()
 {
        delete _shuffler;
+       delete _delay;
 }
 
 void
@@ -114,6 +117,10 @@ Player::setup_pieces ()
        _shuffler = new Shuffler();
        _shuffler->Video.connect(bind(&Player::video, this, _1, _2));
 
+       delete _delay;
+       _delay = new Delay();
+       _delay->Video.connect(bind(&Player::video, this, _1, _2));
+
        BOOST_FOREACH (shared_ptr<Content> i, _playlist->content ()) {
 
                if (!i->paths_valid ()) {
@@ -149,9 +156,13 @@ Player::setup_pieces ()
 
                if (decoder->video) {
                        if (i->video->frame_type() == VIDEO_FRAME_TYPE_3D_LEFT || i->video->frame_type() == VIDEO_FRAME_TYPE_3D_RIGHT) {
+                               /* We need a Shuffler to cope with 3D L/R video data arriving out of sequence */
                                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));
+                               /* We need a Delay to give a little wiggle room to ensure that relevent subtitles arrive at the
+                                  player before the video that requires them.
+                               */
+                               decoder->video->Data.connect (bind (&Delay::video, _delay, weak_ptr<Piece>(piece), _1));
                        }
                }
 
@@ -649,6 +660,7 @@ Player::pass ()
 
        if (done) {
                _shuffler->flush ();
+               _delay->flush ();
        }
        return done;
 }
@@ -931,6 +943,10 @@ Player::seek (DCPTime time, bool accurate)
                _shuffler->clear ();
        }
 
+       if (_delay) {
+               _delay->clear ();
+       }
+
        if (_audio_processor) {
                _audio_processor->flush ();
        }

diff --git a/src/lib/player.h b/src/lib/player.h
index 0b8540c15e046c783ac995acd814352a4a3d999b..58ed4e36959611b8d32618a462506b85f83ed46a 100644 (file)
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -33,6 +33,7 @@
 #include "audio_stream.h"
 #include "audio_merger.h"
 #include "empty.h"
+#include "delay.h"
 #include <boost/shared_ptr.hpp>
 #include <boost/enable_shared_from_this.hpp>
 #include <list>
@@ -159,6 +160,7 @@ private:
 
        AudioMerger _audio_merger;
        Shuffler* _shuffler;
+       Delay* _delay;
 
        class StreamState
        {

diff --git a/src/lib/shuffler.cc b/src/lib/shuffler.cc
index 84bf98ed254cbb43b4078cafa66e988a3e7a5f00..4b8474ab3028eeb5b99e091aba9d426e15302c8b 100644 (file)
--- a/src/lib/shuffler.cc
+++ b/src/lib/shuffler.cc
@@ -78,13 +78,6 @@ Shuffler::video (weak_ptr<Piece> weak_piece, ContentVideo video)
 void
 Shuffler::clear ()
 {
+       VideoAdjuster::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
index 54e13d486fefeb3bf3390f497bc94c7b839cbba1..2c1d0677f9747000de688cf5e70428b8e744e996 100644 (file)
--- a/src/lib/shuffler.h
+++ b/src/lib/shuffler.h
@@ -20,23 +20,18 @@
 
 #include "types.h"
 #include "content_video.h"
+#include "video_adjuster.h"
 #include <boost/signals2.hpp>
 
 class Piece;
 
-class Shuffler
+class Shuffler : public VideoAdjuster
 {
 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/video_adjuster.cc b/src/lib/video_adjuster.cc
new file mode 100644 (file)
index 0000000..c945c66
--- /dev/null
+++ b/src/lib/video_adjuster.cc
@@ -0,0 +1,37 @@
+/*
+    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 "video_adjuster.h"
+#include "content_video.h"
+#include <boost/foreach.hpp>
+
+void
+VideoAdjuster::flush ()
+{
+       BOOST_FOREACH (Store i, _store) {
+               Video (i.first, i.second);
+       }
+}
+
+void
+VideoAdjuster::clear ()
+{
+       _store.clear ();
+}

diff --git a/src/lib/video_adjuster.h b/src/lib/video_adjuster.h
new file mode 100644 (file)
index 0000000..6fde2ba
--- /dev/null
+++ b/src/lib/video_adjuster.h
@@ -0,0 +1,48 @@
+/*
+    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/>.
+
+*/
+
+#ifndef DCPOMATIC_VIDEO_ADJUSTER_H
+#define DCPOMATIC_VIDEO_ADJUSTER_H
+
+#include <boost/signals2.hpp>
+
+class Piece;
+class ContentVideo;
+
+/** Parent class for short delays of video content done by the Player
+ *  to work around various problems.
+ */
+class VideoAdjuster
+{
+public:
+       virtual ~VideoAdjuster() {}
+
+       virtual void clear ();
+       void flush ();
+
+       boost::signals2::signal<void (boost::weak_ptr<Piece>, ContentVideo)> Video;
+
+       typedef std::pair<boost::weak_ptr<Piece>, ContentVideo> Store;
+
+protected:
+       std::list<Store> _store;
+};
+
+#endif

diff --git a/src/lib/wscript b/src/lib/wscript
index ae8ab30cba195e1ca4589a8e59f96629f61541e6..46ced80ab661fd92e93f0290b54fce6d9382be1b 100644 (file)
--- a/src/lib/wscript
+++ b/src/lib/wscript
@@ -63,6 +63,7 @@ sources = """
           decoder.cc
           decoder_factory.cc
           decoder_part.cc
+          delay.cc
           digester.cc
           dkdm_wrapper.cc
           dolby_cp750.cc
@@ -146,6 +147,7 @@ sources = """
           upmixer_a.cc
           upmixer_b.cc
           util.cc
+          video_adjuster.cc
           video_content.cc
           video_content_scale.cc
           video_decoder.cc