From e55b2b3a7eb05b3cc69411b133aeec9772420c83 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 24 Oct 2022 20:37:02 +0200 Subject: [PATCH] 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. --- src/lib/ffmpeg_file_encoder.cc | 16 +++++++++++++--- src/lib/ffmpeg_file_encoder.h | 5 +++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc index 0d5167c49..6f33700a2 100644 --- a/src/lib/ffmpeg_file_encoder.cc +++ b/src/lib/ffmpeg_file_encoder.cc @@ -412,7 +412,13 @@ FFmpegFileEncoder::video (shared_ptr 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 5bf501370..f0b3eac8c 100644 --- a/src/lib/ffmpeg_file_encoder.h +++ b/src/lib/ffmpeg_file_encoder.h @@ -106,9 +106,10 @@ private: std::shared_ptr _pending_audio; /** Store of shared_ptr 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> _pending_images; + std::map, int>> _pending_images; boost::mutex _pending_images_mutex; static int _video_stream_index; -- 2.30.2