Correctly parse \c tags in SSA (with no specified colour) to return to PrimaryColour.

This seems to be what libssa does.

4 files changed, 37 insertions, 26 deletions

diff --git a/src/ssa_reader.cc b/src/ssa_reader.cc
index 07d592a..dad0f26 100644
--- a/src/ssa_reader.cc
+++ b/src/ssa_reader.cc
@@ -200,7 +200,7 @@ SSAReader::parse_time (string t) const
 }
 
 void
-SSAReader::parse_style (RawSubtitle& sub, string style, int play_res_x, int play_res_y)
+SSAReader::parse_style(RawSubtitle& sub, string style, int play_res_x, int play_res_y, Colour primary_colour)
 {
 	if (style == "\\i1") {
 		sub.italic = true;
@@ -254,10 +254,13 @@ SSAReader::parse_style (RawSubtitle& sub, string style, int play_res_x, int play
 		sub.font_size.set_proportional(raw_convert<float>(style.substr(3)) / play_res_y);
 	} else if (boost::starts_with(style, "\\c")) {
 		/* \c&Hbbggrr& */
-		if (style.length() <= 2) {
+		if (style.length() > 2) {
+			sub.colour = h_colour(style.substr(2, style.length() - 3));
+		} else if (style.length() == 2) {
+			sub.colour = primary_colour;
+		} else {
 			throw SSAError(String::compose("Badly formatted colour tag %1", style));
 		}
-		sub.colour = h_colour (style.substr(2, style.length() - 3));
 	}
 }
 
@@ -266,7 +269,7 @@ SSAReader::parse_style (RawSubtitle& sub, string style, int play_res_x, int play
  *  @return List of RawSubtitles to represent line with vertical reference TOP_OF_SUBTITLE.
  */
 vector<RawSubtitle>
-SSAReader::parse_line (RawSubtitle base, string line, int play_res_x, int play_res_y)
+SSAReader::parse_line(RawSubtitle base, string line, int play_res_x, int play_res_y, Colour primary_colour)
 {
 	enum {
 		TEXT,
@@ -328,7 +331,7 @@ SSAReader::parse_line (RawSubtitle base, string line, int play_res_x, int play_r
 					subs.push_back (current);
 					current.text = "";
 				}
-				parse_style (current, style, play_res_x, play_res_y);
+				parse_style(current, style, play_res_x, play_res_y, primary_colour);
 				style = "";
 			}
 
@@ -472,6 +475,7 @@ SSAReader::read (function<optional<string> ()> get_line)
 				SUB_ASSERT (event_format.size() == event.size());
 
 				RawSubtitle sub;
+				optional<Style> style;
 
 				for (size_t i = 0; i < event.size(); ++i) {
 					trim (event[i]);
@@ -485,19 +489,19 @@ SSAReader::read (function<optional<string> ()> get_line)
 						*/
 						trim_left_if (event[i], boost::is_any_of ("*"));
 						SUB_ASSERT (styles.find(event[i]) != styles.end());
-						Style style = styles[event[i]];
-						sub.font = style.font_name;
-						sub.font_size = FontSize::from_proportional(static_cast<float>(style.font_size) / play_res_y);
-						sub.colour = style.primary_colour;
-						sub.effect_colour = style.back_colour;
-						sub.bold = style.bold;
-						sub.italic = style.italic;
-						sub.underline = style.underline;
-						sub.effect = style.effect;
-						sub.horizontal_position.reference = style.horizontal_reference;
-						sub.vertical_position.reference = style.vertical_reference;
+						style = styles[event[i]];
+						sub.font = style->font_name;
+						sub.font_size = FontSize::from_proportional(static_cast<float>(style->font_size) / play_res_y);
+						sub.colour = style->primary_colour;
+						sub.effect_colour = style->back_colour;
+						sub.bold = style->bold;
+						sub.italic = style->italic;
+						sub.underline = style->underline;
+						sub.effect = style->effect;
+						sub.horizontal_position.reference = style->horizontal_reference;
+						sub.vertical_position.reference = style->vertical_reference;
 						if (sub.vertical_position.reference != sub::VERTICAL_CENTRE_OF_SCREEN) {
-							sub.vertical_position.proportional = float(style.vertical_margin) / play_res_y;
+							sub.vertical_position.proportional = float(style->vertical_margin) / play_res_y;
 						}
 					} else if (event_format[i] == "MarginV") {
 						if (event[i] != "0" && sub.vertical_position.reference != sub::VERTICAL_CENTRE_OF_SCREEN) {
@@ -505,7 +509,7 @@ SSAReader::read (function<optional<string> ()> get_line)
 							sub.vertical_position.proportional = raw_convert<float>(event[i]) / play_res_y;
 						}
 					} else if (event_format[i] == "Text") {
-						for (auto j: parse_line (sub, event[i], play_res_x, play_res_y)) {
+						for (auto j: parse_line(sub, event[i], play_res_x, play_res_y, style ? style->primary_colour : Colour(1, 1, 1))) {
 							_subs.push_back (j);
 						}
 					}
diff --git a/src/ssa_reader.h b/src/ssa_reader.h
index e9bb061..45cc271 100644
--- a/src/ssa_reader.h
+++ b/src/ssa_reader.h
@@ -41,8 +41,8 @@ public:
 	SSAReader (FILE* f);
 	SSAReader (std::string subs);
 
-	static std::vector<RawSubtitle> parse_line (RawSubtitle base, std::string line, int play_res_x, int play_res_y);
-	static void parse_style (RawSubtitle& sub, std::string style, int play_res_x, int play_res_y);
+	static std::vector<RawSubtitle> parse_line(RawSubtitle base, std::string line, int play_res_x, int play_res_y, Colour primary_colour);
+	static void parse_style(RawSubtitle& sub, std::string style, int play_res_x, int play_res_y, Colour primary_colour);
 
 private:
 	void read (boost::function<boost::optional<std::string> ()> get_line);
diff --git a/src/subrip_reader.cc b/src/subrip_reader.cc
index 94e1383..7c7b5c2 100644
--- a/src/subrip_reader.cc
+++ b/src/subrip_reader.cc
@@ -268,7 +268,7 @@ SubripReader::convert_line (string t, RawSubtitle& p)
 				++i;
 			}
 			++i;
-			SSAReader::parse_style (p, ssa, 288, 288);
+			SSAReader::parse_style (p, ssa, 288, 288, Colour(1, 1, 1));
 		} else {
 			p.text += t[i];
 			++i;
diff --git a/test/ssa_reader_test.cc b/test/ssa_reader_test.cc
index e6da697..3413e96 100644
--- a/test/ssa_reader_test.cc
+++ b/test/ssa_reader_test.cc
@@ -91,7 +91,8 @@ BOOST_AUTO_TEST_CASE (ssa_reader_line_test1)
 	auto r = sub::SSAReader::parse_line (
 		base,
 		"This is a line with some {\\i1}italics{\\i0} and then\\nthere is a new line.",
-		1920, 1080
+		1920, 1080,
+		sub::Colour(1, 1, 1)
 		);
 
 	auto i = r.begin();
@@ -122,7 +123,8 @@ BOOST_AUTO_TEST_CASE (ssa_reader_line_test2)
 	auto r = sub::SSAReader::parse_line (
 		base,
 		"{\\i1}It's all just italics{\\i0}",
-		1920, 1080
+		1920, 1080,
+		sub::Colour(1, 1, 1)
 		);
 
 	/* Convert a font size in points to a vertical position for this file */
@@ -139,7 +141,8 @@ BOOST_AUTO_TEST_CASE (ssa_reader_line_test2)
 	r = sub::SSAReader::parse_line (
 		base,
 		"{\\i1}Italic{\\i0}\\Nand new line",
-		1920, 1080
+		1920, 1080,
+		sub::Colour(1, 1, 1)
 		);
 
 	i = r.begin ();
@@ -612,7 +615,8 @@ BOOST_AUTO_TEST_CASE (ssa_reader_fs)
 	auto r = sub::SSAReader::parse_line (
 		base,
 		"This is a line with some {\\fs64}font sizing.",
-		1920, 1080
+		1920, 1080,
+		sub::Colour(1, 1, 1)
 		);
 
 	auto i = r.begin ();
@@ -635,7 +639,8 @@ test_c(string command, string colour)
 	auto r = sub::SSAReader::parse_line (
 		base,
 		String::compose("{\\c%1}Hello world", command),
-		1920, 1080
+		1920, 1080,
+		sub::Colour(1, 0, 1)
 		);
 
 	auto i = r.begin ();
@@ -655,5 +660,7 @@ BOOST_AUTO_TEST_CASE (ssa_reader_c)
 	test_c("&HFF00&", "00ff00");
 	test_c("&HFF0000&", "0000ff");
 	test_c("&HFFFFFF&", "ffffff");
+	/* \c with no parameter seems to be parsed as "return to primary colour" */
+	test_c("", "ff00ff");
 }