Skip to content

Fix IR loudness/classification bug with explicit per-block category - #120

Open
Shayshez wants to merge 1 commit into
tone-3000:mainfrom
Shayshez:fix/ir-loudness-classification-89
Open

Shayshez wants to merge 1 commit into
tone-3000:mainfrom
Shayshez:fix/ir-loudness-classification-89

Conversation

@Shayshez

@Shayshez Shayshez commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #89. ChainBlock::irIsLong was doing two unrelated jobs: picking the
convolution engine (a CPU decision) and driving every audible IR default -
the -18 dB cab pad and the block's default mix (a perceptual/content
decision). Both read the same signal: irLengthBaseSamples, which is
juce::dsp::Convolution::getCurrentIRSize() after Trim::yes - JUCE trims
trailing samples below an absolute -80 dBFS threshold and reports what's
left. That threshold exists to save CPU on a convolution kernel; it was never
designed to answer "is this a cab or a reverb/space," and it doesn't: a cab
manually trimmed to a few seconds of file length around tens of ms of real
content reports a long post-trim size, while a genuinely short slap-delay or
small-room IR reports a short one. Two versions of the same cabinet capture
(one with more trailing silence than the other) could land on opposite sides
of the classification and get wildly different perceived level - the
reported bug.

This isn't fixable by tuning the threshold or the trim mechanism: short
reverbs/spaces and short cabs are both legitimately short, and long-file-
length cabs and long reverbs are both legitimately long. Duration - raw or
trimmed - doesn't encode which one a file is; nothing downstream of it can
either.

The fix replaces duration with an explicit, persisted per-block
IrCategory (Cab / IrPlayer) as the sole source of the pad and the
default mix:

  • Tones loaded from tone3000.com resolve category synchronously from the
    catalog's gear metadata (cab -> Cab, everything else -> IrPlayer),
    before the download even starts.
  • Local file drops carry no gear tag, so category is seeded once from a
    load-time RMS content scan instead (a real, editable value from the
    moment it loads, not authoritative going forward) - the split drop-zone
    UX that makes local-load category known up front too is tracked
    separately.
  • Engine selection is fully decoupled and no longer duration-based on raw
    file length either: a load already known to be Cab is unconditionally
    the uniform engine and hard-capped to its first 500 ms at load (a real
    truncation - the actual size limit passed to loadImpulseResponse - not
    just an engine hint), so it's safe regardless of what the source file
    contains. IrPlayer's engine choice (uniform vs. two-stage non-uniform)
    comes from that same RMS content scan, never raw length.
  • setBlockIrCategory lets a block's category be changed at any time
    (V1: resets mix/pad to the new category's fixed default; no memory of a
    prior per-category setting - a tracked fast-follow). The cab pad is now
    a smoothed value pulled every block instead of a load-time constant, so
    this glides instead of clicking.
  • Persisted through the existing save/restore path. State saved before
    this field existed backfills via the same content scan local loads use,
    without touching the mix the user actually saved.

See the demo video and root-cause comment thread on #89 for the audible
before/after and the empirical trace that led here.

Test plan

CI does not run on pull requests.

  • DSP: ./script/test-dsp.sh - 147/147 passing, including a new
    ir_category_tests.cpp that reproduces the original bug scenario
    bit-for-bit: the same cab content, one copy with extra trailing
    silence past the old cutoff, now produces identical processed audio
    (max sample diff < 1e-5) instead of a different pad/mix. Also covers
    the gear -> category mapping, setBlockIrCategory, the local-file
    duration guess, engine selection staying content-based and decoupled
    from category, and the state round-trip (including pre-migration
    backfill).
  • UI: cd ui && npm run lint && npm run build
  • Host validators: not run this pass (no processor lifecycle/format
    surface changed - only per-block IR classification/gain staging)
  • Host smoke: verified in the Standalone build; see the video on [Bug]: Cab/reverb IRs get wrong gain pad + mix defaults based on raw file length #89

Compatibility

  • AU parameter version hints in Processor.cpp are not reused or renumbered
  • LV2 URI and CLAP ID in plugin/CMakeLists.txt are unchanged
  • State format is unchanged (irCategory is a new, optional per-block
    property; its absence is handled explicitly - see the backfill above)
  • README / plugin/docs/ updated if the public build or behavior changed

🤖 Generated with Claude Code

…one-3000#89)

Replaces the duration-derived short/long IR classification with an
explicit, persisted IrCategory (Cab / IrPlayer) as the sole source of
the -18 dB cab pad and default mix. Raw/trimmed file length is
unreliable for this: catalog content includes cabs manually trimmed to
seconds of file length around tens of ms of real content, so two
versions of the same cabinet IR (one padded past the old 1 s cutoff)
could land on wildly different perceived levels despite identical
audible content.

- Site-loaded tones resolve category synchronously from catalog `gear`
  metadata (loadTone), before the download even starts. Local file
  loads (no gear tag) seed it from a one-shot load-time duration guess
  instead - a real, editable value from the start, never re-derived
  afterward.
- Engine selection (uniform vs non-uniform convolution) is fully
  decoupled from category and no longer duration-based on raw file
  length either: a known Cab is unconditionally the uniform engine,
  hard-capped to its first 500 ms at load (a real truncation, not just
  an engine hint); IrPlayer's engine choice comes from a fresh
  load-time RMS content scan instead.
- setBlockIrCategory lets a block's category be changed at any time
  (V1: resets mix/pad to the new category's fixed default, no memory
  of a prior per-category setting).
- Persisted through the existing ValueTree save/restore path; state
  saved before this field existed backfills via the same duration
  guess local loads use, without touching the mix the user actually
  saved.
- UI: Cab / IR Player toggle on the IR block's detail view.

New test/src/ir_category_tests.cpp exercises the actual tone-3000#89 regression
(same content, different trailing silence, now produces bit-identical
processed audio), the gear->category mapping, the category setter, and
that engine selection stays content-based and decoupled from category.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URpcSgm9HiuypH6TfWfqMg

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Cab/reverb IRs get wrong gain pad + mix defaults based on raw file length

1 participant