From 901f1001affa63e20aec6205a64408a145f1ebf3 Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Sun, 9 Aug 2020 14:03:11 +1000 Subject: [PATCH] renderer: improved memory management Clear up more memory when it isn't used for a while. --- app/codec/ffmpeg/ffmpegdecoder.cpp | 24 +++++++------------ app/render/backend/renderworker.cpp | 37 +++++++++++++++++++++++++++++ app/render/backend/renderworker.h | 11 +++++++++ 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/app/codec/ffmpeg/ffmpegdecoder.cpp b/app/codec/ffmpeg/ffmpegdecoder.cpp index 4c54a42df..e8a0b7844 100644 --- a/app/codec/ffmpeg/ffmpegdecoder.cpp +++ b/app/codec/ffmpeg/ffmpegdecoder.cpp @@ -52,7 +52,8 @@ QHash< Stream*, QList > FFmpegDecoder::instance_map_; QMutex FFmpegDecoder::instance_map_lock_; QHash< Stream*, FFmpegFramePool* > FFmpegDecoder::frame_pool_map_; -// FIXME: Hardcoded, ideally this value is dynamically chosen based on memory restraints +// FIXME: Hardcoded value. It seems to work fine, but is there a possibility we should make +// this a dynamic value somehow or a configurable value? const int FFmpegDecoderInstance::kMaxFrameLife = 2000; FFmpegDecoder::FFmpegDecoder() : @@ -94,11 +95,12 @@ bool FFmpegDecoder::Open() if (stream()->type() == Stream::kVideo) { QMutexLocker map_locker(&instance_map_lock_); - // FIXME: Test code, this should be changed later FFmpegFramePool* frame_pool = frame_pool_map_.value(stream().get()); if (!frame_pool) { - frame_pool = new FFmpegFramePool(256, + // FIXME: Hardcoded value. It seems to work fine, but is there a possibility we should make + // this a dynamic value somehow or a configurable value? + frame_pool = new FFmpegFramePool(32, our_instance->stream()->codecpar->width, our_instance->stream()->codecpar->height, static_cast(our_instance->stream()->codecpar->format)); @@ -106,7 +108,6 @@ bool FFmpegDecoder::Open() } our_instance->SetFramePool(frame_pool); - // End test code } // Determine which Olive native pixel format we retrieved @@ -1300,8 +1301,8 @@ FFmpegDecoderInstance::FFmpegDecoderInstance(const char *filename, int stream_in // Start clear timer clear_timer_ = new QTimer(); clear_timer_->setInterval(kMaxFrameLife); - //clear_timer_->moveToThread(qApp->thread()); - connect(clear_timer_, &QTimer::timeout, this, &FFmpegDecoderInstance::ClearTimerEvent); + clear_timer_->moveToThread(qApp->thread()); + connect(clear_timer_, &QTimer::timeout, this, &FFmpegDecoderInstance::ClearTimerEvent, Qt::DirectConnection); QMetaObject::invokeMethod(clear_timer_, "start", Qt::QueuedConnection); } @@ -1330,16 +1331,9 @@ void FFmpegDecoderInstance::ClearResources() // Stop timer if (clear_timer_) { - - if (clear_timer_->thread() == QThread::currentThread()) { - clear_timer_->stop(); - } else { - QMetaObject::invokeMethod(clear_timer_, "stop", Qt::BlockingQueuedConnection); - } - - QMetaObject::invokeMethod(clear_timer_, "deleteLater", Qt::QueuedConnection); + QMetaObject::invokeMethod(clear_timer_, "stop", Qt::QueuedConnection); + clear_timer_->deleteLater(); clear_timer_ = nullptr; - } if (opts_) { diff --git a/app/render/backend/renderworker.cpp b/app/render/backend/renderworker.cpp index 5b670fc0d..252cb1210 100644 --- a/app/render/backend/renderworker.cpp +++ b/app/render/backend/renderworker.cpp @@ -22,6 +22,7 @@ #include #include +#include #include "audio/audiovisualwaveform.h" #include "common/functiontimer.h" @@ -31,12 +32,27 @@ OLIVE_NAMESPACE_ENTER +// FIXME: Hardcoded value. It seems to work fine, but is there a possibility we should make +// this a dynamic value somehow or a configurable value? +const int RenderWorker::kMaxDecoderLife = 6000; + RenderWorker::RenderWorker(RenderBackend* parent) : parent_(parent), available_(true), generate_audio_previews_(false), render_mode_(RenderMode::kOnline) { + cleanup_timer_ = new QTimer(); + cleanup_timer_->setInterval(kMaxDecoderLife); + connect(cleanup_timer_, &QTimer::timeout, this, &RenderWorker::ClearOldDecoders, Qt::DirectConnection); + cleanup_timer_->moveToThread(qApp->thread()); + QMetaObject::invokeMethod(cleanup_timer_, "start", Qt::QueuedConnection); +} + +RenderWorker::~RenderWorker() +{ + QMetaObject::invokeMethod(cleanup_timer_, "stop", Qt::QueuedConnection); + cleanup_timer_->deleteLater(); } void RenderWorker::Hash(RenderTicketPtr ticket, ViewerOutput *viewer, const QVector ×) @@ -72,6 +88,24 @@ QByteArray RenderWorker::HashNode(const Node *n, const VideoParams ¶ms, cons return hasher.result(); } +void RenderWorker::ClearOldDecoders() +{ + QMutexLocker locker(&decoder_lock_); + + QHash::iterator i = decoder_age_.begin(); + + while (i != decoder_age_.end()) { + if (i.value() < QDateTime::currentMSecsSinceEpoch() - kMaxDecoderLife) { + // This decoder is old, remove it + decoder_cache_.remove(i.key()); + + i = decoder_age_.erase(i); + } else { + i++; + } + } +} + void RenderWorker::RenderFrame(RenderTicketPtr ticket, ViewerOutput* viewer, const rational &time) { ticket_ = ticket; @@ -294,6 +328,7 @@ QVariant RenderWorker::GetCachedFrame(const Node* node, const rational& time) DecoderPtr RenderWorker::ResolveDecoderFromInput(StreamPtr stream) { // Access a map of Node inputs and decoder instances and retrieve a frame! + QMutexLocker locker(&decoder_lock_); DecoderPtr decoder = decoder_cache_.value(stream.get()); @@ -311,6 +346,8 @@ DecoderPtr RenderWorker::ResolveDecoderFromInput(StreamPtr stream) } } + decoder_age_.insert(stream.get(), QDateTime::currentMSecsSinceEpoch()); + return decoder; } diff --git a/app/render/backend/renderworker.h b/app/render/backend/renderworker.h index a709663d8..6d4729cb9 100644 --- a/app/render/backend/renderworker.h +++ b/app/render/backend/renderworker.h @@ -38,6 +38,8 @@ class RenderWorker : public QObject, public NodeTraverser public: RenderWorker(RenderBackend* parent); + virtual ~RenderWorker() override; + bool IsAvailable() const { return available_; @@ -169,7 +171,9 @@ private: QMatrix4x4 video_download_matrix_; + QMutex decoder_lock_; DecoderCache decoder_cache_; + QHash decoder_age_; TimeRange audio_render_time_; bool available_; @@ -182,6 +186,13 @@ private: RenderMode::Mode render_mode_; + QTimer* cleanup_timer_; + + static const int kMaxDecoderLife; + +private slots: + void ClearOldDecoders(); + }; OLIVE_NAMESPACE_EXIT