From 1958ef3615d4b24c7d3c4de8cb8d746d232284d8 Mon Sep 17 00:00:00 2001 From: Bao Nguyen Date: Sun, 30 Aug 2026 20:57:59 +0700 Subject: [PATCH] fix: enforce spec limits for ConvolverNode.buffer and allow 32-channel buffers The ConvolverNode buffer setter accepted any impulse response, so a buffer with an unsupported channel count or a sample rate different from the context's reached the engine instead of throwing NotSupportedError. The existing channel-count check only ran for buffers passed through the constructor options. AudioBuffer rejected 32 channels while its own error message and MAX_CHANNEL_COUNT both treat 32 as the inclusive upper bound, and the spec requires implementations to support at least 32 channels. WPT the-convolvernode-interface goes from 170/253 to 203/255 passing assertions; convolver-channels.html now passes all 32 of its assertions and ctor-convolver.html passes the illegal-sample-rate case. --- .../audiodocs/docs/effects/convolver-node.mdx | 2 +- .../src/core/AudioBuffer.ts | 2 +- .../src/core/ConvolverNode.ts | 15 ++- .../src/utils/validation/convolver.ts | 11 +++ .../src/utils/validation/index.ts | 1 + .../src/web-core/AudioBuffer.web.ts | 2 +- .../src/web-core/AudioContext.web.ts | 2 +- .../src/web-core/OfflineAudioContext.web.ts | 2 +- .../tests/convolver-buffer.test.ts | 92 +++++++++++++++++++ 9 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 packages/react-native-audio-api/tests/convolver-buffer.test.ts diff --git a/packages/audiodocs/docs/effects/convolver-node.mdx b/packages/audiodocs/docs/effects/convolver-node.mdx index 025fd45af..75121af7c 100644 --- a/packages/audiodocs/docs/effects/convolver-node.mdx +++ b/packages/audiodocs/docs/effects/convolver-node.mdx @@ -41,7 +41,7 @@ Inherits all properties from [`AudioNode`](../core/audio-node.mdx#properties). | Name | Type | Description | | :----: | :----: | :-------- | -| `buffer` | [`AudioBuffer`](../sources/audio-buffer.mdx) | Associated AudioBuffer. | +| `buffer` | [`AudioBuffer`](../sources/audio-buffer.mdx) | Associated AudioBuffer. Setting it throws `NotSupportedError` unless the buffer has 1, 2 or 4 channels and the same sample rate as the context. | | `normalize` | `boolean` | Whether the impulse response from the buffer will be scaled by an equal-power normalization when the buffer attribute is set. | :::caution diff --git a/packages/react-native-audio-api/src/core/AudioBuffer.ts b/packages/react-native-audio-api/src/core/AudioBuffer.ts index e6d8091c1..b73fcf441 100644 --- a/packages/react-native-audio-api/src/core/AudioBuffer.ts +++ b/packages/react-native-audio-api/src/core/AudioBuffer.ts @@ -92,7 +92,7 @@ export default class AudioBuffer implements AudioBufferLike { options: AudioBufferOptions ): IAudioBuffer { const { numberOfChannels = 1, length, sampleRate } = options; - if (numberOfChannels < 1 || numberOfChannels >= 32) { + if (numberOfChannels < 1 || numberOfChannels > 32) { throw new NotSupportedError( `The number of channels provided (${numberOfChannels}) is outside the range [1, 32]` ); diff --git a/packages/react-native-audio-api/src/core/ConvolverNode.ts b/packages/react-native-audio-api/src/core/ConvolverNode.ts index 16f6c3a47..ead117aff 100644 --- a/packages/react-native-audio-api/src/core/ConvolverNode.ts +++ b/packages/react-native-audio-api/src/core/ConvolverNode.ts @@ -3,7 +3,11 @@ import { ConvolverOptions } from '../types'; import type BaseAudioContext from './BaseAudioContext'; import AudioNode from './AudioNode'; import AudioBuffer from './AudioBuffer'; -import { ConvolverOptionsValidator } from '../utils/validation'; +import { + ConvolverOptionsValidator, + validateConvolverBufferChannelCount, + validateConvolverBufferSampleRate, +} from '../utils/validation'; export default class ConvolverNode extends AudioNode { private _buffer: AudioBuffer | null = null; @@ -31,6 +35,15 @@ export default class ConvolverNode extends AudioNode { this._buffer = null; return; } + + // Spec setter steps: the engine has no guard of its own, so an impulse + // response the convolution matrix is undefined for must be rejected here. + validateConvolverBufferChannelCount(buffer.numberOfChannels); + validateConvolverBufferSampleRate( + buffer.sampleRate, + this.context.sampleRate + ); + (this.node as IConvolverNode).setBuffer(buffer.buffer); this._buffer = buffer; } diff --git a/packages/react-native-audio-api/src/utils/validation/convolver.ts b/packages/react-native-audio-api/src/utils/validation/convolver.ts index 50510f107..876f15260 100644 --- a/packages/react-native-audio-api/src/utils/validation/convolver.ts +++ b/packages/react-native-audio-api/src/utils/validation/convolver.ts @@ -15,6 +15,17 @@ export function validateConvolverBufferChannelCount( } } +export function validateConvolverBufferSampleRate( + bufferSampleRate: number, + contextSampleRate: number +): void { + if (bufferSampleRate !== contextSampleRate) { + throw new NotSupportedError( + `The sample rate of the impulse response for ConvolverNode buffer (${bufferSampleRate}) must match the sample rate of its context (${contextSampleRate}).` + ); + } +} + export const ConvolverOptionsValidator: OptionsValidator = { validate(options?: ConvolverOptions): void { if (!options?.buffer) { diff --git a/packages/react-native-audio-api/src/utils/validation/index.ts b/packages/react-native-audio-api/src/utils/validation/index.ts index e21a0a3d0..e8a9dfa7d 100644 --- a/packages/react-native-audio-api/src/utils/validation/index.ts +++ b/packages/react-native-audio-api/src/utils/validation/index.ts @@ -12,6 +12,7 @@ export { export { ConvolverOptionsValidator, validateConvolverBufferChannelCount, + validateConvolverBufferSampleRate, } from './convolver'; export { OscillatorOptionsValidator } from './oscillator'; diff --git a/packages/react-native-audio-api/src/web-core/AudioBuffer.web.ts b/packages/react-native-audio-api/src/web-core/AudioBuffer.web.ts index 921653541..568a3656d 100644 --- a/packages/react-native-audio-api/src/web-core/AudioBuffer.web.ts +++ b/packages/react-native-audio-api/src/web-core/AudioBuffer.web.ts @@ -73,7 +73,7 @@ export default class AudioBuffer implements AudioBufferLike { options: AudioBufferOptions ): globalThis.AudioBuffer { const { numberOfChannels = 1, length, sampleRate } = options; - if (numberOfChannels < 1 || numberOfChannels >= 32) { + if (numberOfChannels < 1 || numberOfChannels > 32) { throw new NotSupportedError( `The number of channels provided (${numberOfChannels}) is outside the range [1, 32]` ); diff --git a/packages/react-native-audio-api/src/web-core/AudioContext.web.ts b/packages/react-native-audio-api/src/web-core/AudioContext.web.ts index 220b4a85d..83590b974 100644 --- a/packages/react-native-audio-api/src/web-core/AudioContext.web.ts +++ b/packages/react-native-audio-api/src/web-core/AudioContext.web.ts @@ -123,7 +123,7 @@ export default class AudioContext implements BaseAudioContext { length: number, sampleRate: number ): AudioBuffer { - if (numberOfChannels < 1 || numberOfChannels >= 32) { + if (numberOfChannels < 1 || numberOfChannels > 32) { throw new NotSupportedError( `The number of channels provided (${numberOfChannels}) is outside the range [1, 32]` ); diff --git a/packages/react-native-audio-api/src/web-core/OfflineAudioContext.web.ts b/packages/react-native-audio-api/src/web-core/OfflineAudioContext.web.ts index 0084dad5e..51db31418 100644 --- a/packages/react-native-audio-api/src/web-core/OfflineAudioContext.web.ts +++ b/packages/react-native-audio-api/src/web-core/OfflineAudioContext.web.ts @@ -119,7 +119,7 @@ export default class OfflineAudioContext implements BaseAudioContext { length: number, sampleRate: number ): AudioBuffer { - if (numberOfChannels < 1 || numberOfChannels >= 32) { + if (numberOfChannels < 1 || numberOfChannels > 32) { throw new NotSupportedError( `The number of channels provided (${numberOfChannels}) is outside the range [1, 32]` ); diff --git a/packages/react-native-audio-api/tests/convolver-buffer.test.ts b/packages/react-native-audio-api/tests/convolver-buffer.test.ts new file mode 100644 index 000000000..c26ddabf9 --- /dev/null +++ b/packages/react-native-audio-api/tests/convolver-buffer.test.ts @@ -0,0 +1,92 @@ +import AudioBuffer from '../src/core/AudioBuffer'; +import ConvolverNode from '../src/core/ConvolverNode'; +import { NotSupportedError } from '../src/errors'; +import type BaseAudioContext from '../src/core/BaseAudioContext'; + +const CONTEXT_SAMPLE_RATE = 48000; + +function createContext(): BaseAudioContext { + return { + sampleRate: CONTEXT_SAMPLE_RATE, + context: { + createConvolver: () => ({ + numberOfInputs: 1, + numberOfOutputs: 1, + normalize: true, + setBuffer: jest.fn(), + }), + }, + } as unknown as BaseAudioContext; +} + +function createBuffer(numberOfChannels: number, sampleRate: number) { + return new AudioBuffer({ numberOfChannels, length: 4, sampleRate }); +} + +beforeAll(() => { + globalThis.createAudioBuffer = jest.fn( + (numberOfChannels: number, length: number, sampleRate: number) => ({ + length, + duration: length / sampleRate, + sampleRate, + numberOfChannels, + getChannelData: () => new Float32Array(length), + copyFromChannel: jest.fn(), + copyToChannel: jest.fn(), + }) + ) as unknown as typeof globalThis.createAudioBuffer; +}); + +describe('AudioBuffer channel count bounds', () => { + // The spec requires an implementation to support at least 32 channels. + it.each([1, 2, 32])('accepts %i channels', (numberOfChannels) => { + expect( + createBuffer(numberOfChannels, CONTEXT_SAMPLE_RATE).numberOfChannels + ).toBe(numberOfChannels); + }); + + it.each([0, 33])('rejects %i channels', (numberOfChannels) => { + expect(() => createBuffer(numberOfChannels, CONTEXT_SAMPLE_RATE)).toThrow( + NotSupportedError + ); + }); +}); + +describe('ConvolverNode buffer setter', () => { + it.each([1, 2, 4])( + 'accepts an impulse response with %i channels', + (channels) => { + const convolver = new ConvolverNode(createContext()); + expect(() => { + convolver.buffer = createBuffer(channels, CONTEXT_SAMPLE_RATE); + }).not.toThrow(); + expect(convolver.buffer?.numberOfChannels).toBe(channels); + } + ); + + it.each([3, 5, 6, 32])( + 'rejects an impulse response with %i channels', + (channels) => { + const convolver = new ConvolverNode(createContext()); + expect(() => { + convolver.buffer = createBuffer(channels, CONTEXT_SAMPLE_RATE); + }).toThrow(NotSupportedError); + expect(convolver.buffer).toBeNull(); + } + ); + + it('rejects an impulse response whose sample rate differs from the context', () => { + const convolver = new ConvolverNode(createContext()); + expect(() => { + convolver.buffer = createBuffer(1, CONTEXT_SAMPLE_RATE / 2); + }).toThrow(NotSupportedError); + expect(convolver.buffer).toBeNull(); + }); + + it('accepts a null impulse response', () => { + const convolver = new ConvolverNode(createContext()); + convolver.buffer = createBuffer(2, CONTEXT_SAMPLE_RATE); + convolver.buffer = null; + expect(convolver.buffer).toBeNull(); + }); +});