Merge pull request #4672 from ywwg/channelhandle-memory

Copy channelhandle by value instead of pointers
mixxxdj · Feb 14, 2022 · 69bdba5 · 69bdba5
2 parents b519aba + 8bad0e3
commit 69bdba5
Show file tree

Hide file tree

Showing 10 changed files with 36 additions and 46 deletions.
diff --git a/src/effects/backends/effectprocessor.h b/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(ChannelHandle inputChannel) = 0;
 
     // Called from the audio thread
-    virtual bool loadStatesForInputChannel(const ChannelHandle* inputChannel,
+    virtual bool loadStatesForInputChannel(ChannelHandle inputChannel,
             const EffectStatesMap* pStatesMap) = 0;
 
     /// Called from the audio thread
@@ -202,11 +202,11 @@ class EffectProcessorImpl : public EffectProcessor {
         return createSpecificState(engineParameters);
     };
 
-    bool loadStatesForInputChannel(const ChannelHandle* inputChannel,
+    bool loadStatesForInputChannel(ChannelHandle inputChannel,
             const EffectStatesMap* pStatesMap) final {
         if (kEffectDebugOutput) {
             qDebug() << "EffectProcessorImpl::loadStatesForInputChannel" << this
-                     << "input" << *inputChannel;
+                     << "input" << inputChannel;
         }
 
         // 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[inputChannel];
 
         // 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(ChannelHandle inputChannel) final {
         if (kEffectDebugOutput) {
             qDebug() << "EffectProcessorImpl::deleteStatesForInputChannel"
-                     << this << *inputChannel;
+                     << this << inputChannel;
         }
 
         // 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[inputChannel];
         for (EffectSpecificState* pState : stateMap) {
             VERIFY_OR_DEBUG_ASSERT(pState != nullptr) {
                 continue;

diff --git a/src/effects/effectchain.cpp b/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.channelHandle = 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.channelHandle = handleGroup.handle();
     m_pMessenger->writeRequest(request);
 }
 

diff --git a/src/effects/effectsmessenger.cpp b/src/effects/effectsmessenger.cpp
@@ -93,10 +93,10 @@ void EffectsMessenger::collectGarbage(const EffectsRequest* pRequest) {
     } else if (pRequest->type == EffectsRequest::DISABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL) {
         if (kEffectDebugOutput) {
             qDebug() << debugString() << "deleting states for input channel"
-                     << pRequest->DisableInputChannelForChain.pChannelHandle
+                     << pRequest->DisableInputChannelForChain.channelHandle
                      << "for EngineEffectChain" << pRequest->pTargetChain;
         }
         pRequest->pTargetChain->deleteStatesForInputChannel(
-                pRequest->DisableInputChannelForChain.pChannelHandle);
+                pRequest->DisableInputChannelForChain.channelHandle);
     }
 }
diff --git a/src/engine/channelhandle.h b/src/engine/channelhandle.h
@@ -95,7 +95,7 @@ class ChannelHandleAndGroup {
         return m_name;
     }
 
-    inline const ChannelHandle& handle() const {
+    inline ChannelHandle handle() const {
         return m_handle;
     }
 

diff --git a/src/engine/channels/enginechannel.h b/src/engine/channels/enginechannel.h
@@ -30,7 +30,7 @@ class EngineChannel : public EngineObject {
 
     virtual ChannelOrientation getOrientation() const;
 
-    inline const ChannelHandle& getHandle() const {
+    inline ChannelHandle getHandle() const {
         return m_group.handle();
     }
 

diff --git a/src/engine/effects/engineeffect.cpp b/src/engine/effects/engineeffect.cpp
@@ -53,16 +53,16 @@ EffectState* EngineEffect::createState(const mixxx::EngineParameters& enginePara
     return m_pProcessor->createState(engineParameters);
 }
 
-void EngineEffect::loadStatesForInputChannel(const ChannelHandle* inputChannel,
+void EngineEffect::loadStatesForInputChannel(ChannelHandle inputChannel,
         EffectStatesMap* pStatesMap) {
     if (kEffectDebugOutput) {
         qDebug() << "EngineEffect::loadStatesForInputChannel" << this
-                 << "loading states for input" << *inputChannel;
+                 << "loading states for input" << inputChannel;
     }
     m_pProcessor->loadStatesForInputChannel(inputChannel, pStatesMap);
 }
 
-void EngineEffect::deleteStatesForInputChannel(const ChannelHandle* inputChannel) {
+void EngineEffect::deleteStatesForInputChannel(ChannelHandle inputChannel) {
     m_pProcessor->deleteStatesForInputChannel(inputChannel);
 }
 

diff --git a/src/engine/effects/engineeffect.h b/src/engine/effects/engineeffect.h
@@ -35,10 +35,10 @@ class EngineEffect final : public EffectsRequestHandler {
     EffectState* createState(const mixxx::EngineParameters& engineParameters);
 
     /// Called in audio thread to load EffectStates received from the main thread
-    void loadStatesForInputChannel(const ChannelHandle* inputChannel,
+    void loadStatesForInputChannel(ChannelHandle inputChannel,
             EffectStatesMap* pStatesMap);
     /// Called from the main thread for garbage collection after an input channel is disabled
-    void deleteStatesForInputChannel(const ChannelHandle* inputChannel);
+    void deleteStatesForInputChannel(ChannelHandle inputChannel);
 
     /// Called in audio thread
     bool processEffectsRequest(

diff --git a/src/engine/effects/engineeffectchain.cpp b/src/engine/effects/engineeffectchain.cpp
@@ -123,21 +123,21 @@ bool EngineEffectChain::processEffectsRequest(EffectsRequest& message,
             qDebug() << debugString() << this
                      << "ENABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL"
                      << message.pTargetChain
-                     << *message.EnableInputChannelForChain.pChannelHandle;
+                     << message.EnableInputChannelForChain.channelHandle;
         }
         response.success = enableForInputChannel(
-                message.EnableInputChannelForChain.pChannelHandle,
+                message.EnableInputChannelForChain.channelHandle,
                 message.EnableInputChannelForChain.pEffectStatesMapArray);
         break;
     case EffectsRequest::DISABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL:
         if (kEffectDebugOutput) {
             qDebug() << debugString() << this
                      << "DISABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL"
                      << message.pTargetChain
-                     << *message.DisableInputChannelForChain.pChannelHandle;
+                     << message.DisableInputChannelForChain.channelHandle;
         }
         response.success = disableForInputChannel(
-                message.DisableInputChannelForChain.pChannelHandle);
+                message.DisableInputChannelForChain.channelHandle);
         break;
     default:
         return false;
@@ -146,12 +146,12 @@ bool EngineEffectChain::processEffectsRequest(EffectsRequest& message,
     return true;
 }
 
-bool EngineEffectChain::enableForInputChannel(const ChannelHandle* inputHandle,
+bool EngineEffectChain::enableForInputChannel(ChannelHandle inputHandle,
         EffectStatesMapArray* statesForEffectsInChain) {
     if (kEffectDebugOutput) {
         qDebug() << "EngineEffectChain::enableForInputChannel" << this << inputHandle;
     }
-    auto& outputMap = m_chainStatusForChannelMatrix[*inputHandle];
+    auto& outputMap = m_chainStatusForChannelMatrix[inputHandle];
     for (auto&& outputChannelStatus : outputMap) {
         VERIFY_OR_DEBUG_ASSERT(outputChannelStatus.enableState !=
                 EffectEnableState::Enabled) {
@@ -180,8 +180,8 @@ bool EngineEffectChain::enableForInputChannel(const ChannelHandle* inputHandle,
     return true;
 }
 
-bool EngineEffectChain::disableForInputChannel(const ChannelHandle* inputHandle) {
-    auto& outputMap = m_chainStatusForChannelMatrix[*inputHandle];
+bool EngineEffectChain::disableForInputChannel(ChannelHandle inputHandle) {
+    auto& outputMap = m_chainStatusForChannelMatrix[inputHandle];
     for (auto&& outputChannelStatus : outputMap) {
         if (outputChannelStatus.enableState != EffectEnableState::Disabled) {
             outputChannelStatus.enableState = EffectEnableState::Disabling;
@@ -195,7 +195,7 @@ bool EngineEffectChain::disableForInputChannel(const ChannelHandle* inputHandle)
 }
 
 // Called from the main thread for garbage collection after an input channel is disabled
-void EngineEffectChain::deleteStatesForInputChannel(const ChannelHandle* inputChannel) {
+void EngineEffectChain::deleteStatesForInputChannel(const ChannelHandle inputChannel) {
     // If an output channel is not presently being processed, for example when
     // PFL is not active, then process() cannot be relied upon to set this
     // chain's EffectEnableState from Disabling to Disabled. This must be done
@@ -209,7 +209,7 @@ void EngineEffectChain::deleteStatesForInputChannel(const ChannelHandle* inputCh
     // QMap. So it is okay that m_chainStatusForChannelMatrix may be
     // accessed concurrently in the audio engine thread in process(),
     // enableForInputChannel(), or disableForInputChannel().
-    auto& outputMap = m_chainStatusForChannelMatrix[*inputChannel];
+    auto& outputMap = m_chainStatusForChannelMatrix[inputChannel];
     for (auto&& outputChannelStatus : outputMap) {
         outputChannelStatus.enableState = EffectEnableState::Disabled;
     }

diff --git a/src/engine/effects/engineeffectchain.h b/src/engine/effects/engineeffectchain.h
@@ -45,7 +45,7 @@ class EngineEffectChain final : public EffectsRequestHandler {
             const GroupFeatureState& groupFeatures);
 
     /// called from main thread
-    void deleteStatesForInputChannel(const ChannelHandle* channel);
+    void deleteStatesForInputChannel(const ChannelHandle channel);
 
   private:
     struct ChannelStatus {
@@ -64,9 +64,9 @@ class EngineEffectChain final : public EffectsRequestHandler {
     bool updateParameters(const EffectsRequest& message);
     bool addEffect(EngineEffect* pEffect, int iIndex);
     bool removeEffect(EngineEffect* pEffect, int iIndex);
-    bool enableForInputChannel(const ChannelHandle* inputHandle,
+    bool enableForInputChannel(ChannelHandle inputHandle,
             EffectStatesMapArray* statesForEffectsInChain);
-    bool disableForInputChannel(const ChannelHandle* inputHandle);
+    bool disableForInputChannel(ChannelHandle inputHandle);
 
     // Gets or creates a ChannelStatus entry in m_channelStatus for the provided
     // handle.

diff --git a/src/engine/effects/message.h b/src/engine/effects/message.h
@@ -34,23 +34,14 @@ struct EffectsRequest {
         NUM_REQUEST_TYPES
     };
 
+    // Creates a new uninitialized EffectsRequest.  Callers are responsible for making sure that
+    // they initialize all the values of the struct corresponding to the type they select.
     EffectsRequest()
             : type(NUM_REQUEST_TYPES),
               request_id(-1),
               value(0.0) {
         pTargetChain = nullptr;
         pTargetEffect = nullptr;
-#define CLEAR_STRUCT(x) memset(&x, 0, sizeof(x));
-        CLEAR_STRUCT(AddEffectChain);
-        CLEAR_STRUCT(RemoveEffectChain);
-        CLEAR_STRUCT(EnableInputChannelForChain);
-        CLEAR_STRUCT(DisableInputChannelForChain);
-        CLEAR_STRUCT(AddEffectToChain);
-        CLEAR_STRUCT(RemoveEffectFromChain);
-        CLEAR_STRUCT(SetEffectChainParameters);
-        CLEAR_STRUCT(SetEffectParameters);
-        CLEAR_STRUCT(SetParameterParameters);
-#undef CLEAR_STRUCT
     }
 
     // This is called from the main thread by EffectsManager after receiving a
@@ -92,15 +83,14 @@ struct EffectsRequest {
         } AddEffectChain;
         struct {
             EngineEffectChain* pChain;
-            int iIndex;
             SignalProcessingStage signalProcessingStage;
         } RemoveEffectChain;
         struct {
             EffectStatesMapArray* pEffectStatesMapArray;
-            const ChannelHandle* pChannelHandle;
+            ChannelHandle channelHandle;
         } EnableInputChannelForChain;
         struct {
-            const ChannelHandle* pChannelHandle;
+            ChannelHandle channelHandle;
         } DisableInputChannelForChain;
         struct {
             EngineEffect* pEffect;