Skip to content

Fix spectrum-strip peak-block crash when bin count grows between frames#496

Open
patrickrb wants to merge 1 commit into
devfrom
optio/task-63ba1216-c4e5-449b-83db-d2e0d452bbcf
Open

Fix spectrum-strip peak-block crash when bin count grows between frames#496
patrickrb wants to merge 1 commit into
devfrom
optio/task-63ba1216-c4e5-449b-83db-d2e0d452bbcf

Conversation

@patrickrb

Copy link
Copy Markdown
Owner

Summary

ColumnarView (the spectrum strip's bar view) can throw an
IndexOutOfBoundsException on the UI thread — an unrecoverable app crash —
while drawing the peak-hold "falling block" overlay. This fixes the root cause
and adds tests.

Root cause

In ColumnarView.setWaveData(int[]) the peak-block section rebuilt its block
list to the current frame's bin count (binsToShow) whenever the size
changed:

for (int i = 0; i < binsToShow; i++) { blockData.add(new Rect(...)); }
for (int i = 0; i < blockData.size(); i++) {
    blockData.get(i).left = newData.get(i).left;   // <-- newData is the PREVIOUS frame
    ...
}

but the copy loop reads geometry from newData, which at that point still
holds the previous frame's (smaller) bar set — newData is not rebuilt
until later in the method. When the visible bin count grows between two
frames, the loop indexes past the previous bars → IndexOutOfBoundsException.
Drawing is not wrapped in try/catch, so it crashes the app.

The bin count changes whenever binsToShow = min(len, round(spectrumWidth / nyquist * len)) changes — i.e. on a spectrum-width change (the
NumberInputDialog, 2500–5000 Hz, in Radio/Audio settings) or an audio
sample-rate change
— both reachable at runtime while the peak blocks are
shown (setShowBlock(true) is set on the waterfall's ColumnarView).

Fix

Drive the peak-hold state off the bars actually present this frame. The
sizing/geometry decision is extracted into a pure, JVM-testable PeakBlocks
helper (mirroring the existing BinAggregation extraction). It always returns
exactly one block per current bar, so the copy can never over-index. A
bin-count change resets the peak-hold to the floor (matching the legacy
full-rebuild); when the count is unchanged the geometry is bit-for-bit
identical
to the pre-fix code. The Composable/View wrapper stays a thin
caller, per the project's testability guidance.

Testing

  • PeakBlocksTest (pure JVM, no Robolectric):
    • growingBinCountDoesNotThrow — asserts the reconstructed legacy inline
      logic throws on a grown bin count and the fix does not.
    • stableSizeMatchesLegacyMath — bit-for-bit parity with the legacy math
      when the size is unchanged.
    • first-frame floor start, rise-snap, fall-by-one-step, shrinking-bin-count
      reset, empty-bars cases.
  • ./gradlew :app:testDebugUnitTest — full suite green.
  • ./gradlew :app:assembleDebug — native cpp + APK build succeeds.

Risk

Low and localized to ColumnarView's peak-block overlay. No DSP, decoder,
encoder, protocol, or CAT behavior touched. Steady-state rendering is
unchanged (verified by the parity test); only the previously-crashing
size-change transient now renders instead of throwing. Same per-frame Rect
reuse as before, so no allocation/perf regression.

🤖 Generated with Claude Code

…frames

ColumnarView.setWaveData rebuilt the peak-hold "falling block" list to the
CURRENT frame's bin count (binsToShow) but the fall loop read positions from
the PREVIOUS frame's bar list (newData). When the visible bin count grew
between two frames — a spectrum-width change (NumberInputDialog, 2500-5000 Hz)
or an audio-sample-rate change while the peak blocks are shown — the loop
indexed past the previous bars and threw IndexOutOfBoundsException. Drawing
runs on the UI thread with no try/catch, so that is a hard app crash.

Root cause: the rebuild sized to binsToShow while the copy source (newData)
still held the previous frame's, smaller, bar set.

Fix: drive the peak-hold state off the bars actually present this frame. The
geometry/sizing decision is extracted into a pure, JVM-testable PeakBlocks
helper (mirroring the BinAggregation extraction) that always returns one block
per current bar; a bin-count change resets the peak-hold to the floor, matching
the legacy full-rebuild. When the bin count is unchanged the geometry is
bit-for-bit identical to the pre-fix code.

Tests: PeakBlocksTest covers the growing-bin-count crash (asserts the legacy
inline logic throws and the fix does not), first-frame/rise/fall math, the
shrinking-bin-count reset, and bit-for-bit parity with the legacy math for the
stable-size case.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 23.35%. Comparing base (6135d45) to head (9172a7a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                dev     #496   +/-   ##
=========================================
  Coverage     23.35%   23.35%           
  Complexity      162      162           
=========================================
  Files           167      167           
  Lines         21450    21450           
  Branches       3154     3154           
=========================================
  Hits           5009     5009           
  Misses        16248    16248           
  Partials        193      193           
Flag Coverage Δ
android 13.26% <ø> (ø)
native 9.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant