From 5f79a9f280d799757e6685471a83ccf23a45a704 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Wed, 8 Jun 2022 16:12:28 -0400
Subject: [PATCH] renderers: do not crash on update and simplify code
No need to add complex mechanisms for nearly no-ops, the previous
method were bugguy and caused weird crashes. Just re-create the
renderer correctly.
Change-Id: I88738244c7cbaa6f189799c6a28dc9244953667d
---
src/app/videoprovider.cpp | 32 ++++++++++++++++++++++++--------
src/libclient/avmodel.cpp | 24 ++++++------------------
src/libclient/directrenderer.cpp | 10 ----------
src/libclient/directrenderer.h | 1 -
src/libclient/renderer.cpp | 6 ------
src/libclient/renderer.h | 7 -------
src/libclient/shmrenderer.cpp | 24 ++++++++----------------
src/libclient/shmrenderer.h | 1 -
8 files changed, 38 insertions(+), 67 deletions(-)
diff --git a/src/app/videoprovider.cpp b/src/app/videoprovider.cpp
index d7741b770..127a89662 100644
--- a/src/app/videoprovider.cpp
+++ b/src/app/videoprovider.cpp
@@ -34,10 +34,26 @@ VideoProvider::VideoProvider(AVModel& avModel, QObject* parent)
: QObject(parent)
, avModel_(avModel)
{
- connect(&avModel_, &AVModel::rendererStarted, this, &VideoProvider::onRendererStarted);
- connect(&avModel_, &AVModel::frameBufferRequested, this, &VideoProvider::onFrameBufferRequested);
- connect(&avModel_, &AVModel::frameUpdated, this, &VideoProvider::onFrameUpdated);
- connect(&avModel_, &AVModel::rendererStopped, this, &VideoProvider::onRendererStopped);
+ connect(&avModel_,
+ &AVModel::rendererStarted,
+ this,
+ &VideoProvider::onRendererStarted,
+ Qt::DirectConnection);
+ connect(&avModel_,
+ &AVModel::frameBufferRequested,
+ this,
+ &VideoProvider::onFrameBufferRequested,
+ Qt::DirectConnection);
+ connect(&avModel_,
+ &AVModel::frameUpdated,
+ this,
+ &VideoProvider::onFrameUpdated,
+ Qt::DirectConnection);
+ connect(&avModel_,
+ &AVModel::rendererStopped,
+ this,
+ &VideoProvider::onRendererStopped,
+ Qt::DirectConnection);
}
void
@@ -95,10 +111,10 @@ VideoProvider::captureVideoFrame(const QString& id)
auto imageFormat = QVideoFrameFormat::imageFormatFromPixelFormat(
QVideoFrameFormat::Format_RGBA8888);
auto img = QImage(videoFrame->bits(0),
- videoFrame->width(),
- videoFrame->height(),
- videoFrame->bytesPerLine(0),
- imageFormat);
+ videoFrame->width(),
+ videoFrame->height(),
+ videoFrame->bytesPerLine(0),
+ imageFormat);
return Utils::byteArrayToBase64String(Utils::QImageToByteArray(img));
}
return {};
diff --git a/src/libclient/avmodel.cpp b/src/libclient/avmodel.cpp
index 088ff0e25..0254eb0dd 100644
--- a/src/libclient/avmodel.cpp
+++ b/src/libclient/avmodel.cpp
@@ -872,24 +872,12 @@ AVModelPimpl::addRenderer(const QString& id, const QSize& res, const QString& sh
Qt::DirectConnection);
};
std::lock_guard<std::mutex> lk(renderers_mtx_);
- Renderer* renderer {nullptr};
- auto it = renderers_.find(id);
- if (it == renderers_.end()) {
- renderers_.emplace(id, createRenderer(id, res, shmPath));
- renderer = renderers_.at(id).get();
- connectRenderer(renderer, id);
- renderer->startRendering();
- } else {
- renderer = it->second.get();
- if (renderer) {
- renderer->update(res, shmPath);
- } else {
- it->second.reset(createRenderer(id, res, shmPath).get());
- renderer = it->second.get();
- connectRenderer(renderer, id);
- renderer->startRendering();
- }
- }
+ renderers_.erase(id); // Because it should be done before creating the renderer
+ auto renderer = createRenderer(id, res, shmPath);
+ auto& r = renderers_[id];
+ r = std::move(renderer);
+ connectRenderer(r.get(), id);
+ r->startRendering();
}
}
diff --git a/src/libclient/directrenderer.cpp b/src/libclient/directrenderer.cpp
index f3d838326..c6078340e 100644
--- a/src/libclient/directrenderer.cpp
+++ b/src/libclient/directrenderer.cpp
@@ -112,16 +112,6 @@ DirectRenderer::stopRendering()
Q_EMIT stopped();
}
-void
-DirectRenderer::update(const QSize& res, const QString&)
-{
- stopRendering();
- Renderer::update(res);
-
- VideoManager::instance().registerSinkTarget(id(), pimpl_->target);
- startRendering();
-}
-
Frame
DirectRenderer::currentFrame() const
{
diff --git a/src/libclient/directrenderer.h b/src/libclient/directrenderer.h
index dfcd02659..3db7d2859 100644
--- a/src/libclient/directrenderer.h
+++ b/src/libclient/directrenderer.h
@@ -33,7 +33,6 @@ public:
~DirectRenderer();
// Renderer interface.
- void update(const QSize& res, const QString& shmPath) override;
lrc::api::video::Frame currentFrame() const override;
public Q_SLOTS:
diff --git a/src/libclient/renderer.cpp b/src/libclient/renderer.cpp
index 3a2065fd1..22f6fea4a 100644
--- a/src/libclient/renderer.cpp
+++ b/src/libclient/renderer.cpp
@@ -46,11 +46,5 @@ Renderer::size() const
return size_;
}
-void
-Renderer::update(const QSize& size, const QString&)
-{
- size_ = size;
-}
-
} // namespace video
} // namespace lrc
diff --git a/src/libclient/renderer.h b/src/libclient/renderer.h
index 9e91bc29f..7cba0f9ef 100644
--- a/src/libclient/renderer.h
+++ b/src/libclient/renderer.h
@@ -49,13 +49,6 @@ public:
*/
QSize size() const;
- /**
- * Update size and shmPath of a renderer
- * @param size new renderer dimensions
- * @param shmPath new shmPath
- */
- virtual void update(const QSize& size, const QString& shmPath = {});
-
/**
* @return current rendered frame
*/
diff --git a/src/libclient/shmrenderer.cpp b/src/libclient/shmrenderer.cpp
index 8bbd2947d..bdd4eee25 100644
--- a/src/libclient/shmrenderer.cpp
+++ b/src/libclient/shmrenderer.cpp
@@ -114,10 +114,16 @@ public:
constexpr static const int FRAME_CHECK_RATE_HZ = 120;
// Lock the memory while the copy is being made
- bool shmLock() { return ::sem_wait(&shmArea->mutex) >= 0; };
+ bool shmLock()
+ {
+ return ::sem_wait(&shmArea->mutex) >= 0;
+ };
// Remove the lock, allow a new frame to be drawn
- void shmUnlock() { ::sem_post(&shmArea->mutex); };
+ void shmUnlock()
+ {
+ ::sem_post(&shmArea->mutex);
+ };
// Wait for new frame data from shared memory and save pointer.
bool getNewFrame(bool wait)
@@ -242,20 +248,6 @@ ShmRenderer::~ShmRenderer()
stopShm();
}
-void
-ShmRenderer::update(const QSize& res, const QString& shmPath)
-{
- Q_EMIT stopped();
- Renderer::update(res);
-
- if (!pimpl_->thread.isRunning())
- pimpl_->thread.start();
-
- pimpl_->path = shmPath;
- VideoManager::instance().startShmSink(id(), true);
- Q_EMIT started();
-}
-
Frame
ShmRenderer::currentFrame() const
{
diff --git a/src/libclient/shmrenderer.h b/src/libclient/shmrenderer.h
index 7880432a1..3514a6ecb 100644
--- a/src/libclient/shmrenderer.h
+++ b/src/libclient/shmrenderer.h
@@ -32,7 +32,6 @@ public:
~ShmRenderer();
// Renderer interface.
- void update(const QSize& res, const QString& shmPath) override;
lrc::api::video::Frame currentFrame() const override;
void stopShm();
--
GitLab