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 cd03c7b93a1cf762b88add85347525b5bb0d9d40..786f97dd706ddec92394dd391df6834812ba7ecd 100644 (file)
--- 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 4bd264c8bdf6a760cdb251d3b4950557632b8e68..3b0bdd170e35f52da28ce707fdfc8d1b07914527 100644 (file)
--- 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 c11d12a8263442252a33f2b8a241da1b1b5a2074..d15bdabde676723922cba737d6c50839ab085805 100644 (file)
--- 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 1c23d59027c0359f5bb670ad52db2a407e574305..19674d249f94f1bb29ccdaa834bece14398d26a8 100644 (file)
--- 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);