Make burnt-in subtitle outline width configurable (#940).

diff --git a/ChangeLog b/ChangeLog
index 42ac1ea081d08d7012ec2946f5ca4675bfebc776..450f31e49c9a1582ab4190ea75ad6573c8f872f9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 2016-08-25  Carl Hetherington  <cth@carlh.net>
 
+       * Make burnt-in subtitle outline width configurable (#940).
+
        * Updated nl_NL translation from Rob van Nieuwkerk.
 
 2016-08-24  Carl Hetherington  <cth@carlh.net>

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 07bb097c2632a95df233497c7dd5a42aa1783c13..10c92c0736eb87385b5d4e0e20f88ba95c473f43 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -183,6 +183,7 @@ Player::playlist_content_changed (weak_ptr<Content> 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 9e50ea7776c961ba86f3446438e7a131c4ac9f08..a202cf4f846260b1ff4ca729ebb5ec23c315b98f 100644 (file)
--- 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 <dcp/subtitle_string.h>
+#include "subtitle_string.h"
 
 class Font;
 
@@ -41,7 +41,7 @@ public:
 
        /** ImageSubtitles, with their rectangles transformed as specified by their content */
        std::list<ImageSubtitle> image;
-       std::list<dcp::SubtitleString> text;
+       std::list<SubtitleString> text;
 };
 
 #endif

diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 72e10e86ba78a7170e5902c506cd70ab0d4fe788..d742818ae08849bfe81e191c3f922deb01a3b6c0 100644 (file)
--- 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 6e4dcf46ad4958d899eb55eccaacf8690041a06b..c99827d108da8421faa7a65b8b278283a932d089 100644 (file)
--- a/src/lib/render_subtitles.cc
+++ b/src/lib/render_subtitles.cc
@@ -45,13 +45,13 @@ static FcConfig* fc_config = 0;
 static list<pair<FontFiles, string> > fc_config_fonts;
 
 string
-marked_up (list<dcp::SubtitleString> subtitles)
+marked_up (list<SubtitleString> 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 += "<i>";
@@ -96,7 +96,7 @@ marked_up (list<dcp::SubtitleString> subtitles)
  *  at the same time and with the same fade in/out.
  */
 static PositionImage
-render_line (list<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
+render_line (list<SubtitleString> subtitles, list<shared_ptr<Font> > 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<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > 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<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts,
 
 /** @param time Time of the frame that these subtitles are going on */
 list<PositionImage>
-render_subtitles (list<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
+render_subtitles (list<SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
 {
-       list<dcp::SubtitleString> pending;
+       list<SubtitleString> pending;
        list<PositionImage> 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 79194568a9211511da7a675ba5404758d47b073a..f81685a2871813c0a290daa27e74a7df9de5bed0 100644 (file)
--- 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 <dcp/subtitle_string.h>
+#include "subtitle_string.h"
 #include <dcp/util.h>
 
 class Font;
 
-std::string marked_up (std::list<dcp::SubtitleString> subtitles);
+std::string marked_up (std::list<SubtitleString> subtitles);
 std::list<PositionImage> render_subtitles (
-       std::list<dcp::SubtitleString>, std::list<boost::shared_ptr<Font> > fonts, dcp::Size, DCPTime
+       std::list<SubtitleString>, std::list<boost::shared_ptr<Font> > fonts, dcp::Size, DCPTime
        );

diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc
index 34ffa3f4ba3790f278950b6d9d60c50292a330a8..37d431e1aa1701234cc0827c206ca559de0900e1 100644 (file)
--- 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<double>("LineSpacing").get_value_or (1))
        , _fade_in (node->optional_number_child<Frame>("SubtitleFadeIn").get_value_or (0))
        , _fade_out (node->optional_number_child<Frame>("SubtitleFadeOut").get_value_or (0))
+       , _outline_width (node->optional_number_child<int>("OutlineWidth").get_value_or (2))
 {
        if (version >= 32) {
                _use = node->bool_child ("UseSubtitles");
@@ -196,6 +199,10 @@ SubtitleContent::SubtitleContent (Content* parent, vector<shared_ptr<Content> >
                        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<shared_ptr<Font> > 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<shared_ptr<Content> >
        _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<string> (_line_spacing));
        root->add_child("SubtitleFadeIn")->add_child_text (raw_convert<string> (_fade_in.get()));
        root->add_child("SubtitleFadeOut")->add_child_text (raw_convert<string> (_fade_out.get()));
+       root->add_child("OutlineWidth")->add_child_text (raw_convert<string> (_outline_width));
 
        for (list<shared_ptr<Font> >::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<string> (y_offset())
                + "_" + raw_convert<string> (line_spacing())
                + "_" + raw_convert<string> (fade_in().get())
-               + "_" + raw_convert<string> (fade_out().get());
+               + "_" + raw_convert<string> (fade_out().get())
+               + "_" + raw_convert<string> (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<const SubtitleContent> c)
 {
@@ -413,4 +429,5 @@ SubtitleContent::use_template (shared_ptr<const SubtitleContent> 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 ddc97ce9fdd0c69ab7be5813a2396d3d007cdc62..7278b540892681fecff8a9d038c04970b3a79700 100644 (file)
--- 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<SubtitleContent> 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 d00dd2cf16ca6e4075fbe5b6d0b9455eb99a4ee1..292a8ed2ab21248aff8806ee60b1166ebd66b9c3 100644 (file)
--- 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 <wx/wx.h>
 #include <wx/clrpicker.h>
+#include <wx/spinctrl.h>
 
 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> 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<ContentTime> (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 381b36ec7f63c4d765b7083c7c7bdfa43bea421f..c191cc3c01d052c73bfbf56ca1fc7d9a8b79d5e6 100644 (file)
--- 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 <boost/shared_ptr.hpp>
+#include <boost/signals2.hpp>
 
 class wxRadioButton;
 class wxColourPickerCtrl;
@@ -34,11 +35,20 @@ public:
        void apply ();
 
 private:
+       void setup_sensitivity ();
+
        wxColourPickerCtrl* _colour;
        wxChoice* _effect;
        wxColourPickerCtrl* _effect_colour;
        Timecode<ContentTime>* _fade_in;
        Timecode<ContentTime>* _fade_out;
+       wxSpinCtrl* _outline_width;
 
        boost::shared_ptr<Content> _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 9ab4f5debd8bb7084ae6ee3de751886cc6825b9c..91aafb072929e9158f95c81a352cdbe4820de0b1 100644 (file)
--- a/test/render_subtitles_test.cc
+++ b/test/render_subtitles_test.cc
@@ -23,29 +23,31 @@
 #include <boost/test/unit_test.hpp>
 
 static void
-add (std::list<dcp::SubtitleString>& s, std::string text, bool italic, bool bold, bool underline)
+add (std::list<SubtitleString>& s, std::string text, bool italic, bool bold, bool underline)
 {
        s.push_back (
-               dcp::SubtitleString (
-                       boost::optional<std::string> (),
-                       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<std::string> (),
+                               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<dcp::SubtitleString>& s, std::string text, bool italic, bool bold
 /** Test marked_up() in render_subtitles.cc */
 BOOST_AUTO_TEST_CASE (render_markup_test1)
 {
-       std::list<dcp::SubtitleString> s;
+       std::list<SubtitleString> 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<dcp::SubtitleString> s;
+       std::list<SubtitleString> s;
        add (s, "Hello", false, true, false);
        BOOST_CHECK_EQUAL (marked_up (s), "<b>Hello</b>");
 }
@@ -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<dcp::SubtitleString> s;
+       std::list<SubtitleString> s;
        add (s, "Hello", true, true, false);
        BOOST_CHECK_EQUAL (marked_up (s), "<i><b>Hello</b></i>");
 }
@@ -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<dcp::SubtitleString> s;
+       std::list<SubtitleString> s;
        add (s, "Hello", true, true, true);
        BOOST_CHECK_EQUAL (marked_up (s), "<i><b><u>Hello</u></b></i>");
 }
@@ -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<dcp::SubtitleString> s;
+       std::list<SubtitleString> s;
        add (s, "Hello", false, true, false);
        add (s, " world.", false, false, false);
        BOOST_CHECK_EQUAL (marked_up (s), "<b>Hello</b> 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<dcp::SubtitleString> s;
+       std::list<SubtitleString> s;
        add (s, "Hello", true, false, false);
        add (s, " world ", false, false, false);
        add (s, "we are bold.", false, true, false);