Hopefully fix crash when the same frame is encoded twice.

Previously if some frame X was passed to the encoder twice
I think this would happen:

X1 written; X stored to _pending_images
X2 written; _pending_images remains the same
X1 encode finishes; X is discarded from _pending_images
Data for X2 is read by the encoder but was just freed

I think this might have resulted in intermittent crashes, which were
fairly common in the 2.17.x branch with the test
ffmpeg_encoder_prores_regression_1

But I didn't conclusively prove it.

diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc
index 0d5167c496c109ae0b7f4db56d28ee350d4134e8..6f33700a249422314a31cc52102a45cf8e66b4e7 100644 (file)
--- a/src/lib/ffmpeg_file_encoder.cc
+++ b/src/lib/ffmpeg_file_encoder.cc
@@ -412,7 +412,13 @@ FFmpegFileEncoder::video (shared_ptr<PlayerVideo> video, DCPTime time)
 
        {
                boost::mutex::scoped_lock lm (_pending_images_mutex);
-               _pending_images[image->data()[0]] = image;
+               auto key = image->data()[0];
+               auto iter = _pending_images.find(key);
+               if (iter != _pending_images.end()) {
+                       iter->second.second++;
+               } else {
+                       _pending_images[key] = { image, 1 };
+               }
        }
 
        for (int i = 0; i < 3; ++i) {
@@ -503,7 +509,11 @@ void
 FFmpegFileEncoder::buffer_free2 (uint8_t* data)
 {
        boost::mutex::scoped_lock lm (_pending_images_mutex);
-       if (_pending_images.find(data) != _pending_images.end()) {
-               _pending_images.erase (data);
+       auto iter = _pending_images.find(data);
+       if (iter != _pending_images.end()) {
+               iter->second.second--;
+               if (iter->second.second == 0) {
+                       _pending_images.erase(data);
+               }
        }
 }

diff --git a/src/lib/ffmpeg_file_encoder.h b/src/lib/ffmpeg_file_encoder.h
index 5bf501370ab6d9ad917b6808a0ffc1041b176378..f0b3eac8c1fa1f610f3bdbe6108a3190edba6578 100644 (file)
--- a/src/lib/ffmpeg_file_encoder.h
+++ b/src/lib/ffmpeg_file_encoder.h
@@ -106,9 +106,10 @@ private:
        std::shared_ptr<AudioBuffers> _pending_audio;
 
        /** Store of shared_ptr<Image> to keep them alive whilst raw pointers into
-           their data have been passed to FFmpeg.
+           their data have been passed to FFmpeg.  The second part of the pair is
+           a count of how many copies of the same key must be kept.
        */
-       std::map<uint8_t*, std::shared_ptr<const Image>> _pending_images;
+       std::map<uint8_t*, std::pair<std::shared_ptr<const Image>, int>> _pending_images;
        boost::mutex _pending_images_mutex;
 
        static int _video_stream_index;