Add MIDI Machine Control (MMC) support#32741
Merged
RomanPudashkin merged 16 commits intomusescore:masterfrom Apr 6, 2026
Merged
Add MIDI Machine Control (MMC) support#32741RomanPudashkin merged 16 commits intomusescore:masterfrom
RomanPudashkin merged 16 commits intomusescore:masterfrom
Conversation
RomanPudashkin
force-pushed
the
add_mmc_support
branch
2 times, most recently
from
eaae204 to
c627341
Compare
…plugin window Ensure plugin NSWindow is attached to parent window as a child window
RomanPudashkin
force-pushed
the
add_mmc_support
branch
from
c627341 to
6d6f847
Compare
📝 WalkthroughWalkthroughThis pull request introduces a new MIDI Remote module alongside significant refactoring of existing audio and UI systems. The changes include: creating the ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/framework/audio/common/audiotypes.h`:
- Around line 264-280: The file is missing the header for std::unordered_map
used by AUDIO_FX_CATEGORY_TO_STRING_MAP; add `#include` <unordered_map> near the
other includes in src/framework/audio/common/audiotypes.h so the declaration of
AUDIO_FX_CATEGORY_TO_STRING_MAP (and uses of std::unordered_map) compiles—make
sure the include appears before the inline const
std::unordered_map<AudioFxCategory, String> AUDIO_FX_CATEGORY_TO_STRING_MAP
definition and keep existing type names AudioFxCategory and String unchanged.
In `@src/framework/audio/common/audioutils.h`:
- Around line 100-105: When building AudioFxCategories from the split string in
audioutils.h, skip empty tokens before performing the muse::key lookup and
inserting into result: inside the loop iterating over StringList list (variable
name list and loop variable name), add a guard like if (name.empty()) continue;
before calling muse::key(AUDIO_FX_CATEGORY_TO_STRING_MAP, name,
AudioFxCategory::FxOther) and result.insert, so empty tokens do not default to
AudioFxCategory::FxOther.
In `@src/framework/audio/engine/internal/mixer.cpp`:
- Around line 410-413: The paramsChanged handler currently updates
m_masterParams.fxChain but doesn't update the cached
m_shouldProcessMasterFxDuringSilence, so disabling a previously
silence-processing FX leaves processing on; modify the lambda used in
fx->paramsChanged().onReceive (the handler that inserts into
m_masterParams.fxChain and sends m_masterOutputParamsChanged) to recompute
m_shouldProcessMasterFxDuringSilence after insert_or_assign by iterating the
entries in m_masterParams.fxChain and setting the flag true only if any active
FX processor (check the FX objects' active() state or the equivalent in
AudioFxParams) indicates it should process during silence; then emit
m_masterOutputParamsChanged as before.
In `@src/framework/audio/engine/internal/tracksequence.cpp`:
- Around line 257-263: The code currently removes the idle-processing ID only
from the local m_tracksToProcessWhenIdle, but Mixer still retains that ID and
can reassign it to a new track; instead delegate the idle-removal to Mixer so
both sides stay in sync: replace the local removal
(muse::remove(m_tracksToProcessWhenIdle, id)) with a call into Mixer (e.g. add
and call mixer()->removeIdleProcessingForChannel(id) or similar) and implement
that method in Mixer to remove the ID from Mixer’s idle list; keep the existing
mixer()->removeChannel(id), m_tracks.erase(id) and m_prevActiveTrackId =
INVALID_TRACK_ID logic unchanged.
In `@src/framework/midi/internal/platform/win/winmidiinport.cpp`:
- Around line 261-268: In run(), after preparing the SysEx buffer
(m_win->buffer, m_win->header) call and check the return values of
midiInPrepareHeader, midiInAddBuffer, and midiInStart; only set m_running = true
if all three succeed and log/handle errors otherwise. In stop(), symmetrically
call midiInStop and check its return value, then call midiInUnprepareHeader for
m_win->header (only if it was prepared), handling errors; ensure the header is
unprepared on every stop so subsequent run() calls can safely
midiInPrepareHeader again. Update disconnect() logic only if needed to avoid
double-unprepare, and use m_running (or a dedicated flag) to track prepared
state so you don't unprepare an unprepared header. Ensure all MIDI API return
codes are validated before changing state.
- Around line 192-194: The error message uses the wrong variable: replace the
use of ret with openRes when calling wmidi_prv::errorString in the error path
inside winmidiinport.cpp so the actual MMRESULT from midiInOpen() is reported;
update the call that currently builds the Err::MidiFailedConnect return via
make_ret(...) to pass wmidi_prv::errorString(openRes) instead of
wmidi_prv::errorString(ret).
In `@src/framework/midi/midievent.h`:
- Around line 180-184: The current SysEx trimming in midievent.h incorrectly
strips 0xF0/0xF7 only when both delimiters are inside the same buffer (the block
that mutates data and size), which causes partial SysEx fragments to lose
delimiter bytes—either reassemble full SysEx messages before calling this helper
or change the API to carry explicit start/end metadata; implement one of these
fixes by (a) moving delimiter-trimming out of this helper and reassembling
across MIM_LONGDATA callbacks in the caller, or (b) extending the helper's
signature to accept flags like isStart/isEnd (or a fragment state object) and
only strip 0xF0 when isStart is true and only strip 0xF7 when isEnd is true,
referencing the current variables data and size in midievent.h to locate and
adjust the trimming logic.
In `@src/framework/midiremote/internal/midiremote.cpp`:
- Around line 120-126: The handler currently returns make_ret(Ret::Code::Ok) for
any SystemExclusiveData event even when m_mmcParser.process(ev) returns no
value, which marks unparsed/malformed SysEx as handled; change the logic in the
SysEx branch so that you only call processMMC(msg.value()) and return
make_ret(Ret::Code::Ok) when m_mmcParser.process(ev) yields a value
(msg.has_value()), otherwise do not return here (let the function fall
through/unhandled) so other consumers can see the event; apply the same fix to
the equivalent SysEx handling block that calls m_mmcParser.process and returns
Ok (the one around processMMC usage mentioned in the comment).
- Around line 249-257: The release detection misses NoteOn events with velocity
0 (many controllers send key-up as NoteOn velocity 0), so update the logic
around Event::Opcode opcode = event.opcode() and the release boolean to treat a
NoteOn with event.data() == 0 as a release as well; specifically, keep allowing
NoteOn/NoteOff/ControlChange, but change the release computation (the variable
release) to be true when opcode == Event::Opcode::NoteOff OR (opcode ==
Event::Opcode::NoteOn && event.data() == 0) OR (opcode ==
Event::Opcode::ControlChange && event.data() == 0) so that NoteOn zero-velocity
is handled like a release and mapped actions won’t fire on key-up.
In `@src/framework/midiremote/internal/midiremote.h`:
- Around line 80-88: The declaration uses the fully qualified type
midiremote::MMCParser inside the muse::midiremote namespace; replace
midiremote::MMCParser with just MMCParser for consistency and to remove the
redundant namespace qualifier, updating the member declaration m_mmcParser and
any related references (e.g., processMMC signature remains the same) so the code
relies on the local namespace type.
In `@src/framework/midiremote/mmc.cpp`:
- Around line 185-189: Replace the manual new/delete ownership of m_assembler
with a std::unique_ptr to make ownership explicit: change the member declaration
of m_assembler in mmc.h to std::unique_ptr<SysExAssembler>, update construction
sites that currently assign via new to use std::make_unique<SysExAssembler>(...)
(or construct into the unique_ptr), and remove the explicit delete in
MMCParser::~MMCParser() (the destructor can be defaulted). Ensure any
copies/moves of MMCParser are adjusted (delete copy or default move) to respect
unique ownership.
In `@src/framework/vst/internal/fx/vstfxprocessor.cpp`:
- Around line 30-31: The constructor VstFxProcessor currently initializes
m_pluginPtr by copying the by-value parameter instance which causes an extra
shared_ptr refcount increment; change the initialization to move the parameter
(use std::move(instance)) when initializing m_pluginPtr in the
VstFxProcessor::VstFxProcessor constructor to avoid the redundant copy and
refcount operation while keeping semantic correctness.
In `@src/framework/vst/internal/vstaudioclient.cpp`:
- Around line 511-514: Before calling processor->process(m_processData) when
m_needUpdateState is true (e.g., in disableActivity()), call flushBuffers() to
zero/clear the audio buffers so the final process() doesn't reuse previous block
data; specifically, invoke flushBuffers() immediately before
processor->process(m_processData) (retain setting m_needUpdateState = false
afterward) to avoid double-processing identical audio data and state mutation.
In `@src/project/internal/projectactionscontroller.cpp`:
- Around line 727-729: The call to interactive()->closeAllDialogsSync() returns
a Ret status that must be checked before clearing the project; modify the block
handling result to capture and inspect that Ret, and only call
globalContext()->setCurrentProject(nullptr) when closeAllDialogsSync() succeeds
(or otherwise log/propagate the failure), mirroring the pattern used for
openPageIfNeed(); specifically update the code around closeAllDialogsSync() so
it assigns its return to a variable, checks isOk()/isFailure() (or equivalent),
and conditionally clears the project or logs the error and aborts the clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eef40bc2-2214-4395-86ab-0205796206f9
📒 Files selected for processing (109)
SetupConfigure.cmakesrc/app/CMakeLists.txtsrc/app/appfactory.cppsrc/framework/CMakeLists.txtsrc/framework/audio/common/audiotypes.hsrc/framework/audio/common/audioutils.hsrc/framework/audio/engine/ifxprocessor.hsrc/framework/audio/engine/ifxresolver.hsrc/framework/audio/engine/internal/fx/abstractfxresolver.cppsrc/framework/audio/engine/internal/fx/abstractfxresolver.hsrc/framework/audio/engine/internal/fx/equaliser.cppsrc/framework/audio/engine/internal/fx/equaliser.hsrc/framework/audio/engine/internal/fx/reverb/reverbprocessor.cppsrc/framework/audio/engine/internal/fx/reverb/reverbprocessor.hsrc/framework/audio/engine/internal/igetplaybackposition.hsrc/framework/audio/engine/internal/mixer.cppsrc/framework/audio/engine/internal/mixer.hsrc/framework/audio/engine/internal/mixerchannel.cppsrc/framework/audio/engine/internal/mixerchannel.hsrc/framework/audio/engine/internal/tracksequence.cppsrc/framework/audio/engine/internal/tracksequence.hsrc/framework/autobot/internal/autobotinteractive.cppsrc/framework/autobot/internal/autobotinteractive.hsrc/framework/cmake/MuseDeclareOptions.cmakesrc/framework/cmake/MuseModules.cmakesrc/framework/cmake/muse_framework_config.h.insrc/framework/extensions/qml/Muse/Extensions/ExtensionViewerDialog.qmlsrc/framework/interactive/iinteractive.hsrc/framework/interactive/internal/interactive.cppsrc/framework/interactive/internal/interactive.hsrc/framework/interactive/tests/mocks/interactivemock.hsrc/framework/midi/CMakeLists.txtsrc/framework/midi/internal/platform/win/winmidiinport.cppsrc/framework/midi/internal/platform/win/winmidiinport.hsrc/framework/midi/midievent.hsrc/framework/midi/tests/CMakeLists.txtsrc/framework/midi/tests/midievent_tests.cppsrc/framework/midiremote/CMakeLists.txtsrc/framework/midiremote/imidiremote.hsrc/framework/midiremote/imidiremoteconfiguration.hsrc/framework/midiremote/internal/midiremote.cppsrc/framework/midiremote/internal/midiremote.hsrc/framework/midiremote/internal/midiremoteconfiguration.cppsrc/framework/midiremote/internal/midiremoteconfiguration.hsrc/framework/midiremote/midiremotemodule.cppsrc/framework/midiremote/midiremotemodule.hsrc/framework/midiremote/midiremotetypes.hsrc/framework/midiremote/mmc.cppsrc/framework/midiremote/mmc.hsrc/framework/midiremote/qml/Muse/MidiRemote/CMakeLists.txtsrc/framework/midiremote/qml/Muse/MidiRemote/EditMidiMappingDialog.qmlsrc/framework/midiremote/qml/Muse/MidiRemote/MidiDeviceMappingPage.qmlsrc/framework/midiremote/qml/Muse/MidiRemote/editmidimappingmodel.cppsrc/framework/midiremote/qml/Muse/MidiRemote/editmidimappingmodel.hsrc/framework/midiremote/qml/Muse/MidiRemote/internal/MidiMappingBottomPanel.qmlsrc/framework/midiremote/qml/Muse/MidiRemote/internal/MidiMappingTopPanel.qmlsrc/framework/midiremote/qml/Muse/MidiRemote/mididevicemappingmodel.cppsrc/framework/midiremote/qml/Muse/MidiRemote/mididevicemappingmodel.hsrc/framework/midiremote/tests/CMakeLists.txtsrc/framework/midiremote/tests/mmc_tests.cppsrc/framework/shortcuts/CMakeLists.txtsrc/framework/shortcuts/internal/shortcutsconfiguration.cppsrc/framework/shortcuts/internal/shortcutsconfiguration.hsrc/framework/shortcuts/ishortcutsconfiguration.hsrc/framework/shortcuts/qml/Muse/Shortcuts/CMakeLists.txtsrc/framework/shortcuts/shortcutsmodule.cppsrc/framework/shortcuts/shortcutsmodule.hsrc/framework/shortcuts/shortcutstypes.hsrc/framework/stubs/CMakeLists.txtsrc/framework/stubs/midiremote/CMakeLists.txtsrc/framework/stubs/midiremote/midiremoteconfigurationstub.cppsrc/framework/stubs/midiremote/midiremoteconfigurationstub.hsrc/framework/stubs/midiremote/midiremotestub.cppsrc/framework/stubs/midiremote/midiremotestub.hsrc/framework/stubs/midiremote/midiremotestubmodule.cppsrc/framework/stubs/midiremote/midiremotestubmodule.hsrc/framework/stubs/shortcuts/CMakeLists.txtsrc/framework/stubs/shortcuts/shortcutsconfigurationstub.cppsrc/framework/stubs/shortcuts/shortcutsconfigurationstub.hsrc/framework/stubs/shortcuts/shortcutsstubmodule.cppsrc/framework/uicomponents/qml/Muse/UiComponents/CMakeLists.txtsrc/framework/uicomponents/qml/Muse/UiComponents/dialogview.cppsrc/framework/uicomponents/qml/Muse/UiComponents/dialogview.hsrc/framework/uicomponents/qml/Muse/UiComponents/internal/platform/macos/macoschildwindowcontroller.hsrc/framework/uicomponents/qml/Muse/UiComponents/internal/platform/macos/macoschildwindowcontroller.mmsrc/framework/uicomponents/qml/Muse/UiComponents/windowview.cppsrc/framework/uicomponents/qml/Muse/UiComponents/windowview.hsrc/framework/vst/internal/fx/vstfxprocessor.cppsrc/framework/vst/internal/fx/vstfxprocessor.hsrc/framework/vst/internal/fx/vstfxresolver.cppsrc/framework/vst/internal/fx/vstfxresolver.hsrc/framework/vst/internal/synth/vstsynthesiser.cppsrc/framework/vst/internal/synth/vstsynthesiser.hsrc/framework/vst/internal/vstactionscontroller.cppsrc/framework/vst/internal/vstactionscontroller.hsrc/framework/vst/internal/vstaudioclient.cppsrc/framework/vst/internal/vstaudioclient.hsrc/framework/vst/qml/Muse/Vst/VstEditorDialog.qmlsrc/notationscene/internal/midiinputoutputcontroller.hsrc/playback/internal/playbackcontroller.cppsrc/playback/internal/playbackuiactions.cppsrc/playback/qml/MuseScore/Playback/mixerchannelitem.cppsrc/playback/qml/MuseScore/Playback/outputresourceitem.cppsrc/preferences/qml/MuseScore/Preferences/MidiDeviceMappingPreferencesPage.qmlsrc/preferences/qml/MuseScore/Preferences/noteinputpreferencesmodel.cppsrc/preferences/qml/MuseScore/Preferences/noteinputpreferencesmodel.hsrc/preferences/qml/MuseScore/Preferences/preferencesmodel.cppsrc/project/internal/projectactionscontroller.cppsrc/project/internal/projectaudiosettings.cpp
💤 Files with no reviewable changes (8)
- src/framework/stubs/shortcuts/CMakeLists.txt
- src/framework/shortcuts/CMakeLists.txt
- src/framework/stubs/shortcuts/shortcutsconfigurationstub.cpp
- src/framework/shortcuts/internal/shortcutsconfiguration.h
- src/framework/shortcuts/qml/Muse/Shortcuts/CMakeLists.txt
- src/framework/shortcuts/internal/shortcutsconfiguration.cpp
- src/framework/shortcuts/shortcutsmodule.cpp
- src/framework/stubs/shortcuts/shortcutsstubmodule.cpp
Comment on lines
+264
to
280
| inline const std::unordered_map<AudioFxCategory, String> AUDIO_FX_CATEGORY_TO_STRING_MAP { | ||
| { AudioFxCategory::FxEqualizer, u"EQ" }, | ||
| { AudioFxCategory::FxAnalyzer, u"Analyzer" }, | ||
| { AudioFxCategory::FxDelay, u"Delay" }, | ||
| { AudioFxCategory::FxDistortion, u"Distortion" }, | ||
| { AudioFxCategory::FxDynamics, u"Dynamics" }, | ||
| { AudioFxCategory::FxFilter, u"Filter" }, | ||
| { AudioFxCategory::FxGenerator, u"Generator" }, | ||
| { AudioFxCategory::FxMastering, u"Mastering" }, | ||
| { AudioFxCategory::FxModulation, u"Modulation" }, | ||
| { AudioFxCategory::FxPitchShift, u"Pitch Shift" }, | ||
| { AudioFxCategory::FxRestoration, u"Restoration" }, | ||
| { AudioFxCategory::FxReverb, u"Reverb" }, | ||
| { AudioFxCategory::FxSurround, u"Surround" }, | ||
| { AudioFxCategory::FxTools, u"Tools" }, | ||
| { AudioFxCategory::FxOther, u"Fx" }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '(^#include <unordered_map>|std::unordered_map<|AUDIO_FX_CATEGORY_TO_STRING_MAP)' src/framework/audio/common/audiotypes.hRepository: musescore/MuseScore
Length of output: 157
🏁 Script executed:
head -50 src/framework/audio/common/audiotypes.h | cat -nRepository: musescore/MuseScore
Length of output: 1914
🏁 Script executed:
grep -n "unordered_map" src/framework/audio/common/audiotypes.hRepository: musescore/MuseScore
Length of output: 157
🏁 Script executed:
rg -l "unordered_map" --type-list | head -20
# Search for unordered_map in header files that might be included
rg "unordered_map" src/framework/global/types/ src/framework/global/async/ src/framework/global/io/ 2>/dev/null | head -20Repository: musescore/MuseScore
Length of output: 635
🏁 Script executed:
for header in "global/types/number.h" "global/types/secs.h" "global/types/ratio.h" "global/types/string.h" "global/types/ret.h" "global/realfn.h" "global/async/channel.h" "global/io/iodevice.h" "mpe/events.h" "log.h"; do
echo "=== Checking $header ==="
grep -l "unordered_map" "src/framework/$header" 2>/dev/null && echo "Found unordered_map" || echo "No unordered_map"
doneRepository: musescore/MuseScore
Length of output: 585
Add missing #include <unordered_map> header.
Line 264 declares std::unordered_map<AudioFxCategory, String> AUDIO_FX_CATEGORY_TO_STRING_MAP, but the file does not include <unordered_map>. None of the currently included headers provide this type, so the code will fail to compile.
Proposed fix
`#include` <variant>
`#include` <set>
`#include` <string>
+#include <unordered_map>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/framework/audio/common/audiotypes.h` around lines 264 - 280, The file is
missing the header for std::unordered_map used by
AUDIO_FX_CATEGORY_TO_STRING_MAP; add `#include` <unordered_map> near the other
includes in src/framework/audio/common/audiotypes.h so the declaration of
AUDIO_FX_CATEGORY_TO_STRING_MAP (and uses of std::unordered_map) compiles—make
sure the include appears before the inline const
std::unordered_map<AudioFxCategory, String> AUDIO_FX_CATEGORY_TO_STRING_MAP
definition and keep existing type names AudioFxCategory and String unchanged.
Comment on lines
410
to
+413
| fx->paramsChanged().onReceive(this, [this](const AudioFxParams& fxParams) { | ||
| m_masterParams.fxChain.insert_or_assign(fxParams.chainOrder, fxParams); | ||
| m_masterOutputParamsChanged.send(m_masterParams); | ||
| }); | ||
| }, async::Asyncable::Mode::SetReplace); |
There was a problem hiding this comment.
Keep m_shouldProcessMasterFxDuringSilence in sync with active FX.
The cache is computed once here and ignores fx->active(). If the last silence-processing master FX is later disabled via paramsChanged(), process() keeps running silent blocks because this flag stays true. Recompute it in the handler and base it on active processors only.
Suggested fix
+ auto updateShouldProcessMasterFxDuringSilence = [this]() {
+ m_shouldProcessMasterFxDuringSilence = std::any_of(
+ m_masterFxProcessors.cbegin(),
+ m_masterFxProcessors.cend(),
+ [](const IFxProcessorPtr& fx) {
+ return fx->active() && fx->shouldProcessDuringSilence();
+ });
+ };
+
for (IFxProcessorPtr& fx : m_masterFxProcessors) {
fx->setOutputSpec(m_outputSpec);
fx->setPlaying(m_isActive);
fx->paramsChanged().onReceive(this, [this](const AudioFxParams& fxParams) {
m_masterParams.fxChain.insert_or_assign(fxParams.chainOrder, fxParams);
+ updateShouldProcessMasterFxDuringSilence();
m_masterOutputParamsChanged.send(m_masterParams);
}, async::Asyncable::Mode::SetReplace);
}
@@
- m_shouldProcessMasterFxDuringSilence = std::any_of(m_masterFxProcessors.cbegin(), m_masterFxProcessors.cend(),
- [](const IFxProcessorPtr& fx) {
- return fx->shouldProcessDuringSilence();
- });
+ updateShouldProcessMasterFxDuringSilence();Also applies to: 441-444
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/framework/audio/engine/internal/mixer.cpp` around lines 410 - 413, The
paramsChanged handler currently updates m_masterParams.fxChain but doesn't
update the cached m_shouldProcessMasterFxDuringSilence, so disabling a
previously silence-processing FX leaves processing on; modify the lambda used in
fx->paramsChanged().onReceive (the handler that inserts into
m_masterParams.fxChain and sends m_masterOutputParamsChanged) to recompute
m_shouldProcessMasterFxDuringSilence after insert_or_assign by iterating the
entries in m_masterParams.fxChain and setting the flag true only if any active
FX processor (check the FX objects' active() state or the equivalent in
AudioFxParams) indicates it should process during silence; then emit
m_masterOutputParamsChanged as before.
Comment on lines
257
to
+263
| mixer()->removeChannel(id); | ||
| m_tracks.erase(id); | ||
| muse::remove(m_tracksToProcessWhenIdle, id); | ||
|
|
||
| if (m_prevActiveTrackId == id) { | ||
| m_prevActiveTrackId = INVALID_TRACK_ID; | ||
| } |
There was a problem hiding this comment.
Push idle-track removals back into Mixer.
m_tracksToProcessWhenIdle is updated locally, but Mixer keeps the removed ID until some later setter call. Since Line 337 now reuses 0 when the sequence becomes empty, a newly added track can inherit the previous track’s idle-processing slot.
Suggested fix
mixer()->removeChannel(id);
m_tracks.erase(id);
muse::remove(m_tracksToProcessWhenIdle, id);
if (m_prevActiveTrackId == id) {
m_prevActiveTrackId = INVALID_TRACK_ID;
}
+
+ mixer()->setTracksToProcessWhenIdle(m_tracksToProcessWhenIdle);
m_trackRemoved.send(id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/framework/audio/engine/internal/tracksequence.cpp` around lines 257 -
263, The code currently removes the idle-processing ID only from the local
m_tracksToProcessWhenIdle, but Mixer still retains that ID and can reassign it
to a new track; instead delegate the idle-removal to Mixer so both sides stay in
sync: replace the local removal (muse::remove(m_tracksToProcessWhenIdle, id))
with a call into Mixer (e.g. add and call
mixer()->removeIdleProcessingForChannel(id) or similar) and implement that
method in Mixer to remove the ID from Mixer’s idle list; keep the existing
mixer()->removeChannel(id), m_tracks.erase(id) and m_prevActiveTrackId =
INVALID_TRACK_ID logic unchanged.
Comment on lines
+511
to
+514
| if (m_needUpdateState) { | ||
| processor->process(m_processData); | ||
| m_needUpdateState = false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and find the relevant code
ls -la src/framework/vst/internal/vstaudioclient.cpp 2>/dev/null || echo "File not found"
wc -l src/framework/vst/internal/vstaudioclient.cppRepository: musescore/MuseScore
Length of output: 208
🏁 Script executed:
# Let's examine the code around lines 511-514 to understand the context
sed -n '495,530p' src/framework/vst/internal/vstaudioclient.cppRepository: musescore/MuseScore
Length of output: 1000
🏁 Script executed:
# Search for flushBuffers method to see if it exists
rg "flushBuffers" src/framework/vst/internal/Repository: musescore/MuseScore
Length of output: 345
🏁 Script executed:
# Search for m_processData usage and initialization
rg "m_processData" src/framework/vst/internal/vstaudioclient.cpp -A 2 -B 2Repository: musescore/MuseScore
Length of output: 3487
🏁 Script executed:
# Look for disableActivity method implementation
ast-grep --pattern $'void disableActivity() {
$$$
}'Repository: musescore/MuseScore
Length of output: 45
🏁 Script executed:
# Find all calls to flushBuffers to understand the pattern
rg "flushBuffers\(\)" src/framework/vst/internal/vstaudioclient.cpp -B 3 -A 1Repository: musescore/MuseScore
Length of output: 229
🏁 Script executed:
# Check the git history or recent changes to understand if this is a new pattern
git log --oneline -n 20 -- src/framework/vst/internal/vstaudioclient.cpp 2>/dev/null | head -20Repository: musescore/MuseScore
Length of output: 116
🏁 Script executed:
# Check the full disableActivity() method and surrounding context
sed -n '495,540p' src/framework/vst/internal/vstaudioclient.cppRepository: musescore/MuseScore
Length of output: 1429
Call flushBuffers() before the state-sync process() call.
The extra process() call in disableActivity() reuses buffers from the previous audio block. For effect plugins, this means the last block gets processed twice with identical audio data, which can mutate internal plugin state incorrectly. Zero the buffers first to avoid this.
Suggested fix
if (m_needUpdateState) {
+ flushBuffers();
processor->process(m_processData);
m_needUpdateState = false;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/framework/vst/internal/vstaudioclient.cpp` around lines 511 - 514, Before
calling processor->process(m_processData) when m_needUpdateState is true (e.g.,
in disableActivity()), call flushBuffers() to zero/clear the audio buffers so
the final process() doesn't reuse previous block data; specifically, invoke
flushBuffers() immediately before processor->process(m_processData) (retain
setting m_needUpdateState = false afterward) to avoid double-processing
identical audio data and state mutation.
Comment on lines
727
to
729
| if (result) { | ||
| interactive()->closeAllDialogs(); | ||
| interactive()->closeAllDialogsSync(); | ||
| globalContext()->setCurrentProject(nullptr); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Declarations / signatures =="
rg -n -C2 'closeAllDialogsSync\s*\(' src/framework/interactive src/framework/autobot || true
echo
echo "== Implementations =="
rg -n -C4 'closeAllDialogsSync\s*\(' src/framework/interactive || true
echo
echo "== Call sites =="
rg -n -C2 'closeAllDialogsSync\s*\(' src || trueRepository: musescore/MuseScore
Length of output: 7176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full implementation of closeAllDialogsSync() =="
sed -n '794,820p' src/framework/interactive/internal/interactive.cpp
echo
echo "== Ret type definition =="
rg -n 'using Ret\s*=' src/framework --type cpp | head -20
echo
echo "== Context around call site (line 720-735) =="
sed -n '720,735p' src/project/internal/projectactionscontroller.cppRepository: musescore/MuseScore
Length of output: 1184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Context around call site (line 715-740) =="
sed -n '715,740p' src/project/internal/projectactionscontroller.cpp
echo
echo "== Ret type - search in common locations =="
find src/framework -name "*.h" | xargs grep -l "using Ret\s*=" | head -5
echo
echo "== Ret in types.h or ret.h =="
find src -name "*ret*.h" -o -name "*types.h" | xargs grep "using Ret\s*=\|class Ret\|struct Ret" 2>/dev/null | head -20Repository: musescore/MuseScore
Length of output: 844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for Ret type definition more broadly =="
rg -n 'class Ret\b|struct Ret\b|using Ret ' src --type cpp --type h | head -30
echo
echo "== Check ret.h files =="
find src -name "*.h" -path "*ret*" -o -name "ret.h" 2>/dev/null | xargs cat 2>/dev/null | head -100Repository: musescore/MuseScore
Length of output: 2806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read the actual Ret class definition =="
cat src/framework/global/types/ret.h
echo
echo "== Check return value handling in closeObjectsSync =="
rg -n -A15 'closeObjectsSync' src/framework/interactive/internal/interactive.cpp | head -60Repository: musescore/MuseScore
Length of output: 6896
Check the result of closeAllDialogsSync() before clearing the project.
At line 728, the return value of closeAllDialogsSync() is ignored. The method returns a Ret status object that can indicate failure (e.g., if dialogs reject closure). If the close operation fails, the code still proceeds to clear the current project. This is inconsistent with the error handling pattern in the same function (see lines 732–734, which check the return value of openPageIfNeed()). If dialog closure fails, consider whether the project-clearing step should be conditional or logged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/project/internal/projectactionscontroller.cpp` around lines 727 - 729,
The call to interactive()->closeAllDialogsSync() returns a Ret status that must
be checked before clearing the project; modify the block handling result to
capture and inspect that Ret, and only call
globalContext()->setCurrentProject(nullptr) when closeAllDialogsSync() succeeds
(or otherwise log/propagate the failure), mirroring the pattern used for
openPageIfNeed(); specifically update the code around closeAllDialogsSync() so
it assigns its return to a variable, checks isOk()/isFailure() (or equivalent),
and conditionally clears the project or logs the error and aborts the clear.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports: #32679