Merge pull request #448 from dpaulat/hotfix/use-separate-audio-thread

Move media objects to dedicated thread to avoid main thread delays

scwx-qt/source/scwx/qt/manager/media_manager.cpp

@@ -5,13 +5,10 @@
 #include <QAudioOutput>
 #include <QMediaDevices>
 #include <QMediaPlayer>
+#include <QThread>
 #include <QUrl>
 
-namespace scwx
-{
-namespace qt
-{
-namespace manager
+namespace scwx::qt::manager
 {
 
 static const std::string logPrefix_ = "scwx::qt::manager::media_manager";
@@ -20,46 +17,80 @@ static const auto        logger_    = scwx::util::Logger::Create(logPrefix_);
 class MediaManager::Impl
 {
 public:
-   explicit Impl(MediaManager* self) :
-       self_ {self},
-       mediaDevices_ {new QMediaDevices(self)},
-       mediaPlayer_ {new QMediaPlayer(self)},
-       audioOutput_ {new QAudioOutput(self)}
+   explicit Impl()
    {
-      logger_->debug("Audio device: {}",
-                     audioOutput_->device().description().toStdString());
+      mediaParent_ = std::make_unique<QObject>();
+      mediaParent_->moveToThread(&thread_);
 
-      mediaPlayer_->setAudioOutput(audioOutput_);
+      thread_.start();
 
-      ConnectSignals();
+      QMetaObject::invokeMethod(
+         mediaParent_.get(),
+         [this]()
+         {
+            // QObjects are managed by the parent
+            // NOLINTBEGIN(cppcoreguidelines-owning-memory)
+
+            logger_->debug("Creating QMediaDevices");
+            mediaDevices_ = new QMediaDevices(mediaParent_.get());
+            logger_->debug("Creating QMediaPlayer");
+            mediaPlayer_ = new QMediaPlayer(mediaParent_.get());
+            logger_->debug("Creating QAudioOutput");
+            audioOutput_ = new QAudioOutput(mediaParent_.get());
+
+            // NOLINTEND(cppcoreguidelines-owning-memory)
+
+            logger_->debug("Audio device: {}",
+                           audioOutput_->device().description().toStdString());
+
+            mediaPlayer_->setAudioOutput(audioOutput_);
+
+            ConnectSignals();
+         });
    }
 
-   ~Impl() {}
+   ~Impl()
+   {
+      // Delete the media parent
+      mediaParent_.reset();
+
+      thread_.quit();
+      thread_.wait();
+   }
+
+   Impl(const Impl&)             = delete;
+   Impl& operator=(const Impl&)  = delete;
+   Impl(const Impl&&)            = delete;
+   Impl& operator=(const Impl&&) = delete;
 
    void ConnectSignals();
 
-   MediaManager* self_;
+   QThread thread_ {};
 
-   QMediaDevices* mediaDevices_;
-   QMediaPlayer*  mediaPlayer_;
-   QAudioOutput*  audioOutput_;
+   std::unique_ptr<QObject> mediaParent_ {nullptr};
+   QMediaDevices*           mediaDevices_ {nullptr};
+   QMediaPlayer*            mediaPlayer_ {nullptr};
+   QAudioOutput*            audioOutput_ {nullptr};
 };
 
-MediaManager::MediaManager() : p(std::make_unique<Impl>(this)) {}
+MediaManager::MediaManager() : p(std::make_unique<Impl>()) {}
 MediaManager::~MediaManager() = default;
 
+MediaManager::MediaManager(MediaManager&&) noexcept            = default;
+MediaManager& MediaManager::operator=(MediaManager&&) noexcept = default;
+
 void MediaManager::Impl::ConnectSignals()
 {
    QObject::connect(
       mediaDevices_,
       &QMediaDevices::audioOutputsChanged,
-      self_,
+      mediaParent_.get(),
       [this]()
       { audioOutput_->setDevice(QMediaDevices::defaultAudioOutput()); });
 
    QObject::connect(audioOutput_,
                     &QAudioOutput::deviceChanged,
-                    self_,
+                    mediaParent_.get(),
                     [this]()
                     {
                        logger_->debug(
@@ -69,7 +100,7 @@ void MediaManager::Impl::ConnectSignals()
 
    QObject::connect(mediaPlayer_,
                     &QMediaPlayer::errorOccurred,
-                    self_,
+                    mediaParent_.get(),
                     [](QMediaPlayer::Error error, const QString& errorString)
                     {
                        logger_->error("Error {}: {}",
@@ -81,30 +112,46 @@ void MediaManager::Impl::ConnectSignals()
 void MediaManager::Play(types::AudioFile media)
 {
    const std::string path = types::GetMediaPath(media);
+   Play(path);
 }
 
 void MediaManager::Play(const std::string& mediaPath)
 {
    logger_->debug("Playing audio: {}", mediaPath);
 
+   if (p->mediaPlayer_ == nullptr)
+   {
+      logger_->warn("Media player is not yet initialized");
+      return;
+   }
+
    if (mediaPath.starts_with(':'))
    {
-      p->mediaPlayer_->setSource(
+      QMetaObject::invokeMethod(
+         p->mediaPlayer_,
+         &QMediaPlayer::setSource,
          QUrl(QString("qrc%1").arg(QString::fromStdString(mediaPath))));
    }
    else
    {
-      p->mediaPlayer_->setSource(
+      QMetaObject::invokeMethod(
+         p->mediaPlayer_,
+         &QMediaPlayer::setSource,
          QUrl::fromLocalFile(QString::fromStdString(mediaPath)));
    }
 
-   p->mediaPlayer_->setPosition(0);
-
+   QMetaObject::invokeMethod(p->mediaPlayer_, &QMediaPlayer::setPosition, 0);
    QMetaObject::invokeMethod(p->mediaPlayer_, &QMediaPlayer::play);
 }
 
 void MediaManager::Stop()
 {
+   if (p->mediaPlayer_ == nullptr)
+   {
+      logger_->warn("Media player is not yet initialized");
+      return;
+   }
+
    QMetaObject::invokeMethod(p->mediaPlayer_, &QMediaPlayer::stop);
 }
 
@@ -126,6 +173,4 @@ std::shared_ptr<MediaManager> MediaManager::Instance()
    return mediaManager;
 }
 
-} // namespace manager
-} // namespace qt
-} // namespace scwx
+} // namespace scwx::qt::manager

scwx-qt/source/scwx/qt/manager/media_manager.hpp

@@ -4,24 +4,21 @@
 
 #include <memory>
 
-#include <QObject>
-
-namespace scwx
-{
-namespace qt
-{
-namespace manager
+namespace scwx::qt::manager
 {
 
-class MediaManager : public QObject
+class MediaManager
 {
-   Q_OBJECT
-   Q_DISABLE_COPY_MOVE(MediaManager)
-
 public:
    explicit MediaManager();
    ~MediaManager();
 
+   MediaManager(const MediaManager&)            = delete;
+   MediaManager& operator=(const MediaManager&) = delete;
+
+   MediaManager(MediaManager&&) noexcept;
+   MediaManager& operator=(MediaManager&&) noexcept;
+
    void Play(types::AudioFile media);
    void Play(const std::string& mediaPath);
    void Stop();
@@ -33,6 +30,4 @@ private:
    std::unique_ptr<Impl> p;
 };
 
-} // namespace manager
-} // namespace qt
-} // namespace scwx
+} // namespace scwx::qt::manager