From b0c97119743e7127c81359b25cbb795195aa0645 Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Fri, 21 Feb 2020 20:04:36 +1100 Subject: [PATCH] ffmpegdecoder: fixed double mutex lock Error() would call Close() getting stuck in a double mutex lock that could stall workers. This commit fixes that. --- app/codec/ffmpeg/ffmpegdecoder.cpp | 61 +++++++++++++++--------------- app/codec/ffmpeg/ffmpegdecoder.h | 6 +-- app/task/index/index.cpp | 5 --- 3 files changed, 32 insertions(+), 40 deletions(-) diff --git a/app/codec/ffmpeg/ffmpegdecoder.cpp b/app/codec/ffmpeg/ffmpegdecoder.cpp index 22534e472..80289a15e 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.cpp +++ b/app/codec/ffmpeg/ffmpegdecoder.cpp @@ -49,7 +49,6 @@ FFmpegDecoder::FFmpegDecoder() : cache_at_zero_(false), cache_at_eof_(false), opts_(nullptr), - multithreading_(true), allow_clear_event_(false) { connect(this, &FFmpegDecoder::ConsumedMemory, MemoryManager::instance(), &MemoryManager::ConsumedMemory, Qt::DirectConnection); @@ -138,7 +137,7 @@ bool FFmpegDecoder::Open() } // Set multithreading setting - error_code = av_dict_set(&opts_, "threads", multithreading_ ? "auto" : "1", 0); + error_code = av_dict_set(&opts_, "threads", "auto", 0); // Handle failure to set multithreaded decoding if (error_code < 0) { @@ -486,29 +485,7 @@ void FFmpegDecoder::Close() { QMutexLocker locker(&mutex_); - if (opts_) { - av_dict_free(&opts_); - opts_ = nullptr; - } - - ClearFrameCache(); - - if (scale_ctx_) { - sws_freeContext(scale_ctx_); - scale_ctx_ = nullptr; - } - - if (codec_ctx_) { - avcodec_free_context(&codec_ctx_); - codec_ctx_ = nullptr; - } - - if (fmt_ctx_) { - avformat_close_input(&fmt_ctx_); - fmt_ctx_ = nullptr; - } - - open_ = false; + ClearResources(); } QString FFmpegDecoder::id() @@ -526,11 +503,6 @@ bool FFmpegDecoder::SupportsAudio() return true; } -void FFmpegDecoder::SetMultithreading(bool e) -{ - multithreading_ = e; -} - bool FFmpegDecoder::Probe(Footage *f, const QAtomicInt* cancelled) { if (open_) { @@ -702,7 +674,7 @@ void FFmpegDecoder::Error(const QString &s) { qWarning() << s; - Close(); + ClearResources(); } void FFmpegDecoder::Index(const QAtomicInt* cancelled) @@ -985,6 +957,33 @@ void FFmpegDecoder::ClearFrameCache() cache_at_zero_ = false; } +void FFmpegDecoder::ClearResources() +{ + if (opts_) { + av_dict_free(&opts_); + opts_ = nullptr; + } + + ClearFrameCache(); + + if (scale_ctx_) { + sws_freeContext(scale_ctx_); + scale_ctx_ = nullptr; + } + + if (codec_ctx_) { + avcodec_free_context(&codec_ctx_); + codec_ctx_ = nullptr; + } + + if (fmt_ctx_) { + avformat_close_input(&fmt_ctx_); + fmt_ctx_ = nullptr; + } + + open_ = false; +} + void FFmpegDecoder::FreeMemory() { if (!allow_clear_event_) { diff --git a/app/codec/ffmpeg/ffmpegdecoder.h b/app/codec/ffmpeg/ffmpegdecoder.h index 29e19c7f1..be3fcab31 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.h +++ b/app/codec/ffmpeg/ffmpegdecoder.h @@ -62,8 +62,6 @@ public: virtual bool SupportsVideo() override; virtual bool SupportsAudio() override; - void SetMultithreading(bool e); - virtual void Index(const QAtomicInt *cancelled) override; signals: @@ -110,6 +108,8 @@ private: void RemoveLastFromFrameCache(); void ClearFrameCache(); + void ClearResources(); + AVFormatContext* fmt_ctx_; AVCodecContext* codec_ctx_; AVStream* avstream_; @@ -127,8 +127,6 @@ private: AVDictionary* opts_; - bool multithreading_; - QTimer clear_timer_; QAtomicInt allow_clear_event_; diff --git a/app/task/index/index.cpp b/app/task/index/index.cpp index 411f29c0e..539f5fc8a 100644 --- a/app/task/index/index.cpp +++ b/app/task/index/index.cpp @@ -18,11 +18,6 @@ void IndexTask::Action() decoder->set_stream(stream_); - // Force multithreading for faster indexing - if (decoder->id() == "ffmpeg") { - static_cast(decoder.get())->SetMultithreading(true); - } - connect(decoder.get(), &Decoder::IndexProgress, this, &IndexTask::ProgressChanged); decoder->Open();