diff --git a/src/media/local-stream.spec.ts b/src/media/local-stream.spec.ts index 5d52e03..121cd24 100644 --- a/src/media/local-stream.spec.ts +++ b/src/media/local-stream.spec.ts @@ -1,6 +1,7 @@ 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. @@ -15,42 +16,27 @@ describe('LocalStream', () => { localStream = new TestLocalStream(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('setUserMuted', () => { + describe('setMuted', () => { let emitSpy: jest.SpyInstance; beforeEach(() => { localStream = new TestLocalStream(mockStream); - emitSpy = jest.spyOn(localStream[LocalStreamEventNames.UserMuteStateChange], 'emit'); + emitSpy = jest.spyOn(localStream[StreamEventNames.MuteStateChange], 'emit'); }); it('should change the input track enabled state and fire an event', () => { - expect.assertions(8); + expect.assertions(6); // Simulate the default state of the track's enabled state. mockStream.getTracks()[0].enabled = true; - localStream.setUserMuted(true); + localStream.setMuted(true); expect(mockStream.getTracks()[0].enabled).toBe(false); - expect(localStream.userMuted).toBe(true); expect(emitSpy).toHaveBeenCalledTimes(1); expect(emitSpy).toHaveBeenLastCalledWith(true); - localStream.setUserMuted(false); + localStream.setMuted(false); expect(mockStream.getTracks()[0].enabled).toBe(true); - expect(localStream.userMuted).toBe(false); expect(emitSpy).toHaveBeenCalledTimes(2); expect(emitSpy).toHaveBeenLastCalledWith(false); }); @@ -61,7 +47,21 @@ describe('LocalStream', () => { // Simulate the default state of the track's enabled state. mockStream.getTracks()[0].enabled = true; - localStream.setUserMuted(false); + 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); expect(emitSpy).toHaveBeenCalledTimes(0); }); }); diff --git a/src/media/local-stream.ts b/src/media/local-stream.ts index fb62a77..a9b25fd 100644 --- a/src/media/local-stream.ts +++ b/src/media/local-stream.ts @@ -7,16 +7,12 @@ 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>; @@ -26,10 +22,6 @@ 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>(); @@ -53,51 +45,6 @@ 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); } /** @@ -111,48 +58,45 @@ abstract class _LocalStream extends Stream { } /** - * 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. + * @inheritdoc */ - get muted(): boolean { - return this.userMuted || this.systemMuted; + protected handleTrackMuted() { + if (this.inputTrack.enabled) { + super.handleTrackMuted(); + } } /** - * 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. + * @inheritdoc */ - get userMuted(): boolean { - return !this.inputTrack.enabled; + protected handleTrackUnmuted() { + if (this.inputTrack.enabled) { + super.handleTrackUnmuted(); + } } /** - * 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. + * @inheritdoc */ - get systemMuted(): boolean { - return this.inputTrack.muted; + 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; } /** - * 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. + * Set the mute state of this stream. * * @param isMuted - True to mute, false to unmute. */ - setUserMuted(isMuted: boolean): void { + setMuted(isMuted: boolean): void { if (this.inputTrack.enabled === isMuted) { this.inputTrack.enabled = !isMuted; - this[LocalStreamEventNames.UserMuteStateChange].emit(isMuted); + // setting `enabled` will not automatically emit MuteStateChange, so we emit it here + if (!this.inputTrack.muted) { + this[StreamEventNames.MuteStateChange].emit(isMuted); + } } } diff --git a/src/media/local-video-stream.spec.ts b/src/media/local-video-stream.spec.ts index 657b668..dd7ed1a 100644 --- a/src/media/local-video-stream.spec.ts +++ b/src/media/local-video-stream.spec.ts @@ -3,6 +3,7 @@ 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'); @@ -20,7 +21,7 @@ describe('localVideoStream', () => { it('should work', () => { expect.hasAssertions(); - videoStream.on(LocalStreamEventNames.UserMuteStateChange, (muted: boolean) => { + videoStream.on(StreamEventNames.MuteStateChange, (muted: boolean) => { // eslint-disable-next-line no-console console.log(`stream is muted? ${muted}`); }); @@ -29,7 +30,7 @@ describe('localVideoStream', () => { console.log('stream constraints changed'); }); - videoStream.setUserMuted(true); + videoStream.setMuted(true); videoStream.applyConstraints({ height: 720, width: 1280, diff --git a/src/media/remote-stream.spec.ts b/src/media/remote-stream.spec.ts index 8400d10..9893cf8 100644 --- 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 { RemoteMediaState, RemoteStream, RemoteStreamEventNames } from './remote-stream'; +import { RemoteStream } from './remote-stream'; describe('RemoteStream', () => { const mockStream = createMockedStream(); @@ -8,19 +8,6 @@ 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); @@ -43,66 +30,6 @@ 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', () => { diff --git a/src/media/remote-stream.ts b/src/media/remote-stream.ts index 23f78f2..b28ee2d 100644 --- a/src/media/remote-stream.ts +++ b/src/media/remote-stream.ts @@ -1,87 +1,14 @@ -import { AddEvents, TypedEvent, WithEventsDummyType } from '@webex/ts-events'; import { Stream, StreamEventNames } from './stream'; -export enum RemoteMediaState { - Started = 'started', - Stopped = 'stopped', -} - -export enum RemoteStreamEventNames { - MediaStateChange = 'media-state-change', -} - -interface RemoteStreamEvents { - [RemoteStreamEventNames.MediaStateChange]: TypedEvent<(state: RemoteMediaState) => void>; -} - /** * A stream originating from a remote peer. */ -class _RemoteStream extends Stream { - [RemoteStreamEventNames.MediaStateChange] = new TypedEvent<(state: RemoteMediaState) => void>(); - - /** - * Create a RemoteStream from the given values. - * - * @param stream - The initial output MediaStream for this Stream. - */ - constructor(stream: MediaStream) { - super(stream); - this.handleMediaStarted = this.handleMediaStarted.bind(this); - this.handleMediaStopped = this.handleMediaStopped.bind(this); - this.outputTrack.addEventListener('mute', this.handleMediaStopped); - this.outputTrack.addEventListener('unmute', this.handleMediaStarted); - } - - /** - * @inheritdoc - */ - protected handleMediaStarted(): void { - this[RemoteStreamEventNames.MediaStateChange].emit(RemoteMediaState.Started); - } - - /** - * @inheritdoc - */ - protected handleMediaStopped(): void { - this[RemoteStreamEventNames.MediaStateChange].emit(RemoteMediaState.Stopped); - } - - /** - * Helper function to add event handlers to a MediaStreamTrack. See - * {@link Stream.addTrackHandlersForStreamEvents} for why this is useful. - * - * @param track - The MediaStreamTrack. - */ - private addTrackHandlersForRemoteStreamEvents(track: MediaStreamTrack): void { - track.addEventListener('mute', this.handleMediaStopped); - track.addEventListener('unmute', this.handleMediaStarted); - } - - /** - * @inheritdoc - */ - protected addTrackHandlers(track: MediaStreamTrack): void { - super.addTrackHandlers(track); - this.addTrackHandlersForRemoteStreamEvents(track); - } - +export class RemoteStream extends Stream { /** * @inheritdoc */ - protected removeTrackHandlers(track: MediaStreamTrack): void { - super.removeTrackHandlers(track); - track.removeEventListener('mute', this.handleMediaStopped); - track.removeEventListener('unmute', this.handleMediaStarted); - } - - /** - * Get whether the media on this stream has started or stopped. - * - * @returns The state of the media. - */ - get mediaState(): RemoteMediaState { - return this.outputTrack.muted ? RemoteMediaState.Stopped : RemoteMediaState.Started; + get muted(): boolean { + return !this.outputTrack.enabled; } /** @@ -104,14 +31,6 @@ class _RemoteStream extends Stream { this.outputStream.addTrack(newTrack); this.addTrackHandlers(newTrack); - if (oldTrack.muted !== newTrack.muted) { - if (newTrack.muted) { - this.handleMediaStopped(); - } else { - this.handleMediaStarted(); - } - } - // TODO: Chrome/React may not automatically refresh the media element with the new track when // the output track has changed, so we may need to emit an event here if this is the case. // this[StreamEventNames.OutputTrackChange].emit(newTrack); @@ -126,7 +45,3 @@ class _RemoteStream extends Stream { this[StreamEventNames.Ended].emit(); } } - -export const RemoteStream = AddEvents(_RemoteStream); - -export type RemoteStream = _RemoteStream & WithEventsDummyType; diff --git a/src/media/stream.ts b/src/media/stream.ts index 187c8e7..7040d0a 100644 --- a/src/media/stream.ts +++ b/src/media/stream.ts @@ -1,10 +1,12 @@ import { AddEvents, TypedEvent, WithEventsDummyType } from '@webex/ts-events'; export enum StreamEventNames { + MuteStateChange = 'mute-state-change', Ended = 'stream-ended', } interface StreamEvents { + [StreamEventNames.MuteStateChange]: TypedEvent<(muted: boolean) => void>; [StreamEventNames.Ended]: TypedEvent<() => void>; } @@ -17,6 +19,8 @@ abstract class _Stream { // TODO: these should be protected, but we need the helper type in ts-events // to hide the 'emit' method from TypedEvent. + [StreamEventNames.MuteStateChange] = new TypedEvent<(muted: boolean) => void>(); + [StreamEventNames.Ended] = new TypedEvent<() => void>(); /** @@ -26,28 +30,31 @@ abstract class _Stream { */ constructor(stream: MediaStream) { this.outputStream = stream; + this.handleTrackMuted = this.handleTrackMuted.bind(this); + this.handleTrackUnmuted = this.handleTrackUnmuted.bind(this); this.handleTrackEnded = this.handleTrackEnded.bind(this); - this.addTrackHandlersForStreamEvents(this.outputTrack); + this.addTrackHandlers(this.outputTrack); } /** - * Handler which is called when a track's ended event fires. + * Handler which is called when a track's mute event fires. */ - private handleTrackEnded() { - this[StreamEventNames.Ended].emit(); + protected handleTrackMuted() { + this[StreamEventNames.MuteStateChange].emit(true); } /** - * Helper function to add event handlers to a MediaStreamTrack. Unlike the virtual - * {@link addTrackHandlers} function, which can be overridden, this function is internal to this - * class and will only add the event handlers relevant to this class. It prevents, for example, - * accidentally adding the same event handlers multiple times, which could happen if the virtual - * `addTrackHandlers` method was called from a subclass's constructor. - * - * @param track - The MediaStreamTrack. + * Handler which is called when a track's unmute event fires. */ - private addTrackHandlersForStreamEvents(track: MediaStreamTrack) { - track.addEventListener('ended', this.handleTrackEnded); + protected handleTrackUnmuted() { + this[StreamEventNames.MuteStateChange].emit(false); + } + + /** + * Handler which is called when a track's ended event fires. + */ + private handleTrackEnded() { + this[StreamEventNames.Ended].emit(); } /** @@ -56,7 +63,9 @@ abstract class _Stream { * @param track - The MediaStreamTrack. */ protected addTrackHandlers(track: MediaStreamTrack) { - this.addTrackHandlersForStreamEvents(track); + track.addEventListener('mute', this.handleTrackMuted); + track.addEventListener('unmute', this.handleTrackUnmuted); + track.addEventListener('ended', this.handleTrackEnded); } /** @@ -65,6 +74,8 @@ abstract class _Stream { * @param track - The MediaStreamTrack. */ protected removeTrackHandlers(track: MediaStreamTrack) { + track.removeEventListener('mute', this.handleTrackMuted); + track.removeEventListener('unmute', this.handleTrackUnmuted); track.removeEventListener('ended', this.handleTrackEnded); } @@ -77,6 +88,13 @@ abstract class _Stream { return this.outputStream.id; } + /** + * Check whether or not this stream is muted. + * + * @returns True if the stream is muted, false otherwise. + */ + abstract get muted(): boolean; + /** * Get the track of the output stream. *