From 254815cbcf5998df1aa289c7824837e87be12001 Mon Sep 17 00:00:00 2001
From: Michael Weimann <michaelw@matrix.org>
Date: Wed, 7 Dec 2022 10:37:30 +0100
Subject: [PATCH] Tweak voice broadcast chunk decoding (#9713)

---
 .../models/VoiceBroadcastPlayback.ts          | 63 +++++++++++--------
 .../models/VoiceBroadcastPlayback-test.ts     |  3 +
 2 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts
index d21ca49e33..62ad35628c 100644
--- a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts
+++ b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts
@@ -143,13 +143,14 @@ export class VoiceBroadcastPlayback
             return false;
         }
 
+        if (!event.getId() && !event.getTxnId()) {
+            // skip events without id and txn id
+            return false;
+        }
+
         this.chunkEvents.addEvent(event);
         this.setDuration(this.chunkEvents.getLength());
 
-        if (this.getState() !== VoiceBroadcastPlaybackState.Stopped) {
-            await this.enqueueChunk(event);
-        }
-
         if (this.getState() === VoiceBroadcastPlaybackState.Buffering) {
             await this.start();
             this.updateLiveness();
@@ -183,18 +184,7 @@ export class VoiceBroadcastPlayback
         }
     };
 
-    private async enqueueChunks(): Promise<void> {
-        const promises = this.chunkEvents.getEvents().reduce((promises, event: MatrixEvent) => {
-            if (!this.playbacks.has(event.getId() || "")) {
-                promises.push(this.enqueueChunk(event));
-            }
-            return promises;
-        }, [] as Promise<void>[]);
-
-        await Promise.all(promises);
-    }
-
-    private async enqueueChunk(chunkEvent: MatrixEvent): Promise<void> {
+    private async loadPlayback(chunkEvent: MatrixEvent): Promise<void> {
         const eventId = chunkEvent.getId();
 
         if (!eventId) {
@@ -215,6 +205,14 @@ export class VoiceBroadcastPlayback
         });
     }
 
+    private unloadPlayback(event: MatrixEvent): void {
+        const playback = this.playbacks.get(event.getId()!);
+        if (!playback) return;
+
+        playback.destroy();
+        this.playbacks.delete(event.getId()!);
+    }
+
     private onPlaybackPositionUpdate = (
         event: MatrixEvent,
         position: number,
@@ -261,6 +259,7 @@ export class VoiceBroadcastPlayback
         if (newState !== PlaybackState.Stopped) return;
 
         await this.playNext();
+        this.unloadPlayback(event);
     };
 
     private async playNext(): Promise<void> {
@@ -283,10 +282,11 @@ export class VoiceBroadcastPlayback
     private async playEvent(event: MatrixEvent): Promise<void> {
         this.setState(VoiceBroadcastPlaybackState.Playing);
         this.currentlyPlaying = event;
-        await this.getPlaybackForEvent(event)?.play();
+        const playback = await this.getOrLoadPlaybackForEvent(event);
+        playback?.play();
     }
 
-    private getPlaybackForEvent(event: MatrixEvent): Playback | undefined {
+    private async getOrLoadPlaybackForEvent(event: MatrixEvent): Promise<Playback | undefined> {
         const eventId = event.getId();
 
         if (!eventId) {
@@ -294,6 +294,10 @@ export class VoiceBroadcastPlayback
             return;
         }
 
+        if (!this.playbacks.has(eventId)) {
+            await this.loadPlayback(event);
+        }
+
         const playback = this.playbacks.get(eventId);
 
         if (!playback) {
@@ -301,9 +305,18 @@ export class VoiceBroadcastPlayback
             logger.warn("unable to find playback for event", event);
         }
 
+        // try to load the playback for the next event for a smooth(er) playback
+        const nextEvent = this.chunkEvents.getNext(event);
+        if (nextEvent) this.loadPlayback(nextEvent);
+
         return playback;
     }
 
+    private getCurrentPlayback(): Playback | undefined {
+        if (!this.currentlyPlaying) return;
+        return this.playbacks.get(this.currentlyPlaying.getId()!);
+    }
+
     public getLiveness(): VoiceBroadcastLiveness {
         return this.liveness;
     }
@@ -365,11 +378,8 @@ export class VoiceBroadcastPlayback
             return;
         }
 
-        const currentPlayback = this.currentlyPlaying
-            ? this.getPlaybackForEvent(this.currentlyPlaying)
-            : null;
-
-        const skipToPlayback = this.getPlaybackForEvent(event);
+        const currentPlayback = this.getCurrentPlayback();
+        const skipToPlayback = await this.getOrLoadPlaybackForEvent(event);
 
         if (!skipToPlayback) {
             logger.warn("voice broadcast chunk to skip to not found", event);
@@ -396,14 +406,13 @@ export class VoiceBroadcastPlayback
     }
 
     public async start(): Promise<void> {
-        await this.enqueueChunks();
         const chunkEvents = this.chunkEvents.getEvents();
 
         const toPlay = this.getInfoState() === VoiceBroadcastInfoState.Stopped
             ? chunkEvents[0] // start at the beginning for an ended voice broadcast
             : chunkEvents[chunkEvents.length - 1]; // start at the current chunk for an ongoing voice broadcast
 
-        if (this.playbacks.has(toPlay?.getId() || "")) {
+        if (toPlay) {
             return this.playEvent(toPlay);
         }
 
@@ -422,7 +431,7 @@ export class VoiceBroadcastPlayback
 
         this.setState(VoiceBroadcastPlaybackState.Paused);
         if (!this.currentlyPlaying) return;
-        this.getPlaybackForEvent(this.currentlyPlaying)?.pause();
+        this.getCurrentPlayback()?.pause();
     }
 
     public resume(): void {
@@ -433,7 +442,7 @@ export class VoiceBroadcastPlayback
         }
 
         this.setState(VoiceBroadcastPlaybackState.Playing);
-        this.getPlaybackForEvent(this.currentlyPlaying)?.play();
+        this.getCurrentPlayback()?.play();
     }
 
     /**
diff --git a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts
index 64b2362703..269ee1a3e7 100644
--- a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts
+++ b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts
@@ -387,6 +387,9 @@ describe("VoiceBroadcastPlayback", () => {
                 });
 
                 it("should play until the end", () => {
+                    // assert first chunk was unloaded
+                    expect(chunk1Playback.destroy).toHaveBeenCalled();
+
                     // assert that the second chunk is being played
                     expect(chunk2Playback.play).toHaveBeenCalled();