Skip to content

Commit

Permalink
Revert "feat: separate API for enabled and muted states (#72)"
Browse files Browse the repository at this point in the history 
This reverts commit f4ce41d.
  • Loading branch information
@marcin-bazyl
marcin-bazyl committed Mar 20, 2024
1 parent ee46d62 commit a2779d9
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 277 deletions.
42 changes: 21 additions & 21 deletions src/media/local-stream.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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);
});
Expand All @@ -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);
});
});
Expand Down
100 changes: 22 additions & 78 deletions src/media/local-stream.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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>();
Expand All @@ -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);
}

/**
Expand All @@ -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);
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/media/local-video-stream.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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}`);
});
Expand All @@ -29,7 +30,7 @@ describe('localVideoStream', () => {
console.log('stream constraints changed');
});

videoStream.setUserMuted(true);
videoStream.setMuted(true);
videoStream.applyConstraints({
height: 720,
width: 1280,
Expand Down
75 changes: 1 addition & 74 deletions src/media/remote-stream.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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);
Expand All @@ -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', () => {
Expand Down
Loading

0 comments on commit a2779d9

Please sign in to comment.