Ignore and report failures to decode frames during playback (#1593).

6 files changed, 66 insertions, 28 deletions

diff --git a/src/lib/image_proxy.h b/src/lib/image_proxy.h
index 9619fab75..1d57b4e08 100644
--- a/src/lib/image_proxy.h
+++ b/src/lib/image_proxy.h
@@ -64,6 +64,13 @@ public:
 		Result (boost::shared_ptr<Image> image_, int log2_scaling_)
 			: image (image_)
 			, log2_scaling (log2_scaling_)
+			, error (false)
+		{}
+
+		Result (boost::shared_ptr<Image> image_, int log2_scaling_, bool error_)
+			: image (image_)
+			, log2_scaling (log2_scaling_)
+			, error (error_)
 		{}
 
 		/** Image (which will be aligned) */
@@ -73,6 +80,8 @@ public:
 		 *  will be 1.
 		 */
 		int log2_scaling;
+		/** true if there was an error during image decoding, otherwise false */
+		bool error;
 	};
 
 	/** @param log Log to write to, or 0.
diff --git a/src/lib/j2k_image_proxy.cc b/src/lib/j2k_image_proxy.cc
index da3e23caf..0e3fa88f5 100644
--- a/src/lib/j2k_image_proxy.cc
+++ b/src/lib/j2k_image_proxy.cc
@@ -51,6 +51,7 @@ J2KImageProxy::J2KImageProxy (boost::filesystem::path path, dcp::Size size, AVPi
 	: _data (path)
 	, _size (size)
 	, _pixel_format (pixel_format)
+	, _error (false)
 {
 	/* ::image assumes 16bpp */
 	DCPOMATIC_ASSERT (_pixel_format == AV_PIX_FMT_RGB48 || _pixel_format == AV_PIX_FMT_XYZ12LE);
