Attempt to tidy up internal APIs slightly.
authorCarl Hetherington <cth@carlh.net>
Mon, 28 Nov 2016 23:45:34 +0000 (23:45 +0000)
committerCarl Hetherington <cth@carlh.net>
Wed, 19 Apr 2017 22:04:32 +0000 (23:04 +0100)
16 files changed:
doc/design/decoder_structures.tex
src/lib/dcpomatic_time.h
src/lib/encoder.cc
src/lib/encoder.h
src/lib/player.cc
src/lib/player.h
src/lib/player_subtitles.h
src/lib/player_video.cc
src/lib/player_video.h
src/lib/transcoder.cc
src/lib/transcoder.h
src/lib/writer.cc
src/lib/writer.h
src/wx/film_viewer.cc
src/wx/film_viewer.h
test/client_server_test.cc

index b7513859cf3426df176737c18e48ee22529764bf..64a7da7520a65a611db7db824ab88df9809b1e71 100644 (file)
@@ -128,9 +128,9 @@ Resampling also looks fiddly in the v1 code.
     virtual void pass() = 0;
     virtual void seek(ContentTime time, bool accurate) = 0;
 
-    signals2<void (ContentVideo)> Video;
-    signals2<void (ContentAudio, AudioStreamPtr)> Audio;
-    signals2<void (ContentTextSubtitle)> TextSubtitle;
+    signal<void (ContentVideo)> Video;
+    signal<void (ContentAudio, AudioStreamPtr)> Audio;
+    signal<void (ContentTextSubtitle)> TextSubtitle;
   };
 \end{lstlisting}
 
@@ -160,12 +160,18 @@ Questions:
 \subsection{Steps}
 
 \begin{itemize}
+\item Add signals to \texttt{Player}.
+  \begin{itemize}
+    \item \texttt{signal<void (shared\_ptr<PlayerVideo>), DCPTime> Video;}
+    \item \texttt{signal<void (shared\_ptr<AudioBuffers>, DCPTime)> Audio;}
+    \item \texttt{signal<void (PlayerSubtitles, DCPTimePeriod)> Subtitle;}
+  \end{itemize}
   \item Remove \texttt{get()}-based loops and replace with \texttt{pass()} and signal connections.
   \item Remove \texttt{get()} and \texttt{seek()} from decoder parts; add emission signals.
   \item Put \texttt{AudioMerger} back.
   \item Remove \texttt{during} stuff from \texttt{SubtitleDecoder} and decoder classes that use it.
   \item Rename \texttt{give} methods to \texttt{emit}.
-  \item Remove \text{get} methods from \texttt{Player}; replace with \texttt{pass()} and \texttt{seek()}.
+  \item Remove \texttt{get} methods from \texttt{Player}; replace with \texttt{pass()} and \texttt{seek()}.
 \end{itemize}
 
 \end{document}
index 16d93ca28bc83abc01c8273c6ece6cd092b42359..35ddd0199e448ae65420f17ad8bfd637fe24bdc3 100644 (file)
@@ -260,7 +260,7 @@ public:
                return TimePeriod<T> (from + o, to + o);
        }
 
