Fix voice broadcast recording limit (#9478)

src/audio/VoiceRecording.ts

@@ -60,6 +60,7 @@ export class VoiceRecording extends EventEmitter implements IDestroyable {
     private recorderProcessor: ScriptProcessorNode;
     private recording = false;
     private observable: SimpleObservable<IRecordingUpdate>;
+    private targetMaxLength: number | null = TARGET_MAX_LENGTH;
     public amplitudes: number[] = []; // at each second mark, generated
     private liveWaveform = new FixedRollingArray(RECORDING_PLAYBACK_SAMPLES, 0);
     public onDataAvailable: (data: ArrayBuffer) => void;
@@ -83,6 +84,10 @@ export class VoiceRecording extends EventEmitter implements IDestroyable {
         return true; // we don't ever care if the event had listeners, so just return "yes"
     }
 
+    public disableMaxLength(): void {
+        this.targetMaxLength = null;
+    }
+
     private async makeRecorder() {
         try {
             this.recorderStream = await navigator.mediaDevices.getUserMedia({
@@ -203,6 +208,12 @@ export class VoiceRecording extends EventEmitter implements IDestroyable {
         // In testing, recorder time and worker time lag by about 400ms, which is roughly the
         // time needed to encode a sample/frame.
         //
+
+        if (!this.targetMaxLength) {
+            // skip time checks if max length has been disabled
+            return;
+        }
+
         const secondsLeft = TARGET_MAX_LENGTH - this.recorderSeconds;
         if (secondsLeft < 0) { // go over to make sure we definitely capture that last frame
             // noinspection JSIgnoredPromiseFromCall - we aren't concerned with it overlapping

src/voice-broadcast/audio/VoiceBroadcastRecorder.ts

@@ -139,5 +139,7 @@ export class VoiceBroadcastRecorder
 }
 
 export const createVoiceBroadcastRecorder = (): VoiceBroadcastRecorder => {
-    return new VoiceBroadcastRecorder(new VoiceRecording(), getChunkLength());
+    const voiceRecording = new VoiceRecording();
+    voiceRecording.disableMaxLength();
+    return new VoiceBroadcastRecorder(voiceRecording, getChunkLength());
 };

test/audio/VoiceRecording-test.ts

@@ -0,0 +1,105 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { VoiceRecording } from "../../src/audio/VoiceRecording";
+
+/**
+ * The tests here are heavily using access to private props.
+ * While this is not so great, we can at lest test some behaviour easily this way.
+ */
+describe("VoiceRecording", () => {
+    let recording: VoiceRecording;
+    let recorderSecondsSpy: jest.SpyInstance;
+
+    const itShouldNotCallStop = () => {
+        it("should not call stop", () => {
+            expect(recording.stop).not.toHaveBeenCalled();
+        });
+    };
+
+    const simulateUpdate = (recorderSeconds: number) => {
+        beforeEach(() => {
+            recorderSecondsSpy.mockReturnValue(recorderSeconds);
+            // @ts-ignore
+            recording.processAudioUpdate(recorderSeconds);
+        });
+    };
+
+    beforeEach(() => {
+        recording = new VoiceRecording();
+        // @ts-ignore
+        recording.observable = {
+            update: jest.fn(),
+        };
+        jest.spyOn(recording, "stop").mockImplementation();
+        recorderSecondsSpy = jest.spyOn(recording, "recorderSeconds", "get");
+    });
+
+    afterEach(() => {
+        jest.resetAllMocks();
+    });
+
+    describe("when recording", () => {
+        beforeEach(() => {
+            // @ts-ignore
+            recording.recording = true;
+        });
+
+        describe("and there is an audio update and time left", () => {
+            simulateUpdate(42);
+            itShouldNotCallStop();
+        });
+
+        describe("and there is an audio update and time is up", () => {
+            // one second above the limit
+            simulateUpdate(901);
+
+            it("should call stop", () => {
+                expect(recording.stop).toHaveBeenCalled();
+            });
+        });
+
+        describe("and the max length limit has been disabled", () => {
+            beforeEach(() => {
+                recording.disableMaxLength();
+            });
+
+            describe("and there is an audio update and time left", () => {
+                simulateUpdate(42);
+                itShouldNotCallStop();
+            });
+
+            describe("and there is an audio update and time is up", () => {
+                // one second above the limit
+                simulateUpdate(901);
+                itShouldNotCallStop();
+            });
+        });
+    });
+
+    describe("when not recording", () => {
+        describe("and there is an audio update and time left", () => {
+            simulateUpdate(42);
+            itShouldNotCallStop();
+        });
+
+        describe("and there is an audio update and time is up", () => {
+            // one second above the limit
+            simulateUpdate(901);
+            itShouldNotCallStop();
+        });
+    });
+});

test/voice-broadcast/audio/VoiceBroadcastRecorder-test.ts

@@ -26,6 +26,8 @@ import {
     VoiceBroadcastRecorderEvent,
 } from "../../../src/voice-broadcast";
 
+jest.mock("../../../src/audio/VoiceRecording");
+
 describe("VoiceBroadcastRecorder", () => {
     describe("createVoiceBroadcastRecorder", () => {
         beforeEach(() => {
@@ -44,6 +46,7 @@ describe("VoiceBroadcastRecorder", () => {
 
         it("should return a VoiceBroadcastRecorder instance with targetChunkLength from config", () => {
             const voiceBroadcastRecorder = createVoiceBroadcastRecorder();
+            expect(mocked(VoiceRecording).mock.instances[0].disableMaxLength).toHaveBeenCalled();
             expect(voiceBroadcastRecorder).toBeInstanceOf(VoiceBroadcastRecorder);
             expect(voiceBroadcastRecorder.targetChunkLength).toBe(1337);
         });
@@ -72,16 +75,12 @@ describe("VoiceBroadcastRecorder", () => {
         };
 
         beforeEach(() => {
-            voiceRecording = {
+            voiceRecording = new VoiceRecording();
-                contentType,
+            // @ts-ignore
-                start: jest.fn().mockResolvedValue(undefined),
+            voiceRecording.recorderSeconds = 23;
-                stop: jest.fn().mockResolvedValue(undefined),
+            // @ts-ignore
-                on: jest.fn(),
+            voiceRecording.contentType = contentType;
-                off: jest.fn(),
+
-                emit: jest.fn(),
-                destroy: jest.fn(),
-                recorderSeconds: 23,
-            } as unknown as VoiceRecording;
             voiceBroadcastRecorder = new VoiceBroadcastRecorder(voiceRecording, chunkLength);
             jest.spyOn(voiceBroadcastRecorder, "removeAllListeners");
             onChunkRecorded = jest.fn();