@@ -66,6 +67,7 @@ J2KImageProxy::J2KImageProxy (
 	, _size (size)
 	, _pixel_format (pixel_format)
 	, _forced_reduction (forced_reduction)
+	, _error (false)
 {
 	/* ::image assumes 16bpp */
 	DCPOMATIC_ASSERT (_pixel_format == AV_PIX_FMT_RGB48 || _pixel_format == AV_PIX_FMT_XYZ12LE);
@@ -83,6 +85,7 @@ J2KImageProxy::J2KImageProxy (
 	, _eye (eye)
 	, _pixel_format (pixel_format)
 	, _forced_reduction (forced_reduction)
+	, _error (false)
 {
 	/* ::image assumes 16bpp */
 	DCPOMATIC_ASSERT (_pixel_format == AV_PIX_FMT_RGB48 || _pixel_format == AV_PIX_FMT_XYZ12LE);
@@ -99,6 +102,7 @@ J2KImageProxy::J2KImageProxy (
 }
 
 J2KImageProxy::J2KImageProxy (shared_ptr<cxml::Node> xml, shared_ptr<Socket> socket)
+	: _error (false)
 {
 	_size = dcp::Size (xml->number_child<int> ("Width"), xml->number_child<int> ("Height"));
 	if (xml->optional_number_child<int> ("Eye")) {
@@ -136,29 +140,35 @@ J2KImageProxy::prepare (optional<dcp::Size> target_size) const
 		reduce = max (0, reduce);
 	}
 
-	shared_ptr<dcp::OpenJPEGImage> decompressed = dcp::decompress_j2k (const_cast<uint8_t*> (_data.data().get()), _data.size (), reduce);
-	_image.reset (new Image (_pixel_format, decompressed->size(), true));
-
-	int const shift = 16 - decompressed->precision (0);
-
-	/* Copy data in whatever format (sRGB or XYZ) into our Image; I'm assuming
-	   the data is 12-bit either way.
-	*/
-
-	int const width = decompressed->size().width;
-
-	int p = 0;
-	int* decomp_0 = decompressed->data (0);
-	int* decomp_1 = decompressed->data (1);
-	int* decomp_2 = decompressed->data (2);
-	for (int y = 0; y < decompressed->size().height; ++y) {
-		uint16_t* q = (uint16_t *) (_image->data()[0] + y * _image->stride()[0]);
-		for (int x = 0; x < width; ++x) {
-			*q++ = decomp_0[p] << shift;
-			*q++ = decomp_1[p] << shift;
-			*q++ = decomp_2[p] << shift;
-			++p;
+	try {
+		shared_ptr<dcp::OpenJPEGImage> decompressed = dcp::decompress_j2k (const_cast<uint8_t*> (_data.data().get()), _data.size (), reduce);
+		_image.reset (new Image (_pixel_format, decompressed->size(), true));
+
+		int const shift = 16 - decompressed->precision (0);
+
+		/* Copy data in whatever format (sRGB or XYZ) into our Image; I'm assuming
+		   the data is 12-bit either way.
+		   */
+
+		int const width = decompressed->size().width;
+
+		int p = 0;
+		int* decomp_0 = decompressed->data (0);
+		int* decomp_1 = decompressed->data (1);
+		int* decomp_2 = decompressed->data (2);
+		for (int y = 0; y < decompressed->size().height; ++y) {
+			uint16_t* q = (uint16_t *) (_image->data()[0] + y * _image->stride()[0]);
+			for (int x = 0; x < width; ++x) {
+				*q++ = decomp_0[p] << shift;
+				*q++ = decomp_1[p] << shift;
+				*q++ = decomp_2[p] << shift;
+				++p;
+			}
 		}
+	} catch (dcp::J2KDecompressionError& e) {
+		_image.reset (new Image (_pixel_format, _size, true));
+		_image->make_black ();
+		_error = true;
 	}
 
 	_target_size = target_size;
@@ -172,12 +182,14 @@ ImageProxy::Result
 J2KImageProxy::image (optional<dcp::Size> target_size) const
 {
 	int const r = prepare (target_size);
+
 	/* I think this is safe without a lock on mutex.  _image is guaranteed to be
 	   set up when prepare() has happened.
 	*/
-	return Result (_image, r);
+	return Result (_image, r, _error);
 }
 
+
 void
 J2KImageProxy::add_metadata (xmlpp::Node* node) const
 {
diff --git a/src/lib/j2k_image_proxy.h b/src/lib/j2k_image_proxy.h
index ec99e71a9..78f291e5d 100644
--- a/src/lib/j2k_image_proxy.h
+++ b/src/lib/j2k_image_proxy.h
@@ -85,4 +85,6 @@ private:
 	AVPixelFormat _pixel_format;
 	mutable boost::mutex _mutex;
 	boost::optional<int> _forced_reduction;
+	/** true if an error occurred while decoding the JPEG2000 data, false if not */
+	mutable bool _error;
 };
diff --git a/src/lib/player.cc b/src/lib/player.cc
index 304f8c723..d5a558184 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -346,7 +346,8 @@ Player::black_player_video_frame (Eyes eyes) const
 			PresetColourConversion::all().front().conversion,
 			VIDEO_RANGE_FULL,
 			boost::weak_ptr<Content>(),
-			boost::optional<Frame>()
+			boost::optional<Frame>(),
+			false
 		)
 	);
 }
@@ -850,7 +851,8 @@ Player::video (weak_ptr<Piece> wp, ContentVideo video)
 			piece->content->video->colour_conversion(),
 			piece->content->video->range(),
 			piece->content,
-			video.frame
+			video.frame,
+			false
 			)
 		);
 
diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index bd643af60..8d55ffb2e 100644
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -54,7 +54,8 @@ PlayerVideo::PlayerVideo (
 	optional<ColourConversion> colour_conversion,
 	VideoRange video_range,
 	weak_ptr<Content> content,
-	optional<Frame> video_frame
+	optional<Frame> video_frame,
+	bool error
 	)
 	: _in (in)
 	, _crop (crop)
@@ -67,6 +68,7 @@ PlayerVideo::PlayerVideo (
 	, _video_range (video_range)
 	, _content (content)
 	, _video_frame (video_frame)
+	, _error (error)
 {
 
 }
@@ -81,6 +83,7 @@ PlayerVideo::PlayerVideo (shared_ptr<cxml::Node> node, shared_ptr<Socket> socket
 	_eyes = (Eyes) node->number_child<int> ("Eyes");
 	_part = (Part) node->number_child<int> ("Part");
 	_video_range = (VideoRange) node->number_child<int>("VideoRange");
+	_error = node->optional_bool_child("Error").get_value_or (false);
 
 	/* Assume that the ColourConversion uses the current state version */
 	_colour_conversion = ColourConversion::from_xml (node, Film::current_state_version);
@@ -133,6 +136,7 @@ PlayerVideo::make_image (function<AVPixelFormat (AVPixelFormat)> pixel_format, b
 	_image_fade = _fade;
 
 	ImageProxy::Result prox = _in->image (_inter_size);
+	_error = prox.error;
 
 	Crop total_crop = _crop;
 	switch (_part) {
@@ -194,6 +198,7 @@ PlayerVideo::add_metadata (xmlpp::Node* node) const
 	node->add_child("Eyes")->add_child_text (raw_convert<string> (static_cast<int> (_eyes)));
 	node->add_child("Part")->add_child_text (raw_convert<string> (static_cast<int> (_part)));
 	node->add_child("VideoRange")->add_child_text(raw_convert<string>(static_cast<int>(_video_range)));
+	node->add_child("Error")->add_child_text(_error ? "1" : "0");
 	if (_colour_conversion) {
 		_colour_conversion.get().as_xml (node);
 	}
@@ -315,7 +320,8 @@ PlayerVideo::shallow_copy () const
 			_colour_conversion,
 			_video_range,
 			_content,
-			_video_frame
+			_video_frame,
+			_error
 			)
 		);
 }
diff --git a/src/lib/player_video.h b/src/lib/player_video.h
index 3cd559409..0a6a9da67 100644
--- a/src/lib/player_video.h
+++ b/src/lib/player_video.h
@@ -56,7 +56,8 @@ public:
 		boost::optional<ColourConversion>,
 		VideoRange video_range,
 		boost::weak_ptr<Content>,
-		boost::optional<Frame>
+		boost::optional<Frame>,
+		bool error
 		);
 
 	PlayerVideo (boost::shared_ptr<cxml::Node>, boost::shared_ptr<Socket>);
@@ -107,6 +108,10 @@ public:
 		return _content;
 	}
 
+	bool error () const {
+		return _error;
+	}
+
 private:
 	void make_image (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const;
 
@@ -137,6 +142,8 @@ private:
 	mutable dcp::Size _image_out_size;
 	/** _fade that was used to make _image */
 	mutable boost::optional<double> _image_fade;
+	/** true if there was an error when decoding our image */
+	mutable bool _error;
 };
 
 #endif