(fix) FX units not working on all samplers - #15971
Conversation
| auto& outputMap = m_chainStatusForChannelMatrix[inputHandle]; | ||
|
|
||
| if (outputMap.isEmpty()) { | ||
| // Apparently a request to enable an unregistered input. | ||
| // ChannelHandleMap's operator[] does maybeExpand(), so we now have an | ||
| // inputHandle key and we can assign our outputmap to it. | ||
| // Now request the map reference again and we're ready to roll... | ||
| m_chainStatusForChannelMatrix[inputHandle] = m_outputChannelMap; | ||
| outputMap = m_chainStatusForChannelMatrix[inputHandle]; | ||
| } |
There was a problem hiding this comment.
I prefer to avoid this pattern where we create a new variable, and then check to see ok, and then fix it if not. This creates an in-between state where the variable is invalid, and later down the line someone may accidentally put code in between.
By reordering this a little, we can do the assignment once and outputMap is always valid. It does technically do two lookups, but this map lookup is extremely fast, the map is small, and this function is not in a hot path.
| auto& outputMap = m_chainStatusForChannelMatrix[inputHandle]; | |
| if (outputMap.isEmpty()) { | |
| // Apparently a request to enable an unregistered input. | |
| // ChannelHandleMap's operator[] does maybeExpand(), so we now have an | |
| // inputHandle key and we can assign our outputmap to it. | |
| // Now request the map reference again and we're ready to roll... | |
| m_chainStatusForChannelMatrix[inputHandle] = m_outputChannelMap; | |
| outputMap = m_chainStatusForChannelMatrix[inputHandle]; | |
| } | |
| if (m_chainStatusForChannelMatrix[inputHandle].isEmpty()) { | |
| // Apparently a request to enable an unregistered input. | |
| // ChannelHandleMap's operator[] does maybeExpand(), so we now have an | |
| // inputHandle key and we can assign our outputmap to it. | |
| // Now request the map reference again and we're ready to roll... | |
| m_chainStatusForChannelMatrix[inputHandle] = m_outputChannelMap; | |
| } | |
| auto& outputMap = m_chainStatusForChannelMatrix[inputHandle]; |
cafc88e to
421a665
Compare
|
I tested the build artifact on Windows 11 x64. I confirmed these steps work. However I will note that when I first reloaded Mixxx, FX1 was not enabled on sampler 5. I believe that I had enabled it and left it enabled before quitting but I'm not 100% sure.
I didnt confirm the pre-fix (bugged) case. |
|
Thanks for testing!
That is on purpose so you can start playback whitout any potentially hidden effects being enabled. |
|
@ywwg You approved, let's merge? |
Fixes #15799
-> this PR: enjoy the noise : )
-> 2.5: no echo. toggle Fx1 Off and On before engaging Echo & Co, then it works.
The root cause:
short: the fx I/O routing table is not updated instantly when additional samplers are added to the engine (or any inputs actually).
long:
EffectsManagerm_pEffectsManager->setup()which initializes all EngineEffectChains with the registred inputs/outputsEffectsManager, then in each (standard) effect chain and the Fx routing CO states are also restored correctly (see GUI, FX1 on sampler 5)-> BUT the I/O maps used by
EngineEffectChains are not updated, hence engaging an effect has no ..effect for those new inputs-> the I/O maps are extended for the new key, but
ChannelHandleMap::maybeExpandabviously can't initialize the outputs mapThe fix:
EffectsManager::setup()-- afaict)ChannelHandleMap[input]as before but then assign the stored output mapMaybe alternatively we could add a method
EngineEffectChain::registerInputChanneland use a newEffectsRequest::MessageTypeto send the request via the message pipe.But I doubt it has any benefits over the current fix. Does it?