From: Carl Hetherington <cth@carlh.net>
Date: Mon, 17 Aug 2020 11:16:48 +0000 (+0200)
Subject: Tidy up resource management of FFmpegFileEncoders so that they
X-Git-Url: https://git.carlh.net/gitweb/?a=commitdiff_plain;h=dfac61382f1719f0a879747de43cbc5f9115c2d7;p=dcpomatic.git

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

diff --git a/src/lib/ffmpeg_encoder.cc b/src/lib/ffmpeg_encoder.cc
index 8ab56a592..ce9114777 100644
--- a/src/lib/ffmpeg_encoder.cc
+++ b/src/lib/ffmpeg_encoder.cc
@@ -53,6 +53,10 @@ FFmpegEncoder::FFmpegEncoder (
 	)
 	: Encoder (film, job)
 	, _history (1000)
+	, _output (output)
+	, _format (format)
+	, _split_reels (split_reels)
+	, _x264_crf (x264_crf)
 {
 	_player->set_always_burn_open_subtitles ();
 	_player->set_play_referenced ();
@@ -91,11 +95,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;
 
-	int const files = split_reels ? film->reels().size() : 1;
+	std::list<FileEncoderSet> file_encoders;
+
+	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, "");
 
@@ -105,36 +124,24 @@ 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,
-				x264_crf,
+				_format,
+				_x264_crf,
 				_film->three_d(),
 				filename,
 				extension
 				)
 			);
 	}
-}
-
-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());
@@ -143,19 +150,19 @@ 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) {
 			Butler::Error e;
 			pair<shared_ptr<PlayerVideo>, DCPTime> v = _butler->get_video (&e);
 			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) {
@@ -189,7 +196,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 79539acce..542711cc9 100644
--- a/src/lib/ffmpeg_encoder.h
+++ b/src/lib/ffmpeg_encoder.h
@@ -74,13 +74,16 @@ private:
 		std::map<Eyes, boost::shared_ptr<FFmpegFileEncoder> > _encoders;
 	};
 
-	std::list<FileEncoderSet> _file_encoders;
 	int _output_audio_channels;
 
 	mutable boost::mutex _mutex;
 	DCPTime _last_time;
 
 	EventHistory _history;
+	boost::filesystem::path _output;
+	ExportFormat _format;
+	bool _split_reels;
+	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 294b03162..663f791e5 100644
--- a/src/lib/ffmpeg_file_encoder.cc
+++ b/src/lib/ffmpeg_file_encoder.cc
@@ -122,6 +122,15 @@ FFmpegFileEncoder::FFmpegFileEncoder (
 	_pending_audio.reset (new AudioBuffers(channels, 0));
 }
 
+
+FFmpegFileEncoder::~FFmpegFileEncoder ()
+{
+	avcodec_close (_video_codec_context);
+	avcodec_close (_audio_codec_context);
+	avformat_close_input (&_format_context);
+}
+
+
 AVPixelFormat
 FFmpegFileEncoder::pixel_format (ExportFormat format)
 {
@@ -225,11 +234,6 @@ FFmpegFileEncoder::flush ()
 	}
 
 	av_write_trailer (_format_context);
-
-	avcodec_close (_video_codec_context);
-	avcodec_close (_audio_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 a0c17c846..e4ae04a78 100644
--- a/src/lib/ffmpeg_file_encoder.h
+++ b/src/lib/ffmpeg_file_encoder.h
@@ -29,8 +29,9 @@ extern "C" {
 #include <libavcodec/avcodec.h>
 #include <libavformat/avformat.h>
 }
+#include <boost/noncopyable.hpp>
 
-class FFmpegFileEncoder
+class FFmpegFileEncoder : public boost::noncopyable
 {
 public:
 	FFmpegFileEncoder (
@@ -43,6 +44,8 @@ public:
 		boost::filesystem::path output
 		);
 
+	~FFmpegFileEncoder ();
+
 	void video (boost::shared_ptr<PlayerVideo>, DCPTime);
 	void audio (boost::shared_ptr<AudioBuffers>);
 	void subtitle (PlayerText, DCPTimePeriod);