From df52b97f307605aad15ab5f01c8fdcf93afc9d15 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 9 Jul 2013 22:16:46 +0100 Subject: [PATCH] Various fixes; simplification of FilmViewer; make image appear on first load of content. --- src/lib/ffmpeg_decoder.cc | 16 +++- src/lib/ffmpeg_decoder.h | 1 + src/lib/film.cc | 2 + src/lib/image.cc | 1 + src/lib/player.cc | 32 ++++++-- src/lib/player.h | 8 ++ src/wx/film_viewer.cc | 166 +++++++++----------------------------- src/wx/film_viewer.h | 28 +++---- 8 files changed, 100 insertions(+), 154 deletions(-) diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index bf0949130..3b7d727b8 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -67,6 +67,7 @@ FFmpegDecoder::FFmpegDecoder (shared_ptr f, shared_ptrvideo_frame_rate() / av_q2d (_format_context->streams[_video_stream]->time_base); + int64_t const vt = frame / (_ffmpeg_content->video_frame_rate() * av_q2d (_format_context->streams[_video_stream]->time_base)); av_seek_frame (_format_context, _video_stream, vt, backwards ? AVSEEK_FLAG_BACKWARD : 0); - _video_position = frame; avcodec_flush_buffers (video_codec_context()); if (_subtitle_codec_context) { avcodec_flush_buffers (_subtitle_codec_context); } + _just_sought = true; + if (accurate) { while (1) { int r = av_read_frame (_format_context, &_packet); @@ -420,6 +422,15 @@ FFmpegDecoder::decode_video_packet () if (bet != AV_NOPTS_VALUE) { double const pts = bet * av_q2d (_format_context->streams[_video_stream]->time_base) - _pts_offset; + + if (_just_sought) { + /* We just did a seek, so disable any attempts to correct for where we + are / should be. + */ + _video_position = pts * _ffmpeg_content->video_frame_rate (); + _just_sought = false; + } + double const next = _video_position / _ffmpeg_content->video_frame_rate(); double const one_frame = 1 / _ffmpeg_content->video_frame_rate (); double delta = pts - next; @@ -445,6 +456,7 @@ FFmpegDecoder::decode_video_packet () /* This PTS is within a frame of being right; emit this (otherwise it will be dropped) */ video (image, false, _video_position); } + } else { shared_ptr film = _film.lock (); assert (film); diff --git a/src/lib/ffmpeg_decoder.h b/src/lib/ffmpeg_decoder.h index 8f0482aad..2e64d8801 100644 --- a/src/lib/ffmpeg_decoder.h +++ b/src/lib/ffmpeg_decoder.h @@ -85,4 +85,5 @@ private: bool _decode_audio; double _pts_offset; + bool _just_sought; }; diff --git a/src/lib/film.cc b/src/lib/film.cc index fa75ab1f1..7320676d7 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -128,6 +128,7 @@ Film::Film (string d) } set_directory (result.string ()); + _log.reset (new FileLog ("log")); } Film::Film (Film const & o) @@ -793,6 +794,7 @@ Film::add_content (shared_ptr c) c->set_start (_playlist->video_end ()); } + cout << "actually adding content.\n"; _playlist->add (c); } diff --git a/src/lib/image.cc b/src/lib/image.cc index 722ff5d3c..0eabbe84d 100644 --- a/src/lib/image.cc +++ b/src/lib/image.cc @@ -45,6 +45,7 @@ extern "C" { using std::string; using std::min; +using std::cout; using boost::shared_ptr; using libdcp::Size; diff --git a/src/lib/player.cc b/src/lib/player.cc index 9969fbf9e..7488364bd 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -96,6 +96,7 @@ Player::Player (shared_ptr f, shared_ptr p) { _playlist->Changed.connect (bind (&Player::playlist_changed, this)); _playlist->ContentChanged.connect (bind (&Player::content_changed, this, _1, _2)); + _film->Changed.connect (bind (&Player::film_changed, this, _1)); set_video_container_size (_film->container()->size (_film->full_frame ())); } @@ -120,7 +121,7 @@ Player::pass () } #ifdef DEBUG_PLAYER - cout << "= PASS\n"; + cout << "= PASS " << this << "\n"; #endif Time earliest_t = TIME_MAX; @@ -381,7 +382,7 @@ Player::setup_pieces () Playlist::ContentList content = _playlist->content (); sort (content.begin(), content.end(), ContentSorter ()); - + for (Playlist::ContentList::iterator i = content.begin(); i != content.end(); ++i) { shared_ptr piece (new Piece (*i)); @@ -445,8 +446,13 @@ Player::content_changed (weak_ptr w, int p) return; } - if (p == ContentProperty::START || p == ContentProperty::LENGTH) { + if ( + p == ContentProperty::START || p == ContentProperty::LENGTH || + p == VideoContentProperty::VIDEO_CROP || p == VideoContentProperty::VIDEO_RATIO + ) { + _have_valid_pieces = false; + Changed (); } } @@ -454,6 +460,7 @@ void Player::playlist_changed () { _have_valid_pieces = false; + Changed (); } void @@ -495,5 +502,20 @@ Player::emit_silence (OutputAudioFrame most) _audio_position += _film->audio_frames_to_time (N); } - - +void +Player::film_changed (Film::Property p) +{ + /* Here we should notice Film properties that affect our output, and + alert listeners that our output now would be different to how it was + last time we were run. + */ + + if ( + p == Film::SCALER || p == Film::WITH_SUBTITLES || + p == Film::SUBTITLE_SCALE || p == Film::SUBTITLE_OFFSET || + p == Film::CONTAINER + ) { + + Changed (); + } +} diff --git a/src/lib/player.h b/src/lib/player.h index bbdb14ec2..3f8d59d29 100644 --- a/src/lib/player.h +++ b/src/lib/player.h @@ -26,6 +26,7 @@ #include "playlist.h" #include "audio_buffers.h" #include "content.h" +#include "film.h" class Job; class Film; @@ -67,6 +68,12 @@ public: /** Emitted when some audio data is ready */ boost::signals2::signal, Time)> Audio; + /** Emitted when something has changed such that if we went back and emitted + * the last frame again it would look different. This is not emitted after + * a seek. + */ + boost::signals2::signal Changed; + private: void process_video (boost::weak_ptr, boost::shared_ptr, bool, VideoContent::Frame); @@ -79,6 +86,7 @@ private: void emit_black (); void emit_silence (OutputAudioFrame); boost::shared_ptr resampler (boost::shared_ptr); + void film_changed (Film::Property); boost::shared_ptr _film; boost::shared_ptr _playlist; diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index 8ef64d509..6469d0962 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -58,7 +58,7 @@ FilmViewer::FilmViewer (shared_ptr f, wxWindow* p) , _slider (new wxSlider (this, wxID_ANY, 0, 0, 4096)) , _back_button (new wxButton (this, wxID_ANY, wxT("<"))) , _forward_button (new wxButton (this, wxID_ANY, wxT(">"))) - , _frame (new wxStaticText (this, wxID_ANY, wxT(""))) + , _frame_number (new wxStaticText (this, wxID_ANY, wxT(""))) , _timecode (new wxStaticText (this, wxID_ANY, wxT(""))) , _play_button (new wxToggleButton (this, wxID_ANY, _("Play"))) , _got_frame (false) @@ -79,7 +79,7 @@ FilmViewer::FilmViewer (shared_ptr f, wxWindow* p) wxBoxSizer* h_sizer = new wxBoxSizer (wxHORIZONTAL); wxBoxSizer* time_sizer = new wxBoxSizer (wxVERTICAL); - time_sizer->Add (_frame, 0, wxEXPAND); + time_sizer->Add (_frame_number, 0, wxEXPAND); time_sizer->Add (_timecode, 0, wxEXPAND); h_sizer->Add (_back_button, 0, wxALL, 2); @@ -90,7 +90,7 @@ FilmViewer::FilmViewer (shared_ptr f, wxWindow* p) _v_sizer->Add (h_sizer, 0, wxEXPAND | wxALL, 6); - _frame->SetMinSize (wxSize (84, -1)); + _frame_number->SetMinSize (wxSize (84, -1)); _back_button->SetMinSize (wxSize (32, -1)); _forward_button->SetMinSize (wxSize (32, -1)); @@ -111,37 +111,6 @@ FilmViewer::FilmViewer (shared_ptr f, wxWindow* p) ); } -void -FilmViewer::film_changed (Film::Property p) -{ - switch (p) { - case Film::CONTAINER: - calculate_sizes (); - update_from_raw (); - break; - case Film::CONTENT: - { - calculate_sizes (); - wxScrollEvent ev; - slider_moved (ev); - break; - } - case Film::WITH_SUBTITLES: - case Film::SUBTITLE_OFFSET: - case Film::SUBTITLE_SCALE: - update_from_decoder (); - raw_to_display (); - _panel->Refresh (); - _panel->Update (); - break; - case Film::SCALER: - update_from_decoder (); - break; - default: - break; - } -} - void FilmViewer::set_film (shared_ptr f) { @@ -151,10 +120,7 @@ FilmViewer::set_film (shared_ptr f) _film = f; - _raw_frame.reset (); - _display_frame.reset (); - _panel->Refresh (); - _panel->Update (); + _frame.reset (); if (!_film) { return; @@ -162,29 +128,22 @@ FilmViewer::set_film (shared_ptr f) _player = f->player (); _player->disable_audio (); - _player->Video.connect (bind (&FilmViewer::process_video, this, _1, _2, _3)); - - _film->Changed.connect (boost::bind (&FilmViewer::film_changed, this, _1)); - _film->ContentChanged.connect (boost::bind (&FilmViewer::film_content_changed, this, _1, _2)); - - film_changed (Film::CONTENT); - film_changed (Film::CONTAINER); - film_changed (Film::WITH_SUBTITLES); - film_changed (Film::SUBTITLE_OFFSET); - film_changed (Film::SUBTITLE_SCALE); + _player->Video.connect (boost::bind (&FilmViewer::process_video, this, _1, _2, _3)); + _player->Changed.connect (boost::bind (&FilmViewer::player_changed, this)); + + fetch_current_frame_again (); } void -FilmViewer::update_from_decoder () +FilmViewer::fetch_current_frame_again () { if (!_player) { return; } + /* This will cause a Player::Changed to be emitted */ _player->seek (_player->video_position() - _film->video_frames_to_time (1)); - get_frame (); - _panel->Refresh (); - _panel->Update (); + fetch_next_frame (); } void @@ -194,7 +153,7 @@ FilmViewer::timer (wxTimerEvent &) return; } - get_frame (); + fetch_next_frame (); if (_film->length()) { int const new_slider_position = 4096 * _player->video_position() / _film->length(); @@ -202,9 +161,6 @@ FilmViewer::timer (wxTimerEvent &) _slider->SetValue (new_slider_position); } } - - _panel->Refresh (); - _panel->Update (); } @@ -213,12 +169,14 @@ FilmViewer::paint_panel (wxPaintEvent &) { wxPaintDC dc (_panel); - if (!_display_frame || !_film || !_out_size.width || !_out_size.height) { + if (!_frame || !_film || !_out_size.width || !_out_size.height) { dc.Clear (); return; } - wxImage frame (_out_size.width, _out_size.height, _display_frame->data()[0], true); + shared_ptr packed_frame (new SimpleImage (_frame, false)); + + wxImage frame (_out_size.width, _out_size.height, packed_frame->data()[0], true); wxBitmap frame_bitmap (frame); dc.DrawBitmap (frame_bitmap, 0, 0); @@ -243,15 +201,10 @@ FilmViewer::paint_panel (wxPaintEvent &) void FilmViewer::slider_moved (wxScrollEvent &) { - cout << "slider " << _slider->GetValue() << " " << _film->length() << "\n"; - if (_film && _player) { _player->seek (_slider->GetValue() * _film->length() / 4096); + fetch_next_frame (); } - - get_frame (); - _panel->Refresh (); - _panel->Update (); } void @@ -259,34 +212,9 @@ FilmViewer::panel_sized (wxSizeEvent& ev) { _panel_size.width = ev.GetSize().GetWidth(); _panel_size.height = ev.GetSize().GetHeight(); - calculate_sizes (); - update_from_raw (); + fetch_current_frame_again (); } -void -FilmViewer::update_from_raw () -{ - if (!_raw_frame) { - return; - } - - raw_to_display (); - - _panel->Refresh (); - _panel->Update (); -} - -void -FilmViewer::raw_to_display () -{ - if (!_raw_frame || _out_size.width < 64 || _out_size.height < 64 || !_film) { - return; - } - - /* Get a compacted image as we have to feed it to wxWidgets */ - _display_frame.reset (new SimpleImage (_raw_frame, false)); -} - void FilmViewer::calculate_sizes () { @@ -314,7 +242,6 @@ FilmViewer::calculate_sizes () _out_size.height = max (64, _out_size.height); _player->set_video_container_size (_out_size); - update_from_decoder (); } void @@ -340,15 +267,13 @@ FilmViewer::check_play_state () void FilmViewer::process_video (shared_ptr image, bool, Time t) { - _raw_frame = image; - - raw_to_display (); + _frame = image; _got_frame = true; double const fps = _film->dcp_video_frame_rate (); /* Count frame number from 1 ... not sure if this is the best idea */ - _frame->SetLabel (wxString::Format (wxT("%d"), int (rint (t * fps / TIME_HZ)) + 1)); + _frame_number->SetLabel (wxString::Format (wxT("%d"), int (rint (t * fps / TIME_HZ)) + 1)); double w = static_cast(t) / TIME_HZ; int const h = (w / 3600); @@ -361,36 +286,28 @@ FilmViewer::process_video (shared_ptr image, bool, Time t) _timecode->SetLabel (wxString::Format (wxT("%02d:%02d:%02d:%02d"), h, m, s, f)); } -/** Get a new _raw_frame from the decoder and then do - * raw_to_display (). - */ +/** Ask the player to emit its next frame, then update our display */ void -FilmViewer::get_frame () +FilmViewer::fetch_next_frame () { - /* Clear our raw frame in case we don't get a new one */ - _raw_frame.reset (); + /* Clear our frame in case we don't get a new one */ + _frame.reset (); if (!_player) { - _display_frame.reset (); return; } try { _got_frame = false; - while (!_got_frame) { - if (_player->pass ()) { - /* We didn't get a frame before the decoder gave up, - so clear our display frame. - */ - _display_frame.reset (); - break; - } - } + while (!_got_frame && !_player->pass ()); } catch (DecodeError& e) { _play_button->SetValue (false); check_play_state (); error_dialog (this, wxString::Format (_("Could not decode video for view (%s)"), std_to_wx(e.what()).data())); } + + _panel->Refresh (); + _panel->Update (); } void @@ -413,18 +330,6 @@ FilmViewer::active_jobs_changed (bool a) _play_button->Enable (!a); } -void -FilmViewer::film_content_changed (weak_ptr, int p) -{ - if (p == ContentProperty::LENGTH) { - /* Force an update to our frame */ - wxScrollEvent ev; - slider_moved (ev); - } else if (p == VideoContentProperty::VIDEO_CROP || p == VideoContentProperty::VIDEO_RATIO) { - update_from_decoder (); - } -} - void FilmViewer::back_clicked (wxCommandEvent &) { @@ -433,9 +338,7 @@ FilmViewer::back_clicked (wxCommandEvent &) } _player->seek_back (); - get_frame (); - _panel->Refresh (); - _panel->Update (); + fetch_next_frame (); } void @@ -445,7 +348,12 @@ FilmViewer::forward_clicked (wxCommandEvent &) return; } - get_frame (); - _panel->Refresh (); - _panel->Update (); + fetch_next_frame (); +} + +void +FilmViewer::player_changed () +{ + calculate_sizes (); + fetch_current_frame_again (); } diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index 6c18c7c5b..7587911e8 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -34,20 +34,16 @@ class RGBPlusAlphaImage; * * The film takes the following path through the viewer: * - * 1. get_frame() asks our _player to decode some data. If it does, process_video() + * 1. fetch_next_frame() asks our _player to decode some data. If it does, process_video() * will be called. * - * 2. process_video() takes the image from the decoder (_raw_frame) and calls raw_to_display(). - * - * 3. raw_to_display() copies _raw_frame to _display_frame, processing it and scaling it. + * 2. process_video() takes the image from the player (_frame). * - * 4. calling _panel->Refresh() and _panel->Update() results in paint_panel() being called; - * this creates frame_bitmap from _display_frame and blits it to the display. + * 3. fetch_next_frame() calls _panel->Refresh() and _panel->Update() which results in + * paint_panel() being called; this creates frame_bitmap from _frame and blits it to the display. * - * update_from_decoder() asks the player to re-emit its current frame on the next pass(), and then + * fetch_current_frame_again() asks the player to re-emit its current frame on the next pass(), and then * starts from step #1. - * - * update_from_raw() starts at step #3, then calls _panel->Refresh and _panel->Update. */ class FilmViewer : public wxPanel { @@ -57,8 +53,6 @@ public: void set_film (boost::shared_ptr); private: - void film_changed (Film::Property); - void film_content_changed (boost::weak_ptr, int); void paint_panel (wxPaintEvent &); void panel_sized (wxSizeEvent &); void slider_moved (wxScrollEvent &); @@ -67,13 +61,12 @@ private: void process_video (boost::shared_ptr, bool, Time); void calculate_sizes (); void check_play_state (); - void update_from_raw (); - void update_from_decoder (); - void raw_to_display (); - void get_frame (); + void fetch_current_frame_again (); + void fetch_next_frame (); void active_jobs_changed (bool); void back_clicked (wxCommandEvent &); void forward_clicked (wxCommandEvent &); + void player_changed (); boost::shared_ptr _film; boost::shared_ptr _player; @@ -83,13 +76,12 @@ private: wxSlider* _slider; wxButton* _back_button; wxButton* _forward_button; - wxStaticText* _frame; + wxStaticText* _frame_number; wxStaticText* _timecode; wxToggleButton* _play_button; wxTimer _timer; - boost::shared_ptr _raw_frame; - boost::shared_ptr _display_frame; + boost::shared_ptr _frame; bool _got_frame; /** Size of our output (including padding if we have any) */ -- 2.30.2