Skip to content

Commit

Permalink
fix: check muted property for mute getter (#69)
Browse files Browse the repository at this point in the history
* fix: check muted property for mute getter

* fix: update mute event conditions

* test: update unit tests

---------

Co-authored-by: Bryce Tham <btham@cisco.com>
  • Loading branch information
brycetham and Bryce Tham committed Jan 19, 2024
1 parent 7737912 commit f07754c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 6 deletions.
43 changes: 41 additions & 2 deletions src/media/local-stream.spec.ts
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 @@ -10,20 +11,58 @@ class TestLocalStream extends LocalStream {}
describe('LocalStream', () => {
const mockStream = createMockedStream();
let localStream: LocalStream;

beforeEach(() => {
localStream = new TestLocalStream(mockStream);
});

describe('setMuted', () => {
it('should change the input track state based on being muted & unmuted', () => {
expect.assertions(2);
let emitSpy: jest.SpyInstance;

beforeEach(() => {
localStream = new TestLocalStream(mockStream);
emitSpy = jest.spyOn(localStream[StreamEventNames.MuteStateChange], 'emit');
});

it('should change the input track enabled state and fire an event', () => {
expect.assertions(6);

// Simulate the default state of the track's enabled state.
mockStream.getTracks()[0].enabled = true;

localStream.setMuted(true);
expect(mockStream.getTracks()[0].enabled).toBe(false);
expect(emitSpy).toHaveBeenCalledTimes(1);
expect(emitSpy).toHaveBeenLastCalledWith(true);

localStream.setMuted(false);
expect(mockStream.getTracks()[0].enabled).toBe(true);
expect(emitSpy).toHaveBeenCalledTimes(2);
expect(emitSpy).toHaveBeenLastCalledWith(false);
});

it('should not fire an event if the same mute state is set twice', () => {
expect.assertions(1);

// 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);
expect(emitSpy).toHaveBeenCalledTimes(0);
});
});

Expand Down
27 changes: 25 additions & 2 deletions src/media/local-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,32 @@ abstract class _LocalStream extends Stream {
return this.inputStream.getTracks()[0];
}

/**
* @inheritdoc
*/
protected handleTrackMuted() {
if (this.inputTrack.enabled) {
super.handleTrackMuted();
}
}

/**
* @inheritdoc
*/
protected handleTrackUnmuted() {
if (this.inputTrack.enabled) {
super.handleTrackUnmuted();
}
}

/**
* @inheritdoc
*/
get muted(): boolean {
return !this.inputTrack.enabled;
// 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;
}

/**
Expand All @@ -73,7 +94,9 @@ abstract class _LocalStream extends Stream {
if (this.inputTrack.enabled === isMuted) {
this.inputTrack.enabled = !isMuted;
// setting `enabled` will not automatically emit MuteStateChange, so we emit it here
this[StreamEventNames.MuteStateChange].emit(isMuted);
if (!this.inputTrack.muted) {
this[StreamEventNames.MuteStateChange].emit(isMuted);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/media/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ abstract class _Stream {
/**
* Handler which is called when a track's mute event fires.
*/
private handleTrackMuted() {
protected handleTrackMuted() {
this[StreamEventNames.MuteStateChange].emit(true);
}

/**
* Handler which is called when a track's unmute event fires.
*/
private handleTrackUnmuted() {
protected handleTrackUnmuted() {
this[StreamEventNames.MuteStateChange].emit(false);
}

Expand Down

0 comments on commit f07754c

Please sign in to comment.