From d702f4a291f0158c8fa5f9c6b0ed679cef4322ec Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 24 Oct 2022 16:06:58 +0200 Subject: [PATCH] When start listening to a broadcast, pause the others (#9489) --- .../models/VoiceBroadcastPlayback.ts | 17 ++- .../stores/VoiceBroadcastPlaybacksStore.ts | 56 ++++++++- .../models/VoiceBroadcastPlayback-test.ts | 67 +++++++---- .../VoiceBroadcastPlaybacksStore-test.ts | 107 +++++++++++++----- 4 files changed, 186 insertions(+), 61 deletions(-) diff --git a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts index 641deb66ad1..dcecaa72820 100644 --- a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts +++ b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts @@ -47,7 +47,10 @@ export enum VoiceBroadcastPlaybackEvent { interface EventMap { [VoiceBroadcastPlaybackEvent.LengthChanged]: (length: number) => void; - [VoiceBroadcastPlaybackEvent.StateChanged]: (state: VoiceBroadcastPlaybackState) => void; + [VoiceBroadcastPlaybackEvent.StateChanged]: ( + state: VoiceBroadcastPlaybackState, + playback: VoiceBroadcastPlayback + ) => void; [VoiceBroadcastPlaybackEvent.InfoStateChanged]: (state: VoiceBroadcastInfoState) => void; } @@ -217,14 +220,20 @@ export class VoiceBroadcastPlayback } public pause(): void { - if (!this.currentlyPlaying) return; + // stopped voice broadcasts cannot be paused + if (this.getState() === VoiceBroadcastPlaybackState.Stopped) return; this.setState(VoiceBroadcastPlaybackState.Paused); + if (!this.currentlyPlaying) return; this.currentlyPlaying.pause(); } public resume(): void { - if (!this.currentlyPlaying) return; + if (!this.currentlyPlaying) { + // no playback to resume, start from the beginning + this.start(); + return; + } this.setState(VoiceBroadcastPlaybackState.Playing); this.currentlyPlaying.play(); @@ -260,7 +269,7 @@ export class VoiceBroadcastPlayback } this.state = state; - this.emit(VoiceBroadcastPlaybackEvent.StateChanged, state); + this.emit(VoiceBroadcastPlaybackEvent.StateChanged, state, this); } public getInfoState(): VoiceBroadcastInfoState { diff --git a/src/voice-broadcast/stores/VoiceBroadcastPlaybacksStore.ts b/src/voice-broadcast/stores/VoiceBroadcastPlaybacksStore.ts index 38d774e088a..03378d9492a 100644 --- a/src/voice-broadcast/stores/VoiceBroadcastPlaybacksStore.ts +++ b/src/voice-broadcast/stores/VoiceBroadcastPlaybacksStore.ts @@ -17,7 +17,8 @@ limitations under the License. import { MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix"; import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter"; -import { VoiceBroadcastPlayback } from ".."; +import { VoiceBroadcastPlayback, VoiceBroadcastPlaybackEvent, VoiceBroadcastPlaybackState } from ".."; +import { IDestroyable } from "../../utils/IDestroyable"; export enum VoiceBroadcastPlaybacksStoreEvent { CurrentChanged = "current_changed", @@ -28,10 +29,16 @@ interface EventMap { } /** - * This store provides access to the current and specific Voice Broadcast playbacks. + * This store manages VoiceBroadcastPlaybacks: + * - access the currently playing voice broadcast + * - ensures that only once broadcast is playing at a time */ -export class VoiceBroadcastPlaybacksStore extends TypedEventEmitter { +export class VoiceBroadcastPlaybacksStore + extends TypedEventEmitter + implements IDestroyable { private current: VoiceBroadcastPlayback | null; + + /** Playbacks indexed by their info event id. */ private playbacks = new Map(); public constructor() { @@ -42,7 +49,7 @@ export class VoiceBroadcastPlaybacksStore extends TypedEventEmitter { + if ([ + VoiceBroadcastPlaybackState.Buffering, + VoiceBroadcastPlaybackState.Playing, + ].includes(state)) { + this.pauseExcept(playback); + } + }; + + private pauseExcept(playbackNotToPause: VoiceBroadcastPlayback): void { + for (const playback of this.playbacks.values()) { + if (playback !== playbackNotToPause) { + playback.pause(); + } + } + } + + public destroy(): void { + this.removeAllListeners(); + + for (const playback of this.playbacks.values()) { + playback.off(VoiceBroadcastPlaybackEvent.StateChanged, this.onPlaybackStateChanged); + } + + this.playbacks = new Map(); + } + public static readonly _instance = new VoiceBroadcastPlaybacksStore(); /** diff --git a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts index 7e7722321cd..ec28a95fd6a 100644 --- a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts +++ b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts @@ -74,7 +74,25 @@ describe("VoiceBroadcastPlayback", () => { const itShouldEmitAStateChangedEvent = (state: VoiceBroadcastPlaybackState) => { it(`should emit a ${state} state changed event`, () => { - expect(mocked(onStateChanged)).toHaveBeenCalledWith(state); + expect(mocked(onStateChanged)).toHaveBeenCalledWith(state, playback); + }); + }; + + const startPlayback = () => { + beforeEach(async () => { + await playback.start(); + }); + }; + + const pausePlayback = () => { + beforeEach(() => { + playback.pause(); + }); + }; + + const stopPlayback = () => { + beforeEach(() => { + playback.stop(); }); }; @@ -180,14 +198,28 @@ describe("VoiceBroadcastPlayback", () => { }); describe("and calling start", () => { - beforeEach(async () => { - await playback.start(); - }); + startPlayback(); it("should be in buffering state", () => { expect(playback.getState()).toBe(VoiceBroadcastPlaybackState.Buffering); }); + describe("and calling stop", () => { + stopPlayback(); + itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Stopped); + + describe("and calling pause", () => { + pausePlayback(); + // stopped voice broadcasts cannot be paused + itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Stopped); + }); + }); + + describe("and calling pause", () => { + pausePlayback(); + itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Paused); + }); + describe("and receiving the first chunk", () => { beforeEach(() => { // TODO Michael W: Use RelationsHelper @@ -212,9 +244,7 @@ describe("VoiceBroadcastPlayback", () => { }); describe("and calling start", () => { - beforeEach(async () => { - await playback.start(); - }); + startPlayback(); it("should play the last chunk", () => { // assert that the last chunk is played first @@ -258,10 +288,7 @@ describe("VoiceBroadcastPlayback", () => { }); describe("and calling start", () => { - beforeEach(async () => { - await playback.start(); - }); - + startPlayback(); itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Buffering); }); }); @@ -278,9 +305,7 @@ describe("VoiceBroadcastPlayback", () => { itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Stopped); describe("and calling start", () => { - beforeEach(async () => { - await playback.start(); - }); + startPlayback(); itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Playing); @@ -303,13 +328,15 @@ describe("VoiceBroadcastPlayback", () => { }); describe("and calling pause", () => { - beforeEach(() => { - playback.pause(); - }); - + pausePlayback(); itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Paused); itShouldEmitAStateChangedEvent(VoiceBroadcastPlaybackState.Paused); }); + + describe("and calling stop", () => { + stopPlayback(); + itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Stopped); + }); }); describe("and calling toggle for the first time", () => { @@ -337,9 +364,7 @@ describe("VoiceBroadcastPlayback", () => { }); describe("and calling stop", () => { - beforeEach(() => { - playback.stop(); - }); + stopPlayback(); itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Stopped); diff --git a/test/voice-broadcast/stores/VoiceBroadcastPlaybacksStore-test.ts b/test/voice-broadcast/stores/VoiceBroadcastPlaybacksStore-test.ts index 8b994ef9c6c..07c7e2fe63c 100644 --- a/test/voice-broadcast/stores/VoiceBroadcastPlaybacksStore-test.ts +++ b/test/voice-broadcast/stores/VoiceBroadcastPlaybacksStore-test.ts @@ -22,24 +22,24 @@ import { } from "matrix-js-sdk/src/matrix"; import { - VoiceBroadcastInfoEventType, + VoiceBroadcastInfoState, VoiceBroadcastPlayback, + VoiceBroadcastPlaybackEvent, VoiceBroadcastPlaybacksStore, VoiceBroadcastPlaybacksStoreEvent, + VoiceBroadcastPlaybackState, } from "../../../src/voice-broadcast"; -import { mkEvent, mkStubRoom, stubClient } from "../../test-utils"; - -jest.mock("../../../src/voice-broadcast/models/VoiceBroadcastPlayback", () => ({ - ...jest.requireActual("../../../src/voice-broadcast/models/VoiceBroadcastPlayback") as object, - VoiceBroadcastPlayback: jest.fn().mockImplementation((infoEvent: MatrixEvent) => ({ infoEvent })), -})); +import { mkStubRoom, stubClient } from "../../test-utils"; +import { mkVoiceBroadcastInfoStateEvent } from "../utils/test-utils"; describe("VoiceBroadcastPlaybacksStore", () => { const roomId = "!room:example.com"; let client: MatrixClient; let room: Room; - let infoEvent: MatrixEvent; - let playback: VoiceBroadcastPlayback; + let infoEvent1: MatrixEvent; + let infoEvent2: MatrixEvent; + let playback1: VoiceBroadcastPlayback; + let playback2: VoiceBroadcastPlayback; let playbacks: VoiceBroadcastPlaybacksStore; let onCurrentChanged: (playback: VoiceBroadcastPlayback) => void; @@ -51,17 +51,26 @@ describe("VoiceBroadcastPlaybacksStore", () => { return room; } }); - infoEvent = mkEvent({ - event: true, - type: VoiceBroadcastInfoEventType, - user: client.getUserId(), - room: roomId, - content: {}, - }); - playback = { - infoEvent, - } as unknown as VoiceBroadcastPlayback; + + infoEvent1 = mkVoiceBroadcastInfoStateEvent( + roomId, + VoiceBroadcastInfoState.Started, + client.getUserId(), + client.getDeviceId(), + ); + infoEvent2 = mkVoiceBroadcastInfoStateEvent( + roomId, + VoiceBroadcastInfoState.Started, + client.getUserId(), + client.getDeviceId(), + ); + playback1 = new VoiceBroadcastPlayback(infoEvent1, client); + jest.spyOn(playback1, "off"); + playback2 = new VoiceBroadcastPlayback(infoEvent2, client); + jest.spyOn(playback2, "off"); + playbacks = new VoiceBroadcastPlaybacksStore(); + jest.spyOn(playbacks, "removeAllListeners"); onCurrentChanged = jest.fn(); playbacks.on(VoiceBroadcastPlaybacksStoreEvent.CurrentChanged, onCurrentChanged); }); @@ -72,31 +81,69 @@ describe("VoiceBroadcastPlaybacksStore", () => { describe("when setting a current Voice Broadcast playback", () => { beforeEach(() => { - playbacks.setCurrent(playback); + playbacks.setCurrent(playback1); }); it("should return it as current", () => { - expect(playbacks.getCurrent()).toBe(playback); + expect(playbacks.getCurrent()).toBe(playback1); }); it("should return it by id", () => { - expect(playbacks.getByInfoEvent(infoEvent, client)).toBe(playback); + expect(playbacks.getByInfoEvent(infoEvent1, client)).toBe(playback1); }); it("should emit a CurrentChanged event", () => { - expect(onCurrentChanged).toHaveBeenCalledWith(playback); + expect(onCurrentChanged).toHaveBeenCalledWith(playback1); }); describe("and setting the same again", () => { beforeEach(() => { mocked(onCurrentChanged).mockClear(); - playbacks.setCurrent(playback); + playbacks.setCurrent(playback1); }); it("should not emit a CurrentChanged event", () => { expect(onCurrentChanged).not.toHaveBeenCalled(); }); }); + + describe("and setting another playback and start both", () => { + beforeEach(() => { + playbacks.setCurrent(playback2); + playback1.start(); + playback2.start(); + }); + + it("should set playback1 to paused", () => { + expect(playback1.getState()).toBe(VoiceBroadcastPlaybackState.Paused); + }); + + it("should set playback2 to buffering", () => { + // buffering because there are no chunks, yet + expect(playback2.getState()).toBe(VoiceBroadcastPlaybackState.Buffering); + }); + + describe("and calling destroy", () => { + beforeEach(() => { + playbacks.destroy(); + }); + + it("should remove all listeners", () => { + expect(playbacks.removeAllListeners).toHaveBeenCalled(); + }); + + it("should deregister the listeners on the playbacks", () => { + expect(playback1.off).toHaveBeenCalledWith( + VoiceBroadcastPlaybackEvent.StateChanged, + expect.any(Function), + ); + expect(playback2.off).toHaveBeenCalledWith( + VoiceBroadcastPlaybackEvent.StateChanged, + expect.any(Function), + ); + }); + }); + }); }); describe("getByInfoEventId", () => { @@ -104,24 +151,22 @@ describe("VoiceBroadcastPlaybacksStore", () => { describe("when retrieving a known playback", () => { beforeEach(() => { - playbacks.setCurrent(playback); - returnedPlayback = playbacks.getByInfoEvent(infoEvent, client); + playbacks.setCurrent(playback1); + returnedPlayback = playbacks.getByInfoEvent(infoEvent1, client); }); it("should return the playback", () => { - expect(returnedPlayback).toBe(playback); + expect(returnedPlayback).toBe(playback1); }); }); describe("when retrieving an unknown playback", () => { beforeEach(() => { - returnedPlayback = playbacks.getByInfoEvent(infoEvent, client); + returnedPlayback = playbacks.getByInfoEvent(infoEvent1, client); }); it("should return the playback", () => { - expect(returnedPlayback).toEqual({ - infoEvent, - }); + expect(returnedPlayback.infoEvent).toBe(infoEvent1); }); }); });