Recognize JUCE-exported sidechain port groups by name when pg:sideChainOf is absent - #560
Draft
diamondsea11 wants to merge 1 commit into
Draft
Conversation
LV2's proper way to mark a port group as a sidechain is pg:sideChainOf, which is what this detection already checks. JUCE-generated LV2 exports (e.g. Dusk Multi-Comp) give their sidechain input a named port group but currently omit pg:sideChainOf entirely, so the existing check silently fails to recognize it and the plugin's sidechain input is treated as an ordinary audio input. Adds isSidechainGroupName() as a narrow, explicit fallback: it only fires when the proper property is absent, and only recognizes group names/symbols containing "sidechain", "side chain" or "side_chain" (case-insensitive). It does not change behavior for any plugin that already declares pg:sideChainOf correctly. Correction for context: an earlier message in this review thread stated that PiPedal's fork adds sidechain *input selection* (choosing a specific plugin's output as another plugin's sidechain source) as new functionality. That was wrong -- checked against upstream directly, and the entire mechanism (sideChainInputId, its GetEffect()-based resolution in Lv2Pedalboard.cpp, the IEffect/Lv2Effect plumbing, and the SideChainSelectControl.tsx UI) already exists in upstream, byte-for-byte identical to the fork. There is nothing to contribute there. This commit is the one actual, narrow gap: the JUCE-style port-group naming fallback needed for plugins like Dusk Multi-Comp to be recognized at all. Extracted as a single, minimal change from the diamondsea11/pipedal fork (feature/multipath-v1) for independent review, per the maintainer's request in rerdavies#555 to evaluate fork changes one topic at a time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
diamondsea11
pushed a commit
to diamondsea11/pipedal
that referenced
this pull request
Aug 6, 2026
All four extracted branches are now open as drafts against rerdavies/pipedal: rerdavies#558 ALSA multichannel, rerdavies#559 S24_LE scaling, rerdavies#560 JUCE sidechain group fallback, rerdavies#561 LV2 category patching. All target main, which is what the branches were cut from; retargeting to dev is offered in each PR body. rerdavies#558 already existed as a non-draft and was converted, with its auto-populated commit-message body replaced by one written for review. Each PR body states the provenance Robin asked about in rerdavies#555 (not a developer, AI-implemented from descriptions) so he can calibrate review effort, and states verification honestly per PR -- only rerdavies#561 has actually been compiled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
First, a correction
In #555 I implied the fork adds sidechain input selection — choosing a specific plugin's output as another plugin's sidechain source. That was wrong, and I should have checked before saying it.
Diffing against
upstream/maindirectly:sideChainInputId, itsGetEffect()-based resolution inLv2Pedalboard.cpp, theIEffect/Lv2Effectplumbing andSideChainSelectControl.tsxare all already in upstream, byte-for-byte identical to the fork. There is nothing to contribute there. Sorry for the noise.Your question about a send from a lower split branch to an upper one is also answered by that check: it isn't implemented, in the fork or upstream. Processing is strictly top-to-bottom in both, so that direction still incurs the one-buffer delay you described. That would be new work, not an extraction, and I'd want your input on the intended semantics before anyone attempts it.
What this PR actually is
The one genuine gap I found while checking: LV2's proper way to mark a port group as a sidechain is
pg:sideChainOf, which the existing detection inPluginHost.cppchecks. JUCE-generated LV2 exports — Dusk Multi-Comp is the case I hit — give their sidechain input a named port group but omitpg:sideChainOfentirely. The check silently fails and the plugin's sidechain input is treated as an ordinary audio input.isSidechainGroupName()is a narrow fallback: it runs only when the proper property is absent, and matches only group names/symbols containing "sidechain", "side chain" or "side_chain", case-insensitively. Behaviour is unchanged for any plugin that declarespg:sideChainOfcorrectly.Three lines of real logic plus a helper and one
#include.Caveat worth your judgement
This is heuristic matching on a human-readable name, which is exactly the kind of thing that ages badly. It's here because JUCE's exporter omits a property it should emit — arguably the right fix is upstream in JUCE, and this is a workaround for their bug. If you'd rather not carry that, I understand; the alternative is that JUCE-generated plugins with sidechains stay unusable until JUCE changes.
Verification
Not built. The change is small and mechanical, but I haven't compiled this branch, so I won't claim otherwise.
Provenance
As discussed in #555: I'm not a developer, this is AI-implemented from my descriptions. Please review accordingly.
Targeting
mainsince that's what the branch is based on — happy to retarget todev.