From dc1c279211773ad68cb0348eb802cf5c9594d8dd Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 20 Jun 2014 23:01:12 +0100 Subject: [PATCH 1/1] Various subtitle UI tweaks. --- src/lib/ffmpeg_content.cc | 2 +- src/lib/film.cc | 20 -------- src/lib/film.h | 9 ---- src/lib/player.cc | 9 +++- src/lib/playlist.cc | 18 ------- src/lib/playlist.h | 2 - src/lib/subtitle_content.cc | 20 ++++++++ src/lib/subtitle_content.h | 8 +++ src/wx/film_editor.cc | 6 +-- src/wx/subtitle_panel.cc | 97 +++++++++++++++++++------------------ src/wx/subtitle_panel.h | 4 +- src/wx/subtitle_view.cc | 2 +- test/burnt_subtitle_test.cc | 2 +- 13 files changed, 91 insertions(+), 108 deletions(-) diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc index d3e0fa7b2..a12f45a30 100644 --- a/src/lib/ffmpeg_content.cc +++ b/src/lib/ffmpeg_content.cc @@ -110,7 +110,7 @@ FFmpegContent::FFmpegContent (shared_ptr f, vector fc = dynamic_pointer_cast (c[i]); - if (f->with_subtitles() && *(fc->_subtitle_stream.get()) != *(ref->_subtitle_stream.get())) { + if (fc->subtitle_use() && *(fc->_subtitle_stream.get()) != *(ref->_subtitle_stream.get())) { throw JoinError (_("Content to be joined must use the same subtitle stream.")); } diff --git a/src/lib/film.cc b/src/lib/film.cc index 14735d1b1..b01a70b72 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -110,7 +110,6 @@ Film::Film (boost::filesystem::path dir, bool log) , _container (Config::instance()->default_container ()) , _resolution (RESOLUTION_2K) , _scaler (Scaler::from_id ("bicubic")) - , _with_subtitles (false) , _signed (true) , _encrypted (false) , _j2k_bandwidth (Config::instance()->default_j2k_bandwidth ()) @@ -187,10 +186,6 @@ Film::video_identifier () const s << "_3D"; } - if (_with_subtitles) { - s << "_WS"; - } - return s.str (); } @@ -370,7 +365,6 @@ Film::metadata () const root->add_child("Resolution")->add_child_text (resolution_to_string (_resolution)); root->add_child("Scaler")->add_child_text (_scaler->id ()); - root->add_child("WithSubtitles")->add_child_text (_with_subtitles ? "1" : "0"); root->add_child("J2KBandwidth")->add_child_text (raw_convert (_j2k_bandwidth)); _isdcf_metadata.as_xml (root->add_child ("ISDCFMetadata")); root->add_child("VideoFrameRate")->add_child_text (raw_convert (_video_frame_rate)); @@ -442,7 +436,6 @@ Film::read_metadata () _resolution = string_to_resolution (f.string_child ("Resolution")); _scaler = Scaler::from_id (f.string_child ("Scaler")); - _with_subtitles = f.bool_child ("WithSubtitles"); _j2k_bandwidth = f.number_child ("J2KBandwidth"); _video_frame_rate = f.number_child ("VideoFrameRate"); _signed = f.optional_bool_child("Signed").get_value_or (true); @@ -713,13 +706,6 @@ Film::set_scaler (Scaler const * s) signal_changed (SCALER); } -void -Film::set_with_subtitles (bool w) -{ - _with_subtitles = w; - signal_changed (WITH_SUBTITLES); -} - void Film::set_j2k_bandwidth (int b) { @@ -969,12 +955,6 @@ Film::length () const return _playlist->length (); } -bool -Film::has_subtitles () const -{ - return _playlist->has_subtitles (); -} - int Film::best_video_frame_rate () const { diff --git a/src/lib/film.h b/src/lib/film.h index 67d00aa54..a44c606d6 100644 --- a/src/lib/film.h +++ b/src/lib/film.h @@ -115,7 +115,6 @@ public: ContentList content () const; DCPTime length () const; - bool has_subtitles () const; int best_video_frame_rate () const; FrameRateChange active_frame_rate_change (DCPTime) const; @@ -155,7 +154,6 @@ public: CONTAINER, RESOLUTION, SCALER, - WITH_SUBTITLES, SIGNED, ENCRYPTED, J2K_BANDWIDTH, @@ -199,10 +197,6 @@ public: return _scaler; } - bool with_subtitles () const { - return _with_subtitles; - } - /* signed is a reserved word */ bool is_signed () const { return _signed; @@ -256,7 +250,6 @@ public: void set_container (Ratio const *); void set_resolution (Resolution); void set_scaler (Scaler const *); - void set_with_subtitles (bool); void set_signed (bool); void set_encrypted (bool); void set_j2k_bandwidth (int); @@ -309,8 +302,6 @@ private: Resolution _resolution; /** Scaler algorithm to use */ Scaler const * _scaler; - /** True if subtitles should be shown for this film */ - bool _with_subtitles; bool _signed; bool _encrypted; /** bandwidth for J2K files in bits per second */ diff --git a/src/lib/player.cc b/src/lib/player.cc index 418f360fe..bb1a4cdeb 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -179,6 +179,7 @@ Player::content_changed (weak_ptr w, int property, bool frequent) Changed (frequent); } else if ( + property == SubtitleContentProperty::SUBTITLE_USE || property == SubtitleContentProperty::SUBTITLE_X_OFFSET || property == SubtitleContentProperty::SUBTITLE_Y_OFFSET || property == SubtitleContentProperty::SUBTITLE_SCALE || @@ -216,7 +217,7 @@ Player::film_changed (Film::Property p) last time we were run. */ - if (p == Film::SCALER || p == Film::WITH_SUBTITLES || p == Film::CONTAINER || p == Film::VIDEO_FRAME_RATE) { + if (p == Film::SCALER || p == Film::CONTAINER || p == Film::VIDEO_FRAME_RATE) { Changed (false); } } @@ -375,8 +376,12 @@ Player::get_video (DCPTime time, bool accurate) list sub_images; for (list >::const_iterator j = subs.begin(); j != subs.end(); ++j) { - shared_ptr subtitle_decoder = dynamic_pointer_cast ((*j)->decoder); shared_ptr subtitle_content = dynamic_pointer_cast ((*j)->content); + if (!subtitle_content->subtitle_use ()) { + continue; + } + + shared_ptr subtitle_decoder = dynamic_pointer_cast ((*j)->decoder); ContentTime const from = dcp_to_content_subtitle (*j, time); /* XXX: this video_frame_rate() should be the rate that the subtitle content has been prepared for */ ContentTime const to = from + ContentTime::from_frames (1, _film->video_frame_rate ()); diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc index e555db9ba..1c268ca11 100644 --- a/src/lib/playlist.cc +++ b/src/lib/playlist.cc @@ -186,24 +186,6 @@ Playlist::remove (ContentList c) Changed (); } -bool -Playlist::has_subtitles () const -{ - for (ContentList::const_iterator i = _content.begin(); i != _content.end(); ++i) { - shared_ptr fc = dynamic_pointer_cast (*i); - if (fc && !fc->subtitle_streams().empty()) { - return true; - } - - shared_ptr sc = dynamic_pointer_cast (*i); - if (sc) { - return true; - } - } - - return false; -} - class FrameRateCandidate { public: diff --git a/src/lib/playlist.h b/src/lib/playlist.h index 7c29b8588..3e5093aca 100644 --- a/src/lib/playlist.h +++ b/src/lib/playlist.h @@ -63,8 +63,6 @@ public: void move_earlier (boost::shared_ptr); void move_later (boost::shared_ptr); - bool has_subtitles () const; - ContentList content () const; std::string video_identifier () const; diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc index 068bf7516..f839a56d0 100644 --- a/src/lib/subtitle_content.cc +++ b/src/lib/subtitle_content.cc @@ -35,9 +35,11 @@ using dcp::raw_convert; int const SubtitleContentProperty::SUBTITLE_X_OFFSET = 500; int const SubtitleContentProperty::SUBTITLE_Y_OFFSET = 501; int const SubtitleContentProperty::SUBTITLE_SCALE = 502; +int const SubtitleContentProperty::SUBTITLE_USE = 503; SubtitleContent::SubtitleContent (shared_ptr f, boost::filesystem::path p) : Content (f, p) + , _subtitle_use (false) , _subtitle_x_offset (0) , _subtitle_y_offset (0) , _subtitle_scale (1) @@ -47,11 +49,13 @@ SubtitleContent::SubtitleContent (shared_ptr f, boost::filesystem::p SubtitleContent::SubtitleContent (shared_ptr f, cxml::ConstNodePtr node, int version) : Content (f, node) + , _subtitle_use (false) , _subtitle_x_offset (0) , _subtitle_y_offset (0) , _subtitle_scale (1) { if (version >= 7) { + _subtitle_use = node->bool_child ("SubtitleUse"); _subtitle_x_offset = node->number_child ("SubtitleXOffset"); _subtitle_y_offset = node->number_child ("SubtitleYOffset"); } else { @@ -70,6 +74,10 @@ SubtitleContent::SubtitleContent (shared_ptr f, vector sc = dynamic_pointer_cast (c[i]); + if (sc->subtitle_use() != ref->subtitle_use()) { + throw JoinError (_("Content to be joined must have the same 'use subtitles' setting.")); + } + if (sc->subtitle_x_offset() != ref->subtitle_x_offset()) { throw JoinError (_("Content to be joined must have the same subtitle X offset.")); } @@ -83,6 +91,7 @@ SubtitleContent::SubtitleContent (shared_ptr f, vectorsubtitle_use (); _subtitle_x_offset = ref->subtitle_x_offset (); _subtitle_y_offset = ref->subtitle_y_offset (); _subtitle_scale = ref->subtitle_scale (); @@ -91,11 +100,22 @@ SubtitleContent::SubtitleContent (shared_ptr f, vectoradd_child("SubtitleUse")->add_child_text (raw_convert (_subtitle_use)); root->add_child("SubtitleXOffset")->add_child_text (raw_convert (_subtitle_x_offset)); root->add_child("SubtitleYOffset")->add_child_text (raw_convert (_subtitle_y_offset)); root->add_child("SubtitleScale")->add_child_text (raw_convert (_subtitle_scale)); } +void +SubtitleContent::set_subtitle_use (bool u) +{ + { + boost::mutex::scoped_lock lm (_mutex); + _subtitle_use = u; + } + signal_changed (SubtitleContentProperty::SUBTITLE_USE); +} + void SubtitleContent::set_subtitle_x_offset (double o) { diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h index 7d4a385c9..b73119bdb 100644 --- a/src/lib/subtitle_content.h +++ b/src/lib/subtitle_content.h @@ -28,6 +28,7 @@ public: static int const SUBTITLE_X_OFFSET; static int const SUBTITLE_Y_OFFSET; static int const SUBTITLE_SCALE; + static int const SUBTITLE_USE; }; class SubtitleContent : public virtual Content @@ -39,10 +40,16 @@ public: void as_xml (xmlpp::Node *) const; + void set_subtitle_use (bool); void set_subtitle_x_offset (double); void set_subtitle_y_offset (double); void set_subtitle_scale (double); + bool subtitle_use () const { + boost::mutex::scoped_lock lm (_mutex); + return _subtitle_use; + } + double subtitle_x_offset () const { boost::mutex::scoped_lock lm (_mutex); return _subtitle_x_offset; @@ -61,6 +68,7 @@ public: private: friend class ffmpeg_pts_offset_test; + bool _subtitle_use; /** x offset for placing subtitles, as a proportion of the container width; * +ve is further right, -ve is further left. */ diff --git a/src/wx/film_editor.cc b/src/wx/film_editor.cc index 252a89719..002f89604 100644 --- a/src/wx/film_editor.cc +++ b/src/wx/film_editor.cc @@ -439,9 +439,6 @@ FilmEditor::film_changed (Film::Property p) checked_set (_name, _film->name()); setup_dcp_name (); break; - case Film::WITH_SUBTITLES: - setup_dcp_name (); - break; case Film::DCP_CONTENT_TYPE: checked_set (_dcp_content_type, DCPContentType::as_index (_film->dcp_content_type ())); setup_dcp_name (); @@ -528,7 +525,7 @@ FilmEditor::film_content_changed (int property) (*i)->film_content_changed (property); } - if (property == FFmpegContentProperty::AUDIO_STREAM) { + if (property == FFmpegContentProperty::AUDIO_STREAM || property == SubtitleContentProperty::SUBTITLE_USE) { setup_dcp_name (); } else if (property == ContentProperty::PATH) { setup_content (); @@ -615,7 +612,6 @@ FilmEditor::set_film (shared_ptr f) film_changed (Film::CONTAINER); film_changed (Film::RESOLUTION); film_changed (Film::SCALER); - film_changed (Film::WITH_SUBTITLES); film_changed (Film::SIGNED); film_changed (Film::ENCRYPTED); film_changed (Film::J2K_BANDWIDTH); diff --git a/src/wx/subtitle_panel.cc b/src/wx/subtitle_panel.cc index b68b491d7..4b7f1ca53 100644 --- a/src/wx/subtitle_panel.cc +++ b/src/wx/subtitle_panel.cc @@ -40,12 +40,12 @@ SubtitlePanel::SubtitlePanel (FilmEditor* e) wxFlexGridSizer* grid = new wxFlexGridSizer (2, DCPOMATIC_SIZER_X_GAP, DCPOMATIC_SIZER_Y_GAP); _sizer->Add (grid, 0, wxALL, 8); - _with_subtitles = new wxCheckBox (this, wxID_ANY, _("With Subtitles")); - grid->Add (_with_subtitles, 1); + _use = new wxCheckBox (this, wxID_ANY, _("Use subtitles")); + grid->Add (_use); grid->AddSpacer (0); - + { - add_label_to_sizer (grid, this, _("Subtitle X Offset"), true); + add_label_to_sizer (grid, this, _("X Offset"), true); wxBoxSizer* s = new wxBoxSizer (wxHORIZONTAL); _x_offset = new wxSpinCtrl (this); s->Add (_x_offset); @@ -54,7 +54,7 @@ SubtitlePanel::SubtitlePanel (FilmEditor* e) } { - add_label_to_sizer (grid, this, _("Subtitle Y Offset"), true); + add_label_to_sizer (grid, this, _("Y Offset"), true); wxBoxSizer* s = new wxBoxSizer (wxHORIZONTAL); _y_offset = new wxSpinCtrl (this); s->Add (_y_offset); @@ -63,7 +63,7 @@ SubtitlePanel::SubtitlePanel (FilmEditor* e) } { - add_label_to_sizer (grid, this, _("Subtitle Scale"), true); + add_label_to_sizer (grid, this, _("Scale"), true); wxBoxSizer* s = new wxBoxSizer (wxHORIZONTAL); _scale = new wxSpinCtrl (this); s->Add (_scale); @@ -71,7 +71,7 @@ SubtitlePanel::SubtitlePanel (FilmEditor* e) grid->Add (s); } - add_label_to_sizer (grid, this, _("Subtitle Stream"), true); + add_label_to_sizer (grid, this, _("Stream"), true); _stream = new wxChoice (this, wxID_ANY); grid->Add (_stream, 1, wxEXPAND); @@ -83,27 +83,19 @@ SubtitlePanel::SubtitlePanel (FilmEditor* e) _scale->SetRange (1, 1000); _scale->SetValue (100); - _with_subtitles->Bind (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&SubtitlePanel::with_subtitles_toggled, this)); - _x_offset->Bind (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::x_offset_changed, this)); - _y_offset->Bind (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::y_offset_changed, this)); - _scale->Bind (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::scale_changed, this)); - _stream->Bind (wxEVT_COMMAND_CHOICE_SELECTED, boost::bind (&SubtitlePanel::stream_changed, this)); - _view_button->Bind (wxEVT_COMMAND_BUTTON_CLICKED, boost::bind (&SubtitlePanel::view_clicked, this)); + _use->Bind (wxEVT_COMMAND_CHECKBOX_CLICKED, boost::bind (&SubtitlePanel::use_toggled, this)); + _x_offset->Bind (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::x_offset_changed, this)); + _y_offset->Bind (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::y_offset_changed, this)); + _scale->Bind (wxEVT_COMMAND_SPINCTRL_UPDATED, boost::bind (&SubtitlePanel::scale_changed, this)); + _stream->Bind (wxEVT_COMMAND_CHOICE_SELECTED, boost::bind (&SubtitlePanel::stream_changed, this)); + _view_button->Bind (wxEVT_COMMAND_BUTTON_CLICKED, boost::bind (&SubtitlePanel::view_clicked, this)); } void SubtitlePanel::film_changed (Film::Property property) { - switch (property) { - case Film::CONTENT: + if (property == Film::CONTENT) { setup_sensitivity (); - break; - case Film::WITH_SUBTITLES: - checked_set (_with_subtitles, _editor->film()->with_subtitles ()); - setup_sensitivity (); - break; - default: - break; } } @@ -122,7 +114,7 @@ SubtitlePanel::film_content_changed (int property) if (sc.size() == 1) { scs = sc.front (); } - + if (property == FFmpegContentProperty::SUBTITLE_STREAMS) { _stream->Clear (); if (fcs) { @@ -138,6 +130,9 @@ SubtitlePanel::film_content_changed (int property) } } setup_sensitivity (); + } else if (property == SubtitleContentProperty::SUBTITLE_USE) { + checked_set (_use, scs ? scs->subtitle_use() : false); + setup_sensitivity (); } else if (property == SubtitleContentProperty::SUBTITLE_X_OFFSET) { checked_set (_x_offset, scs ? (scs->subtitle_x_offset() * 100) : 0); } else if (property == SubtitleContentProperty::SUBTITLE_Y_OFFSET) { @@ -148,33 +143,40 @@ SubtitlePanel::film_content_changed (int property) } void -SubtitlePanel::with_subtitles_toggled () +SubtitlePanel::use_toggled () { - if (!_editor->film()) { - return; + SubtitleContentList c = _editor->selected_subtitle_content (); + for (SubtitleContentList::iterator i = c.begin(); i != c.end(); ++i) { + (*i)->set_subtitle_use (_use->GetValue()); } - - _editor->film()->set_with_subtitles (_with_subtitles->GetValue ()); } void SubtitlePanel::setup_sensitivity () { - bool h = false; - bool j = false; - if (_editor->film()) { - h = _editor->film()->has_subtitles (); - j = _editor->film()->with_subtitles (); + int any_subs = 0; + int ffmpeg_subs = 0; + SubtitleContentList c = _editor->selected_subtitle_content (); + for (SubtitleContentList::const_iterator i = c.begin(); i != c.end(); ++i) { + shared_ptr fc = boost::dynamic_pointer_cast (*i); + if (fc) { + if (!fc->subtitle_streams().empty ()) { + ++ffmpeg_subs; + ++any_subs; + } + } else { + ++any_subs; + } } + + _use->Enable (any_subs > 0); + bool const use = _use->GetValue (); - _with_subtitles->Enable (h); - _x_offset->Enable (j); - _y_offset->Enable (j); - _scale->Enable (j); - _stream->Enable (j); - - SubtitleContentList c = _editor->selected_subtitle_content (); - _view_button->Enable (c.size() == 1); + _x_offset->Enable (any_subs > 0 && use); + _y_offset->Enable (any_subs > 0 && use); + _scale->Enable (any_subs > 0 && use); + _stream->Enable (ffmpeg_subs == 1); + _view_button->Enable (any_subs == 1); } void @@ -203,8 +205,8 @@ void SubtitlePanel::x_offset_changed () { SubtitleContentList c = _editor->selected_subtitle_content (); - if (c.size() == 1) { - c.front()->set_subtitle_x_offset (_x_offset->GetValue() / 100.0); + for (SubtitleContentList::iterator i = c.begin(); i != c.end(); ++i) { + (*i)->set_subtitle_x_offset (_x_offset->GetValue() / 100.0); } } @@ -212,8 +214,8 @@ void SubtitlePanel::y_offset_changed () { SubtitleContentList c = _editor->selected_subtitle_content (); - if (c.size() == 1) { - c.front()->set_subtitle_y_offset (_y_offset->GetValue() / 100.0); + for (SubtitleContentList::iterator i = c.begin(); i != c.end(); ++i) { + (*i)->set_subtitle_y_offset (_y_offset->GetValue() / 100.0); } } @@ -221,8 +223,8 @@ void SubtitlePanel::scale_changed () { SubtitleContentList c = _editor->selected_subtitle_content (); - if (c.size() == 1) { - c.front()->set_subtitle_scale (_scale->GetValue() / 100.0); + for (SubtitleContentList::iterator i = c.begin(); i != c.end(); ++i) { + (*i)->set_subtitle_scale (_scale->GetValue() / 100.0); } } @@ -230,6 +232,7 @@ void SubtitlePanel::content_selection_changed () { film_content_changed (FFmpegContentProperty::SUBTITLE_STREAMS); + film_content_changed (SubtitleContentProperty::SUBTITLE_USE); film_content_changed (SubtitleContentProperty::SUBTITLE_X_OFFSET); film_content_changed (SubtitleContentProperty::SUBTITLE_Y_OFFSET); film_content_changed (SubtitleContentProperty::SUBTITLE_SCALE); diff --git a/src/wx/subtitle_panel.h b/src/wx/subtitle_panel.h index 1ee775025..c5665098b 100644 --- a/src/wx/subtitle_panel.h +++ b/src/wx/subtitle_panel.h @@ -33,7 +33,7 @@ public: void content_selection_changed (); private: - void with_subtitles_toggled (); + void use_toggled (); void x_offset_changed (); void y_offset_changed (); void scale_changed (); @@ -42,7 +42,7 @@ private: void setup_sensitivity (); - wxCheckBox* _with_subtitles; + wxCheckBox* _use; wxSpinCtrl* _x_offset; wxSpinCtrl* _y_offset; wxSpinCtrl* _scale; diff --git a/src/wx/subtitle_view.cc b/src/wx/subtitle_view.cc index b8b62b586..e4604ccde 100644 --- a/src/wx/subtitle_view.cc +++ b/src/wx/subtitle_view.cc @@ -60,7 +60,7 @@ SubtitleView::SubtitleView (wxWindow* parent, shared_ptr film, shared_ptr< wxBoxSizer* sizer = new wxBoxSizer (wxVERTICAL); sizer->Add (_list, 1, wxEXPAND); - wxSizer* buttons = CreateSeparatedButtonSizer (wxOK | wxCANCEL); + wxSizer* buttons = CreateSeparatedButtonSizer (wxOK); if (buttons) { sizer->Add (buttons, wxSizerFlags().Expand().DoubleBorder()); } diff --git a/test/burnt_subtitle_test.cc b/test/burnt_subtitle_test.cc index c5d6c4c14..acd30d030 100644 --- a/test/burnt_subtitle_test.cc +++ b/test/burnt_subtitle_test.cc @@ -38,8 +38,8 @@ BOOST_AUTO_TEST_CASE (burnt_subtitle_test) film->set_container (Ratio::from_id ("185")); film->set_dcp_content_type (DCPContentType::from_isdcf_name ("TLR")); film->set_name ("frobozz"); - film->set_with_subtitles (true); shared_ptr content (new SubRipContent (film, "test/data/subrip2.srt")); + content->set_subtitle_use (true); film->examine_and_add_content (content); wait_for_jobs (); film->make_dcp (); -- 2.30.2