Stabilise seekign in broadcast (#9968)

Author: weeman1337

src/voice-broadcast/models/VoiceBroadcastPlayback.ts

@@ -25,6 +25,7 @@ import {
 import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter";
 import { SimpleObservable } from "matrix-widget-api";
 import { logger } from "matrix-js-sdk/src/logger";
+import { defer, IDeferred } from "matrix-js-sdk/src/utils";
 
 import { Playback, PlaybackInterface, PlaybackState } from "../../audio/Playback";
 import { PlaybackManager } from "../../audio/PlaybackManager";
@@ -96,6 +97,9 @@ export class VoiceBroadcastPlayback
     private chunkRelationHelper!: RelationsHelper;
     private infoRelationHelper!: RelationsHelper;
 
+    private skipToNext?: number;
+    private skipToDeferred?: IDeferred<void>;
+
     public constructor(
         public readonly infoEvent: MatrixEvent,
         private client: MatrixClient,
@@ -370,6 +374,28 @@ export class VoiceBroadcastPlayback
     }
 
     public async skipTo(timeSeconds: number): Promise<void> {
+        this.skipToNext = timeSeconds;
+
+        if (this.skipToDeferred) {
+            // Skip to position is already in progress. Return the promise for that.
+            return this.skipToDeferred.promise;
+        }
+
+        this.skipToDeferred = defer();
+
+        while (this.skipToNext !== undefined) {
+            // Skip to position until skipToNext is undefined.
+            // skipToNext can be set if skipTo is called while already skipping.
+            const skipToNext = this.skipToNext;
+            this.skipToNext = undefined;
+            await this.doSkipTo(skipToNext);
+        }
+
+        this.skipToDeferred.resolve();
+        this.skipToDeferred = undefined;
+    }
+
+    private async doSkipTo(timeSeconds: number): Promise<void> {
         const time = timeSeconds * 1000;
         const event = this.chunkEvents.findByTime(time);
 
@@ -379,6 +405,7 @@ export class VoiceBroadcastPlayback
         }
 
         const currentPlayback = this.getCurrentPlayback();
+        const currentPlaybackEvent = this.currentlyPlaying;
         const skipToPlayback = await this.getOrLoadPlaybackForEvent(event);
 
         if (!skipToPlayback) {
@@ -388,10 +415,12 @@ export class VoiceBroadcastPlayback
 
         this.currentlyPlaying = event;
 
-        if (currentPlayback && currentPlayback !== skipToPlayback) {
+        if (currentPlayback && currentPlaybackEvent && currentPlayback !== skipToPlayback) {
+            // only stop and unload the playback here without triggering other effects, e.g. play next
             currentPlayback.off(UPDATE_EVENT, this.onPlaybackStateChange);
             await currentPlayback.stop();
             currentPlayback.on(UPDATE_EVENT, this.onPlaybackStateChange);
+            this.unloadPlayback(currentPlaybackEvent);
         }
 
         const offsetInChunk = time - this.chunkEvents.getLengthTo(event);

test/voice-broadcast/models/VoiceBroadcastPlayback-test.tsx

@@ -31,7 +31,7 @@ import {
     VoiceBroadcastPlaybackState,
     VoiceBroadcastRecording,
 } from "../../../src/voice-broadcast";
-import { flushPromises, stubClient } from "../../test-utils";
+import { filterConsole, flushPromises, stubClient } from "../../test-utils";
 import { createTestPlayback } from "../../test-utils/audio";
 import { mkVoiceBroadcastChunkEvent, mkVoiceBroadcastInfoStateEvent } from "../utils/test-utils";
 
@@ -64,6 +64,8 @@ describe("VoiceBroadcastPlayback", () => {
     let chunk1Playback: Playback;
     let chunk2Playback: Playback;
     let chunk3Playback: Playback;
+    let middleOfSecondChunk!: number;
+    let middleOfThirdChunk!: number;
 
     const queryConfirmListeningDialog = () => {
         return screen.queryByText(
@@ -164,6 +166,9 @@ describe("VoiceBroadcastPlayback", () => {
         chunk2Playback = createTestPlayback();
         chunk3Playback = createTestPlayback();
 
+        middleOfSecondChunk = (chunk1Length + chunk2Length / 2) / 1000;
+        middleOfThirdChunk = (chunk1Length + chunk2Length + chunk3Length / 2) / 1000;
+
         jest.spyOn(PlaybackManager.instance, "createPlaybackInstance").mockImplementation(
             (buffer: ArrayBuffer, _waveForm?: number[]) => {
                 if (buffer === chunk1Data) return chunk1Playback;
@@ -181,10 +186,11 @@ describe("VoiceBroadcastPlayback", () => {
         });
     };
 
+    filterConsole("Starting load of AsyncWrapper for modal");
+
     beforeEach(() => {
         client = stubClient();
         deviceId = client.getDeviceId() || "";
-        jest.clearAllMocks();
         room = new Room(roomId, client, client.getSafeUserId());
         mocked(client.getRoom).mockImplementation((roomId: string): Room | null => {
             if (roomId === room.roomId) return room;
@@ -425,8 +431,8 @@ describe("VoiceBroadcastPlayback", () => {
         beforeEach(async () => {
             infoEvent = mkInfoEvent(VoiceBroadcastInfoState.Stopped);
             createChunkEvents();
-            setUpChunkEvents([chunk2Event, chunk1Event]);
-            room.addLiveEvents([infoEvent, chunk1Event, chunk2Event]);
+            setUpChunkEvents([chunk2Event, chunk1Event, chunk3Event]);
+            room.addLiveEvents([infoEvent, chunk1Event, chunk2Event, chunk3Event]);
             playback = await mkPlayback();
         });
 
@@ -472,6 +478,7 @@ describe("VoiceBroadcastPlayback", () => {
 
                 it("should play the second chunk", () => {
                     expect(chunk1Playback.stop).toHaveBeenCalled();
+                    expect(chunk1Playback.destroy).toHaveBeenCalled();
                     expect(chunk2Playback.play).toHaveBeenCalled();
                 });
 
@@ -484,9 +491,10 @@ describe("VoiceBroadcastPlayback", () => {
                         await playback.skipTo(0);
                     });
 
-                    it("should play the second chunk", () => {
-                        expect(chunk1Playback.play).toHaveBeenCalled();
+                    it("should play the first chunk", () => {
                         expect(chunk2Playback.stop).toHaveBeenCalled();
+                        expect(chunk2Playback.destroy).toHaveBeenCalled();
+                        expect(chunk1Playback.play).toHaveBeenCalled();
                     });
 
                     it("should update the time", () => {
@@ -495,6 +503,28 @@ describe("VoiceBroadcastPlayback", () => {
                 });
             });
 
+            describe("and skipping multiple times", () => {
+                beforeEach(async () => {
+                    return Promise.all([
+                        playback.skipTo(middleOfSecondChunk),
+                        playback.skipTo(middleOfThirdChunk),
+                        playback.skipTo(0),
+                    ]);
+                });
+
+                it("should only skip to the first and last position", () => {
+                    expect(chunk1Playback.stop).toHaveBeenCalled();
+                    expect(chunk1Playback.destroy).toHaveBeenCalled();
+                    expect(chunk2Playback.play).toHaveBeenCalled();
+
+                    expect(chunk3Playback.play).not.toHaveBeenCalled();
+
+                    expect(chunk2Playback.stop).toHaveBeenCalled();
+                    expect(chunk2Playback.destroy).toHaveBeenCalled();
+                    expect(chunk1Playback.play).toHaveBeenCalled();
+                });
+            });
+
             describe("and the first chunk ends", () => {
                 beforeEach(() => {
                     chunk1Playback.emit(PlaybackState.Stopped);
@@ -507,8 +537,9 @@ describe("VoiceBroadcastPlayback", () => {
                     // assert that the second chunk is being played
                     expect(chunk2Playback.play).toHaveBeenCalled();
 
-                    // simulate end of second chunk
+                    // simulate end of second and third chunk
                     chunk2Playback.emit(PlaybackState.Stopped);
+                    chunk3Playback.emit(PlaybackState.Stopped);
 
                     // assert that the entire playback is now in stopped state
                     expect(playback.getState()).toBe(VoiceBroadcastPlaybackState.Stopped);