-       boost::optional<TimePeriod<T> > overlap (TimePeriod<T> const & other) {
+       boost::optional<TimePeriod<T> > overlap (TimePeriod<T> const & other) const {
                T const max_from = std::max (from, other.from);
                T const min_to = std::min (to, other.to);
 
index f468f91e113a2235ab3c69d17e35a2d6a92bf064..f8a2ee3bde878c117f9948015d23951082d82fa5 100644 (file)
@@ -165,11 +165,11 @@ Encoder::current_encoding_rate () const
 int
 Encoder::video_frames_enqueued () const
 {
-       if (!_last_player_video) {
+       if (!_last_player_video_time) {
                return 0;
        }
 
-       return _last_player_video->time().frames_floor (_film->video_frame_rate ());
+       return _last_player_video_time->frames_floor (_film->video_frame_rate ());
 }
 
 /** Should be called when a frame has been encoded successfully.
@@ -194,7 +194,7 @@ Encoder::frame_done ()
  *  for this DCP frame.
  */
 void
-Encoder::encode (shared_ptr<PlayerVideo> pv)
+Encoder::encode (shared_ptr<PlayerVideo> pv, DCPTime time)
 {
        _waker.nudge ();
 
@@ -222,7 +222,7 @@ Encoder::encode (shared_ptr<PlayerVideo> pv)
        */
        rethrow ();
 
-       Frame const position = pv->time().frames_floor(_film->video_frame_rate());
+       Frame const position = time.frames_floor(_film->video_frame_rate());
 
        if (_writer->can_fake_write (position)) {
                /* We can fake-write this frame */
@@ -258,6 +258,7 @@ Encoder::encode (shared_ptr<PlayerVideo> pv)
        }
 
        _last_player_video = pv;
+       _last_player_video_time = time;
 }
 
 void
index 5816fe12f64c88c69d0c19ef47ece2b2744fe8e5..27ae64aac589ba577578fb874fcafdffe8557b40 100644 (file)
@@ -62,7 +62,7 @@ public:
        void begin ();
 
        /** Called to pass a bit of video to be encoded as the next DCP frame */
-       void encode (boost::shared_ptr<PlayerVideo> f);
+       void encode (boost::shared_ptr<PlayerVideo> f, DCPTime time);
 
        /** Called when a processing run has finished */
        void end ();
@@ -107,6 +107,7 @@ private:
        Waker _waker;
 
        boost::shared_ptr<PlayerVideo> _last_player_video;
+       boost::optional<DCPTime> _last_player_video_time;
 
        boost::signals2::scoped_connection _server_found_connection;
 };
index 31c3c26e496db5d6bc808d0d1c3e266d18d5e9dd..d9fb8dfaec67c949f196c00d26f3f0a14a8a1481 100644 (file)
@@ -286,12 +286,11 @@ Player::transform_image_subtitles (list<ImageSubtitle> subs) const
 }
 
 shared_ptr<PlayerVideo>
-Player::black_player_video_frame (DCPTime time) const
+Player::black_player_video_frame () const
 {
        return shared_ptr<PlayerVideo> (
                new PlayerVideo (
                        shared_ptr<const ImageProxy> (new RawImageProxy (_black_image)),
-                       time,
                        Crop (),
                        optional<double> (),
                        _video_container_size,
@@ -563,21 +562,21 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
 
        optional<PositionImage> subtitles;
 
-       BOOST_FOREACH (PlayerSubtitles i, _subtitles) {
+       for (list<pair<PlayerSubtitles, DCPTimePeriod> >::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
 
-               if (!i.period.overlap (period)) {
+               if (!i->second.overlap (period)) {
                        continue;
                }
 
                list<PositionImage> sub_images;
 
                /* Image subtitles */
-               list<PositionImage> c = transform_image_subtitles (i.image);
+               list<PositionImage> c = transform_image_subtitles (i->first.image);
                copy (c.begin(), c.end(), back_inserter (sub_images));
 
                /* Text subtitles (rendered to an image) */
-               if (!i.text.empty ()) {
-                       list<PositionImage> s = render_subtitles (i.text, i.fonts, _video_container_size, time);
+               if (!i->first.text.empty ()) {
+                       list<PositionImage> s = render_subtitles (i->first.text, i->first.fonts, _video_container_size, time);
                        copy (s.begin (), s.end (), back_inserter (sub_images));
                }
 
@@ -591,9 +590,9 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
        if (_last_video_time) {
                for (DCPTime i = _last_video_time.get(); i < time; i += DCPTime::from_frames (1, _film->video_frame_rate())) {
                        if (_playlist->video_content_at(i) && _last_video) {
-                               Video (_last_video->clone (i));
+                               Video (shared_ptr<PlayerVideo> (new PlayerVideo (*_last_video)), i);
                        } else {
-                               Video (black_player_video_frame (i));
+                               Video (black_player_video_frame (), i);
                        }
                }
        }
@@ -601,7 +600,6 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
        _last_video.reset (
                new PlayerVideo (
                        video.image,
-                       time,
                        piece->content->video->crop (),
                        piece->content->video->fade (video.frame.index()),
                        piece->content->video->scale().size (
@@ -621,15 +619,15 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
        _last_video_time = time;
 
        cout << "Video @ " << to_string(_last_video_time.get()) << "\n";
-       Video (_last_video);
+       Video (_last_video, *_last_video_time);
 
        /* Discard any subtitles we no longer need */
 
-       for (list<PlayerSubtitles>::iterator i = _subtitles.begin (); i != _subtitles.end(); ) {
-               list<PlayerSubtitles>::iterator tmp = i;
+       for (list<pair<PlayerSubtitles, DCPTimePeriod> >::iterator i = _subtitles.begin (); i != _subtitles.end(); ) {
+               list<pair<PlayerSubtitles, DCPTimePeriod> >::iterator tmp = i;
                ++tmp;
 
-               if (i->period.to < time) {
+               if (i->second.to < time) {
                        _subtitles.erase (i);
                }
 
@@ -711,12 +709,12 @@ Player::image_subtitle (weak_ptr<Piece> wp, ContentImageSubtitle subtitle)
 
        PlayerSubtitles ps;
        ps.image.push_back (subtitle.sub);
-       ps.period = DCPTimePeriod (content_time_to_dcp (piece, subtitle.period().from), content_time_to_dcp (piece, subtitle.period().to));
+       DCPTimePeriod period (content_time_to_dcp (piece, subtitle.period().from), content_time_to_dcp (piece, subtitle.period().to));
 
        if (piece->content->subtitle->use() && (piece->content->subtitle->burn() || _always_burn_subtitles)) {
-               _subtitles.push_back (ps);
+               _subtitles.push_back (make_pair (ps, period));
        } else {
-               Subtitle (ps);
+               Subtitle (ps, period);
        }
 }
 
@@ -729,6 +727,7 @@ Player::text_subtitle (weak_ptr<Piece> wp, ContentTextSubtitle subtitle)
        }
 
        PlayerSubtitles ps;
+       DCPTimePeriod const period (content_time_to_dcp (piece, subtitle.period().from), content_time_to_dcp (piece, subtitle.period().to));
 
        BOOST_FOREACH (dcp::SubtitleString s, subtitle.subs) {
                s.set_h_position (s.h_position() + piece->content->subtitle->x_offset ());
@@ -750,18 +749,16 @@ Player::text_subtitle (weak_ptr<Piece> wp, ContentTextSubtitle subtitle)
                        s.set_aspect_adjust (xs / ys);
                }
 
-               ps.period = DCPTimePeriod (content_time_to_dcp (piece, subtitle.period().from), content_time_to_dcp (piece, subtitle.period().to));
-
-               s.set_in (dcp::Time(ps.period.from.seconds(), 1000));
-               s.set_out (dcp::Time(ps.period.to.seconds(), 1000));
+               s.set_in (dcp::Time(period.from.seconds(), 1000));
+               s.set_out (dcp::Time(period.to.seconds(), 1000));
                ps.text.push_back (SubtitleString (s, piece->content->subtitle->outline_width()));
                ps.add_fonts (piece->content->subtitle->fonts ());
        }
 
        if (piece->content->subtitle->use() && (piece->content->subtitle->burn() || _always_burn_subtitles)) {
-               _subtitles.push_back (ps);
+               _subtitles.push_back (make_pair (ps, period));
        } else {
-               Subtitle (ps);
+               Subtitle (ps, period);
        }
 }
 
index 95db8756be6f2337aeae147f1dcb07a67070e730..05e994d0bb105aa8b797762e96a285e3de4e4624 100644 (file)
@@ -75,9 +75,9 @@ public:
         */
        boost::signals2::signal<void (bool)> Changed;
 
-       boost::signals2::signal<void (boost::shared_ptr<PlayerVideo>)> Video;
+       boost::signals2::signal<void (boost::shared_ptr<PlayerVideo>, DCPTime)> Video;
        boost::signals2::signal<void (boost::shared_ptr<AudioBuffers>, DCPTime)> Audio;
-       boost::signals2::signal<void (PlayerSubtitles)> Subtitle;
+       boost::signals2::signal<void (PlayerSubtitles, DCPTimePeriod)> Subtitle;
 
 private:
        friend class PlayerWrapper;
@@ -99,7 +99,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 (DCPTime) const;
+       boost::shared_ptr<PlayerVideo> black_player_video_frame () const;
        std::list<boost::shared_ptr<Piece> > overlaps (DCPTime from, DCPTime to, boost::function<bool (Content *)> valid);
        void video (boost::weak_ptr<Piece>, ContentVideo);
        void audio (boost::weak_ptr<Piece>, AudioStreamPtr, ContentAudio);
@@ -136,7 +136,7 @@ private:
        AudioMerger _audio_merger;
        DCPTime _last_audio_time;
 
-       std::list<PlayerSubtitles> _subtitles;
+       std::list<std::pair<PlayerSubtitles, DCPTimePeriod> > _subtitles;
 
        boost::shared_ptr<AudioProcessor> _audio_processor;
 
index 9ec7b10561a3f229489209d83b32dc301fa2a1e8..e1a104f2127c89a4d56e865f513f5f5320baab7b 100644 (file)
@@ -34,8 +34,6 @@ public:
        void add_fonts (std::list<boost::shared_ptr<Font> > fonts_);
        std::list<boost::shared_ptr<Font> > fonts;
 
-       DCPTimePeriod period;
-
        /** ImageSubtitles, with their rectangles transformed as specified by their content */
        std::list<ImageSubtitle> image;
        std::list<SubtitleString> text;
index 7c9a4ee66fdeb5c08f747673df81c355d1e64eac..b7fb52e3aa8d8b2c27645a5688b5c96aae62bf06 100644 (file)
@@ -41,7 +41,6 @@ using dcp::raw_convert;
 
 PlayerVideo::PlayerVideo (
        shared_ptr<const ImageProxy> in,
-       DCPTime time,
        Crop crop,
        boost::optional<double> fade,
        dcp::Size inter_size,
@@ -51,7 +50,6 @@ PlayerVideo::PlayerVideo (
        optional<ColourConversion> colour_conversion
        )
        : _in (in)
-       , _time (time)
        , _crop (crop)
        , _fade (fade)
        , _inter_size (inter_size)
@@ -65,7 +63,6 @@ PlayerVideo::PlayerVideo (
 
 PlayerVideo::PlayerVideo (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket)
 {
-       _time = DCPTime (node->number_child<DCPTime::Type> ("Time"));
        _crop = Crop (node);
        _fade = node->optional_number_child<double> ("Fade");
 
@@ -149,7 +146,6 @@ PlayerVideo::image (dcp::NoteHandler note, function<AVPixelFormat (AVPixelFormat
 void
 PlayerVideo::add_metadata (xmlpp::Node* node) const
 {
-       node->add_child("Time")->add_child_text (raw_convert<string> (_time.get ()));
        _crop.as_xml (node);
        if (_fade) {
                node->add_child("Fade")->add_child_text (raw_convert<string> (_fade.get ()));
@@ -208,9 +204,7 @@ PlayerVideo::inter_position () const
        return Position<int> ((_out_size.width - _inter_size.width) / 2, (_out_size.height - _inter_size.height) / 2);
 }
 
-/** @return true if this PlayerVideo is definitely the same as another
- * (apart from _time), false if it is probably not
- */
+/** @return true if this PlayerVideo is definitely the same as another, false if it is probably not */
 bool
 PlayerVideo::same (shared_ptr<const PlayerVideo> other) const
 {
@@ -250,11 +244,3 @@ PlayerVideo::keep_xyz_or_rgb (AVPixelFormat p)
 {
        return p == AV_PIX_FMT_XYZ12LE ? AV_PIX_FMT_XYZ12LE : AV_PIX_FMT_RGB48LE;
 }
-
-shared_ptr<PlayerVideo>
-PlayerVideo::clone (DCPTime time) const
-{
-       shared_ptr<PlayerVideo> c (new PlayerVideo (*this));
-       c->_time = time;
-       return c;
-}
index a5ca9b865705648bc298db707166d3614968873a..60c9224b0e5254978397e271fb1206584c970c68 100644 (file)
@@ -41,7 +41,6 @@ class PlayerVideo
 public:
        PlayerVideo (
                boost::shared_ptr<const ImageProxy>,
-               DCPTime,
                Crop,
                boost::optional<double>,
                dcp::Size,
@@ -66,10 +65,6 @@ public:
        bool has_j2k () const;
        dcp::Data j2k () const;
 
-       DCPTime time () const {
-               return _time;
-       }
-
        Eyes eyes () const {
                return _eyes;
        }
@@ -92,11 +87,8 @@ public:
 
        bool same (boost::shared_ptr<const PlayerVideo> other) const;
 
-       boost::shared_ptr<PlayerVideo> clone (DCPTime time) const;
-
 private:
        boost::shared_ptr<const ImageProxy> _in;
-       DCPTime _time;
        Crop _crop;
        boost::optional<double> _fade;
        dcp::Size _inter_size;
index da6b08c5ff1af87b2158112ed979c3d2f45509e5..de2fb1d3388b73d94b1b5889c8b91b7d153083be 100644 (file)
@@ -63,9 +63,9 @@ Transcoder::Transcoder (shared_ptr<const Film> film, weak_ptr<Job> j)
        , _finishing (false)
        , _non_burnt_subtitles (false)
 {
-       _player->Video.connect (bind (&Transcoder::video, this, _1));
+       _player->Video.connect (bind (&Transcoder::video, this, _1, _2));
        _player->Audio.connect (bind (&Transcoder::audio, this, _1, _2));
-       _player->Subtitle.connect (bind (&Transcoder::subtitle, this, _1));
+       _player->Subtitle.connect (bind (&Transcoder::subtitle, this, _1, _2));
 
        BOOST_FOREACH (shared_ptr<const Content> c, _film->content ()) {
                if (c->subtitle && c->subtitle->use() && !c->subtitle->burn()) {
@@ -102,14 +102,14 @@ Transcoder::go ()
 }
 
 void
-Transcoder::video (shared_ptr<PlayerVideo> data)
+Transcoder::video (shared_ptr<PlayerVideo> data, DCPTime time)
 {
        if (!_film->three_d() && data->eyes() == EYES_LEFT) {
                /* Use left-eye images for both eyes */
                data->set_eyes (EYES_BOTH);
        }
 
-       _encoder->encode (data);
+       _encoder->encode (data, time);
 }
 
 void
@@ -123,10 +123,10 @@ Transcoder::audio (shared_ptr<AudioBuffers> data, DCPTime time)
 }
 
 void
-Transcoder::subtitle (PlayerSubtitles data)
+Transcoder::subtitle (PlayerSubtitles data, DCPTimePeriod period)
 {
        if (_non_burnt_subtitles) {
-               _writer->write (data);
+               _writer->write (data, period);
        }
 }
 
index 1b20bbffc535500ede23f9d2c41e9b3efb6b4813..0095ad9d1111ca9883d33b48f8b6fc62adcc1bfa 100644 (file)
@@ -48,9 +48,9 @@ public:
 
 private:
 
-       void video (boost::shared_ptr<PlayerVideo>);
+       void video (boost::shared_ptr<PlayerVideo>, DCPTime);
        void audio (boost::shared_ptr<AudioBuffers>, DCPTime);
-       void subtitle (PlayerSubtitles);
+       void subtitle (PlayerSubtitles, DCPTimePeriod);
 
        boost::shared_ptr<const Film> _film;
        boost::weak_ptr<Job> _job;
index 67b00987d4ea5eab9b335530256471f831ad84e2..88925cbbdbeb8596c1a809faf45884dad913b586 100644 (file)
@@ -542,13 +542,13 @@ Writer::can_fake_write (Frame frame) const
 }
 
 void
-Writer::write (PlayerSubtitles subs)
+Writer::write (PlayerSubtitles subs, DCPTimePeriod period)
 {
        if (subs.text.empty ()) {
                return;
        }
 
-       if (_subtitle_reel->period().to <= subs.period.from) {
+       if (_subtitle_reel->period().to <= period.from) {
                ++_subtitle_reel;
        }
 
index c1d7e4f8bdd41e9afc96f73147676cd3e6283756..14b21f35975c84b8a791bf7c25730f37bb1b9f1a 100644 (file)
@@ -101,7 +101,7 @@ public:
        bool can_repeat (Frame) const;
        void repeat (Frame, Eyes);
        void write (boost::shared_ptr<const AudioBuffers>);
-       void write (PlayerSubtitles subs);
+       void write (PlayerSubtitles subs, DCPTimePeriod period);
        void write (std::list<boost::shared_ptr<Font> > fonts);
        void write (ReferencedReelAsset asset);
        void finish ();
index bddd3abba280316ae58409b1247a1994cd26c501..c17927ac5bc0de8d483004ed4c41dcd69c9bbb16 100644 (file)
@@ -181,7 +181,7 @@ FilmViewer::set_film (shared_ptr<Film> film)
        _film->Changed.connect (boost::bind (&FilmViewer::film_changed, this, _1));
 
        _player->Changed.connect (boost::bind (&FilmViewer::player_changed, this, _1));
-       _player->Video.connect (boost::bind (&FilmViewer::video, this, _1));
+       _player->Video.connect (boost::bind (&FilmViewer::video, this, _1, _2));
 
        calculate_sizes ();
        refresh ();
@@ -197,7 +197,7 @@ FilmViewer::refresh_panel ()
 }
 
 void
-FilmViewer::video (shared_ptr<PlayerVideo> pv)
+FilmViewer::video (shared_ptr<PlayerVideo> pv, DCPTime time)
 {
        if (!_player) {
                return;
@@ -235,7 +235,7 @@ FilmViewer::video (shared_ptr<PlayerVideo> pv)
 
        ImageChanged (pv);
 
-       _position = pv->time ();
+       _position = time;
        _inter_position = pv->inter_position ();
        _inter_size = pv->inter_size ();
 
index 399441f18357cf4d1595e88b35a72d56aec3cb01..d01e002906be3522bad6ee72595987d55ee3b7fc 100644 (file)
@@ -67,7 +67,7 @@ private:
        void player_changed (bool);
        void update_position_label ();
        void update_position_slider ();
-       void video (boost::shared_ptr<PlayerVideo>);
+       void video (boost::shared_ptr<PlayerVideo>, DCPTime time);
        void get ();
        void seek (DCPTime t, bool accurate);
        void refresh_panel ();
index f12c5335d42f3480ffddfe4fc828bd767cc6c016..e21f41c7998163ec7c5290e1b6aa0b3ba16797aa 100644 (file)
@@ -87,7 +87,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_rgb)
        shared_ptr<PlayerVideo> pvf (
                new PlayerVideo (
                        shared_ptr<ImageProxy> (new RawImageProxy (image)),
-                       DCPTime (),
                        Crop (),
                        optional<double> (),
                        dcp::Size (1998, 1080),
@@ -167,7 +166,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_yuv)
        shared_ptr<PlayerVideo> pvf (
                new PlayerVideo (
                        shared_ptr<ImageProxy> (new RawImageProxy (image)),
-                       DCPTime (),
                        Crop (),
                        optional<double> (),
                        dcp::Size (1998, 1080),
@@ -234,7 +232,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k)
        shared_ptr<PlayerVideo> raw_pvf (
                new PlayerVideo (
                        shared_ptr<ImageProxy> (new RawImageProxy (image)),
-                       DCPTime (),
                        Crop (),
                        optional<double> (),
                        dcp::Size (1998, 1080),
@@ -261,7 +258,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k)
        shared_ptr<PlayerVideo> j2k_pvf (
                new PlayerVideo (
                        shared_ptr<ImageProxy> (new J2KImageProxy (raw_locally_encoded, dcp::Size (1998, 1080), AV_PIX_FMT_XYZ12LE)),
-                       DCPTime (),
                        Crop (),
                        optional<double> (),
                        dcp::Size (1998, 1080),