Skip to content
Snippets Groups Projects
Commit 6d8adc94 authored by Andreas Traczyk's avatar Andreas Traczyk
Browse files

avmodel: avoid write-locking the mutex when deleting the renderer

During a resize event the the addRenderer function will regenerate a Renderer of the new size. The Renderer destructor will eventually trigger a write-lock in VideoProvider which may deadlock with frame updates to other renderers.

Gitlab: #1161
Change-Id: I3f3b99f6f78ac94218213130d42efcf0884b38fa
parent e8b69145
No related branches found
No related tags found
No related merge requests found
...@@ -103,10 +103,12 @@ public: ...@@ -103,10 +103,12 @@ public:
void addRenderer(const QString& id, const QSize& res, const QString& shmPath = {}); void addRenderer(const QString& id, const QSize& res, const QString& shmPath = {});
/** /**
* Remove renderer from renderers_ * Remove renderer from renderers_. If the returned renderer is ignored, it
* will be deleted.
* @param id * @param id
* @return the renderer
*/ */
void removeRenderer(const QString& id); std::unique_ptr<Renderer> removeRenderer(const QString& id);
bool hasRenderer(const QString& id); bool hasRenderer(const QString& id);
QSize getRendererSize(const QString& id); QSize getRendererSize(const QString& id);
...@@ -947,7 +949,7 @@ void ...@@ -947,7 +949,7 @@ void
AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& shmPath) AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& shmPath)
{ {
// Remove the renderer if it already exists. // Remove the renderer if it already exists.
removeRenderer(id); // Will write-lock renderersMutex_. std::ignore = removeRenderer(id);
{ {
QWriteLocker lk(&renderersMutex_); QWriteLocker lk(&renderersMutex_);
...@@ -955,13 +957,7 @@ AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& sh ...@@ -955,13 +957,7 @@ AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& sh
} }
QReadLocker lk(&renderersMutex_); QReadLocker lk(&renderersMutex_);
auto it = renderers_.find(id); if (auto* renderer = renderers_.find(id)->second.get()) {
if (it == renderers_.end()) {
qWarning() << Q_FUNC_INFO << "Renderer not found for id:" << id;
return;
}
if (auto* renderer = it->second.get()) {
connect( connect(
renderer, renderer,
&Renderer::fpsChanged, &Renderer::fpsChanged,
...@@ -999,16 +995,18 @@ AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& sh ...@@ -999,16 +995,18 @@ AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& sh
} }
} }
void std::unique_ptr<Renderer>
AVModelPimpl::removeRenderer(const QString& id) AVModelPimpl::removeRenderer(const QString& id)
{ {
QWriteLocker lk(&renderersMutex_); QWriteLocker lk(&renderersMutex_);
auto it = renderers_.find(id); auto it = renderers_.find(id);
if (it == renderers_.end()) { if (it == renderers_.end()) {
qWarning() << "Cannot remove renderer. " << id << "not found"; qWarning() << "Cannot remove renderer. " << id << "not found";
return; return {};
} }
renderers_.erase(id); auto removed = std::move(it->second);
renderers_.erase(it);
return removed;
} }
bool bool
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment