Skip to content

OutputConvolver re-assigns .buffer on every update, restarting the convolution on each level switch; bufferSize from storage is unvalidated #198

Closed

Description

@Nawid3333

Severity: medium

Where: chrome/player/ui/audio/OutputConvolver.mjs (updateNodes: ch.convolverNode.normalize = channelConfig.normalize; ch.convolverNode.buffer = ch.impulseBuffer; unconditionally per active channel), config/AudioConvolverControl.mjs (fromObj takes obj.bufferSize as is)

What happens

AudioConfigManager.updateChannelCount() calls updateNodes() at LOADEDDATA and on every audio/video level change, and every checkbox/toggle calls it too. Each assignment makes Gecko rebuild the reverb for each channel: the tail is cut and restarts (an audible dropout or click per ABR switch); for a 16384-tap impulse response the FFT partitions are rebuilt each time. A NaN bufferSize from localStorage makes channelData.length > IMPULSE_LENGTH false, so a multi-minute file is used whole as the impulse response.

Suggested fix

Assign buffer/normalize only when they changed (keep the last applied pair on ch); clamp bufferSize to [128, 16384] in fromObj as the input handler does.

Confidence: high.


From a line-by-line code review of the tree at 41363fd (2026-10-02). Each claim above was checked against the source before filing; the reproduction steps and line references are as of that commit.


Generated by Claude Code

Activity

  1. added
    bugSomething isn't working
    code-reviewFound by the 2026-10-02 line-by-line review
    on Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcode-reviewFound by the 2026-10-02 line-by-line review

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions