Use an enum for the effect in SubtitleContent.

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 499aa2a389c0c1545d82b915f17daef218d8df4e..8e56991f816eb10a843fdf90261d561937fcaa5d 100644 (file)
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -186,8 +186,7 @@ Player::playlist_content_changed (weak_ptr<Content> w, int property, bool freque
                property == DCPContentProperty::NEEDS_ASSETS ||
                property == DCPContentProperty::NEEDS_KDM ||
                property == SubtitleContentProperty::COLOUR ||
-               property == SubtitleContentProperty::OUTLINE ||
-               property == SubtitleContentProperty::SHADOW ||
+               property == SubtitleContentProperty::EFFECT ||
                property == SubtitleContentProperty::EFFECT_COLOUR ||
                property == FFmpegContentProperty::SUBTITLE_STREAM ||
                property == FFmpegContentProperty::FILTERS ||

diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc
index 0083ff8a2baf85dfd65e891fbd19ac007c5c3ba4..df05b356afda68b43a53b5f2be04df0fe935039b 100644 (file)
--- a/src/lib/subtitle_content.cc
+++ b/src/lib/subtitle_content.cc
@@ -49,13 +49,12 @@ int const SubtitleContentProperty::BURN = 505;
 int const SubtitleContentProperty::LANGUAGE = 506;
 int const SubtitleContentProperty::FONTS = 507;
 int const SubtitleContentProperty::COLOUR = 508;
-int const SubtitleContentProperty::OUTLINE = 509;
-int const SubtitleContentProperty::SHADOW = 510;
-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;
+int const SubtitleContentProperty::EFFECT = 509;
+int const SubtitleContentProperty::EFFECT_COLOUR = 510;
+int const SubtitleContentProperty::LINE_SPACING = 511;
+int const SubtitleContentProperty::FADE_IN = 512;
+int const SubtitleContentProperty::FADE_OUT = 513;
+int const SubtitleContentProperty::OUTLINE_WIDTH = 514;
 
 SubtitleContent::SubtitleContent (Content* parent)
        : ContentPart (parent)
@@ -65,8 +64,7 @@ SubtitleContent::SubtitleContent (Content* parent)
        , _y_offset (0)
        , _x_scale (1)
        , _y_scale (1)
-       , _outline (false)
-       , _shadow (false)
+       , _effect (dcp::NONE)
        , _line_spacing (1)
        , _outline_width (2)
 {
@@ -102,8 +100,6 @@ SubtitleContent::SubtitleContent (Content* parent, cxml::ConstNodePtr node, int
        , _y_offset (0)
        , _x_scale (1)
        , _y_scale (1)
-       , _outline (node->optional_bool_child("Outline").get_value_or(false))
-       , _shadow (node->optional_bool_child("Shadow").get_value_or(false))
        , _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))
@@ -121,6 +117,25 @@ SubtitleContent::SubtitleContent (Content* parent, cxml::ConstNodePtr node, int
                _y_offset = node->number_child<double> ("SubtitleOffset");
        }
 
+       if (node->optional_bool_child("Outline").get_value_or(false)) {
+               _effect = dcp::BORDER;
+       } else if (node->optional_bool_child("Shadow").get_value_or(false)) {
+               _effect = dcp::SHADOW;
+       } else {
+               _effect = dcp::NONE;
+       }
+
+       optional<string> effect = node->optional_string_child("Effect");
+       if (effect) {
+               if (*effect == "none") {
+                       _effect = dcp::NONE;
+               } else if (*effect == "outline") {
+                       _effect = dcp::BORDER;
+               } else if (*effect == "shadow") {
+                       _effect = dcp::SHADOW;
+               }
+       }
+
        if (version >= 10) {
                _x_scale = node->number_child<double> ("SubtitleXScale");
                _y_scale = node->number_child<double> ("SubtitleYScale");
@@ -256,8 +271,17 @@ SubtitleContent::as_xml (xmlpp::Node* root) const
                root->add_child("Green")->add_child_text (raw_convert<string> (_colour->g));
                root->add_child("Blue")->add_child_text (raw_convert<string> (_colour->b));
        }
-       root->add_child("Outline")->add_child_text (_outline ? "1" : "0");
-       root->add_child("Shadow")->add_child_text (_shadow ? "1" : "0");
+       switch (_effect) {
+       case dcp::NONE:
+               root->add_child("none");
+               break;
+       case dcp::BORDER:
+               root->add_child("outline");
+               break;
+       case dcp::SHADOW:
+               root->add_child("shadow");
+               break;
+       }
        if (_effect_colour) {
                root->add_child("EffectRed")->add_child_text (raw_convert<string> (_effect_colour->r));
                root->add_child("EffectGreen")->add_child_text (raw_convert<string> (_effect_colour->g));
@@ -341,15 +365,9 @@ SubtitleContent::unset_colour ()
 }
 
 void
-SubtitleContent::set_outline (bool o)
-{
-       maybe_set (_outline, o, SubtitleContentProperty::OUTLINE);
-}
-
-void
-SubtitleContent::set_shadow (bool s)
+SubtitleContent::set_effect (dcp::Effect e)
 {
-       maybe_set (_shadow, s, SubtitleContentProperty::SHADOW);
+       maybe_set (_effect, e, SubtitleContentProperty::EFFECT);
 }
 
 void
@@ -445,8 +463,7 @@ SubtitleContent::take_settings_from (shared_ptr<const SubtitleContent> c)
        } else {
                unset_colour ();
        }
-       set_outline (c->_outline);
-       set_shadow (c->_shadow);
+       set_effect (c->_effect);
        if (c->_effect_colour) {
                set_effect_colour (*c->_effect_colour);
        } else {

diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h
index baf412bb6aafb28ea3ad8af21b15c2d6b97ebddb..73e53e446ba59c8ef5cebc20bb8d26e645e8a6de 100644 (file)
--- a/src/lib/subtitle_content.h
+++ b/src/lib/subtitle_content.h
@@ -40,8 +40,7 @@ public:
        static int const LANGUAGE;
        static int const FONTS;
        static int const COLOUR;
-       static int const OUTLINE;
-       static int const SHADOW;
+       static int const EFFECT;
        static int const EFFECT_COLOUR;
        static int const LINE_SPACING;
        static int const FADE_IN;
@@ -76,8 +75,7 @@ public:
        void set_language (std::string language);
        void set_colour (dcp::Colour);
        void unset_colour ();
-       void set_outline (bool);
-       void set_shadow (bool);
+       void set_effect (dcp::Effect);
        void set_effect_colour (dcp::Colour);
        void unset_effect_colour ();
        void set_line_spacing (double s);
@@ -130,14 +128,9 @@ public:
                return _colour;
        }
 
-       bool outline () const {
+       dcp::Effect effect () const {
                boost::mutex::scoped_lock lm (_mutex);
-               return _outline;
-       }
-
-       bool shadow () const {
-               boost::mutex::scoped_lock lm (_mutex);
-               return _shadow;
+               return _effect;
        }
 
        boost::optional<dcp::Colour> effect_colour () const {
@@ -196,8 +189,7 @@ private:
        double _y_scale;
        std::list<boost::shared_ptr<Font> > _fonts;
        boost::optional<dcp::Colour> _colour;
-       bool _outline;
-       bool _shadow;
+       dcp::Effect _effect;
        boost::optional<dcp::Colour> _effect_colour;
        /** scaling factor for line spacing; 1 is "standard", < 1 is closer together, > 1 is further apart */
        double _line_spacing;

diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc
index 8badf5183892a19107678d7c4b532250dfd5a426..90bedf0ce8996925bc1c52d90704b43a4565b98e 100644 (file)
--- a/src/lib/subtitle_decoder.cc
+++ b/src/lib/subtitle_decoder.cc
@@ -82,11 +82,7 @@ SubtitleDecoder::emit_text_start (ContentTime from, list<dcp::SubtitleString> s)
                if (content()->effect_colour()) {
                        i.set_effect_colour (*content()->effect_colour());
                }
-               if (content()->outline()) {
-                       i.set_effect (dcp::BORDER);
-               } else if (content()->shadow()) {
-                       i.set_effect (dcp::SHADOW);
-               }
+               i.set_effect (content()->effect());
                i.set_fade_up_time (dcp::Time(content()->fade_in().seconds(), 1000));
                i.set_fade_down_time (dcp::Time(content()->fade_out().seconds(), 1000));
        }

diff --git a/src/wx/subtitle_appearance_dialog.cc b/src/wx/subtitle_appearance_dialog.cc
index 5cb668e7ba69b6466aef4981346b2a39d3fc8c60..929bc6006b6c7e8dc2bec3d4ee3ece33f508d38b 100644 (file)
--- a/src/wx/subtitle_appearance_dialog.cc
+++ b/src/wx/subtitle_appearance_dialog.cc
@@ -157,12 +157,16 @@ SubtitleAppearanceDialog::SubtitleAppearanceDialog (wxWindow* parent, shared_ptr
                _colour->SetColour (wxColour (255, 255, 255));
        }
 
-       if (_content->subtitle->outline()) {
+       switch (_content->subtitle->effect()) {
+       case dcp::NONE:
+               _effect->SetSelection (NONE);
+               break;
+       case dcp::BORDER:
                _effect->SetSelection (OUTLINE);
-       } else if (_content->subtitle->shadow()) {
+               break;
+       case dcp::SHADOW:
                _effect->SetSelection (SHADOW);
-       } else {
-               _effect->SetSelection (NONE);
+               break;
        }
 
        optional<dcp::Colour> effect_colour = _content->subtitle->effect_colour();
@@ -195,8 +199,17 @@ SubtitleAppearanceDialog::apply ()
        } else {
                _content->subtitle->unset_colour ();
        }
-       _content->subtitle->set_outline (_effect->GetSelection() == OUTLINE);
-       _content->subtitle->set_shadow (_effect->GetSelection() == SHADOW);
+       switch (_effect->GetSelection()) {
+       case NONE:
+               _content->subtitle->set_effect (dcp::NONE);
+               break;
+       case OUTLINE:
+               _content->subtitle->set_effect (dcp::BORDER);
+               break;
+       case SHADOW:
+               _content->subtitle->set_effect (dcp::SHADOW);
+               break;
+       }
        if (_force_effect_colour->GetValue ()) {
                wxColour const ec = _effect_colour->GetColour ();
                _content->subtitle->set_effect_colour (dcp::Colour (ec.Red(), ec.Green(), ec.Blue()));

diff --git a/test/ffmpeg_encoder_test.cc b/test/ffmpeg_encoder_test.cc
index e6cff9b8fdfb57df583e57e3d7342a56ff8f9bb5..df64a2c486dce626c95006206753b48b427f30a3 100644 (file)
--- a/test/ffmpeg_encoder_test.cc
+++ b/test/ffmpeg_encoder_test.cc
@@ -76,7 +76,7 @@ BOOST_AUTO_TEST_CASE (ffmpeg_encoder_test_subs_h264_1)
        film->examine_and_add_content (s);
        BOOST_REQUIRE (!wait_for_jobs ());
        s->subtitle->set_colour (dcp::Colour (255, 255, 0));
-       s->subtitle->set_shadow (true);
+       s->subtitle->set_effect (dcp::SHADOW);
        s->subtitle->set_effect_colour (dcp::Colour (0, 255, 255));
        film->write_metadata();
 
@@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE (ffmpeg_encoder_test_subs_h264_2)
        film->examine_and_add_content (s);
        BOOST_REQUIRE (!wait_for_jobs ());
        s->subtitle->set_colour (dcp::Colour (255, 255, 0));
-       s->subtitle->set_shadow (true);
+       s->subtitle->set_effect (dcp::SHADOW);
        s->subtitle->set_effect_colour (dcp::Colour (0, 255, 255));
        film->write_metadata();
 
@@ -122,7 +122,7 @@ BOOST_AUTO_TEST_CASE (ffmpeg_encoder_test_subs_prores_1)
        film->examine_and_add_content (s);
        BOOST_REQUIRE (!wait_for_jobs ());
        s->subtitle->set_colour (dcp::Colour (255, 255, 0));
-       s->subtitle->set_shadow (true);
+       s->subtitle->set_effect (dcp::SHADOW);
        s->subtitle->set_effect_colour (dcp::Colour (0, 255, 255));
        film->write_metadata();
 
@@ -147,7 +147,7 @@ BOOST_AUTO_TEST_CASE (ffmpeg_encoder_test_subs_prores_2)
        film->examine_and_add_content (s);
        BOOST_REQUIRE (!wait_for_jobs ());
        s->subtitle->set_colour (dcp::Colour (255, 255, 0));
-       s->subtitle->set_shadow (true);
+       s->subtitle->set_effect (dcp::SHADOW);
        s->subtitle->set_effect_colour (dcp::Colour (0, 255, 255));
 
        shared_ptr<Job> job (new TranscodeJob (film));