From 4a5fcd19ca16447ba8b204986a378072a9c730b2 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 25 Aug 2016 10:50:19 +0100 Subject: [PATCH] Make burnt-in subtitle outline width configurable (#940). --- ChangeLog | 2 + src/lib/player.cc | 3 +- src/lib/player_subtitles.h | 4 +- src/lib/reel_writer.cc | 2 +- src/lib/render_subtitles.cc | 15 +++--- src/lib/render_subtitles.h | 6 +-- src/lib/subtitle_content.cc | 19 +++++++- src/lib/subtitle_content.h | 8 ++++ src/wx/text_subtitle_appearance_dialog.cc | 44 +++++++++++++++--- src/wx/text_subtitle_appearance_dialog.h | 10 ++++ test/render_subtitles_test.cc | 56 ++++++++++++----------- 11 files changed, 119 insertions(+), 50 deletions(-) diff --git a/ChangeLog b/ChangeLog index 42ac1ea08..450f31e49 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 2016-08-25 Carl Hetherington + * Make burnt-in subtitle outline width configurable (#940). + * Updated nl_NL translation from Rob van Nieuwkerk. 2016-08-24 Carl Hetherington diff --git a/src/lib/player.cc b/src/lib/player.cc index 07bb097c2..10c92c073 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -183,6 +183,7 @@ Player::playlist_content_changed (weak_ptr w, int property, bool freque } else if ( property == SubtitleContentProperty::LINE_SPACING || + property == SubtitleContentProperty::OUTLINE_WIDTH || property == SubtitleContentProperty::Y_SCALE ) { @@ -648,7 +649,7 @@ Player::get_subtitles (DCPTime time, DCPTime length, bool starting, bool burnt, } s.set_in (dcp::Time(content_subtitle_to_dcp (*j, ts.period().from).seconds(), 1000)); s.set_out (dcp::Time(content_subtitle_to_dcp (*j, ts.period().to).seconds(), 1000)); - ps.text.push_back (s); + ps.text.push_back (SubtitleString (s, (*j)->content->subtitle->outline_width())); ps.add_fonts ((*j)->content->subtitle->fonts ()); } } diff --git a/src/lib/player_subtitles.h b/src/lib/player_subtitles.h index 9e50ea777..a202cf4f8 100644 --- a/src/lib/player_subtitles.h +++ b/src/lib/player_subtitles.h @@ -23,7 +23,7 @@ #include "image_subtitle.h" #include "dcpomatic_time.h" -#include +#include "subtitle_string.h" class Font; @@ -41,7 +41,7 @@ public: /** ImageSubtitles, with their rectangles transformed as specified by their content */ std::list image; - std::list text; + std::list text; }; #endif diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc index 72e10e86b..d742818ae 100644 --- a/src/lib/reel_writer.cc +++ b/src/lib/reel_writer.cc @@ -491,7 +491,7 @@ ReelWriter::write (PlayerSubtitles subs) } } - BOOST_FOREACH (dcp::SubtitleString i, subs.text) { + BOOST_FOREACH (SubtitleString i, subs.text) { i.set_in (i.in() - dcp::Time (_period.from.seconds(), i.in().tcr)); i.set_out (i.out() - dcp::Time (_period.from.seconds(), i.out().tcr)); _subtitle_asset->add (i); diff --git a/src/lib/render_subtitles.cc b/src/lib/render_subtitles.cc index 6e4dcf46a..c99827d10 100644 --- a/src/lib/render_subtitles.cc +++ b/src/lib/render_subtitles.cc @@ -45,13 +45,13 @@ static FcConfig* fc_config = 0; static list > fc_config_fonts; string -marked_up (list subtitles) +marked_up (list subtitles) { string out; bool italic = false; bool bold = false; bool underline = false; - BOOST_FOREACH (dcp::SubtitleString const & i, subtitles) { + BOOST_FOREACH (SubtitleString const & i, subtitles) { if (i.italic() && !italic) { out += ""; @@ -96,7 +96,7 @@ marked_up (list subtitles) * at the same time and with the same fade in/out. */ static PositionImage -render_line (list subtitles, list > fonts, dcp::Size target, DCPTime time) +render_line (list subtitles, list > fonts, dcp::Size target, DCPTime time) { /* XXX: this method can only handle italic / bold changes mid-line, nothing else yet. @@ -285,8 +285,7 @@ render_line (list subtitles, list > fonts, /* Border effect; stroke the subtitle with a large (arbitrarily chosen) line width */ dcp::Colour ec = subtitles.front().effect_colour (); context->set_source_rgba (float(ec.r) / 255, float(ec.g) / 255, float(ec.b) / 255, fade_factor); - /* This 300.0 is a magic number chosen to make the outline look good */ - context->set_line_width (target.width / 300.0); + context->set_line_width (subtitles.front().outline_width * target.width / 2048.0); context->set_line_join (Cairo::LINE_JOIN_ROUND); context->move_to (x_offset, 0); layout->add_to_cairo_context (context); @@ -350,12 +349,12 @@ render_line (list subtitles, list > fonts, /** @param time Time of the frame that these subtitles are going on */ list -render_subtitles (list subtitles, list > fonts, dcp::Size target, DCPTime time) +render_subtitles (list subtitles, list > fonts, dcp::Size target, DCPTime time) { - list pending; + list pending; list images; - BOOST_FOREACH (dcp::SubtitleString const & i, subtitles) { + BOOST_FOREACH (SubtitleString const & i, subtitles) { if (!pending.empty() && fabs (i.v_position() - pending.back().v_position()) > 1e-4) { images.push_back (render_line (pending, fonts, target, time)); pending.clear (); diff --git a/src/lib/render_subtitles.h b/src/lib/render_subtitles.h index 79194568a..f81685a28 100644 --- a/src/lib/render_subtitles.h +++ b/src/lib/render_subtitles.h @@ -20,12 +20,12 @@ #include "position_image.h" #include "dcpomatic_time.h" -#include +#include "subtitle_string.h" #include class Font; -std::string marked_up (std::list subtitles); +std::string marked_up (std::list subtitles); std::list render_subtitles ( - std::list, std::list > fonts, dcp::Size, DCPTime + std::list, std::list > fonts, dcp::Size, DCPTime ); diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc index 34ffa3f4b..37d431e1a 100644 --- a/src/lib/subtitle_content.cc +++ b/src/lib/subtitle_content.cc @@ -54,6 +54,7 @@ int const SubtitleContentProperty::EFFECT_COLOUR = 511; int const SubtitleContentProperty::LINE_SPACING = 512; int const SubtitleContentProperty::FADE_IN = 513; int const SubtitleContentProperty::FADE_OUT = 514; +int const SubtitleContentProperty::OUTLINE_WIDTH = 515; SubtitleContent::SubtitleContent (Content* parent) : ContentPart (parent) @@ -68,6 +69,7 @@ SubtitleContent::SubtitleContent (Content* parent) , _shadow (false) , _effect_colour (0, 0, 0) , _line_spacing (1) + , _outline_width (2) { } @@ -111,6 +113,7 @@ SubtitleContent::SubtitleContent (Content* parent, cxml::ConstNodePtr node, int , _line_spacing (node->optional_number_child("LineSpacing").get_value_or (1)) , _fade_in (node->optional_number_child("SubtitleFadeIn").get_value_or (0)) , _fade_out (node->optional_number_child("SubtitleFadeOut").get_value_or (0)) + , _outline_width (node->optional_number_child("OutlineWidth").get_value_or (2)) { if (version >= 32) { _use = node->bool_child ("UseSubtitles"); @@ -196,6 +199,10 @@ SubtitleContent::SubtitleContent (Content* parent, vector > throw JoinError (_("Content to be joined must have the same subtitle fades.")); } + if ((c[i]->subtitle->outline_width() != ref->outline_width())) { + throw JoinError (_("Content to be joined must have the same outline width.")); + } + list > fonts = c[i]->subtitle->fonts (); if (fonts.size() != ref_fonts.size()) { throw JoinError (_("Content to be joined must use the same fonts.")); @@ -224,6 +231,7 @@ SubtitleContent::SubtitleContent (Content* parent, vector > _line_spacing = ref->line_spacing (); _fade_in = ref->fade_in (); _fade_out = ref->fade_out (); + _outline_width = ref->outline_width (); connect_to_fonts (); } @@ -252,6 +260,7 @@ SubtitleContent::as_xml (xmlpp::Node* root) const root->add_child("LineSpacing")->add_child_text (raw_convert (_line_spacing)); root->add_child("SubtitleFadeIn")->add_child_text (raw_convert (_fade_in.get())); root->add_child("SubtitleFadeOut")->add_child_text (raw_convert (_fade_out.get())); + root->add_child("OutlineWidth")->add_child_text (raw_convert (_outline_width)); for (list >::const_iterator i = _fonts.begin(); i != _fonts.end(); ++i) { (*i)->as_xml (root->add_child("Font")); @@ -267,7 +276,8 @@ SubtitleContent::identifier () const + "_" + raw_convert (y_offset()) + "_" + raw_convert (line_spacing()) + "_" + raw_convert (fade_in().get()) - + "_" + raw_convert (fade_out().get()); + + "_" + raw_convert (fade_out().get()) + + "_" + raw_convert (outline_width()); /* XXX: I suppose really _fonts shouldn't be in here, since not all types of subtitle content involve fonts. @@ -396,6 +406,12 @@ SubtitleContent::set_fade_out (ContentTime t) maybe_set (_fade_out, t, SubtitleContentProperty::FADE_OUT); } +void +SubtitleContent::set_outline_width (int w) +{ + maybe_set (_outline_width, w, SubtitleContentProperty::OUTLINE_WIDTH); +} + void SubtitleContent::use_template (shared_ptr c) { @@ -413,4 +429,5 @@ SubtitleContent::use_template (shared_ptr c) _line_spacing = c->_line_spacing; _fade_in = c->_fade_in; _fade_out = c->_fade_out; + _outline_width = c->_outline_width; } diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h index ddc97ce9f..7278b5408 100644 --- a/src/lib/subtitle_content.h +++ b/src/lib/subtitle_content.h @@ -46,6 +46,7 @@ public: static int const LINE_SPACING; static int const FADE_IN; static int const FADE_OUT; + static int const OUTLINE_WIDTH; }; /** @class SubtitleContent @@ -80,6 +81,7 @@ public: void set_line_spacing (double s); void set_fade_in (ContentTime); void set_fade_out (ContentTime); + void set_outline_width (int); bool use () const { boost::mutex::scoped_lock lm (_mutex); @@ -156,6 +158,11 @@ public: return _fade_out; } + int outline_width () const { + boost::mutex::scoped_lock lm (_mutex); + return _outline_width; + } + static boost::shared_ptr from_xml (Content* parent, cxml::ConstNodePtr, int version); protected: @@ -194,6 +201,7 @@ private: double _line_spacing; ContentTime _fade_in; ContentTime _fade_out; + int _outline_width; }; #endif diff --git a/src/wx/text_subtitle_appearance_dialog.cc b/src/wx/text_subtitle_appearance_dialog.cc index d00dd2cf1..292a8ed2a 100644 --- a/src/wx/text_subtitle_appearance_dialog.cc +++ b/src/wx/text_subtitle_appearance_dialog.cc @@ -23,8 +23,14 @@ #include "lib/subtitle_content.h" #include #include +#include using boost::shared_ptr; +using boost::bind; + +int const TextSubtitleAppearanceDialog::NONE = 0; +int const TextSubtitleAppearanceDialog::OUTLINE = 1; +int const TextSubtitleAppearanceDialog::SHADOW = 2; TextSubtitleAppearanceDialog::TextSubtitleAppearanceDialog (wxWindow* parent, shared_ptr content) : TableDialog (parent, _("Subtitle appearance"), 2, 1, true) @@ -38,8 +44,10 @@ TextSubtitleAppearanceDialog::TextSubtitleAppearanceDialog (wxWindow* parent, sh add (_effect = new wxChoice (this, wxID_ANY)); add (_("Outline / shadow colour"), true); - _effect_colour = new wxColourPickerCtrl (this, wxID_ANY); - add (_effect_colour); + add (_effect_colour = new wxColourPickerCtrl (this, wxID_ANY)); + + add (_("Outline width"), true); + add (_outline_width = new wxSpinCtrl (this, wxID_ANY)); add (_("Fade in time"), true); _fade_in = new Timecode (this); @@ -51,23 +59,30 @@ TextSubtitleAppearanceDialog::TextSubtitleAppearanceDialog (wxWindow* parent, sh layout (); + /* Keep these Appends() up to date with NONE/OUTLINE/SHADOW variables */ _effect->Append (_("None")); _effect->Append (_("Outline")); _effect->Append (_("Shadow"));; _colour->SetColour (wxColour (_content->subtitle->colour().r, _content->subtitle->colour().g, _content->subtitle->colour().b)); if (_content->subtitle->outline()) { - _effect->SetSelection (1); + _effect->SetSelection (OUTLINE); } else if (_content->subtitle->shadow()) { - _effect->SetSelection (2); + _effect->SetSelection (SHADOW); } else { - _effect->SetSelection (0); + _effect->SetSelection (NONE); } _effect_colour->SetColour ( wxColour (_content->subtitle->effect_colour().r, _content->subtitle->effect_colour().g, _content->subtitle->effect_colour().b) ); _fade_in->set (_content->subtitle->fade_in(), _content->active_video_frame_rate ()); _fade_out->set (_content->subtitle->fade_out(), _content->active_video_frame_rate ()); + _outline_width->SetValue (_content->subtitle->outline_width ()); + + _effect->Bind (wxEVT_COMMAND_CHOICE_SELECTED, bind (&TextSubtitleAppearanceDialog::setup_sensitivity, this)); + _content_connection = _content->Changed.connect (bind (&TextSubtitleAppearanceDialog::setup_sensitivity, this)); + + setup_sensitivity (); } void @@ -75,10 +90,25 @@ TextSubtitleAppearanceDialog::apply () { wxColour const c = _colour->GetColour (); _content->subtitle->set_colour (dcp::Colour (c.Red(), c.Green(), c.Blue())); - _content->subtitle->set_outline (_effect->GetSelection() == 1); - _content->subtitle->set_shadow (_effect->GetSelection() == 2); + _content->subtitle->set_outline (_effect->GetSelection() == OUTLINE); + _content->subtitle->set_shadow (_effect->GetSelection() == SHADOW); wxColour const ec = _effect_colour->GetColour (); _content->subtitle->set_effect_colour (dcp::Colour (ec.Red(), ec.Green(), ec.Blue())); _content->subtitle->set_fade_in (_fade_in->get (_content->active_video_frame_rate ())); _content->subtitle->set_fade_out (_fade_out->get (_content->active_video_frame_rate ())); + _content->subtitle->set_outline_width (_outline_width->GetValue ()); +} + +void +TextSubtitleAppearanceDialog::setup_sensitivity () +{ + _effect_colour->Enable (_effect->GetSelection() != NONE); + + bool const can_outline_width = _effect->GetSelection() == OUTLINE && _content->subtitle->burn (); + _outline_width->Enable (can_outline_width); + if (can_outline_width) { + _outline_width->SetToolTip (_("Outline width cannot be set unless you are burning in subtitles")); + } else { + _outline_width->UnsetToolTip (); + } } diff --git a/src/wx/text_subtitle_appearance_dialog.h b/src/wx/text_subtitle_appearance_dialog.h index 381b36ec7..c191cc3c0 100644 --- a/src/wx/text_subtitle_appearance_dialog.h +++ b/src/wx/text_subtitle_appearance_dialog.h @@ -21,6 +21,7 @@ #include "table_dialog.h" #include "timecode.h" #include +#include class wxRadioButton; class wxColourPickerCtrl; @@ -34,11 +35,20 @@ public: void apply (); private: + void setup_sensitivity (); + wxColourPickerCtrl* _colour; wxChoice* _effect; wxColourPickerCtrl* _effect_colour; Timecode* _fade_in; Timecode* _fade_out; + wxSpinCtrl* _outline_width; boost::shared_ptr _content; + + boost::signals2::scoped_connection _content_connection; + + static int const NONE; + static int const OUTLINE; + static int const SHADOW; }; diff --git a/test/render_subtitles_test.cc b/test/render_subtitles_test.cc index 9ab4f5deb..91aafb072 100644 --- a/test/render_subtitles_test.cc +++ b/test/render_subtitles_test.cc @@ -23,29 +23,31 @@ #include static void -add (std::list& s, std::string text, bool italic, bool bold, bool underline) +add (std::list& s, std::string text, bool italic, bool bold, bool underline) { s.push_back ( - dcp::SubtitleString ( - boost::optional (), - italic, - bold, - underline, - dcp::Colour (255, 255, 255), - 42, - 1, - dcp::Time (), - dcp::Time (), - 1, - dcp::HALIGN_LEFT, - 1, - dcp::VALIGN_TOP, - dcp::DIRECTION_LTR, - text, - dcp::NONE, - dcp::Colour (0, 0, 0), - dcp::Time (), - dcp::Time () + SubtitleString ( + dcp::SubtitleString ( + boost::optional (), + italic, + bold, + underline, + dcp::Colour (255, 255, 255), + 42, + 1, + dcp::Time (), + dcp::Time (), + 1, + dcp::HALIGN_LEFT, + 1, + dcp::VALIGN_TOP, + dcp::DIRECTION_LTR, + text, + dcp::NONE, + dcp::Colour (0, 0, 0), + dcp::Time (), + dcp::Time () + ) ) ); } @@ -53,7 +55,7 @@ add (std::list& s, std::string text, bool italic, bool bold /** Test marked_up() in render_subtitles.cc */ BOOST_AUTO_TEST_CASE (render_markup_test1) { - std::list s; + std::list s; add (s, "Hello", false, false, false); BOOST_CHECK_EQUAL (marked_up (s), "Hello"); } @@ -61,7 +63,7 @@ BOOST_AUTO_TEST_CASE (render_markup_test1) /** Test marked_up() in render_subtitles.cc */ BOOST_AUTO_TEST_CASE (render_markup_test2) { - std::list s; + std::list s; add (s, "Hello", false, true, false); BOOST_CHECK_EQUAL (marked_up (s), "Hello"); } @@ -70,7 +72,7 @@ BOOST_AUTO_TEST_CASE (render_markup_test2) /** Test marked_up() in render_subtitles.cc */ BOOST_AUTO_TEST_CASE (render_markup_test3) { - std::list s; + std::list s; add (s, "Hello", true, true, false); BOOST_CHECK_EQUAL (marked_up (s), "Hello"); } @@ -78,7 +80,7 @@ BOOST_AUTO_TEST_CASE (render_markup_test3) /** Test marked_up() in render_subtitles.cc */ BOOST_AUTO_TEST_CASE (render_markup_test4) { - std::list s; + std::list s; add (s, "Hello", true, true, true); BOOST_CHECK_EQUAL (marked_up (s), "Hello"); } @@ -86,7 +88,7 @@ BOOST_AUTO_TEST_CASE (render_markup_test4) /** Test marked_up() in render_subtitles.cc */ BOOST_AUTO_TEST_CASE (render_markup_test5) { - std::list s; + std::list s; add (s, "Hello", false, true, false); add (s, " world.", false, false, false); BOOST_CHECK_EQUAL (marked_up (s), "Hello world."); @@ -95,7 +97,7 @@ BOOST_AUTO_TEST_CASE (render_markup_test5) /** Test marked_up() in render_subtitles.cc */ BOOST_AUTO_TEST_CASE (render_markup_test6) { - std::list s; + std::list s; add (s, "Hello", true, false, false); add (s, " world ", false, false, false); add (s, "we are bold.", false, true, false); -- 2.30.2