From: Carl Hetherington <cth@carlh.net>
Date: Mon, 17 Aug 2020 14:42:46 +0000 (+0200)
Subject: Tidy up resource management of FFmpegFileEncoders so that they
X-Git-Tag: v2.15.99~10
X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=5d3c9573914a61db10b24ce7e0cef00902c2912c

Tidy up resource management of FFmpegFileEncoders so that they
are cleaned up correctly when an error occurs.

Adapted from dfac61382f1719f0a879747de43cbc5f9115c2d7 in master.
---

diff --git a/src/lib/ffmpeg_encoder.cc b/src/lib/ffmpeg_encoder.cc
index cd03c7b93..786f97dd7 100644
--- a/src/lib/ffmpeg_encoder.cc
+++ b/src/lib/ffmpeg_encoder.cc
@@ -61,6 +61,11 @@ FFmpegEncoder::FFmpegEncoder (
 	)
 	: Encoder (film, job)
 	, _history (200)
+	, _output (output)
+	, _format (format)
+	, _split_reels (split_reels)
+	, _audio_stream_per_channel (audio_stream_per_channel)
+	, _x264_crf (x264_crf)
 {
 	_player->set_always_burn_open_subtitles ();
 	_player->set_play_referenced ();
@@ -103,11 +108,26 @@ FFmpegEncoder::FFmpegEncoder (
 	}
 
 	_butler.reset (new Butler(_player, map, _output_audio_channels, bind(&PlayerVideo::force, _1, FFmpegFileEncoder::pixel_format(format)), true, false));
+}
+
+
+void
+FFmpegEncoder::go ()
+{
+	{
+		shared_ptr<Job> job = _job.lock ();
+		DCPOMATIC_ASSERT (job);
+		job->sub (_("Encoding"));
+	}
+
+	Waker waker;
+
+	list<FileEncoderSet> file_encoders;
 
-	int const files = split_reels ? film->reels().size() : 1;
+	int const files = _split_reels ? _film->reels().size() : 1;
 	for (int i = 0; i < files; ++i) {
 
-		boost::filesystem::path filename = output;
+		boost::filesystem::path filename = _output;
 		string extension = boost::filesystem::extension (filename);
 		filename = boost::filesystem::change_extension (filename, "");
 
@@ -117,15 +137,15 @@ FFmpegEncoder::FFmpegEncoder (
 			filename = filename.string() + String::compose(_("_reel%1"), i + 1);
 		}
 
-		_file_encoders.push_back (
+		file_encoders.push_back (
 			FileEncoderSet (
 				_film->frame_size(),
 				_film->video_frame_rate(),
 				_film->audio_frame_rate(),
 				_output_audio_channels,
-				format,
-				audio_stream_per_channel,
-				x264_crf,
+				_format,
+				_audio_stream_per_channel,
+				_x264_crf,
 				_film->three_d(),
 				filename,
 				extension
@@ -136,23 +156,10 @@ FFmpegEncoder::FFmpegEncoder (
 				)
 			);
 	}
-}
-
-
-void
-FFmpegEncoder::go ()
-{
-	{
-		shared_ptr<Job> job = _job.lock ();
-		DCPOMATIC_ASSERT (job);
-		job->sub (_("Encoding"));
-	}
-
-	Waker waker;
 
 	list<DCPTimePeriod> reel_periods = _film->reels ();
 	list<DCPTimePeriod>::const_iterator reel = reel_periods.begin ();
-	list<FileEncoderSet>::iterator encoder = _file_encoders.begin ();
+	list<FileEncoderSet>::iterator encoder = file_encoders.begin ();
 
 	DCPTime const video_frame = DCPTime::from_frames (1, _film->video_frame_rate ());
 	int const audio_frames = video_frame.frames_round(_film->audio_frame_rate());
@@ -161,12 +168,12 @@ FFmpegEncoder::go ()
 	int const gets_per_frame = _film->three_d() ? 2 : 1;
 	for (DCPTime i; i < _film->length(); i += video_frame) {
 
-		if (_file_encoders.size() > 1 && !reel->contains(i)) {
+		if (file_encoders.size() > 1 && !reel->contains(i)) {
 			/* Next reel and file */
 			++reel;
 			++encoder;
 			DCPOMATIC_ASSERT (reel != reel_periods.end());
-			DCPOMATIC_ASSERT (encoder != _file_encoders.end());
+			DCPOMATIC_ASSERT (encoder != file_encoders.end());
 		}
 
 		for (int j = 0; j < gets_per_frame; ++j) {
@@ -174,7 +181,7 @@ FFmpegEncoder::go ()
 			pair<shared_ptr<PlayerVideo>, DCPTime> v = _butler->get_video (true, &e);
 			_butler->rethrow ();
 			if (!v.first) {
-				throw ProgrammingError(__FILE__, __LINE__, String::compose("butler returned no video; error was %1", e.summary()));
+				throw DecodeError(String::compose("Error during decoding: %1", e.summary()));
 			}
 			shared_ptr<FFmpegFileEncoder> fe = encoder->get (v.first->eyes());
 			if (fe) {
@@ -208,7 +215,7 @@ FFmpegEncoder::go ()
 	}
 	delete[] interleaved;
 
-	BOOST_FOREACH (FileEncoderSet i, _file_encoders) {
+	BOOST_FOREACH (FileEncoderSet i, file_encoders) {
 		i.flush ();
 	}
 }
diff --git a/src/lib/ffmpeg_encoder.h b/src/lib/ffmpeg_encoder.h
index 4bd264c8b..3b0bdd170 100644
--- a/src/lib/ffmpeg_encoder.h
+++ b/src/lib/ffmpeg_encoder.h
@@ -84,7 +84,6 @@ private:
 		std::map<Eyes, boost::shared_ptr<FFmpegFileEncoder> > _encoders;
 	};
 
-	std::list<FileEncoderSet> _file_encoders;
 	int _output_audio_channels;
 
 	mutable boost::mutex _mutex;
@@ -92,6 +91,12 @@ private:
 
 	EventHistory _history;
 
+	boost::filesystem::path _output;
+	ExportFormat _format;
+	bool _split_reels;
+	bool _audio_stream_per_channel;
+	int _x264_crf;
+
 	boost::shared_ptr<Butler> _butler;
 };
 
diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc
index c11d12a82..d15bdabde 100644
--- a/src/lib/ffmpeg_file_encoder.cc
+++ b/src/lib/ffmpeg_file_encoder.cc
@@ -295,6 +295,15 @@ FFmpegFileEncoder::FFmpegFileEncoder (
 	_pending_audio.reset (new AudioBuffers(channels, 0));
 }
 
+
+FFmpegFileEncoder::~FFmpegFileEncoder ()
+{
+	_audio_streams.clear ();
+	avcodec_close (_video_codec_context);
+	avformat_free_context (_format_context);
+}
+
+
 AVPixelFormat
 FFmpegFileEncoder::pixel_format (ExportFormat format)
 {
@@ -403,11 +412,6 @@ DCPOMATIC_ENABLE_WARNINGS
 	}
 
 	av_write_trailer (_format_context);
-
-	_audio_streams.clear ();
-	avcodec_close (_video_codec_context);
-	avio_close (_format_context->pb);
-	avformat_free_context (_format_context);
 }
 
 void
diff --git a/src/lib/ffmpeg_file_encoder.h b/src/lib/ffmpeg_file_encoder.h
index 1c23d5902..19674d249 100644
--- a/src/lib/ffmpeg_file_encoder.h
+++ b/src/lib/ffmpeg_file_encoder.h
@@ -53,6 +53,8 @@ public:
 #endif
 		);
 
+	~FFmpegFileEncoder ();
+
 	void video (boost::shared_ptr<PlayerVideo>, dcpomatic::DCPTime);
 	void audio (boost::shared_ptr<AudioBuffers>);
 	void subtitle (PlayerText, dcpomatic::DCPTimePeriod);