Pin ChannelHandle to permanent memory address and use pointers

src/effects/backends/effectprocessor.h

@@ -74,10 +74,10 @@ class EffectProcessor {
     virtual void loadEngineEffectParameters(
             const QMap<QString, EngineEffectParameterPointer>& parameters) = 0;
     virtual EffectState* createState(const mixxx::EngineParameters& engineParameters) = 0;
-    virtual void deleteStatesForInputChannel(const ChannelHandle* inputChannel) = 0;
+    virtual void deleteStatesForInputChannel(const ChannelHandle* pInputChannel) = 0;
 
     // Called from the audio thread
-    virtual bool loadStatesForInputChannel(const ChannelHandle* inputChannel,
+    virtual bool loadStatesForInputChannel(const ChannelHandle* pInputChannel,
             const EffectStatesMap* pStatesMap) = 0;
 
     /// Called from the audio thread
@@ -91,8 +91,8 @@ class EffectProcessor {
     /// EffectProcessorImpl::process to fetch the appropriate EffectState and
     /// pass it on to EffectProcessorImpl::processChannel, allowing one
     /// EffectProcessor instance to process multiple signals simultaneously.
-    virtual void process(const ChannelHandle& inputHandle,
-            const ChannelHandle& outputHandle,
+    virtual void process(const ChannelHandle* pInputHandle,
+            const ChannelHandle* pOutputHandle,
             const CSAMPLE* pInput,
             CSAMPLE* pOutput,
             const mixxx::EngineParameters& engineParameters,
@@ -149,25 +149,25 @@ class EffectProcessorImpl : public EffectProcessor {
             const EffectEnableState enableState,
             const GroupFeatureState& groupFeatures) = 0;
 
-    void process(const ChannelHandle& inputHandle,
-            const ChannelHandle& outputHandle,
+    void process(const ChannelHandle* pInputHandle,
+            const ChannelHandle* pOutputHandle,
             const CSAMPLE* pInput,
             CSAMPLE* pOutput,
             const mixxx::EngineParameters& engineParameters,
             const EffectEnableState enableState,
             const GroupFeatureState& groupFeatures) final {
-        EffectSpecificState* pState = m_channelStateMatrix[inputHandle][outputHandle];
+        EffectSpecificState* pState = m_channelStateMatrix[pInputHandle][pOutputHandle];
         VERIFY_OR_DEBUG_ASSERT(pState != nullptr) {
             if (kEffectDebugOutput) {
                 qWarning() << "EffectProcessorImpl::process could not retrieve"
                               "EffectState for input"
-                           << inputHandle
-                           << "and output" << outputHandle
+                           << *pInputHandle
+                           << "and output" << *pOutputHandle
                            << "EffectState should have been preallocated in the"
                               "main thread.";
             }
             pState = createSpecificState(engineParameters);
-            m_channelStateMatrix[inputHandle][outputHandle] = pState;
+            m_channelStateMatrix[pInputHandle][pOutputHandle] = pState;
         }
         processChannel(pState, pInput, pOutput, engineParameters, enableState, groupFeatures);
     }
@@ -202,11 +202,11 @@ class EffectProcessorImpl : public EffectProcessor {
         return createSpecificState(engineParameters);
     };
 
-    bool loadStatesForInputChannel(const ChannelHandle* inputChannel,
+    bool loadStatesForInputChannel(const ChannelHandle* pInputHandle,
             const EffectStatesMap* pStatesMap) final {
         if (kEffectDebugOutput) {
             qDebug() << "EffectProcessorImpl::loadStatesForInputChannel" << this
-                     << "input" << *inputChannel;
+                     << "input" << *pInputHandle;
         }
 
         // NOTE: ChannelHandleMap is like a map in that it associates an
@@ -220,7 +220,7 @@ class EffectProcessorImpl : public EffectProcessor {
         // pStatesMap to build a new ChannelHandleMap with
         // dynamic_cast'ed states.
         ChannelHandleMap<EffectSpecificState*>& effectSpecificStatesMap =
-                m_channelStateMatrix[*inputChannel];
+                m_channelStateMatrix[pInputHandle];
 
         // deleteStatesForInputChannel should have been called before a new
         // map of EffectStates was sent to this function, or this is the first
@@ -256,10 +256,10 @@ class EffectProcessorImpl : public EffectProcessor {
     };
 
     /// Called from main thread for garbage collection after an input channel is disabled
-    void deleteStatesForInputChannel(const ChannelHandle* inputChannel) final {
+    void deleteStatesForInputChannel(const ChannelHandle* pInputHandle) final {
         if (kEffectDebugOutput) {
             qDebug() << "EffectProcessorImpl::deleteStatesForInputChannel"
-                     << this << *inputChannel;
+                     << this << *pInputHandle;
         }
 
         // NOTE: ChannelHandleMap is like a map in that it associates an
@@ -269,7 +269,7 @@ class EffectProcessorImpl : public EffectProcessor {
         // engine thread in loadStatesForInputChannel.
 
         ChannelHandleMap<EffectSpecificState*>& stateMap =
-                m_channelStateMatrix[*inputChannel];
+                m_channelStateMatrix[pInputHandle];
         for (EffectSpecificState* pState : stateMap) {
             VERIFY_OR_DEBUG_ASSERT(pState != nullptr) {
                 continue;

src/effects/effectchain.cpp

@@ -394,7 +394,7 @@ void EffectChain::enableForInputChannel(const ChannelHandleAndGroup& handleGroup
     EffectsRequest* request = new EffectsRequest();
     request->type = EffectsRequest::ENABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL;
     request->pTargetChain = m_pEngineEffectChain;
-    request->EnableInputChannelForChain.pChannelHandle = &handleGroup.handle();
+    request->EnableInputChannelForChain.pChannelHandle = handleGroup.handle();
 
     // Allocate EffectStates here in the main thread to avoid allocating
     // memory in the realtime audio callback thread. Pointers to the
@@ -428,7 +428,7 @@ void EffectChain::disableForInputChannel(const ChannelHandleAndGroup& handleGrou
     EffectsRequest* request = new EffectsRequest();
     request->type = EffectsRequest::DISABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL;
     request->pTargetChain = m_pEngineEffectChain;
-    request->DisableInputChannelForChain.pChannelHandle = &handleGroup.handle();
+    request->DisableInputChannelForChain.pChannelHandle = handleGroup.handle();
     m_pMessenger->writeRequest(request);
 }
 

src/effects/effectsmanager.h

@@ -40,7 +40,7 @@ class EffectsManager {
         return m_pEngineEffectsManager;
     }
 
-    const ChannelHandle getMasterHandle() const {
+    const ChannelHandle* getMasterHandle() const {
         return m_pChannelHandleFactory->getOrCreateHandle("[Master]");
     }
 

src/engine/channelhandle.h

@@ -9,6 +9,8 @@
 #include "util/assert.h"
 #include "util/compatibility/qhash.h"
 
+constexpr int kMaxExpectedChannelGroups = 256;
+
 // ChannelHandle defines a unique identifier for channels of audio in the engine
 // (e.g. headphone output, master output, deck 1, microphone 3). Previously we
 // used the group string of the channel in the engine to uniquely identify it
@@ -28,27 +30,31 @@
 ///
 /// A helper class, ChannelHandleFactory, keeps a running count of handles that
 /// have been assigned.
-class ChannelHandle {
+///
+/// Channel handles are passed as pointers to permanent memory addresses and
+/// can neither be copied nor moved!!! This is required to pass them from/to
+/// the real-time thread, e.g. for managing effect configurations.
+class ChannelHandle final {
   public:
-    ChannelHandle() : m_iHandle(-1) {
-    }
-
-    inline bool valid() const {
+    bool valid() const {
         return m_iHandle >= 0;
     }
 
-    inline int handle() const {
+    int handle() const {
+        DEBUG_ASSERT(valid());
         return m_iHandle;
     }
 
   private:
-    ChannelHandle(int iHandle)
-            : m_iHandle(iHandle) {
-    }
-
-    void setHandle(int iHandle) {
-        m_iHandle = iHandle;
+    ChannelHandle()
+            : m_iHandle(-1) {
     }
+    // Channel handles are pinned at a fixed memory address and must
+    // neither be copied nor moved!!!
+    ChannelHandle(ChannelHandle&&) = delete;
+    ChannelHandle(const ChannelHandle&) = delete;
+    ChannelHandle& operator=(ChannelHandle&&) = delete;
+    ChannelHandle& operator=(const ChannelHandle&) = delete;
 
     int m_iHandle;
 
@@ -86,20 +92,20 @@ inline qhash_seed_t qHash(
 // custom equality and hash methods that save the cost of touching the QString.
 class ChannelHandleAndGroup {
   public:
-    ChannelHandleAndGroup(const ChannelHandle& handle, const QString& name)
-            : m_handle(handle),
+    ChannelHandleAndGroup(const ChannelHandle* pHandle, const QString& name)
+            : m_pHandle(pHandle),
               m_name(name) {
     }
 
-    inline const QString& name() const {
+    const QString& name() const {
         return m_name;
     }
 
-    inline const ChannelHandle& handle() const {
-        return m_handle;
+    const ChannelHandle* handle() const {
+        return m_pHandle;
     }
 
-    const ChannelHandle m_handle;
+    const ChannelHandle* m_pHandle;
     const QString m_name;
 };
 
@@ -127,78 +133,107 @@ inline qhash_seed_t qHash(
 // objects are not compatible and will produce incorrect results when compared,
 // stored in the same container, etc. In practice we only use one instance in
 // EngineMaster.
-class ChannelHandleFactory {
+class ChannelHandleFactory final {
   public:
-    ChannelHandleFactory() : m_iNextHandle(0) {
-    }
-
-    ChannelHandle getOrCreateHandle(const QString& group) {
-        ChannelHandle& handle = m_groupToHandle[group];
-        if (!handle.valid()) {
-            handle.setHandle(m_iNextHandle++);
-            DEBUG_ASSERT(handle.valid());
-            DEBUG_ASSERT(!m_handleToGroup.contains(handle));
-            m_handleToGroup.insert(handle, group);
+    ChannelHandleFactory() = default;
+
+    /// Obtain a a channel handle for a group
+    const ChannelHandle* getOrCreateHandle(const QString& group) {
+        auto groupToHandleIter = m_groupToHandle.find(group);
+        if (groupToHandleIter == m_groupToHandle.end()) {
+            groupToHandleIter = m_groupToHandle.insert(group, m_groupToHandle.size());
+            DEBUG_ASSERT(!m_handleToGroup.contains(groupToHandleIter.value()));
+            m_handleToGroup.insert(groupToHandleIter.value(), group);
+        }
+        DEBUG_ASSERT(groupToHandleIter != m_groupToHandle.end());
+        // Return reference from the map that is pinned to a fixed
+        // memory address!!!
+        const auto handleIndex = groupToHandleIter.value();
+        DEBUG_ASSERT(handleIndex >= 0);
+        DEBUG_ASSERT(handleIndex < static_cast<int>(std::size(m_groupHandles)));
+        if (!m_groupHandles[handleIndex].valid()) {
+            m_groupHandles[handleIndex].m_iHandle = handleIndex;
+            qInfo() << "Created" << m_groupHandles[handleIndex] << "for group" << group;
         }
-        return handle;
+        DEBUG_ASSERT(m_groupHandles[handleIndex].m_iHandle == handleIndex);
+        return &m_groupHandles[handleIndex];
     }
 
-    ChannelHandle handleForGroup(const QString& group) const {
-        return m_groupToHandle.value(group, ChannelHandle());
+    const ChannelHandle* handleForGroup(const QString& group) const {
+        auto groupToHandleIter = m_groupToHandle.find(group);
+        if (groupToHandleIter == m_groupToHandle.end()) {
+            return nullptr;
+        }
+        // Return reference from the map that is pinned to a fixed
+        // memory address!!!
+        const auto handleIndex = groupToHandleIter.value();
+        DEBUG_ASSERT(handleIndex >= 0);
+        DEBUG_ASSERT(handleIndex < static_cast<int>(std::size(m_groupHandles)));
+        return &m_groupHandles[handleIndex];
     }
 
-    QString groupForHandle(const ChannelHandle& handle) const {
-        return m_handleToGroup.value(handle, QString());
+    QString groupForHandle(const ChannelHandle* pHandle) const {
+        VERIFY_OR_DEBUG_ASSERT(pHandle) {
+            return {};
+        }
+        return m_handleToGroup.value(pHandle->handle());
     }
 
   private:
-    int m_iNextHandle;
-    QHash<QString, ChannelHandle> m_groupToHandle;
-    QHash<ChannelHandle, QString> m_handleToGroup;
+    ChannelHandleFactory(ChannelHandleFactory&&) = delete;
+    ChannelHandleFactory(const ChannelHandleFactory&) = delete;
+    ChannelHandleFactory& operator=(ChannelHandleFactory&&) = delete;
+    ChannelHandleFactory& operator=(const ChannelHandleFactory&) = delete;
+
+    ChannelHandle m_groupHandles[kMaxExpectedChannelGroups];
+    QHash<QString, int> m_groupToHandle;
+    QHash<int, QString> m_handleToGroup;
 };
 
 typedef std::shared_ptr<ChannelHandleFactory> ChannelHandleFactoryPointer;
 
 // An associative container mapping ChannelHandle to a template type T. Backed
-// by a QVarLengthArray with ChannelHandleMap::kMaxExpectedGroups pre-allocated
-// entries. Insertions are amortized O(1) time (if less than kMaxExpectedGroups
+// by a QVarLengthArray with ChannelHandleMap::kMaxExpectedChannelGroups pre-allocated
+// entries. Insertions are amortized O(1) time (if less than kMaxExpectedChannelGroups
 // exist then no allocation will occur -- insertion is a mere copy). Lookups are
 // O(1) and quite fast -- a simple index into an array using the handle's
 // integer value.
 template <class T>
 class ChannelHandleMap {
-    static constexpr int kMaxExpectedGroups = 256;
-    typedef QVarLengthArray<T, kMaxExpectedGroups> container_type;
+    typedef QVarLengthArray<T, kMaxExpectedChannelGroups> container_type;
+
   public:
-    typedef typename QVarLengthArray<T, kMaxExpectedGroups>::const_iterator const_iterator;
-    typedef typename QVarLengthArray<T, kMaxExpectedGroups>::iterator iterator;
+    typedef typename QVarLengthArray<T, kMaxExpectedChannelGroups>::const_iterator const_iterator;
+    typedef typename QVarLengthArray<T, kMaxExpectedChannelGroups>::iterator iterator;
 
     ChannelHandleMap()
             : m_dummy{} {
     }
 
-    const T& at(const ChannelHandle& handle) const {
-        if (!handle.valid()) {
+    const T& at(const ChannelHandle* pHandle) const {
+        if (!pHandle) {
             return m_dummy;
         }
-        return m_data.at(handle.handle());
+        DEBUG_ASSERT(pHandle->valid());
+        return m_data.at(pHandle->handle());
     }
 
-    void insert(const ChannelHandle& handle, const T& value) {
-        if (!handle.valid()) {
+    void insert(const ChannelHandle* pHandle, const T& value) {
+        if (!pHandle) {
             return;
         }
-
-        int iHandle = handle.handle();
+        DEBUG_ASSERT(pHandle->valid());
+        int iHandle = pHandle->handle();
         maybeExpand(iHandle + 1);
         m_data[iHandle] = value;
     }
 
-    T& operator[](const ChannelHandle& handle) {
-        if (!handle.valid()) {
+    T& operator[](const ChannelHandle* pHandle) {
+        if (!pHandle) {
             return m_dummy;
         }
-        int iHandle = handle.handle();
+        DEBUG_ASSERT(pHandle->valid());
+        int iHandle = pHandle->handle();
         maybeExpand(iHandle + 1);
         return m_data[iHandle];
     }

src/engine/channelmixer.cpp

@@ -8,7 +8,7 @@ void ChannelMixer::applyEffectsAndMixChannels(const EngineMaster::GainCalculator
         const QVarLengthArray<EngineMaster::ChannelInfo*, kPreallocatedChannels>& activeChannels,
         QVarLengthArray<EngineMaster::GainCache, kPreallocatedChannels>* channelGainCache,
         CSAMPLE* pOutput,
-        const ChannelHandle& outputHandle,
+        const ChannelHandle* pOutputHandle,
         unsigned int iBufferSize,
         unsigned int iSampleRate,
         EngineEffectsManager* pEngineEffectsManager) {
@@ -34,8 +34,10 @@ void ChannelMixer::applyEffectsAndMixChannels(const EngineMaster::GainCalculator
             newGain = gainCalculator.getGain(pChannelInfo);
         }
         gainCache.m_gain = newGain;
-        pEngineEffectsManager->processPostFaderAndMix(pChannelInfo->m_handle,
-                outputHandle,
+        DEBUG_ASSERT(pChannelInfo->m_pHandle);
+        pEngineEffectsManager->processPostFaderAndMix(
+                pChannelInfo->m_pHandle,
+                pOutputHandle,
                 pChannelInfo->m_pBuffer,
                 pOutput,
                 iBufferSize,
@@ -53,7 +55,7 @@ void ChannelMixer::applyEffectsInPlaceAndMixChannels(
         QVarLengthArray<EngineMaster::GainCache, kPreallocatedChannels>*
                 channelGainCache,
         CSAMPLE* pOutput,
-        const ChannelHandle& outputHandle,
+        const ChannelHandle* pOutputHandle,
         unsigned int iBufferSize,
         unsigned int iSampleRate,
         EngineEffectsManager* pEngineEffectsManager) {
@@ -76,8 +78,10 @@ void ChannelMixer::applyEffectsInPlaceAndMixChannels(
             newGain = gainCalculator.getGain(pChannelInfo);
         }
         gainCache.m_gain = newGain;
-        pEngineEffectsManager->processPostFaderInPlace(pChannelInfo->m_handle,
-                outputHandle,
+        DEBUG_ASSERT(pChannelInfo->m_pHandle);
+        pEngineEffectsManager->processPostFaderInPlace(
+                pChannelInfo->m_pHandle,
+                pOutputHandle,
                 pChannelInfo->m_pBuffer,
                 iBufferSize,
                 iSampleRate,