Repository navigation
Follow-ups to #1058: Auto Pan keeps the stereo image, paste and Strip Silence fixes - #1060
Conversation
…ad of panning their mono sum
…instead of on every frame of it
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Stereo processing and envelope level crates/dsp/src/plugins/auto_pan.rs |
The first two channels receive separate constant-power gains. Envelope following uses the louder absolute level of those channels. |
Tests and behavior records crates/dsp/tests/it/auto_pan.rs, ROADMAP.md, docs/plugin-parity.md |
Integration tests check center and half-left output and anti-phase envelope response. The roadmap and plugin-parity record describe the stereo behavior. |
Clipboard updates during paste
| Layer / File(s) | Summary |
|---|---|
Prepare clipboard without committing it crates/engine/src/cmd/edit.rs, crates/engine/src/cmd/auto_rename.rs |
Clipboard preparation no longer commits changes immediately. A test checks that a refused paste leaves the clipboard unchanged and a successful paste updates it. |
Commit clipboard after paste processing crates/engine/src/cmd/edit.rs, crates/engine/src/cmd/edit_more.rs |
Paste, repeat-to-fill, and MIDI merge store the prepared clipboard after processing. |
Browser Strip Silence scan timing
| Layer / File(s) | Summary |
|---|---|
Defer scans during pointer interaction crates/ui-egui/src/dialogs.rs |
On wasm32, browser scans wait while a pointer button is down. The documentation describes scan timing and stopping conditions. |
AudioSuite parameter display
| Layer / File(s) | Summary |
|---|---|
Remove the varispeed slider crates/ui-egui/src/dialogs.rs |
The varispeed slider branch is removed. The process falls through to the existing “No parameters” display. |
Subharmonic drive test coverage
| Layer / File(s) | Summary |
|---|---|
Check output at two filter Q values crates/dsp/tests/it/plugins.rs |
The test checks each character’s default-Q peak bounds and verifies finite output with Q set to 5. |
Clip Looping open-gap record
| Layer / File(s) | Summary |
|---|---|
Record crossfade and loop-count limits docs/gaps.md |
The G-04 entry records the missing crossfade option and the differing loop-count limits. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Possibly related issues
storytold/soundcraft#915— This PR changes the repeat-to-fill paste path, but does not address the reported quadratic growth in clip-multiplying edit commands.
Possibly related PRs
- storytold/soundcraft#1058 — Introduced the Auto Pan, clipboard, and browser Strip Silence behavior changed here.
- storytold/soundcraft#1016 — Added the AudioSuite workflow that retains the varispeed engine process.
- storytold/soundcraft#1055 — Added the Strip Silence preview and scan path that this PR adjusts.
Suggested reviewers: dotcoocoo
Merge Risk: 
Minimal · up to e97e4
-
The changed paste, audio, and dialog behavior has no identified issue that needs resolution before merge; normal checks remain appropriate.
Security architecture risk: 
Low · up to e97e4
-
Paste now preserves clipboard contents on refusal, and stereo processing retains the existing safety checks. No new security issue was established, but not every affected caller was fully assessed.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — Within the inspected paths, controllable audio samples affect existing plugin buffers, while paste parameters and clipboard data affect the current editing session. No expanded authority or new cross-service path was identified in those flows; this conclusion does not cover every caller in the truncated graph.
Trust Boundaries and Controls
- observed — Auto Pan retains finite-sample sanitization and bounded buffer access. Mixer post-processing containment remains in place to zero nonfinite plugin output and mark the slot failed. Independent stereo processing does not bypass these existing controls.
-
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passedFull details: Docstring Coverage
-
Explanation
Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 7 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.99.0)
-
Clippy execution failed
Usage-based review receipt
- Mode: Continue automatically
- Reviewed files: 10
- Waived: $2.50 (charged $0.00)
- View usage details
Note
This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.
Comment @coderabbitai help to get the list of available commands.
Follow-ups from the review of #1058.
Changes
paste,repeat_to_fillandpaste_merge_midistore the remapped clipboard only after they succeed.varispeedcontrols, which no menu entry reached. The engine'svarispeedprocess stays.Tests
auto_pan_balances_a_stereo_signal_without_summing_it: different left and right content, including anti-phase.a_refused_paste_leaves_the_clipboard_as_it_was.subharmonic_drive_keeps_every_character_boundedgains the Q 5 case.cargo xtask cipasses locally.Summary by CodeRabbit