Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VBLOCKS-2032 | AudioProcessor insights and reusing AudioHelper #213

Merged
merged 1 commit into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 56 additions & 12 deletions lib/twilio/audiohelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
import { EventEmitter } from 'events';
import AudioProcessor from './audioprocessor';
import { AudioProcessorEventObserver } from './audioprocessoreventobserver';
import Device from './device';
import { InvalidArgumentError, NotSupportedError } from './errors';
import Log from './log';
Expand Down Expand Up @@ -90,6 +91,11 @@ class AudioHelper extends EventEmitter {
*/
private _audioContext?: AudioContext;

/**
* The AudioProcessorEventObserver instance to use
*/
private _audioProcessorEventObserver: AudioProcessorEventObserver;

/**
* The audio stream of the default device.
* This is populated when _openDefaultDeviceWithConstraints is called,
Expand Down Expand Up @@ -184,12 +190,10 @@ class AudioHelper extends EventEmitter {
* @private
* @param onActiveOutputsChanged - A callback to be called when the user changes the active output devices.
* @param onActiveInputChanged - A callback to be called when the user changes the active input device.
* @param getUserMedia - The getUserMedia method to use.
* @param [options]
*/
constructor(onActiveOutputsChanged: (type: 'ringtone' | 'speaker', outputIds: string[]) => Promise<void>,
onActiveInputChanged: (stream: MediaStream | null) => Promise<void>,
getUserMedia: (constraints: MediaStreamConstraints) => Promise<MediaStream>,
options?: AudioHelper.Options) {
super();

Expand All @@ -198,7 +202,9 @@ class AudioHelper extends EventEmitter {
setSinkId: typeof HTMLAudioElement !== 'undefined' && (HTMLAudioElement.prototype as any).setSinkId,
}, options);

this._getUserMedia = getUserMedia;
this._updateUserOptions(options);

this._audioProcessorEventObserver = options.audioProcessorEventObserver;
this._mediaDevices = options.mediaDevices || navigator.mediaDevices;
this._onActiveInputChanged = onActiveInputChanged;
this._enumerateDevices = typeof options.enumerateDevices === 'function'
Expand Down Expand Up @@ -262,11 +268,16 @@ class AudioHelper extends EventEmitter {
}

/**
* Current state of the enabled sounds
* Destroy this AudioHelper instance
* @private
*/
_getEnabledSounds(): Record<Device.ToggleableSound, boolean> {
return this._enabledSounds;
_destroy(): void {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_destroy(): void {
private _destroy(): void {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is public internal. Meaning, customers cannot use it (not visible in public API), but internal classes can. See device.

this._stopDefaultInputDeviceStream();
this._stopSelectedInputDeviceStream();
this._destroyProcessedStream();
this._maybeStopPollingVolume();
this.removeAllListeners();
this._unbind();
}

/**
Expand Down Expand Up @@ -399,6 +410,19 @@ class AudioHelper extends EventEmitter {
});
}

/**
* Update AudioHelper options that can be changed by the user
* @private
*/
_updateUserOptions(options: AudioHelper.Options): void {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_updateUserOptions(options: AudioHelper.Options): void {
private _updateUserOptions(options: AudioHelper.Options): void {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is public internal. Meaning, customers cannot use it (not visible in public API), but internal classes can. See device.

if (typeof options.enumerateDevices === 'function') {
this._enumerateDevices = options.enumerateDevices;
}
if (typeof options.getUserMedia === 'function') {
this._getUserMedia = options.getUserMedia;
}
}

/**
* Adds an {@link AudioProcessor} object.
* The AudioHelper will route the input audio stream through the processor
Expand Down Expand Up @@ -427,6 +451,7 @@ class AudioHelper extends EventEmitter {
this._log.debug('Adding processor');
this._processor = processor;
this._restartStreams();
this._audioProcessorEventObserver.emit('add');
}

/**
Expand Down Expand Up @@ -478,6 +503,7 @@ class AudioHelper extends EventEmitter {
this._destroyProcessedStream();
this._processor = null;
this._restartStreams();
this._audioProcessorEventObserver.emit('remove');
}

/**
Expand Down Expand Up @@ -543,6 +569,7 @@ class AudioHelper extends EventEmitter {
this._processedStream.getTracks().forEach(track => track.stop());
this._processedStream = null;
this._processor.destroyProcessedStream(processedStream);
this._audioProcessorEventObserver.emit('destroy');
}
}

Expand Down Expand Up @@ -596,6 +623,7 @@ class AudioHelper extends EventEmitter {
this._log.debug('Creating processed stream');
return this._processor.createProcessedStream(stream).then((processedStream: MediaStream) => {
this._processedStream = processedStream;
this._audioProcessorEventObserver.emit('create');
return this._processedStream;
});
}
Expand Down Expand Up @@ -659,9 +687,7 @@ class AudioHelper extends EventEmitter {
this._log.debug('Replacing with new stream.');
if (this._selectedInputDeviceStream) {
this._log.debug('Old stream detected. Stopping tracks.');
this._selectedInputDeviceStream.getTracks().forEach(track => {
track.stop();
});
this._stopSelectedInputDeviceStream();
}

this._selectedInputDeviceStream = stream;
Expand Down Expand Up @@ -714,9 +740,7 @@ class AudioHelper extends EventEmitter {
// If the currently active track is still in readyState `live`, gUM may return the same track
// rather than returning a fresh track.
this._log.debug('Same track detected on setInputDevice, stopping old tracks.');
this._selectedInputDeviceStream.getTracks().forEach(track => {
track.stop();
});
this._stopSelectedInputDeviceStream();
}

// Release the default device in case it was created previously
Expand All @@ -739,6 +763,16 @@ class AudioHelper extends EventEmitter {
});
}

/**
* Stop the selected audio stream
*/
private _stopSelectedInputDeviceStream(): void {
if (this._selectedInputDeviceStream) {
this._log.debug('Stopping selected device stream');
this._selectedInputDeviceStream.getTracks().forEach(track => track.stop());
}
}

/**
* Update a set of devices.
* @param updatedDevices - An updated list of available Devices
Expand Down Expand Up @@ -886,6 +920,11 @@ namespace AudioHelper {
*/
audioContext?: AudioContext;

/**
* AudioProcessorEventObserver to use
*/
audioProcessorEventObserver: AudioProcessorEventObserver,

/**
* Whether each sound is enabled.
*/
Expand All @@ -896,6 +935,11 @@ namespace AudioHelper {
*/
enumerateDevices?: any;

/**
* The getUserMedia method to use
*/
getUserMedia: (constraints: MediaStreamConstraints) => Promise<MediaStream>,

/**
* A custom MediaDevices instance to use.
*/
Expand Down
36 changes: 36 additions & 0 deletions lib/twilio/audioprocessoreventobserver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* @packageDocumentation
* @module Voice
* @internalapi
*/

import { EventEmitter } from 'events';
import Log from './log';

/**
* AudioProcessorEventObserver observes {@link AudioProcessor}
* related operations and re-emits them as generic events.
* @private
*/
export class AudioProcessorEventObserver extends EventEmitter {

private _log: Log = Log.getInstance();

constructor() {
super();
this._log.debug('Creating AudioProcessorEventObserver instance');
this.on('add', () => this._reEmitEvent('add'));
this.on('remove', () => this._reEmitEvent('remove'));
this.on('create', () => this._reEmitEvent('create-processed-stream'));
this.on('destroy', () => this._reEmitEvent('destroy-processed-stream'));
}

destroy(): void {
this.removeAllListeners();
}

private _reEmitEvent(name: string): void {
this._log.debug(`AudioProcessor:${name}`);
this.emit('event', { name, group: 'audio-processor' });
}
}
30 changes: 20 additions & 10 deletions lib/twilio/device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { EventEmitter } from 'events';
import { levels as LogLevels, LogLevelDesc } from 'loglevel';
import AudioHelper from './audiohelper';
import { AudioProcessorEventObserver } from './audioprocessoreventobserver';
import Call from './call';
import * as C from './constants';
import DialtonePlayer from './dialtonePlayer';
Expand Down Expand Up @@ -298,6 +299,11 @@ class Device extends EventEmitter {
*/
private _audio: AudioHelper | null = null;

/**
* The AudioProcessorEventObserver instance to use
*/
private _audioProcessorEventObserver: AudioProcessorEventObserver | null = null;

/**
* {@link Device._confirmClose} bound to the specific {@link Device} instance.
*/
Expand Down Expand Up @@ -589,13 +595,10 @@ class Device extends EventEmitter {
this.disconnectAll();
this._stopRegistrationTimer();

if (this._audio) {
this._audio._unbind();
}

this._destroyStream();
this._destroyPublisher();
this._destroyAudioHelper();
this._audioProcessorEventObserver?.destroy();

if (this._networkInformation && typeof this._networkInformation.removeEventListener === 'function') {
this._networkInformation.removeEventListener('change', this._publishNetworkChange);
Expand Down Expand Up @@ -887,8 +890,7 @@ class Device extends EventEmitter {
*/
private _destroyAudioHelper() {
if (!this._audio) { return; }

this._audio.removeAllListeners();
this._audio._destroy();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this why we didn't actually mark _destroy as a private function? Should we just remove the _?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. We cannot remove _ because that implies it's a customer API.

this._audio = null;
}

Expand Down Expand Up @@ -1325,21 +1327,29 @@ class Device extends EventEmitter {
* Set up an audio helper for usage by this {@link Device}.
*/
private _setupAudioHelper(): void {
if (!this._audioProcessorEventObserver) {
this._audioProcessorEventObserver = new AudioProcessorEventObserver();
this._audioProcessorEventObserver.on('event', ({ name, group }) => {
this._publisher.info(group, name, {}, this._activeCall);
});
}

const audioOptions: AudioHelper.Options = {
audioContext: Device.audioContext,
audioProcessorEventObserver: this._audioProcessorEventObserver,
enumerateDevices: this._options.enumerateDevices,
getUserMedia: this._options.getUserMedia || getUserMedia,
};

if (this._audio) {
this._log.info('Found existing audio helper; destroying...');
audioOptions.enabledSounds = this._audio._getEnabledSounds();
this._destroyAudioHelper();
this._log.info('Found existing audio helper; updating options...');
this._audio._updateUserOptions(audioOptions);
return;
}

this._audio = new (this._options.AudioHelper || AudioHelper)(
this._updateSinkIds,
this._updateInputStream,
this._options.getUserMedia || getUserMedia,
audioOptions,
);

Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.