Refactor the speaker detection logic into observeSpeaker and add tests (#2814)

* Refactor the speaker detection logic into observeSpeaker and add tests

@robintown the tests pass, but some of the values were off by 1ms from what I was expecting. Please can you sanity check them?

* Extra test cases and clean up

* Make distinctUntilChanged part of the observable itself

* More suggestions from code review

src/state/CallViewModel.ts

@@ -27,7 +27,6 @@ import {
   EMPTY,
   Observable,
   Subject,
-  audit,
   combineLatest,
   concat,
   distinctUntilChanged,
@@ -76,6 +75,7 @@ import { spotlightExpandedLayout } from "./SpotlightExpandedLayout";
 import { oneOnOneLayout } from "./OneOnOneLayout";
 import { pipLayout } from "./PipLayout";
 import { EncryptionSystem } from "../e2ee/sharedKeyManagement";
+import { observeSpeaker } from "./observeSpeaker";
 
 // How long we wait after a focus switch before showing the real participant
 // list again
@@ -248,22 +248,7 @@ class UserMedia {
           livekitRoom,
         );
 
-    this.speaker = this.vm.speaking.pipe(
+    this.speaker = observeSpeaker(this.vm.speaking).pipe(this.scope.state());
-      // Require 1 s of continuous speaking to become a speaker, and 60 s of
-      // continuous silence to stop being considered a speaker
-      audit((s) =>
-        merge(
-          timer(s ? 1000 : 60000),
-          // If the speaking flag resets to its original value during this time,
-          // end the silencing window to stick with that original value
-          this.vm.speaking.pipe(filter((s1) => s1 !== s)),
-        ),
-      ),
-      startWith(false),
-      // Make this Observable hot so that the timers don't reset when you
-      // resubscribe
-      this.scope.state(),
-    );
 
     this.presenter = observeParticipantEvents(
       participant,

src/state/observeSpeaker.test.ts

@@ -0,0 +1,119 @@
+/*
+Copyright 2024 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only
+Please see LICENSE in the repository root for full details.
+*/
+
+import { describe, test } from "vitest";
+
+import { withTestScheduler } from "../utils/test";
+import { observeSpeaker } from "./observeSpeaker";
+
+const yesNo = {
+  y: true,
+  n: false,
+};
+
+describe("observeSpeaker", () => {
+  describe("does not activate", () => {
+    const expectedOutputMarbles = "n";
+    test("starts correctly", () => {
+      // should default to false when no input is given
+      const speakingInputMarbles = "";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("after no speaking", () => {
+      const speakingInputMarbles = "n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("with speaking for 1ms", () => {
+      const speakingInputMarbles = "y n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("with speaking for 999ms", () => {
+      const speakingInputMarbles = "y 999ms n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("with speaking intermittently", () => {
+      const speakingInputMarbles =
+        "y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n 199ms y 199ms n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("with consecutive speaking then stops speaking", () => {
+      const speakingInputMarbles = "y y y y y y y y y y n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+  });
+
+  describe("activates", () => {
+    test("after 1s", () => {
+      // this will active after 1s as no `n` follows it:
+      const speakingInputMarbles = " y";
+      const expectedOutputMarbles = "n 999ms y";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("speaking for 1001ms activates for 60s", () => {
+      const speakingInputMarbles = " y 1s    n      ";
+      const expectedOutputMarbles = "n 999ms y 60s n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+
+    test("speaking for 5s activates for 64s", () => {
+      const speakingInputMarbles = " y 5s    n      ";
+      const expectedOutputMarbles = "n 999ms y 64s n";
+      withTestScheduler(({ hot, expectObservable }) => {
+        expectObservable(observeSpeaker(hot(speakingInputMarbles, yesNo))).toBe(
+          expectedOutputMarbles,
+          yesNo,
+        );
+      });
+    });
+  });
+});

src/state/observeSpeaker.ts

@@ -0,0 +1,36 @@
+/*
+Copyright 2024 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only
+Please see LICENSE in the repository root for full details.
+*/
+import {
+  Observable,
+  audit,
+  merge,
+  timer,
+  filter,
+  startWith,
+  distinctUntilChanged,
+} from "rxjs";
+
+/**
+ * Require 1 second of continuous speaking to become a speaker, and 60 second of
+ * continuous silence to stop being considered a speaker
+ */
+export function observeSpeaker(
+  isSpeakingObservable: Observable<boolean>,
+): Observable<boolean> {
+  const distinct = isSpeakingObservable.pipe(distinctUntilChanged());
+
+  return distinct.pipe(
+    // Either change to the new value after the timer or re-emit the same value if it toggles back
+    // (audit will return the latest (toggled back) value) before the timeout.
+    audit((s) =>
+      merge(timer(s ? 1000 : 60000), distinct.pipe(filter((s1) => s1 !== s))),
+    ),
+    // Filter the re-emissions (marked as: | ) that happen if we toggle quickly (<1s) from false->true->false|->..
+    startWith(false),
+    distinctUntilChanged(),
+  );
+}