Use optional<> for PositionImage in PlayerVideo to make things a bit clearer.

2 files changed, 26 insertions, 16 deletions

diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index c2ba1362a..7b6ea574e 100644
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -72,13 +72,13 @@ PlayerVideo::PlayerVideo (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket
 
 	if (node->optional_number_child<int> ("SubtitleX")) {
 
-		_subtitle.position = Position<int> (node->number_child<int> ("SubtitleX"), node->number_child<int> ("SubtitleY"));
-
-		_subtitle.image.reset (
+		shared_ptr<Image> image (
 			new Image (PIX_FMT_RGBA, dcp::Size (node->number_child<int> ("SubtitleWidth"), node->number_child<int> ("SubtitleHeight")), true)
 			);
 
-		_subtitle.image->read_from_socket (socket);
+		image->read_from_socket (socket);
+
+		_subtitle = PositionImage (image, Position<int> (node->number_child<int> ("SubtitleX"), node->number_child<int> ("SubtitleY")));
 	}
 }
 
@@ -118,8 +118,8 @@ PlayerVideo::image (AVPixelFormat pixel_format, bool burn_subtitle, dcp::NoteHan
 
 	shared_ptr<Image> out = im->crop_scale_window (total_crop, _inter_size, _out_size, yuv_to_rgb, pixel_format, true);
 
-	if (burn_subtitle && _subtitle.image) {
-		out->alpha_blend (_subtitle.image, _subtitle.position);
+	if (burn_subtitle && _subtitle) {
+		out->alpha_blend (_subtitle->image, _subtitle->position);
 	}
 
 	if (_fade) {
@@ -147,11 +147,11 @@ PlayerVideo::add_metadata (xmlpp::Node* node, bool send_subtitles) const
 	if (_colour_conversion) {
 		_colour_conversion.get().as_xml (node);
 	}
-	if (send_subtitles && _subtitle.image) {
-		node->add_child ("SubtitleWidth")->add_child_text (raw_convert<string> (_subtitle.image->size().width));
-		node->add_child ("SubtitleHeight")->add_child_text (raw_convert<string> (_subtitle.image->size().height));
-		node->add_child ("SubtitleX")->add_child_text (raw_convert<string> (_subtitle.position.x));
-		node->add_child ("SubtitleY")->add_child_text (raw_convert<string> (_subtitle.position.y));
+	if (send_subtitles && _subtitle) {
+		node->add_child ("SubtitleWidth")->add_child_text (raw_convert<string> (_subtitle->image->size().width));
+		node->add_child ("SubtitleHeight")->add_child_text (raw_convert<string> (_subtitle->image->size().height));
+		node->add_child ("SubtitleX")->add_child_text (raw_convert<string> (_subtitle->position.x));
+		node->add_child ("SubtitleY")->add_child_text (raw_convert<string> (_subtitle->position.y));
 	}
 }
 
@@ -159,8 +159,8 @@ void
 PlayerVideo::send_binary (shared_ptr<Socket> socket, bool send_subtitles) const
 {
 	_in->send_binary (socket);
-	if (send_subtitles && _subtitle.image) {
-		_subtitle.image->write_to_socket (socket);
+	if (send_subtitles && _subtitle) {
+		_subtitle->image->write_to_socket (socket);
 	}
 }
 
@@ -203,11 +203,21 @@ PlayerVideo::same (shared_ptr<const PlayerVideo> other) const
 	    _out_size != other->_out_size ||
 	    _eyes != other->_eyes ||
 	    _part != other->_part ||
-	    _colour_conversion != other->_colour_conversion ||
-	    !_subtitle.same (other->_subtitle)) {
+	    _colour_conversion != other->_colour_conversion) {
+		return false;
+	}
 
+	if ((!_subtitle && other->_subtitle) || (_subtitle && !other->_subtitle)) {
+		/* One has a subtitle and the other doesn't */
 		return false;
 	}
 
+	if (_subtitle && other->_subtitle && !_subtitle->same (other->_subtitle.get ())) {
+		/* They both have subtitles but they are different */
+		return false;
+	}
+
+	/* Now neither has subtitles */
+
 	return _in->same (other->_in);
 }
diff --git a/src/lib/player_video.h b/src/lib/player_video.h
index 276c97948..50c27bebe 100644
--- a/src/lib/player_video.h
+++ b/src/lib/player_video.h
@@ -94,5 +94,5 @@ private:
 	Eyes _eyes;
 	Part _part;
 	boost::optional<ColourConversion> _colour_conversion;
-	PositionImage _subtitle;
+	boost::optional<PositionImage> _subtitle;
 };