Merge pull request #10941 from sethalves/audio-injector-smart-pointers

Audio injector smart pointers

interface/src/Application.h

@@ -678,7 +678,7 @@ private:
     QTimer _addAssetToWorldErrorTimer;
 
     FileScriptingInterface* _fileDownload;
-    AudioInjector* _snapshotSoundInjector { nullptr };
+    AudioInjectorPointer _snapshotSoundInjector;
     SharedSoundPointer _snapshotSound;
 
     DisplayPluginPointer _autoSwitchDisplayModeSupportedHMDPlugin;

interface/src/avatar/AvatarManager.cpp

@@ -430,8 +430,7 @@ void AvatarManager::handleCollisionEvents(const CollisionEvents& collisionEvents
                 // but most avatars are roughly the same size, so let's not be so fancy yet.
                 const float AVATAR_STRETCH_FACTOR = 1.0f;
 
-
-                _collisionInjectors.remove_if([](QPointer<AudioInjector>& injector) {
+                _collisionInjectors.remove_if([](const AudioInjectorPointer& injector) {
                     return !injector || injector->isFinished();
                 });
 

interface/src/avatar/AvatarManager.h

@@ -22,11 +22,11 @@
 #include <SimpleMovingAverage.h>
 #include <shared/RateCounter.h>
 #include <avatars-renderer/ScriptAvatar.h>
+#include <AudioInjector.h>
 
 #include "AvatarMotionState.h"
 #include "MyAvatar.h"
 
-class AudioInjector;
 
 class AvatarManager : public AvatarHashMap {
     Q_OBJECT
@@ -104,7 +104,7 @@ private:
     std::shared_ptr<MyAvatar> _myAvatar;
     quint64 _lastSendAvatarDataTime = 0; // Controls MyAvatar send data rate.
 
-    std::list<QPointer<AudioInjector>> _collisionInjectors;
+    std::list<AudioInjectorPointer> _collisionInjectors;
 
     RateCounter<> _myAvatarSendRate;
     int _numAvatarsUpdated { 0 };

libraries/audio-client/src/AudioClient.cpp

@@ -1203,11 +1203,11 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) {
     // lock the injectors
     Lock lock(_injectorsMutex);
 
-    QVector<AudioInjector*> injectorsToRemove;
+    QVector<AudioInjectorPointer> injectorsToRemove;
 
     memset(mixBuffer, 0, AudioConstants::NETWORK_FRAME_SAMPLES_STEREO * sizeof(float));
 
-    for (AudioInjector* injector : _activeLocalAudioInjectors) {
+    for (const AudioInjectorPointer& injector : _activeLocalAudioInjectors) {
         // the lock guarantees that injectorBuffer, if found, is invariant
         AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer();
         if (injectorBuffer) {
@@ -1278,7 +1278,7 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) {
         }
     }
 
-    for (AudioInjector* injector : injectorsToRemove) {
+    for (const AudioInjectorPointer& injector : injectorsToRemove) {
         qCDebug(audioclient) << "removing injector";
         _activeLocalAudioInjectors.removeOne(injector);
     }
@@ -1369,7 +1369,7 @@ void AudioClient::setIsStereoInput(bool isStereoInput) {
     }
 }
 
-bool AudioClient::outputLocalInjector(AudioInjector* injector) {
+bool AudioClient::outputLocalInjector(const AudioInjectorPointer& injector) {
     AudioInjectorLocalBuffer* injectorBuffer = injector->getLocalBuffer();
     if (injectorBuffer) {
         // local injectors are on the AudioInjectorsThread, so we must guard access

libraries/audio-client/src/AudioClient.h

@@ -143,7 +143,7 @@ public:
 
     Q_INVOKABLE void setAvatarBoundingBoxParameters(glm::vec3 corner, glm::vec3 scale);
 
-    bool outputLocalInjector(AudioInjector* injector) override;
+    bool outputLocalInjector(const AudioInjectorPointer& injector) override;
 
     QAudioDeviceInfo getActiveAudioDevice(QAudio::Mode mode) const;
     QList<QAudioDeviceInfo> getAudioDevices(QAudio::Mode mode) const;
@@ -380,7 +380,7 @@ private:
 
     bool _hasReceivedFirstPacket { false };
 
-    QVector<AudioInjector*> _activeLocalAudioInjectors;
+    QVector<AudioInjectorPointer> _activeLocalAudioInjectors;
 
     bool _isPlayingBackRecording { false };
 

libraries/audio/src/AbstractAudioInterface.h

@@ -18,6 +18,7 @@
 #include <udt/PacketHeaders.h>
 
 #include "AudioInjectorOptions.h"
+#include "AudioInjector.h"
 
 class AudioInjector;
 class AudioInjectorLocalBuffer;
@@ -35,7 +36,7 @@ public:
     // threadsafe
     // moves injector->getLocalBuffer() to another thread (so removes its parent)
     // take care to delete it when ~AudioInjector, as parenting Qt semantics will not work
-    virtual bool outputLocalInjector(AudioInjector* injector) = 0;
+    virtual bool outputLocalInjector(const AudioInjectorPointer& injector) = 0;
 
 public slots:
     virtual bool shouldLoopbackInjectors() { return false; }

libraries/audio/src/AudioInjector.cpp

@@ -92,11 +92,6 @@ void AudioInjector::finish() {
     emit finished();
 
     deleteLocalBuffer();
-
-    if (stateHas(AudioInjectorState::PendingDelete)) {
-        // we've been asked to delete after finishing, trigger a deleteLater here
-        deleteLater();
-    }
 }
 
 void AudioInjector::restart() {
@@ -132,7 +127,7 @@ void AudioInjector::restart() {
     }
 }
 
-bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjector*)) {
+bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(const AudioInjectorPointer&)) {
     _state = AudioInjectorState::NotFinished;
 
     int byteOffset = 0;
@@ -150,7 +145,7 @@ bool AudioInjector::inject(bool(AudioInjectorManager::*injection)(AudioInjector*
     bool success = true;
     if (!_options.localOnly) {
         auto injectorManager = DependencyManager::get<AudioInjectorManager>();
-        if (!(*injectorManager.*injection)(this)) {
+        if (!(*injectorManager.*injection)(sharedFromThis())) {
             success = false;
             finishNetworkInjection();
         }
@@ -173,7 +168,7 @@ bool AudioInjector::injectLocally() {
 
             // call this function on the AudioClient's thread
             // this will move the local buffer's thread to the LocalInjectorThread
-            success = _localAudioInterface->outputLocalInjector(this);
+            success = _localAudioInterface->outputLocalInjector(sharedFromThis());
 
             if (!success) {
                 qCDebug(audio) << "AudioInjector::injectLocally could not output locally via _localAudioInterface";
@@ -418,20 +413,16 @@ void AudioInjector::triggerDeleteAfterFinish() {
     }
 
     if (stateHas(AudioInjectorState::Finished)) {
-        stopAndDeleteLater();
+        stop();
     } else {
         _state |= AudioInjectorState::PendingDelete;
     }
 }
 
-void AudioInjector::stopAndDeleteLater() {
-    stop();
-    QMetaObject::invokeMethod(this, "deleteLater", Qt::QueuedConnection);
-}
-
-AudioInjector* AudioInjector::playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position) {
+AudioInjectorPointer AudioInjector::playSound(SharedSoundPointer sound, const float volume,
+                                              const float stretchFactor, const glm::vec3 position) {
     if (!sound || !sound->isReady()) {
-        return nullptr;
+        return AudioInjectorPointer();
     }
 
     AudioInjectorOptions options;
@@ -462,8 +453,8 @@ AudioInjector* AudioInjector::playSound(SharedSoundPointer sound, const float vo
     return playSoundAndDelete(resampled, options);
 }
 
-AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options) {
-    AudioInjector* sound = playSound(buffer, options);
+AudioInjectorPointer AudioInjector::playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options) {
+    AudioInjectorPointer sound = playSound(buffer, options);
 
     if (sound) {
         sound->_state |= AudioInjectorState::PendingDelete;
@@ -473,8 +464,9 @@ AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const
 }
 
 
-AudioInjector* AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) {
-    AudioInjector* injector = new AudioInjector(buffer, options);
+AudioInjectorPointer AudioInjector::playSound(const QByteArray& buffer, const AudioInjectorOptions options) {
+    AudioInjectorPointer injector = AudioInjectorPointer::create(buffer, options);
+
     if (!injector->inject(&AudioInjectorManager::threadInjector)) {
         qWarning() << "AudioInjector::playSound failed to thread injector";
     }

libraries/audio/src/AudioInjector.h

@@ -32,6 +32,8 @@
 
 class AbstractAudioInterface;
 class AudioInjectorManager;
+class AudioInjector;
+using AudioInjectorPointer = QSharedPointer<AudioInjector>;
 
 
 enum class AudioInjectorState : uint8_t {
@@ -47,7 +49,7 @@ AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs)
 
 // In order to make scripting cleaner for the AudioInjector, the script now holds on to the AudioInjector object
 // until it dies.
-class AudioInjector : public QObject {
+class AudioInjector : public QObject, public QEnableSharedFromThis<AudioInjector> {
     Q_OBJECT
 public:
     AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions);
@@ -72,16 +74,16 @@ public:
 
     bool stateHas(AudioInjectorState state) const ;
     static void setLocalAudioInterface(AbstractAudioInterface* audioInterface) { _localAudioInterface = audioInterface; }
-    static AudioInjector* playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options);
-    static AudioInjector* playSound(const QByteArray& buffer, const AudioInjectorOptions options);
-    static AudioInjector* playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position);
+    static AudioInjectorPointer playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options);
+    static AudioInjectorPointer playSound(const QByteArray& buffer, const AudioInjectorOptions options);
+    static AudioInjectorPointer playSound(SharedSoundPointer sound, const float volume,
+                                          const float stretchFactor, const glm::vec3 position);
 
 public slots:
     void restart();
 
     void stop();
     void triggerDeleteAfterFinish();
-    void stopAndDeleteLater();
 
     const AudioInjectorOptions& getOptions() const { return _options; }
     void setOptions(const AudioInjectorOptions& options);
@@ -98,7 +100,7 @@ signals:
 
 private:
     int64_t injectNextFrame();
-    bool inject(bool(AudioInjectorManager::*injection)(AudioInjector*));
+    bool inject(bool(AudioInjectorManager::*injection)(const AudioInjectorPointer&));
     bool injectLocally();
     void deleteLocalBuffer();
 
@@ -123,6 +125,6 @@ private:
     friend class AudioInjectorManager;
 };
 
-Q_DECLARE_METATYPE(AudioInjector*)
+Q_DECLARE_METATYPE(AudioInjectorPointer)
 
 #endif // hifi_AudioInjector_h

libraries/audio/src/AudioInjectorManager.cpp

@@ -30,7 +30,7 @@ AudioInjectorManager::~AudioInjectorManager() {
         auto& timePointerPair = _injectors.top();
 
         // ask it to stop and be deleted
-        timePointerPair.second->stopAndDeleteLater();
+        timePointerPair.second->stop();
 
         _injectors.pop();
     }
@@ -139,7 +139,7 @@ bool AudioInjectorManager::wouldExceedLimits() { // Should be called inside of a
     return false;
 }
 
-bool AudioInjectorManager::threadInjector(AudioInjector* injector) {
+bool AudioInjectorManager::threadInjector(const AudioInjectorPointer& injector) {
     if (_shouldStop) {
         qCDebug(audio)  << "AudioInjectorManager::threadInjector asked to thread injector but is shutting down.";
         return false;
@@ -159,7 +159,7 @@ bool AudioInjectorManager::threadInjector(AudioInjector* injector) {
         injector->moveToThread(_thread);
 
         // add the injector to the queue with a send timestamp of now
-        _injectors.emplace(usecTimestampNow(), InjectorQPointer { injector });
+        _injectors.emplace(usecTimestampNow(), injector);
 
         // notify our wait condition so we can inject two frames for this injector immediately
         _injectorReady.notify_one();
@@ -168,7 +168,7 @@ bool AudioInjectorManager::threadInjector(AudioInjector* injector) {
     }
 }
 
-bool AudioInjectorManager::restartFinishedInjector(AudioInjector* injector) {
+bool AudioInjectorManager::restartFinishedInjector(const AudioInjectorPointer& injector) {
     if (_shouldStop) {
         qCDebug(audio)  << "AudioInjectorManager::threadInjector asked to thread injector but is shutting down.";
         return false;
@@ -181,7 +181,7 @@ bool AudioInjectorManager::restartFinishedInjector(AudioInjector* injector) {
         return false;
     } else {
         // add the injector to the queue with a send timestamp of now
-        _injectors.emplace(usecTimestampNow(), InjectorQPointer { injector });
+        _injectors.emplace(usecTimestampNow(), injector);
 
         // notify our wait condition so we can inject two frames for this injector immediately
         _injectorReady.notify_one();

libraries/audio/src/AudioInjectorManager.h

@@ -23,7 +23,7 @@
 
 #include <DependencyManager.h>
 
-class AudioInjector;
+#include "AudioInjector.h"
 
 class AudioInjectorManager : public QObject, public Dependency {
     Q_OBJECT
@@ -34,8 +34,7 @@ private slots:
     void run();
 private:
 
-    using InjectorQPointer = QPointer<AudioInjector>;
-    using TimeInjectorPointerPair = std::pair<uint64_t, InjectorQPointer>;
+    using TimeInjectorPointerPair = std::pair<uint64_t, AudioInjectorPointer>;
 
     struct greaterTime {
         bool operator() (const TimeInjectorPointerPair& x, const TimeInjectorPointerPair& y) const {
@@ -49,8 +48,8 @@ private:
     using Mutex = std::mutex;
     using Lock = std::unique_lock<Mutex>;
 
-    bool threadInjector(AudioInjector* injector);
-    bool restartFinishedInjector(AudioInjector* injector);
+    bool threadInjector(const AudioInjectorPointer& injector);
+    bool restartFinishedInjector(const AudioInjectorPointer& injector);
     void notifyInjectorReadyCondition() { _injectorReady.notify_one(); }
     bool wouldExceedLimits();
 

libraries/script-engine/src/ScriptAudioInjector.cpp

@@ -29,10 +29,10 @@ void injectorFromScriptValue(const QScriptValue& object, ScriptAudioInjector*& o
     out = qobject_cast<ScriptAudioInjector*>(object.toQObject());
 }
 
-ScriptAudioInjector::ScriptAudioInjector(AudioInjector* injector) :
+ScriptAudioInjector::ScriptAudioInjector(const AudioInjectorPointer& injector) :
     _injector(injector)
 {
-    QObject::connect(injector, &AudioInjector::finished, this, &ScriptAudioInjector::finished);
+    QObject::connect(injector.data(), &AudioInjector::finished, this, &ScriptAudioInjector::finished);
 }
 
 ScriptAudioInjector::~ScriptAudioInjector() {
@@ -44,5 +44,5 @@ ScriptAudioInjector::~ScriptAudioInjector() {
 
 void ScriptAudioInjector::stopInjectorImmediately() {
     qCDebug(scriptengine) << "ScriptAudioInjector::stopInjectorImmediately called to stop audio injector immediately.";
-    _injector->stopAndDeleteLater();
+    _injector->stop();
 }

libraries/script-engine/src/ScriptAudioInjector.h

@@ -23,7 +23,7 @@ class ScriptAudioInjector : public QObject {
     Q_PROPERTY(float loudness READ getLoudness)
     Q_PROPERTY(AudioInjectorOptions options WRITE setOptions READ getOptions)
 public:
-    ScriptAudioInjector(AudioInjector* injector);
+    ScriptAudioInjector(const AudioInjectorPointer& injector);
     ~ScriptAudioInjector();
 public slots:
     void restart() { _injector->restart(); }
@@ -41,7 +41,7 @@ signals:
 protected slots:
     void stopInjectorImmediately();
 private:
-    QPointer<AudioInjector> _injector;
+    AudioInjectorPointer _injector;
 
     friend QScriptValue injectorToScriptValue(QScriptEngine* engine, ScriptAudioInjector* const& in);
 };

libraries/ui/src/ui/types/SoundEffect.h

@@ -15,6 +15,7 @@
 #include <SoundCache.h>
 
 class AudioInjector;
+using AudioInjectorPointer = QSharedPointer<AudioInjector>;
 
 // SoundEffect object, exposed to qml only, not interface JavaScript.
 // This is used to play spatial sound effects on tablets/web entities from within QML.
@@ -38,7 +39,7 @@ protected:
     QUrl _url;
     float _volume { 1.0f };
     SharedSoundPointer _sound;
-    AudioInjector* _injector { nullptr };
+    AudioInjectorPointer _injector;
 };
 
 #endif  // hifi_SoundEffect_h