wip: Error when failing to read MXF frame.bad-mxf

8 files changed, 73 insertions, 8 deletions

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index b2fbc6c60..4efe4fbfb 100644
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -36,6 +36,7 @@ using std::make_pair;
 using std::pair;
 using std::shared_ptr;
 using std::string;
+using std::vector;
 using std::weak_ptr;
 using boost::bind;
 using boost::optional;
@@ -92,6 +93,7 @@ Butler::Butler (
 	_player_video_connection = _player.Video.connect(bind(&Butler::video, this, _1, _2));
 	_player_audio_connection = _player.Audio.connect(bind(&Butler::audio, this, _1, _2, _3));
 	_player_text_connection = _player.Text.connect(bind(&Butler::text, this, _1, _2, _3, _4));
+	_player_error_connection = _player.Error.connect(bind(&Butler::error, this, _1));
 	/* The butler must hear about things first, otherwise it might not sort out suspensions in time for
 	   get_video() to be called in response to this signal.
 	*/
@@ -481,3 +483,19 @@ Butler::Error::summary () const
 	return "";
 }
 
+
+void
+Butler::error(string message)
+{
+	boost::mutex::scoped_lock lm(_mutex);
+	_errors.push_back(message);
+}
+
+
+vector<string>
+Butler::errors() const
+{
+	boost::mutex::scoped_lock lm(_mutex);
+	return _errors;
+}
+
diff --git a/src/lib/butler.h b/src/lib/butler.h
index 6bb0467af..446909cac 100644
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -94,12 +94,14 @@ public:
 	boost::optional<TextRingBuffers::Data> get_closed_caption ();
 
 	std::pair<size_t, std::string> memory_used () const;
+	std::vector<std::string> errors() const;
 
 private:
 	void thread ();
 	void video (std::shared_ptr<PlayerVideo> video, dcpomatic::DCPTime time);
 	void audio (std::shared_ptr<AudioBuffers> audio, dcpomatic::DCPTime time, int frame_rate);
 	void text (PlayerText pt, TextType type, boost::optional<DCPTextTrack> track, dcpomatic::DCPTimePeriod period);
+	void error(std::string message);
 	bool should_run () const;
 	void prepare (std::weak_ptr<PlayerVideo> video);
 	void player_change (ChangeType type, int property);
@@ -117,8 +119,8 @@ private:
 	boost::asio::io_service _prepare_service;
 	std::shared_ptr<boost::asio::io_service::work> _prepare_work;
 
-	/** mutex to protect _pending_seek_position, _pending_seek_accurate, _finished, _died, _stop_thread */
-	boost::mutex _mutex;
+	/** mutex to protect _pending_seek_position, _pending_seek_accurate, _finished, _died, _stop_thread, _errors */
+	mutable boost::mutex _mutex;
 	boost::condition _summon;
 	boost::condition _arrived;
 	boost::optional<dcpomatic::DCPTime> _pending_seek_position;
@@ -128,6 +130,7 @@ private:
 	bool _died;
 	std::string _died_message;
 	bool _stop_thread;
+	std::vector<std::string> _errors;
 
 	AudioMapping _audio_mapping;
 	int _audio_channels;
@@ -155,6 +158,7 @@ private:
 	boost::signals2::scoped_connection _player_video_connection;
 	boost::signals2::scoped_connection _player_audio_connection;
 	boost::signals2::scoped_connection _player_text_connection;
+	boost::signals2::scoped_connection _player_error_connection;
 	boost::signals2::scoped_connection _player_change_connection;
 };
 
diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc
index 9f1a80160..c00341bc7 100644
--- a/src/lib/dcp_decoder.cc
+++ b/src/lib/dcp_decoder.cc
@@ -228,6 +228,8 @@ DCPDecoder::pass ()
 		return PassResult::finished();
 	}
 
+	PassResult result = PassResult::ok();
+
 	auto const vfr = _dcp_content->active_video_frame_rate (film());
 
 	/* Frame within the (played part of the) reel that is coming up next */
