fix(processor): insert aresample between loudnorm and adeclick in Pass 4#98
Merged
Conversation
loudnorm in linear mode upsamples internally to 192 kHz and emits at that rate. Without resampling back to source rate, adeclick and downstream analysis filters processed 4× sample count, causing ~4.5× slowdown on Pass 4. - Add sourceSampleRate parameter to buildLoudnormFilterSpec(), guard aresample - Move Pass 4 progress reporting to OnInputFrame for steady cadence - Add TestBuildLoudnormFilterSpecSourceRateResample guard test (two subtests) - Update docs and AGENTS.md with rate discovery and impact analysis Measured impact: 4.21s → 0.94s per 60s segment (4.5× faster). Signed-off-by: Martin Wimpress <code@wimpress.io>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Confidence score: 4/5
- This PR looks safe to merge overall; the only reported issue is documentation wording, not a runtime or functional regression risk.
- In
docs/FilterLimiter-CBS-Volumax.md, the heading "Filter chain when not clamped (unchanged):" conflicts with the actual chain update (insertion ofaresamplebetweenloudnormandadeclick), which could mislead readers about current behavior. - Because the issue is limited to doc accuracy (severity 5/10) and has clear, straightforward remediation, merge risk remains minimal.
- Pay close attention to
docs/FilterLimiter-CBS-Volumax.md- fix the "(unchanged)" annotation so the documented filter chain matches the updated sequence.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The "(unchanged)" annotation could be misread as unchanged by the aresample edit; it actually meant "same chain as the clamped variant". Reword to "(no pre-gain)" to state the real distinction (the not-clamped path drops the leading volume pre-gain stage). Addresses cubic review feedback on PR #98.
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This change modifies the core audio processing filter chain in normalise.go by inserting an aresample filter between loudnorm and adeclick, which is a functional logic change that could affect audio quality and processing correctness, so a human reviewer should verify the insertion and the new...
Re-trigger cubic
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.
loudnorm in linear mode upsamples internally to 192 kHz and emits at that rate. Without resampling back to source rate, adeclick and downstream analysis filters processed 4× sample count, causing ~4.5× slowdown on Pass 4.
Measured impact: 4.21s → 0.94s per 60s segment (4.5× faster).