From a78b741c43830c84bcb4d18e3147746f13a668e5 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 28 Nov 2016 23:45:34 +0000 Subject: [PATCH] Attempt to tidy up internal APIs slightly. --- doc/design/decoder_structures.tex | 14 +++++++--- src/lib/dcpomatic_time.h | 2 +- src/lib/encoder.cc | 9 ++++--- src/lib/encoder.h | 3 ++- src/lib/player.cc | 43 ++++++++++++++----------------- src/lib/player.h | 8 +++--- src/lib/player_subtitles.h | 2 -- src/lib/player_video.cc | 16 +----------- src/lib/player_video.h | 8 ------ src/lib/transcoder.cc | 12 ++++----- src/lib/transcoder.h | 4 +-- src/lib/writer.cc | 4 +-- src/lib/writer.h | 2 +- src/wx/film_viewer.cc | 6 ++--- src/wx/film_viewer.h | 2 +- test/client_server_test.cc | 4 --- 16 files changed, 58 insertions(+), 81 deletions(-) diff --git a/doc/design/decoder_structures.tex b/doc/design/decoder_structures.tex index b7513859c..64a7da752 100644 --- a/doc/design/decoder_structures.tex +++ b/doc/design/decoder_structures.tex @@ -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 Video; - signals2 Audio; - signals2 TextSubtitle; + signal Video; + signal Audio; + signal TextSubtitle; }; \end{lstlisting} @@ -160,12 +160,18 @@ Questions: \subsection{Steps} \begin{itemize} +\item Add signals to \texttt{Player}. + \begin{itemize} + \item \texttt{signal), DCPTime> Video;} + \item \texttt{signal, DCPTime)> Audio;} + \item \texttt{signal 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} diff --git a/src/lib/dcpomatic_time.h b/src/lib/dcpomatic_time.h index 16d93ca28..35ddd0199 100644 --- a/src/lib/dcpomatic_time.h +++ b/src/lib/dcpomatic_time.h @@ -260,7 +260,7 @@ public: return TimePeriod (from + o, to + o); } - boost::optional > overlap (TimePeriod const & other) { + boost::optional > overlap (TimePeriod const & other) const { T const max_from = std::max (from, other.from); T const min_to = std::min (to, other.to); diff --git a/src/lib/encoder.cc b/src/lib/encoder.cc index f468f91e1..f8a2ee3bd 100644 --- a/src/lib/encoder.cc +++ b/src/lib/encoder.cc @@ -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 pv) +Encoder::encode (shared_ptr pv, DCPTime time) { _waker.nudge (); @@ -222,7 +222,7 @@ Encoder::encode (shared_ptr 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 pv) } _last_player_video = pv; + _last_player_video_time = time; } void diff --git a/src/lib/encoder.h b/src/lib/encoder.h index 5816fe12f..27ae64aac 100644 --- a/src/lib/encoder.h +++ b/src/lib/encoder.h @@ -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 f); + void encode (boost::shared_ptr f, DCPTime time); /** Called when a processing run has finished */ void end (); @@ -107,6 +107,7 @@ private: Waker _waker; boost::shared_ptr _last_player_video; + boost::optional _last_player_video_time; boost::signals2::scoped_connection _server_found_connection; }; diff --git a/src/lib/player.cc b/src/lib/player.cc index 31c3c26e4..d9fb8dfae 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -286,12 +286,11 @@ Player::transform_image_subtitles (list subs) const } shared_ptr -Player::black_player_video_frame (DCPTime time) const +Player::black_player_video_frame () const { return shared_ptr ( new PlayerVideo ( shared_ptr (new RawImageProxy (_black_image)), - time, Crop (), optional (), _video_container_size, @@ -563,21 +562,21 @@ Player::video (weak_ptr wp, ContentVideo video) optional subtitles; - BOOST_FOREACH (PlayerSubtitles i, _subtitles) { + for (list >::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) { - if (!i.period.overlap (period)) { + if (!i->second.overlap (period)) { continue; } list sub_images; /* Image subtitles */ - list c = transform_image_subtitles (i.image); + list 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 s = render_subtitles (i.text, i.fonts, _video_container_size, time); + if (!i->first.text.empty ()) { + list 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 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 (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 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 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::iterator i = _subtitles.begin (); i != _subtitles.end(); ) { - list::iterator tmp = i; + for (list >::iterator i = _subtitles.begin (); i != _subtitles.end(); ) { + list >::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 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 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 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); } } diff --git a/src/lib/player.h b/src/lib/player.h index 95db8756b..05e994d0b 100644 --- a/src/lib/player.h +++ b/src/lib/player.h @@ -75,9 +75,9 @@ public: */ boost::signals2::signal Changed; - boost::signals2::signal)> Video; + boost::signals2::signal, DCPTime)> Video; boost::signals2::signal, DCPTime)> Audio; - boost::signals2::signal Subtitle; + boost::signals2::signal Subtitle; private: friend class PlayerWrapper; @@ -99,7 +99,7 @@ private: DCPTime resampled_audio_to_dcp (boost::shared_ptr piece, Frame f) const; ContentTime dcp_to_content_time (boost::shared_ptr piece, DCPTime t) const; DCPTime content_time_to_dcp (boost::shared_ptr piece, ContentTime t) const; - boost::shared_ptr black_player_video_frame (DCPTime) const; + boost::shared_ptr black_player_video_frame () const; std::list > overlaps (DCPTime from, DCPTime to, boost::function valid); void video (boost::weak_ptr, ContentVideo); void audio (boost::weak_ptr, AudioStreamPtr, ContentAudio); @@ -136,7 +136,7 @@ private: AudioMerger _audio_merger; DCPTime _last_audio_time; - std::list _subtitles; + std::list > _subtitles; boost::shared_ptr _audio_processor; diff --git a/src/lib/player_subtitles.h b/src/lib/player_subtitles.h index 9ec7b1056..e1a104f21 100644 --- a/src/lib/player_subtitles.h +++ b/src/lib/player_subtitles.h @@ -34,8 +34,6 @@ public: void add_fonts (std::list > fonts_); std::list > fonts; - DCPTimePeriod period; - /** ImageSubtitles, with their rectangles transformed as specified by their content */ std::list image; std::list text; diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc index 7c9a4ee66..b7fb52e3a 100644 --- a/src/lib/player_video.cc +++ b/src/lib/player_video.cc @@ -41,7 +41,6 @@ using dcp::raw_convert; PlayerVideo::PlayerVideo ( shared_ptr in, - DCPTime time, Crop crop, boost::optional fade, dcp::Size inter_size, @@ -51,7 +50,6 @@ PlayerVideo::PlayerVideo ( optional colour_conversion ) : _in (in) - , _time (time) , _crop (crop) , _fade (fade) , _inter_size (inter_size) @@ -65,7 +63,6 @@ PlayerVideo::PlayerVideo ( PlayerVideo::PlayerVideo (shared_ptr node, shared_ptr socket) { - _time = DCPTime (node->number_child ("Time")); _crop = Crop (node); _fade = node->optional_number_child ("Fade"); @@ -149,7 +146,6 @@ PlayerVideo::image (dcp::NoteHandler note, functionadd_child("Time")->add_child_text (raw_convert (_time.get ())); _crop.as_xml (node); if (_fade) { node->add_child("Fade")->add_child_text (raw_convert (_fade.get ())); @@ -208,9 +204,7 @@ PlayerVideo::inter_position () const return Position ((_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 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::clone (DCPTime time) const -{ - shared_ptr c (new PlayerVideo (*this)); - c->_time = time; - return c; -} diff --git a/src/lib/player_video.h b/src/lib/player_video.h index a5ca9b865..60c9224b0 100644 --- a/src/lib/player_video.h +++ b/src/lib/player_video.h @@ -41,7 +41,6 @@ class PlayerVideo public: PlayerVideo ( boost::shared_ptr, - DCPTime, Crop, boost::optional, 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 other) const; - boost::shared_ptr clone (DCPTime time) const; - private: boost::shared_ptr _in; - DCPTime _time; Crop _crop; boost::optional _fade; dcp::Size _inter_size; diff --git a/src/lib/transcoder.cc b/src/lib/transcoder.cc index da6b08c5f..de2fb1d33 100644 --- a/src/lib/transcoder.cc +++ b/src/lib/transcoder.cc @@ -63,9 +63,9 @@ Transcoder::Transcoder (shared_ptr film, weak_ptr 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 c, _film->content ()) { if (c->subtitle && c->subtitle->use() && !c->subtitle->burn()) { @@ -102,14 +102,14 @@ Transcoder::go () } void -Transcoder::video (shared_ptr data) +Transcoder::video (shared_ptr 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 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); } } diff --git a/src/lib/transcoder.h b/src/lib/transcoder.h index 1b20bbffc..0095ad9d1 100644 --- a/src/lib/transcoder.h +++ b/src/lib/transcoder.h @@ -48,9 +48,9 @@ public: private: - void video (boost::shared_ptr); + void video (boost::shared_ptr, DCPTime); void audio (boost::shared_ptr, DCPTime); - void subtitle (PlayerSubtitles); + void subtitle (PlayerSubtitles, DCPTimePeriod); boost::shared_ptr _film; boost::weak_ptr _job; diff --git a/src/lib/writer.cc b/src/lib/writer.cc index 67b00987d..88925cbbd 100644 --- a/src/lib/writer.cc +++ b/src/lib/writer.cc @@ -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; } diff --git a/src/lib/writer.h b/src/lib/writer.h index c1d7e4f8b..14b21f359 100644 --- a/src/lib/writer.h +++ b/src/lib/writer.h @@ -101,7 +101,7 @@ public: bool can_repeat (Frame) const; void repeat (Frame, Eyes); void write (boost::shared_ptr); - void write (PlayerSubtitles subs); + void write (PlayerSubtitles subs, DCPTimePeriod period); void write (std::list > fonts); void write (ReferencedReelAsset asset); void finish (); diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index bddd3abba..c17927ac5 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -181,7 +181,7 @@ FilmViewer::set_film (shared_ptr 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 pv) +FilmViewer::video (shared_ptr pv, DCPTime time) { if (!_player) { return; @@ -235,7 +235,7 @@ FilmViewer::video (shared_ptr pv) ImageChanged (pv); - _position = pv->time (); + _position = time; _inter_position = pv->inter_position (); _inter_size = pv->inter_size (); diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index 399441f18..d01e00290 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -67,7 +67,7 @@ private: void player_changed (bool); void update_position_label (); void update_position_slider (); - void video (boost::shared_ptr); + void video (boost::shared_ptr, DCPTime time); void get (); void seek (DCPTime t, bool accurate); void refresh_panel (); diff --git a/test/client_server_test.cc b/test/client_server_test.cc index f12c5335d..e21f41c79 100644 --- a/test/client_server_test.cc +++ b/test/client_server_test.cc @@ -87,7 +87,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_rgb) shared_ptr pvf ( new PlayerVideo ( shared_ptr (new RawImageProxy (image)), - DCPTime (), Crop (), optional (), dcp::Size (1998, 1080), @@ -167,7 +166,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_yuv) shared_ptr pvf ( new PlayerVideo ( shared_ptr (new RawImageProxy (image)), - DCPTime (), Crop (), optional (), dcp::Size (1998, 1080), @@ -234,7 +232,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k) shared_ptr raw_pvf ( new PlayerVideo ( shared_ptr (new RawImageProxy (image)), - DCPTime (), Crop (), optional (), dcp::Size (1998, 1080), @@ -261,7 +258,6 @@ BOOST_AUTO_TEST_CASE (client_server_test_j2k) shared_ptr j2k_pvf ( new PlayerVideo ( shared_ptr (new J2KImageProxy (raw_locally_encoded, dcp::Size (1998, 1080), AV_PIX_FMT_XYZ12LE)), - DCPTime (), Crop (), optional (), dcp::Size (1998, 1080), -- 2.30.2