From 91e8f3bdae4acbe2a5b45aafcc71032e6fc43f0d Mon Sep 17 00:00:00 2001 From: hayyaksi <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:08:59 +0300 Subject: [PATCH] Keep a copy of the audio buffer so the WAV fallback can run (#34481) * Keep a copy of the audio buffer so the WAV fallback can run decodeAudioData detaches the buffer it is given, so the catch block handed an already detached buffer to decodeOgg and it threw "Cannot perform Construct on a detached ArrayBuffer" before it could re-encode. The fallback could never run and the user saw an error instead of their voice message. Fixes https://github.com/element-hq/element-web/issues/24904 * Assert the fallback buffer is usable instead of constructing a view for its side effect --------- Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- apps/web/src/audio/Playback.ts | 5 +++- .../test/unit-tests/audio/Playback-test.ts | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/apps/web/src/audio/Playback.ts b/apps/web/src/audio/Playback.ts index 0181943cb9..93dad591fe 100644 --- a/apps/web/src/audio/Playback.ts +++ b/apps/web/src/audio/Playback.ts @@ -168,6 +168,9 @@ export class Playback extends EventEmitter implements IDestroyable, PlaybackInte this.element.src = URL.createObjectURL(new Blob([this.buf])); await deferred.promise; // make sure the audio element is ready for us } else { + // decodeAudioData detaches the buffer it is given, so the copy the fallback needs has + // to be taken before we call it rather than inside the error handler. + const fallbackBuf = this.buf.slice(0); try { this.audioBuf = await this.context.decodeAudioData(this.buf); } catch (e) { @@ -176,7 +179,7 @@ export class Playback extends EventEmitter implements IDestroyable, PlaybackInte try { // This error handler is largely for Safari, which doesn't support Opus/Ogg very well. - const wav = await decodeOgg(this.buf); + const wav = await decodeOgg(fallbackBuf); this.audioBuf = await this.context.decodeAudioData(wav); } catch (e) { logger.error("Error decoding recording:", e); diff --git a/apps/web/test/unit-tests/audio/Playback-test.ts b/apps/web/test/unit-tests/audio/Playback-test.ts index 3f06ef9342..3ea66938d0 100644 --- a/apps/web/test/unit-tests/audio/Playback-test.ts +++ b/apps/web/test/unit-tests/audio/Playback-test.ts @@ -176,6 +176,34 @@ describe("Playback", () => { expect(playback.currentState).toEqual(PlaybackState.Stopped); }); + it("hands the ogg fallback a buffer which decodeAudioData has not detached", async () => { + // stub logger to keep console clean from expected error + jest.spyOn(logger, "error").mockReturnValue(undefined); + jest.spyOn(logger, "warn").mockReturnValue(undefined); + + const buffer = new ArrayBuffer(8); + mockAudioContext.decodeAudioData + .mockImplementationOnce((buf: ArrayBuffer) => { + // The real decodeAudioData detaches the buffer it is handed, even when it fails. + structuredClone(buf, { transfer: [buf] }); + return Promise.reject(new Error("test")); + }) + .mockResolvedValueOnce(mockAudioBuffer); + // Constructing a view over a detached buffer throws, which is what decodeOgg does first. + mocked(decodeOgg).mockImplementationOnce(async (audioBuffer: ArrayBuffer) => { + expect(() => new Uint8Array(audioBuffer)).not.toThrow(); + return new ArrayBuffer(1); + }); + + const playback = new Playback(buffer); + + await playback.prepare(); + + expect(decodeOgg).toHaveBeenCalled(); + expect(mockAudioContext.decodeAudioData).toHaveBeenCalledTimes(2); + expect(playback.currentState).toEqual(PlaybackState.Stopped); + }); + it("does not try to re-decode audio", async () => { const buffer = new ArrayBuffer(8); const playback = new Playback(buffer);