@@ -242,15 +244,27 @@ DCPDecoder::pass ()
 	pass_texts (_next, picture_asset->size());
 
 	if ((_mono_reader || _stereo_reader) && (_decode_referenced || !_dcp_content->reference_video())) {
-		pass_video(frame, picture_asset->size());
+		try {
+			pass_video(frame, picture_asset->size());
+		} catch (dcp::ReadError const &e) {
+			result = PassResult::error(e.what());
+		}
 	}
 
 	if (_sound_reader && (_decode_referenced || !_dcp_content->reference_audio())) {
-		pass_audio(frame, vfr);
+		try {
+			pass_audio(frame, vfr);
+		} catch (dcp::ReadError const &e) {
+			result = PassResult::error(e.what());
+		}
 	}
 
 	if (_atmos_reader) {
-		pass_atmos(frame);
+		try {
+			pass_atmos(frame);
+		} catch (dcp::ReadError const &e) {
+			result = PassResult::error(e.what());
+		}
 	}
 
 	_next += ContentTime::from_frames (1, vfr);
@@ -262,7 +276,7 @@ DCPDecoder::pass ()
 		}
 	}
 
-	return PassResult::ok();
+	return result;
 }
 
 
diff --git a/src/lib/decoder.h b/src/lib/decoder.h
index 327fdc5af..ae0c97ea6 100644
--- a/src/lib/decoder.h
+++ b/src/lib/decoder.h
@@ -68,6 +68,7 @@ public:
 		{
 			OK,        ///< there was no error and more data may be emitted on the next pass()
 			FINISHED,  ///< this decoder will emit no more data unless a seek() happens
+			ERROR,     ///< some survivable error occurred; output may not be correct
 		};
 
 		static PassResult ok() {
@@ -78,6 +79,10 @@ public:
 			return { Type::FINISHED };
 		}
 
+		static PassResult error(std::string message) {
+			return { Type::ERROR, message };
+		};
+
 		bool is_ok() const {
 			return _type == Type::OK;
 		}
@@ -86,12 +91,27 @@ public:
 			return _type == Type::FINISHED;
 		}
 
+		bool is_error() const {
+			return _type == Type::ERROR;
+		}
+
+		std::string error_message() const {
+			DCPOMATIC_ASSERT(is_error());
+			return _error_message;
+		}
+
 	private:
 		PassResult(Type type)
 			: _type(type)
 		{}
 
+		PassResult(Type type, std::string error_message)
+			: _type(type)
+			, _error_message(error_message)
+		{}
+
 		Type _type;
+		std::string _error_message;
 	};
 
 	/** Do some decoding and perhaps emit video, audio or subtitle data */
diff --git a/src/lib/player.cc b/src/lib/player.cc
index fbf2ef7d6..dce495eb0 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -757,7 +757,11 @@ Player::pass ()
 	case CONTENT:
 	{
 		LOG_DEBUG_PLAYER ("Calling pass() on %1", earliest_content->content->path(0));
-		earliest_content->done = earliest_content->decoder->pass().is_finished();
+		auto result = earliest_content->decoder->pass();
+		earliest_content->done = result.is_finished();
+		if (result.is_error()) {
+			Error(result.error_message());
+		}
 		auto dcp = dynamic_pointer_cast<DCPContent>(earliest_content->content);
 		if (dcp && !_play_referenced && dcp->reference_audio()) {
 			/* We are skipping some referenced DCP audio content, so we need to update _next_audio_time
diff --git a/src/lib/player.h b/src/lib/player.h
index e8f768521..2cfbcd161 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -127,6 +127,9 @@ public:
 	boost::signals2::signal<void (PlayerText, TextType, boost::optional<DCPTextTrack>, dcpomatic::DCPTimePeriod)> Text;
 	boost::signals2::signal<void (std::shared_ptr<const dcp::AtmosFrame>, dcpomatic::DCPTime, AtmosMetadata)> Atmos;
 
+	/** Emitted when some error has occurred; the string is the message */
+	boost::signals2::signal<void (std::string)> Error;
+
 private:
 	friend class PlayerWrapper;
 	friend class Piece;
diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index fb02f0a0f..2fda1c93e 100644
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -805,7 +805,7 @@ FilmViewer::dropped () const
 int
 FilmViewer::errored () const
 {
-	return _video_view->errored ();
+	return _video_view->errored() + (_butler ? _butler->errors().size() : 0);
 }
 
 
diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc
index c271cb65e..91bd0d6af 100644
--- a/src/wx/video_view.cc
+++ b/src/wx/video_view.cc
@@ -90,6 +90,8 @@ VideoView::get_next_frame (bool non_blocking)
 		_player_video.first->eyes() != Eyes::BOTH
 		);
 
+	// XXX: this is too early to check this; error() is only set up when image/make_image is called
+	// on the _player_video, which might not have happened yet (though it could have, with the prepare() stuff)
 	if (_player_video.first && _player_video.first->error()) {
 		++_errored;
 	}