feat: separate API for enabled and muted states (#72)

* feat: add mute reason to MuteStateChange event * feat: separate API for enabled and muted states * fix: emit MediaStateChange on RemoteStream.replaceTrack --------- Co-authored-by: Bryce Tham <[email protected]>
webex · Mar 12, 2024 · f4ce41d · f4ce41d
1 parent 23e6a65
commit f4ce41d
Show file tree

Hide file tree

Showing 6 changed files with 277 additions and 82 deletions.
diff --git a/src/media/local-stream.spec.ts b/src/media/local-stream.spec.ts
@@ -1,7 +1,6 @@
 import { WebrtcCoreError } from '../errors';
 import { createMockedStream } from '../util/test-utils';
 import { LocalStream, LocalStreamEventNames, TrackEffect } from './local-stream';
-import { StreamEventNames } from './stream';
 
 /**
  * A dummy LocalStream implementation, so we can instantiate it for testing.
@@ -16,27 +15,42 @@ describe('LocalStream', () => {
     localStream = new TestLocalStream(mockStream);
   });
 
-  describe('setMuted', () => {
+  describe('constructor', () => {
+    it('should add the correct event handlers on the track', () => {
+      expect.assertions(4);
+
+      const addEventListenerSpy = jest.spyOn(mockStream.getTracks()[0], 'addEventListener');
+
+      expect(addEventListenerSpy).toHaveBeenCalledTimes(3);
+      expect(addEventListenerSpy).toHaveBeenCalledWith('ended', expect.anything());
+      expect(addEventListenerSpy).toHaveBeenCalledWith('mute', expect.anything());
+      expect(addEventListenerSpy).toHaveBeenCalledWith('unmute', expect.anything());
+    });
+  });
+
+  describe('setUserMuted', () => {
     let emitSpy: jest.SpyInstance;
 
     beforeEach(() => {
       localStream = new TestLocalStream(mockStream);
-      emitSpy = jest.spyOn(localStream[StreamEventNames.MuteStateChange], 'emit');
+      emitSpy = jest.spyOn(localStream[LocalStreamEventNames.UserMuteStateChange], 'emit');
     });
 
     it('should change the input track enabled state and fire an event', () => {
-      expect.assertions(6);
+      expect.assertions(8);
 
       // Simulate the default state of the track's enabled state.
       mockStream.getTracks()[0].enabled = true;
 
-      localStream.setMuted(true);
+      localStream.setUserMuted(true);
       expect(mockStream.getTracks()[0].enabled).toBe(false);
+      expect(localStream.userMuted).toBe(true);
       expect(emitSpy).toHaveBeenCalledTimes(1);
       expect(emitSpy).toHaveBeenLastCalledWith(true);
 
-      localStream.setMuted(false);
+      localStream.setUserMuted(false);
       expect(mockStream.getTracks()[0].enabled).toBe(true);
+      expect(localStream.userMuted).toBe(false);
       expect(emitSpy).toHaveBeenCalledTimes(2);
       expect(emitSpy).toHaveBeenLastCalledWith(false);
     });
@@ -47,21 +61,7 @@ describe('LocalStream', () => {
       // Simulate the default state of the track's enabled state.
       mockStream.getTracks()[0].enabled = true;
 
-      localStream.setMuted(false);
-      expect(emitSpy).toHaveBeenCalledTimes(0);
-    });
-
-    it('should not fire an event if the track has been muted by the browser', () => {
-      expect.assertions(2);
-
-      // Simulate the default state of the track's enabled state.
-      mockStream.getTracks()[0].enabled = true;
-
-      // Simulate the track being muted by the browser.
-      Object.defineProperty(mockStream.getTracks()[0], 'muted', { value: true });
-
-      localStream.setMuted(true);
-      expect(mockStream.getTracks()[0].enabled).toBe(false);
+      localStream.setUserMuted(false);
       expect(emitSpy).toHaveBeenCalledTimes(0);
     });
   });

diff --git a/src/media/local-stream.ts b/src/media/local-stream.ts
@@ -7,12 +7,16 @@ import { Stream, StreamEventNames } from './stream';
 export type TrackEffect = BaseEffect;
 
 export enum LocalStreamEventNames {
+  UserMuteStateChange = 'user-mute-state-change',
+  SystemMuteStateChange = 'system-mute-state-change',
   ConstraintsChange = 'constraints-change',
   OutputTrackChange = 'output-track-change',
   EffectAdded = 'effect-added',
 }
 
 interface LocalStreamEvents {
+  [LocalStreamEventNames.UserMuteStateChange]: TypedEvent<(muted: boolean) => void>;
+  [LocalStreamEventNames.SystemMuteStateChange]: TypedEvent<(muted: boolean) => void>;
   [LocalStreamEventNames.ConstraintsChange]: TypedEvent<() => void>;
   [LocalStreamEventNames.OutputTrackChange]: TypedEvent<(track: MediaStreamTrack) => void>;
   [LocalStreamEventNames.EffectAdded]: TypedEvent<(effect: TrackEffect) => void>;
@@ -22,6 +26,10 @@ interface LocalStreamEvents {
  * A stream which originates on the local device.
  */
 abstract class _LocalStream extends Stream {
+  [LocalStreamEventNames.UserMuteStateChange] = new TypedEvent<(muted: boolean) => void>();
+
+  [LocalStreamEventNames.SystemMuteStateChange] = new TypedEvent<(muted: boolean) => void>();
+
   [LocalStreamEventNames.ConstraintsChange] = new TypedEvent<() => void>();
 
   [LocalStreamEventNames.OutputTrackChange] = new TypedEvent<(track: MediaStreamTrack) => void>();
@@ -45,6 +53,51 @@ abstract class _LocalStream extends Stream {
   constructor(stream: MediaStream) {
     super(stream);
     this.inputStream = stream;
+    this.handleTrackMutedBySystem = this.handleTrackMutedBySystem.bind(this);
+    this.handleTrackUnmutedBySystem = this.handleTrackUnmutedBySystem.bind(this);
+    this.addTrackHandlersForLocalStreamEvents(this.inputTrack);
+  }
+
+  /**
+   * Handler which is called when a track's mute event fires.
+   */
+  private handleTrackMutedBySystem(): void {
+    this[LocalStreamEventNames.SystemMuteStateChange].emit(true);
+  }
+
+  /**
+   * Handler which is called when a track's unmute event fires.
+   */
+  private handleTrackUnmutedBySystem(): void {
+    this[LocalStreamEventNames.SystemMuteStateChange].emit(false);
+  }
+
+  /**
+   * Helper function to add event handlers to a MediaStreamTrack. See
+   * {@link Stream.addTrackHandlersForStreamEvents} for why this is useful.
+   *
+   * @param track - The MediaStreamTrack.
+   */
+  private addTrackHandlersForLocalStreamEvents(track: MediaStreamTrack): void {
+    track.addEventListener('mute', this.handleTrackMutedBySystem);
+    track.addEventListener('unmute', this.handleTrackUnmutedBySystem);
+  }
+
+  /**
+   * @inheritdoc
+   */
+  protected addTrackHandlers(track: MediaStreamTrack): void {
+    super.addTrackHandlers(track);
+    this.addTrackHandlersForLocalStreamEvents(track);
+  }
+
+  /**
+   * @inheritdoc
+   */
+  protected removeTrackHandlers(track: MediaStreamTrack): void {
+    super.removeTrackHandlers(track);
+    track.removeEventListener('mute', this.handleTrackMutedBySystem);
+    track.removeEventListener('unmute', this.handleTrackUnmutedBySystem);
   }
 
   /**
@@ -58,45 +111,48 @@ abstract class _LocalStream extends Stream {
   }
 
   /**
-   * @inheritdoc
+   * Check whether or not this stream is muted. This considers both whether the stream has been
+   * muted by the user (see {@link userMuted}) and whether the stream has been muted by the system
+   * (see {@link systemMuted}).
+   *
+   * @returns True if the stream is muted, false otherwise.
    */
-  protected handleTrackMuted() {
-    if (this.inputTrack.enabled) {
-      super.handleTrackMuted();
-    }
+  get muted(): boolean {
+    return this.userMuted || this.systemMuted;
   }
 
   /**
-   * @inheritdoc
+   * Check whether or not this stream has been muted by the user. This is equivalent to checking the
+   * MediaStreamTrack "enabled" state.
+   *
+   * @returns True if the stream has been muted by the user, false otherwise.
    */
-  protected handleTrackUnmuted() {
-    if (this.inputTrack.enabled) {
-      super.handleTrackUnmuted();
-    }
+  get userMuted(): boolean {
+    return !this.inputTrack.enabled;
   }
 
   /**
-   * @inheritdoc
+   * Check whether or not this stream has been muted by the user. This is equivalent to checking the
+   * MediaStreamTrack "muted" state.
+   *
+   * @returns True if the stream has been muted by the system, false otherwise.
    */
-  get muted(): boolean {
-    // Calls to `setMuted` will only affect the "enabled" state, but there are specific cases in
-    // which the browser may mute the track, which will affect the "muted" state but not the
-    // "enabled" state, e.g. minimizing a shared window or unplugging a shared screen.
-    return !this.inputTrack.enabled || this.inputTrack.muted;
+  get systemMuted(): boolean {
+    return this.inputTrack.muted;
   }
 
   /**
-   * Set the mute state of this stream.
+   * Set the user mute state of this stream.
+   *
+   * Note: This sets the user-toggled mute state, equivalent to changing the "enabled" state of the
+   * track. It is separate from the system-toggled mute state.
    *
    * @param isMuted - True to mute, false to unmute.
    */
-  setMuted(isMuted: boolean): void {
+  setUserMuted(isMuted: boolean): void {
     if (this.inputTrack.enabled === isMuted) {
       this.inputTrack.enabled = !isMuted;
-      // setting `enabled` will not automatically emit MuteStateChange, so we emit it here
-      if (!this.inputTrack.muted) {
-        this[StreamEventNames.MuteStateChange].emit(isMuted);
-      }
+      this[LocalStreamEventNames.UserMuteStateChange].emit(isMuted);
     }
   }
 

diff --git a/src/media/local-video-stream.spec.ts b/src/media/local-video-stream.spec.ts
@@ -3,7 +3,6 @@ import MediaStreamStub from '../mocks/media-stream-stub';
 import { mocked } from '../mocks/mock';
 import { LocalStreamEventNames } from './local-stream';
 import { LocalVideoStream } from './local-video-stream';
-import { StreamEventNames } from './stream';
 
 jest.mock('../mocks/media-stream-stub');
 
@@ -21,7 +20,7 @@ describe('localVideoStream', () => {
 
   it('should work', () => {
     expect.hasAssertions();
-    videoStream.on(StreamEventNames.MuteStateChange, (muted: boolean) => {
+    videoStream.on(LocalStreamEventNames.UserMuteStateChange, (muted: boolean) => {
       // eslint-disable-next-line no-console
       console.log(`stream is muted? ${muted}`);
     });
@@ -30,7 +29,7 @@ describe('localVideoStream', () => {
       console.log('stream constraints changed');
     });
 
-    videoStream.setMuted(true);
+    videoStream.setUserMuted(true);
     videoStream.applyConstraints({
       height: 720,
       width: 1280,

diff --git a/src/media/remote-stream.spec.ts b/src/media/remote-stream.spec.ts
@@ -1,5 +1,5 @@
 import { createMockedStream } from '../util/test-utils';
-import { RemoteStream } from './remote-stream';
+import { RemoteMediaState, RemoteStream, RemoteStreamEventNames } from './remote-stream';
 
 describe('RemoteStream', () => {
   const mockStream = createMockedStream();
@@ -8,6 +8,19 @@ describe('RemoteStream', () => {
     remoteStream = new RemoteStream(mockStream);
   });
 
+  describe('constructor', () => {
+    it('should add the correct event handlers on the track', () => {
+      expect.assertions(4);
+
+      const addEventListenerSpy = jest.spyOn(mockStream.getTracks()[0], 'addEventListener');
+
+      expect(addEventListenerSpy).toHaveBeenCalledTimes(3);
+      expect(addEventListenerSpy).toHaveBeenCalledWith('ended', expect.anything());
+      expect(addEventListenerSpy).toHaveBeenCalledWith('mute', expect.anything());
+      expect(addEventListenerSpy).toHaveBeenCalledWith('unmute', expect.anything());
+    });
+  });
+
   describe('getSettings', () => {
     it('should get the settings of the output track', () => {
       expect.assertions(1);
@@ -30,6 +43,66 @@ describe('RemoteStream', () => {
       expect(removeTrackSpy).toHaveBeenCalledWith(mockStream.getTracks()[0]);
       expect(addTrackSpy).toHaveBeenCalledWith(newTrack);
     });
+
+    it('should replace the event handlers on the output track', () => {
+      expect.assertions(8);
+
+      const removeEventListenerSpy = jest.spyOn(mockStream.getTracks()[0], 'removeEventListener');
+      const newTrack = new MediaStreamTrack();
+      const addEventListenerSpy = jest.spyOn(newTrack, 'addEventListener');
+
+      remoteStream.replaceTrack(newTrack);
+
+      expect(removeEventListenerSpy).toHaveBeenCalledTimes(3);
+      expect(removeEventListenerSpy).toHaveBeenCalledWith('ended', expect.anything());
+      expect(removeEventListenerSpy).toHaveBeenCalledWith('mute', expect.anything());
+      expect(removeEventListenerSpy).toHaveBeenCalledWith('unmute', expect.anything());
+
+      expect(addEventListenerSpy).toHaveBeenCalledTimes(3);
+      expect(addEventListenerSpy).toHaveBeenCalledWith('ended', expect.anything());
+      expect(addEventListenerSpy).toHaveBeenCalledWith('mute', expect.anything());
+      expect(addEventListenerSpy).toHaveBeenCalledWith('unmute', expect.anything());
+    });
+
+    it('should emit MediaStateChange event if new track is muted but old track is not', () => {
+      expect.assertions(2);
+
+      // Set old track to be unmuted.
+      Object.defineProperty(mockStream.getTracks()[0], 'muted', {
+        value: false,
+        configurable: true,
+      });
+
+      // Set new track to be muted.
+      const newTrack = new MediaStreamTrack();
+      Object.defineProperty(newTrack, 'muted', { value: true });
+
+      const emitSpy = jest.spyOn(remoteStream[RemoteStreamEventNames.MediaStateChange], 'emit');
+      remoteStream.replaceTrack(newTrack);
+
+      expect(emitSpy).toHaveBeenCalledTimes(1);
+      expect(emitSpy).toHaveBeenCalledWith(RemoteMediaState.Stopped);
+    });
+
+    it('should emit MediaStateChange event if old track is muted but new track is not', () => {
+      expect.assertions(2);
+
+      // Set old track to be muted.
+      Object.defineProperty(mockStream.getTracks()[0], 'muted', {
+        value: true,
+        configurable: true,
+      });
+
+      // Set new track to be unmuted.
+      const newTrack = new MediaStreamTrack();
+      Object.defineProperty(newTrack, 'muted', { value: false });
+
+      const emitSpy = jest.spyOn(remoteStream[RemoteStreamEventNames.MediaStateChange], 'emit');
+      remoteStream.replaceTrack(newTrack);
+
+      expect(emitSpy).toHaveBeenCalledTimes(1);
+      expect(emitSpy).toHaveBeenCalledWith(RemoteMediaState.Started);
+    });
   });
 
   describe('stop', () => {