diff --git a/package.json b/package.json index 9912c23..5c50f8c 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "dependencies": { "@webex/ts-events": "^1.1.0", "@webex/web-capabilities": "^1.1.0", - "@webex/web-media-effects": "^2.15.2", + "@webex/web-media-effects": "^2.15.6", "events": "^3.3.0", "js-logger": "^1.6.1", "typed-emitter": "^2.1.0", diff --git a/src/media/local-stream.spec.ts b/src/media/local-stream.spec.ts index 65d9dda..1d6bb05 100644 --- a/src/media/local-stream.spec.ts +++ b/src/media/local-stream.spec.ts @@ -56,6 +56,7 @@ describe('LocalStream', () => { effect = { id: 'test-id', kind: 'test-kind', + isEnabled: false, dispose: jest.fn().mockResolvedValue(undefined), load: jest.fn().mockResolvedValue(undefined), on: jest.fn(), @@ -76,11 +77,15 @@ describe('LocalStream', () => { expect(emitSpy).toHaveBeenCalledWith(effect); }); - it('should load and add multiple effects', async () => { + it('should load and add multiple effects with different IDs and kinds', async () => { expect.hasAssertions(); const firstEffect = effect; - const secondEffect = { ...effect, kind: 'another-kind' } as unknown as TrackEffect; + const secondEffect = { + ...effect, + id: 'another-id', + kind: 'another-kind', + } as unknown as TrackEffect; await localStream.addEffect(firstEffect); await localStream.addEffect(secondEffect); @@ -89,13 +94,13 @@ describe('LocalStream', () => { expect(emitSpy).toHaveBeenCalledTimes(2); }); - it('should throw an error if the effect is already added', async () => { + it('should not load an effect with the same ID twice', async () => { expect.hasAssertions(); await localStream.addEffect(effect); const secondAddEffectPromise = localStream.addEffect(effect); - await expect(secondAddEffectPromise).rejects.toBeInstanceOf(WebrtcCoreError); + await expect(secondAddEffectPromise).resolves.toBeUndefined(); // no-op expect(loadSpy).toHaveBeenCalledTimes(1); expect(localStream.getEffects()).toStrictEqual([effect]); expect(emitSpy).toHaveBeenCalledTimes(1); @@ -116,6 +121,20 @@ describe('LocalStream', () => { expect(emitSpy).toHaveBeenCalledTimes(1); }); + it('should replace the effect if an effect of the same kind is added after loading', async () => { + expect.hasAssertions(); + + const firstEffect = effect; + const secondEffect = { ...effect, id: 'another-id' } as unknown as TrackEffect; // same kind + await localStream.addEffect(firstEffect); + const secondAddEffectPromise = localStream.addEffect(secondEffect); + + await expect(secondAddEffectPromise).resolves.toBeUndefined(); + expect(loadSpy).toHaveBeenCalledTimes(2); + expect(localStream.getEffects()).toStrictEqual([secondEffect]); + expect(emitSpy).toHaveBeenCalledTimes(2); + }); + it('should throw an error if effects are cleared while loading', async () => { expect.hasAssertions(); diff --git a/src/media/local-stream.ts b/src/media/local-stream.ts index f20e781..190053e 100644 --- a/src/media/local-stream.ts +++ b/src/media/local-stream.ts @@ -1,6 +1,7 @@ import { AddEvents, TypedEvent, WithEventsDummyType } from '@webex/ts-events'; import { BaseEffect, EffectEvent } from '@webex/web-media-effects'; import { WebrtcCoreError, WebrtcCoreErrorType } from '../errors'; +import { logger } from '../util/logger'; import { Stream, StreamEventNames } from './stream'; export type TrackEffect = BaseEffect; @@ -148,11 +149,8 @@ abstract class _LocalStream extends Stream { */ async addEffect(effect: TrackEffect): Promise { // Check if the effect has already been added. - if (this.effects.includes(effect)) { - throw new WebrtcCoreError( - WebrtcCoreErrorType.ADD_EFFECT_FAILED, - `Effect ${effect.id} has already been added to this stream.` - ); + if (this.effects.some((e) => e.id === effect.id)) { + return; } // Load the effect. Because loading is asynchronous, keep track of the loading effects. @@ -172,21 +170,62 @@ abstract class _LocalStream extends Stream { } this.loadingEffects.delete(effect.kind); - // Add the effect to the effects list. - this.effects.push(effect); - - // When the effect's track is updated, update the next effect or output stream. - // TODO: using EffectEvent.TrackUpdated will cause the entire web-media-effects lib to be built - // and makes the size of the webrtc-core build much larger, so we use type assertion here as a - // temporary workaround. - effect.on('track-updated' as EffectEvent, (track: MediaStreamTrack) => { - const effectIndex = this.effects.indexOf(effect); + /** + * Handle when the effect's output track has been changed. This will update the input of the + * next effect in the effects list of the output of the stream. + * + * @param track - The new output track of the effect. + */ + const handleEffectTrackUpdated = (track: MediaStreamTrack) => { + const effectIndex = this.effects.findIndex((e) => e.id === effect.id); if (effectIndex === this.effects.length - 1) { this.changeOutputTrack(track); - } else { + } else if (effectIndex >= 0) { this.effects[effectIndex + 1]?.replaceInputTrack(track); + } else { + logger.error(`Effect with ID ${effect.id} not found in effects list.`); + } + }; + + /** + * Handle when the effect has been disposed. This will remove all event listeners from the + * effect. + */ + const handleEffectDisposed = () => { + effect.off('track-updated' as EffectEvent, handleEffectTrackUpdated); + effect.off('disposed' as EffectEvent, handleEffectDisposed); + }; + + // TODO: using EffectEvent.TrackUpdated or EffectEvent.Disposed will cause the entire + // web-media-effects lib to be rebuilt and inflates the size of the webrtc-core build, so + // we use type assertion here as a temporary workaround. + effect.on('track-updated' as EffectEvent, handleEffectTrackUpdated); + effect.on('disposed' as EffectEvent, handleEffectDisposed); + + // Add the effect to the effects list. If an effect of the same kind has already been added, + // dispose the existing effect and replace it with the new effect. If the existing effect was + // enabled, also enable the new effect. + const existingEffectIndex = this.effects.findIndex((e) => e.kind === effect.kind); + if (existingEffectIndex >= 0) { + const [existingEffect] = this.effects.splice(existingEffectIndex, 1, effect); + if (existingEffect.isEnabled) { + // If the existing effect is not the first effect in the effects list, then the input of the + // new effect should be the output of the previous effect in the effects list. We know the + // output track of the previous effect must exist because it must have been loaded (and all + // loaded effects have an output track). + const inputTrack = + existingEffectIndex === 0 + ? this.inputTrack + : (this.effects[existingEffectIndex - 1].getOutputTrack() as MediaStreamTrack); + await effect.replaceInputTrack(inputTrack); + // Enabling the new effect will trigger the track-updated event, which will handle the new + // effect's updated output track. + await effect.enable(); } - }); + await existingEffect.dispose(); + } else { + this.effects.push(effect); + } // Emit an event with the effect so others can listen to the effect events. this[LocalStreamEventNames.EffectAdded].emit(effect); @@ -203,13 +242,13 @@ abstract class _LocalStream extends Stream { } /** - * Get all the effects from the effects list with the given kind. + * Get an effect from the effects list by kind. * - * @param kind - The kind of the effects you want to get. - * @returns A list of effects. + * @param kind - The kind of the effect you want to get. + * @returns The effect or undefined. */ - getEffectsByKind(kind: string): TrackEffect[] { - return this.effects.filter((effect) => effect.kind === kind); + getEffectByKind(kind: string): TrackEffect | undefined { + return this.effects.find((effect) => effect.kind === kind); } /** diff --git a/yarn.lock b/yarn.lock index 61d7beb..0ab7de1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2687,10 +2687,10 @@ dependencies: bowser "^2.11.0" -"@webex/web-media-effects@^2.15.2": - version "2.15.2" - resolved "https://registry.yarnpkg.com/@webex/web-media-effects/-/web-media-effects-2.15.2.tgz#846efa44d8c88aeadad0856805662807e2f162c0" - integrity sha512-+a5DU45D0Evdl8fi8xx39tlHRhLWEHbsxB//co90q0MHH1Hxc6iNkKuqK1zl56iYIROhdwCrl1gt5h0cKHDEUA== +"@webex/web-media-effects@^2.15.6": + version "2.15.6" + resolved "https://registry.yarnpkg.com/@webex/web-media-effects/-/web-media-effects-2.15.6.tgz#9506963447a6c96435bd1b01acab0f87a6b7d7df" + integrity sha512-uK8sg14FtCwGvpRKi3guJuA6/I/qSI0v8lGblWvpYBqnfgNAki0pzHfJn6H1oiVK03zWqYQ3uzVDvS07hMAghg== dependencies: "@webex/ladon-ts" "^4.2.4" events "^3.3.0"