From 3074f5352aac992f604e7b33848aaf4f58f1cdac Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Tue, 10 May 2022 16:57:03 -0700 Subject: [PATCH 1/7] Clear srcObject in unbindVideoElement API --- CHANGELOG.md | 1 + docs/classes/defaultvideotilecontroller.html | 48 +++++++++---------- .../DefaultVideoTileController.ts | 10 +++- test/dommock/DOMMockBuilder.ts | 3 ++ .../DefaultVideoTileController.test.ts | 19 ++++++++ 5 files changed, 56 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c76d763da..d93724a9e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### Changed +- Clear srcObject in unbindVideoElement API. ### Fixed - Fix issue where video resolution and framerate changes when toggle video transform. diff --git a/docs/classes/defaultvideotilecontroller.html b/docs/classes/defaultvideotilecontroller.html index 66530a65d6..c2475a1e11 100644 --- a/docs/classes/defaultvideotilecontroller.html +++ b/docs/classes/defaultvideotilecontroller.html @@ -138,7 +138,7 @@

constructor

  • Parameters

    @@ -167,7 +167,7 @@

    keepLastFrameWhenPaused

    @@ -184,7 +184,7 @@

    addVideoTile

  • Parameters

    @@ -207,7 +207,7 @@

    bindVideoElement

  • Parameters

    @@ -234,7 +234,7 @@

    captureVideoTile

    Parameters

    @@ -258,7 +258,7 @@

    getAllRemoteVideoTiles

    Returns VideoTile[]

    @@ -276,7 +276,7 @@

    getAllVideoTiles

    Returns VideoTile[]

    @@ -294,7 +294,7 @@

    getLocalVideoTile

    Returns VideoTile | null

    @@ -312,7 +312,7 @@

    getVideoTile

    Parameters

    @@ -336,7 +336,7 @@

    getVideoTileArea

    Parameters

    @@ -360,7 +360,7 @@

    getVideoTileForAttendeeId

    Parameters

    @@ -384,7 +384,7 @@

    hasStartedLocalVideoTile

    Returns boolean

    @@ -402,7 +402,7 @@

    haveVideoTileForAttendeeId

    Parameters

    @@ -426,7 +426,7 @@

    haveVideoTilesWithStreams

    Returns boolean

    @@ -444,7 +444,7 @@

    pauseVideoTile

    Parameters

    @@ -468,7 +468,7 @@

    removeAllVideoTiles

    Returns void

    @@ -486,7 +486,7 @@

    removeLocalVideoTile

    Returns void

    @@ -504,7 +504,7 @@

    removeVideoTile

    Parameters

    @@ -528,7 +528,7 @@

    removeVideoTilesByAttendeeId

    Parameters

    @@ -552,7 +552,7 @@

    sendTileStateUpdate

    Parameters

    @@ -576,7 +576,7 @@

    startLocalVideoTile

    Returns number

    @@ -594,7 +594,7 @@

    stopLocalVideoTile

    Returns void

    @@ -612,7 +612,7 @@

    unbindVideoElement

    Parameters

    @@ -636,7 +636,7 @@

    unpauseVideoTile

    Parameters

    diff --git a/src/videotilecontroller/DefaultVideoTileController.ts b/src/videotilecontroller/DefaultVideoTileController.ts index c2e35ce3fa..9e274b9bb4 100644 --- a/src/videotilecontroller/DefaultVideoTileController.ts +++ b/src/videotilecontroller/DefaultVideoTileController.ts @@ -7,6 +7,7 @@ import DefaultDevicePixelRatioMonitor from '../devicepixelratiomonitor/DefaultDe import DevicePixelRatioWindowSource from '../devicepixelratiosource/DevicePixelRatioWindowSource'; import Logger from '../logger/Logger'; import { Maybe } from '../utils/Types'; +import DefaultVideoTile from '../videotile/DefaultVideoTile'; import VideoTile from '../videotile/VideoTile'; import VideoTileState from '../videotile/VideoTileState'; import VideoTileFactory from '../videotilefactory/VideoTileFactory'; @@ -57,7 +58,14 @@ export default class DefaultVideoTileController implements VideoTileController { } unbindVideoElement(tileId: number): void { - this.bindVideoElement(tileId, null); + const tile = this.getVideoTile(tileId); + if (tile === null) { + this.logger.warn(`Ignoring video element un-binding for unknown tile id ${tileId}`); + return; + } + const videoElement = tile.stateRef().boundVideoElement; + tile.bindVideoElement(null); + DefaultVideoTile.disconnectVideoStreamFromVideoElement(videoElement, false); } startLocalVideoTile(): number { diff --git a/test/dommock/DOMMockBuilder.ts b/test/dommock/DOMMockBuilder.ts index 1241b1ce5b..4ef313eaa3 100644 --- a/test/dommock/DOMMockBuilder.ts +++ b/test/dommock/DOMMockBuilder.ts @@ -1327,6 +1327,9 @@ export default class DOMMockBuilder { videoHeight: number; videoWidth: number; private listeners: { [type: string]: MockListener[] } = {}; + style: { [key: string]: string } = { + transform: '', + }; private clearAttribute(): void { this.videoHeight = 0; diff --git a/test/videotilecontroller/DefaultVideoTileController.test.ts b/test/videotilecontroller/DefaultVideoTileController.test.ts index ac221cb7e5..82049d916e 100644 --- a/test/videotilecontroller/DefaultVideoTileController.test.ts +++ b/test/videotilecontroller/DefaultVideoTileController.test.ts @@ -117,6 +117,25 @@ describe('DefaultVideoTileController', () => { tileController.bindVideoElement(tileId, videoElement); tileController.unbindVideoElement(tileId); }); + + it('clears srcObject with unbinding a video element', () => { + let videoElement = document.createElement('video'); + const tileId = tileController.addVideoTile().id(); + tileController.bindVideoElement(tileId, videoElement); + const tile = tileController.getVideoTile(tileId); + tile.bindVideoStream('attendee', false, mockMediaStream, 1, 1, 1); + expect(videoElement.srcObject).to.eq(mockMediaStream); + tileController.unbindVideoElement(tileId); + expect(videoElement.srcObject).to.eq(null); + videoElement = null; + }); + + it('ignores if no tile bound to a tileId', () => { + const loggerSpy = sinon.spy(audioVideoController.logger, 'warn'); + tileController.unbindVideoElement(0); + expect(loggerSpy.calledWith('Ignoring video element un-binding for unknown tile id 0')).to.be + .true; + }); }); describe('startLocalVideoTile', () => { From 0a8066f9703feef616ad1d3e2d6e9925b93115cf Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Tue, 10 May 2022 17:25:25 -0700 Subject: [PATCH 2/7] Cover POSTLogger unload event code path in unit test --- test/logger/POSTLogger.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/logger/POSTLogger.test.ts b/test/logger/POSTLogger.test.ts index 37fd8fc381..6d4ee120f9 100644 --- a/test/logger/POSTLogger.test.ts +++ b/test/logger/POSTLogger.test.ts @@ -278,16 +278,16 @@ describe('POSTLogger', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const GlobalAny = global as any; let added = false; - let callbackToCall = (): void => {}; - GlobalAny['window']['addEventListener'] = (type: string, callback: () => void) => { + let callbackToCall = (_e: Event): void => {}; + GlobalAny['window']['addEventListener'] = (type: string, callback: (e: Event) => void) => { expect(type).to.equal('unload'); added = true; callbackToCall = callback; }; + const logger = new POSTLogger({ url, batchSize, intervalMs }); await wait(60); - callbackToCall(); + callbackToCall(new Event('unload')); await wait(80); - const logger = new POSTLogger({ url, batchSize, intervalMs }); expect(added).to.be.true; delete GlobalAny['window']['addEventListener']; await logger.destroy(); From 1fe4a5c28dcd0ec4c63faa915cf2d38cdc22c8ce Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Wed, 11 May 2022 09:30:32 -0700 Subject: [PATCH 3/7] Update unbindVideoElement documentation --- CHANGELOG.md | 3 ++- docs/interfaces/videotilecontroller.html | 2 +- src/videotilecontroller/VideoTileController.ts | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d93724a9e3..ff1cd64798 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### Changed -- Clear srcObject in unbindVideoElement API. + +- Clean up the HTML video element bound using `bindVideoElement` as part of `unbindVideoElement` API to fix Safari memory leak. Check [PR#2217](https://github.com/aws/amazon-chime-sdk-js/pull/2217) for detailed information. ### Fixed - Fix issue where video resolution and framerate changes when toggle video transform. diff --git a/docs/interfaces/videotilecontroller.html b/docs/interfaces/videotilecontroller.html index 8f8033d216..d29543b9d1 100644 --- a/docs/interfaces/videotilecontroller.html +++ b/docs/interfaces/videotilecontroller.html @@ -679,7 +679,7 @@

    unbindVideoElement

    Unbinds the video element from the tile if it exists for the provided tileId. - The video tile's bounded video element and that element's width and height are set to null. + The video tile's bounded video element, that element's srcObject, width and height are set to null. This does not remove the provided tileId mapping from the tile map in the DefaultVideoTileController. To remove the mapping and destroy the tile for this tileId, you can use removeVideoTile.

    diff --git a/src/videotilecontroller/VideoTileController.ts b/src/videotilecontroller/VideoTileController.ts index 203c0e71e8..16f9bc4205 100644 --- a/src/videotilecontroller/VideoTileController.ts +++ b/src/videotilecontroller/VideoTileController.ts @@ -22,7 +22,7 @@ export default interface VideoTileController { /** * Unbinds the video element from the tile if it exists for the provided tileId. - * The video tile's bounded video element and that element's width and height are set to null. + * The video tile's bounded video element, that element's srcObject, width and height are set to null. * This does not remove the provided tileId mapping from the tile map in the [[DefaultVideoTileController]]. * To remove the mapping and destroy the tile for this tileId, you can use [[removeVideoTile]]. */ From 2f402ce7844d02dde6ae2167697c76b9cbe83333 Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Wed, 11 May 2022 13:24:30 -0700 Subject: [PATCH 4/7] Add control flag for the srcObject clearing - Update the unbindVideoElement API to take `cleanUpVideoElement` as an optional parameter with default set to true. - When cleanUpVideoElement is true, bounded video element's srcObject is set to null. This helps in memory leaks. The stream wont remain bound to the video element once unbound. - When cleanUpVideoElement is false, the stream remains bound to the video element once unbound. - Since, this is a change in behavior the control flag is added. - Update documentation to match v3 for some methods. - Add corresponding unit tests. --- docs/classes/defaultvideotile.html | 35 ++++++------ docs/classes/defaultvideotilecontroller.html | 45 ++++++++------- docs/interfaces/videotilecontroller.html | 57 +++++++++++-------- src/videotile/DefaultVideoTile.ts | 5 +- .../DefaultVideoTileController.ts | 14 +++-- .../VideoTileController.ts | 6 +- .../DefaultVideoTileController.test.ts | 14 ++++- 7 files changed, 102 insertions(+), 74 deletions(-) diff --git a/docs/classes/defaultvideotile.html b/docs/classes/defaultvideotile.html index 8116b8630e..81be0e58ed 100644 --- a/docs/classes/defaultvideotile.html +++ b/docs/classes/defaultvideotile.html @@ -126,7 +126,7 @@

    constructor

  • Parameters

    @@ -162,7 +162,7 @@

    bindVideoElement

    Parameters

    @@ -186,7 +186,7 @@

    bindVideoStream

    Parameters

    @@ -228,7 +228,7 @@

    capture

    Returns ImageData | null

    @@ -246,7 +246,7 @@

    destroy

    Returns void

    @@ -264,7 +264,7 @@

    devicePixelRatioChanged

    Parameters

    @@ -288,7 +288,7 @@

    id

    Returns number

    @@ -306,7 +306,7 @@

    markPoorConnection

    Returns boolean

    @@ -324,7 +324,7 @@

    pause

    Returns void

    @@ -342,7 +342,7 @@

    setStreamId

    Parameters

    @@ -366,7 +366,7 @@

    state

    Returns VideoTileState

    @@ -384,7 +384,7 @@

    stateRef

    Returns VideoTileState

    @@ -402,7 +402,7 @@

    unmarkPoorConnection

    Returns boolean

    @@ -420,7 +420,7 @@

    unpause

    Returns void

    @@ -480,13 +480,12 @@

    Static disconnectVideo
  • -

    Disconnect a video stream to a video element by clearing the srcObject of the video element. - This will also stop all the tracks of the current stream in the srcObject.

    +

    Disconnect a video stream from a video element by clearing the srcObject of the video element.

    Parameters

    @@ -501,7 +500,7 @@
    videoElement: HTMLVideoElementdueToPause: boolean

    A flag to indicate whether this function is called due to pausing video tile. - If true, then we will not stop the stream's tracks and just clearing out the srcObject.

    + Based on keepLastFrameWhenPaused, it clears out the videoElement's srcObject.

  • diff --git a/docs/classes/defaultvideotilecontroller.html b/docs/classes/defaultvideotilecontroller.html index c2475a1e11..05908b8e6c 100644 --- a/docs/classes/defaultvideotilecontroller.html +++ b/docs/classes/defaultvideotilecontroller.html @@ -184,7 +184,7 @@

    addVideoTile

  • Parameters

    @@ -234,7 +234,7 @@

    captureVideoTile

    Parameters

    @@ -258,7 +258,7 @@

    getAllRemoteVideoTiles

    Returns VideoTile[]

    @@ -276,7 +276,7 @@

    getAllVideoTiles

    Returns VideoTile[]

    @@ -294,7 +294,7 @@

    getLocalVideoTile

    Returns VideoTile | null

    @@ -312,7 +312,7 @@

    getVideoTile

    Parameters

    @@ -336,7 +336,7 @@

    getVideoTileArea

    Parameters

    @@ -360,7 +360,7 @@

    getVideoTileForAttendeeId

    Parameters

    @@ -384,7 +384,7 @@

    hasStartedLocalVideoTile

    Returns boolean

    @@ -402,7 +402,7 @@

    haveVideoTileForAttendeeId

    Parameters

    @@ -426,7 +426,7 @@

    haveVideoTilesWithStreams

    Returns boolean

    @@ -444,7 +444,7 @@

    pauseVideoTile

    Parameters

    @@ -468,7 +468,7 @@

    removeAllVideoTiles

    Returns void

    @@ -486,7 +486,7 @@

    removeLocalVideoTile

    Returns void

    @@ -504,7 +504,7 @@

    removeVideoTile

    Parameters

    @@ -528,7 +528,7 @@

    removeVideoTilesByAttendeeId

    Parameters

    @@ -552,7 +552,7 @@

    sendTileStateUpdate

    Parameters

    @@ -576,7 +576,7 @@

    startLocalVideoTile

    Returns number

    @@ -594,7 +594,7 @@

    stopLocalVideoTile

    Returns void

    @@ -605,7 +605,7 @@

    Returns void

    unbindVideoElement

      -
    • unbindVideoElement(tileId: number): void
      • +
    • unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void
    • @@ -620,6 +620,9 @@

      Parameters

    • tileId: number
      • +
    • +
      Default value cleanUpVideoElement: boolean = true
      +

    Returns void

    • @@ -636,7 +639,7 @@

    unpauseVideoTile

    Parameters

    diff --git a/docs/interfaces/videotilecontroller.html b/docs/interfaces/videotilecontroller.html index d29543b9d1..9d4f2b2908 100644 --- a/docs/interfaces/videotilecontroller.html +++ b/docs/interfaces/videotilecontroller.html @@ -155,7 +155,7 @@

    addVideoTile

  • @@ -210,7 +210,7 @@

    Optional captureVideo
    @@ -238,7 +238,7 @@

    getAllRemoteVideoTiles

  • @@ -260,7 +260,7 @@

    getAllVideoTiles

  • @@ -282,7 +282,7 @@

    getLocalVideoTile

  • @@ -304,7 +304,7 @@

    getVideoTile

  • @@ -332,7 +332,7 @@

    getVideoTileArea

  • @@ -360,7 +360,7 @@

    Optional getVideoTil
  • @@ -391,7 +391,7 @@

    hasStartedLocalVideoTile

  • @@ -413,7 +413,7 @@

    haveVideoTileForAttendeeId

  • @@ -441,7 +441,7 @@

    haveVideoTilesWithStreams

  • @@ -463,7 +463,7 @@

    pauseVideoTile

  • @@ -495,7 +495,7 @@

    removeAllVideoTiles

  • @@ -517,7 +517,7 @@

    removeLocalVideoTile

  • @@ -540,7 +540,7 @@

    removeVideoTile

  • @@ -570,7 +570,7 @@

    removeVideoTilesByAttendeeId

  • @@ -599,7 +599,7 @@

    sendTileStateUpdate

  • @@ -627,7 +627,7 @@

    startLocalVideoTile

  • @@ -650,7 +650,7 @@

    stopLocalVideoTile

  • @@ -667,21 +667,19 @@

    Returns void

    unbindVideoElement

      -
    • unbindVideoElement(tileId: number): void
      • +
    • unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void

    • Unbinds the video element from the tile if it exists for the provided tileId. - The video tile's bounded video element, that element's srcObject, width and height are set to null. - This does not remove the provided tileId mapping from the tile map in the DefaultVideoTileController. - To remove the mapping and destroy the tile for this tileId, you can use removeVideoTile.

      + The video tile's bounded video element and that element's width and height are set to null.

      Parameters

      @@ -689,6 +687,15 @@

      Parameters

    • tileId: number
      • +
    • +
      Optional cleanUpVideoElement: boolean
      +
      +

      By default, the bounded video element's srcObject is also set to null using disconnectVideoStreamFromVideoElement. + Pass false for cleanUpVideoElement, if you do not intend to clear the bounded video element's srcObject. + This does not remove the provided tileId mapping from the tile map in the DefaultVideoTileController. + To remove the mapping and destroy the tile for this tileId, you can use removeVideoTile.

      +
      +

    Returns void

    • @@ -704,7 +711,7 @@

    unpauseVideoTile

  • diff --git a/src/videotile/DefaultVideoTile.ts b/src/videotile/DefaultVideoTile.ts index f3e469a118..6d118bbd01 100644 --- a/src/videotile/DefaultVideoTile.ts +++ b/src/videotile/DefaultVideoTile.ts @@ -62,11 +62,10 @@ export default class DefaultVideoTile implements DevicePixelRatioObserver, Video } /** - * Disconnect a video stream to a video element by clearing the srcObject of the video element. - * This will also stop all the tracks of the current stream in the srcObject. + * Disconnect a video stream from a video element by clearing the srcObject of the video element. * @param videoElement The video element input. * @param dueToPause A flag to indicate whether this function is called due to pausing video tile. - * If true, then we will not stop the stream's tracks and just clearing out the srcObject. + * Based on keepLastFrameWhenPaused, it clears out the videoElement's srcObject. * @param keepLastFrameWhenPaused If true and dueToPause is also true, then we will not clear out the srcObject of the * video element when it is paused and therefore, the last frame of the stream will be shown. */ diff --git a/src/videotilecontroller/DefaultVideoTileController.ts b/src/videotilecontroller/DefaultVideoTileController.ts index 9e274b9bb4..a4f0208a07 100644 --- a/src/videotilecontroller/DefaultVideoTileController.ts +++ b/src/videotilecontroller/DefaultVideoTileController.ts @@ -57,15 +57,21 @@ export default class DefaultVideoTileController implements VideoTileController { tile.bindVideoElement(videoElement); } - unbindVideoElement(tileId: number): void { + unbindVideoElement(tileId: number, cleanUpVideoElement: boolean = true): void { const tile = this.getVideoTile(tileId); if (tile === null) { this.logger.warn(`Ignoring video element un-binding for unknown tile id ${tileId}`); return; } - const videoElement = tile.stateRef().boundVideoElement; - tile.bindVideoElement(null); - DefaultVideoTile.disconnectVideoStreamFromVideoElement(videoElement, false); + if (cleanUpVideoElement) { + this.logger.info(`Unbinding and cleaning up the video element`); + const videoElement = tile.stateRef().boundVideoElement; + tile.bindVideoElement(null); + DefaultVideoTile.disconnectVideoStreamFromVideoElement(videoElement, false); + } else { + this.logger.info(`Unbinding the video element`); + tile.bindVideoElement(null); + } } startLocalVideoTile(): number { diff --git a/src/videotilecontroller/VideoTileController.ts b/src/videotilecontroller/VideoTileController.ts index 16f9bc4205..e6d07b63fe 100644 --- a/src/videotilecontroller/VideoTileController.ts +++ b/src/videotilecontroller/VideoTileController.ts @@ -22,11 +22,13 @@ export default interface VideoTileController { /** * Unbinds the video element from the tile if it exists for the provided tileId. - * The video tile's bounded video element, that element's srcObject, width and height are set to null. + * The video tile's bounded video element and that element's width and height are set to null. + * @param cleanUpVideoElement By default, the bounded video element's srcObject is also set to null using [[disconnectVideoStreamFromVideoElement]]. + * Pass false for cleanUpVideoElement, if you do not intend to clear the bounded video element's srcObject. * This does not remove the provided tileId mapping from the tile map in the [[DefaultVideoTileController]]. * To remove the mapping and destroy the tile for this tileId, you can use [[removeVideoTile]]. */ - unbindVideoElement(tileId: number): void; + unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void; /** * Starts sharing the local video tile by creating a new video tile if one does not already exist. diff --git a/test/videotilecontroller/DefaultVideoTileController.test.ts b/test/videotilecontroller/DefaultVideoTileController.test.ts index 82049d916e..2bc54ffaff 100644 --- a/test/videotilecontroller/DefaultVideoTileController.test.ts +++ b/test/videotilecontroller/DefaultVideoTileController.test.ts @@ -118,7 +118,7 @@ describe('DefaultVideoTileController', () => { tileController.unbindVideoElement(tileId); }); - it('clears srcObject with unbinding a video element', () => { + it('default - clears srcObject with unbindVideoElement', () => { let videoElement = document.createElement('video'); const tileId = tileController.addVideoTile().id(); tileController.bindVideoElement(tileId, videoElement); @@ -130,6 +130,18 @@ describe('DefaultVideoTileController', () => { videoElement = null; }); + it('does not clear srcObject with unbindVideoElement when cleanUpVideoElement parameter is false', () => { + let videoElement = document.createElement('video'); + const tileId = tileController.addVideoTile().id(); + tileController.bindVideoElement(tileId, videoElement); + const tile = tileController.getVideoTile(tileId); + tile.bindVideoStream('attendee', false, mockMediaStream, 1, 1, 1); + expect(videoElement.srcObject).to.eq(mockMediaStream); + tileController.unbindVideoElement(tileId, false); + expect(videoElement.srcObject).to.eq(mockMediaStream); + videoElement = null; + }); + it('ignores if no tile bound to a tileId', () => { const loggerSpy = sinon.spy(audioVideoController.logger, 'warn'); tileController.unbindVideoElement(0); From f944041c5923d8e5b7edc3777ebfee4df8cd9be2 Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Wed, 11 May 2022 14:16:53 -0700 Subject: [PATCH 5/7] Update AudioVideoFacade - Update docs. - Tested in video_test app. - Updated unit tests. --- CHANGELOG.md | 2 +- docs/classes/defaultaudiovideofacade.html | 5 ++++- docs/interfaces/audiovideofacade.html | 5 ++++- docs/interfaces/videotilecontrollerfacade.html | 5 ++++- docs/modules/apioverview.html | 2 +- guides/03_API_Overview.md | 2 +- integration/js/app/package-lock.json | 2 +- src/audiovideofacade/DefaultAudioVideoFacade.ts | 6 +++--- src/videotilecontroller/DefaultVideoTileController.ts | 2 +- src/videotilecontroller/VideoTileControllerFacade.ts | 2 +- test/audiovideofacade/DefaultAudioVideoFacade.test.ts | 11 +++++++++-- .../DefaultVideoTileController.test.ts | 2 +- 12 files changed, 31 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff1cd64798..4a7d98c2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Clean up the HTML video element bound using `bindVideoElement` as part of `unbindVideoElement` API to fix Safari memory leak. Check [PR#2217](https://github.com/aws/amazon-chime-sdk-js/pull/2217) for detailed information. +- Clean up the HTML video element bounded to `VideoTileState` using `unbindVideoElement` API to fix Safari memory leak. If you do not intend to clean the video element, call `unbindVideoElement` API with `cleanUpVideoElement` set to `false`. Check [PR#2217](https://github.com/aws/amazon-chime-sdk-js/pull/2217) for detailed information. ### Fixed - Fix issue where video resolution and framerate changes when toggle video transform. diff --git a/docs/classes/defaultaudiovideofacade.html b/docs/classes/defaultaudiovideofacade.html index f53a1ed59a..dbe094c142 100644 --- a/docs/classes/defaultaudiovideofacade.html +++ b/docs/classes/defaultaudiovideofacade.html @@ -2236,7 +2236,7 @@

    Returns void

    unbindVideoElement

      -
    • unbindVideoElement(tileId: number): void
      • +
    • unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void
    • @@ -2251,6 +2251,9 @@

      Parameters

    • tileId: number
      • +
    • +
      Default value cleanUpVideoElement: boolean = true
      +

    Returns void

    • diff --git a/docs/interfaces/audiovideofacade.html b/docs/interfaces/audiovideofacade.html index faf580ff4b..4ef0508cb9 100644 --- a/docs/interfaces/audiovideofacade.html +++ b/docs/interfaces/audiovideofacade.html @@ -2426,7 +2426,7 @@

    Returns void

    unbindVideoElement

      -
    • unbindVideoElement(tileId: number): void
      • +
    • unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void
    • @@ -2441,6 +2441,9 @@

      Parameters

    • tileId: number
      • +
    • +
      Optional cleanUpVideoElement: boolean
      +

    Returns void

    • diff --git a/docs/interfaces/videotilecontrollerfacade.html b/docs/interfaces/videotilecontrollerfacade.html index c7c4e3272e..6ac3f51a82 100644 --- a/docs/interfaces/videotilecontrollerfacade.html +++ b/docs/interfaces/videotilecontrollerfacade.html @@ -408,7 +408,7 @@

    Returns void

    unbindVideoElement

      -
    • unbindVideoElement(tileId: number): void
      • +
    • unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void
    • @@ -422,6 +422,9 @@

      Parameters

    • tileId: number
      • +
    • +
      Optional cleanUpVideoElement: boolean
      +

    Returns void

    diff --git a/docs/modules/apioverview.html b/docs/modules/apioverview.html index aa742aff14..948904f48a 100644 --- a/docs/modules/apioverview.html +++ b/docs/modules/apioverview.html @@ -318,7 +318,7 @@

    7a. Share local video

    7b. Display local and remote video

    You are responsible for maintaining HTMLVideoElement objects in the DOM and arranging their layout within the web page. To display a video, you must handle the videoTileDidUpdate and videoTileWasRemoved callbacks in an AudioVideoObserver. In the implementation of videoTileDidUpdate, bind the tile ID from the provided VideoTileState with the HTMLVideoElement in your DOM by calling meetingSession.audioVideo.bindVideoElement(tileId, videoElement).

    -

    To unbind a tile, call meetingSession.audioVideo.unbindVideoElement(tileId).

    +

    To unbind a tile, call meetingSession.audioVideo.unbindVideoElement(tileId). Note that, this will also cleanup the HTML video element's srcObject by default. Call unbindVideoElement(tileId, false) to avoid the video element cleanup. Check this PR description for more details.

    A tileId is a unique identifier representing a video stream. When you stop and start, it generates a new tileId. You can have tileIds exceeding 25; they merely identify a particular stream uniquely. When you start video it consumes a video publishing slot, when you stop video it releases that video publishing slot. Pausing does not affect video publishing slots; it allows a remote to choose to not receive a video stream (and thus not consume bandwidth and CPU for that stream).

    7c. Pause and unpause video (optional)

    diff --git a/guides/03_API_Overview.md b/guides/03_API_Overview.md index 5f1fb926bc..46175d5cd9 100644 --- a/guides/03_API_Overview.md +++ b/guides/03_API_Overview.md @@ -300,7 +300,7 @@ To stop sharing video with others, call meetingSession.audioVideo.[stopLocalVide You are responsible for maintaining HTMLVideoElement objects in the DOM and arranging their layout within the web page. To display a video, you must handle the [videoTileDidUpdate](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotiledidupdate) and [videoTileWasRemoved](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotilewasremoved) callbacks in an [AudioVideoObserver](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html). In the implementation of [videoTileDidUpdate](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotiledidupdate), bind the tile ID from the provided VideoTileState with the HTMLVideoElement in your DOM by calling meetingSession.audioVideo.[bindVideoElement(tileId, videoElement)](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#bindvideoelement). -To unbind a tile, call meetingSession.audioVideo.[unbindVideoElement(tileId)](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#unbindvideoelement). +To unbind a tile, call meetingSession.audioVideo.[unbindVideoElement(tileId)](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#unbindvideoelement). Note that, this will also cleanup the HTML video element's `srcObject` by default. Call `unbindVideoElement(tileId, false)` to avoid the video element cleanup. Check [this PR](https://github.com/aws/amazon-chime-sdk-js/pull/2217) description for more details. A `tileId` is a unique identifier representing a video stream. When you stop and start, it generates a new `tileId`. You can have tileIds exceeding 25; they merely identify a particular stream uniquely. When you start video it consumes a video publishing slot, when you stop video it releases that video publishing slot. Pausing does not affect video publishing slots; it allows a remote to choose to not receive a video stream (and thus not consume bandwidth and CPU for that stream). diff --git a/integration/js/app/package-lock.json b/integration/js/app/package-lock.json index ac50212e58..197f67b731 100644 --- a/integration/js/app/package-lock.json +++ b/integration/js/app/package-lock.json @@ -21,7 +21,7 @@ } }, "../../..": { - "version": "3.0.0-beta.2", + "version": "3.2.0", "license": "Apache-2.0", "dependencies": { "@aws-crypto/sha256-js": "^2.0.1", diff --git a/src/audiovideofacade/DefaultAudioVideoFacade.ts b/src/audiovideofacade/DefaultAudioVideoFacade.ts index 22af165722..9dc024ef65 100644 --- a/src/audiovideofacade/DefaultAudioVideoFacade.ts +++ b/src/audiovideofacade/DefaultAudioVideoFacade.ts @@ -105,9 +105,9 @@ export default class DefaultAudioVideoFacade implements AudioVideoFacade, AudioV this.trace('bindVideoElement', { tileId: tileId, videoElementId: videoElement.id }); } - unbindVideoElement(tileId: number): void { - this.videoTileController.unbindVideoElement(tileId); - this.trace('unbindVideoElement', tileId); + unbindVideoElement(tileId: number, cleanUpVideoElement: boolean = true): void { + this.videoTileController.unbindVideoElement(tileId, cleanUpVideoElement); + this.trace('unbindVideoElement', { tileId: tileId, cleanUpVideoElement: cleanUpVideoElement }); } startLocalVideoTile(): number { diff --git a/src/videotilecontroller/DefaultVideoTileController.ts b/src/videotilecontroller/DefaultVideoTileController.ts index a4f0208a07..71fff64f4e 100644 --- a/src/videotilecontroller/DefaultVideoTileController.ts +++ b/src/videotilecontroller/DefaultVideoTileController.ts @@ -60,7 +60,7 @@ export default class DefaultVideoTileController implements VideoTileController { unbindVideoElement(tileId: number, cleanUpVideoElement: boolean = true): void { const tile = this.getVideoTile(tileId); if (tile === null) { - this.logger.warn(`Ignoring video element un-binding for unknown tile id ${tileId}`); + this.logger.warn(`Ignoring video element unbinding for unknown tile id ${tileId}`); return; } if (cleanUpVideoElement) { diff --git a/src/videotilecontroller/VideoTileControllerFacade.ts b/src/videotilecontroller/VideoTileControllerFacade.ts index 46cfaaef9a..3eb4055eae 100644 --- a/src/videotilecontroller/VideoTileControllerFacade.ts +++ b/src/videotilecontroller/VideoTileControllerFacade.ts @@ -5,7 +5,7 @@ import VideoTile from '../videotile/VideoTile'; export default interface VideoTileControllerFacade { bindVideoElement(tileId: number, videoElement: HTMLVideoElement): void; - unbindVideoElement(tileId: number): void; + unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void; startLocalVideoTile(): number; stopLocalVideoTile(): void; hasStartedLocalVideoTile(): boolean; diff --git a/test/audiovideofacade/DefaultAudioVideoFacade.test.ts b/test/audiovideofacade/DefaultAudioVideoFacade.test.ts index 1a97756604..a892477399 100644 --- a/test/audiovideofacade/DefaultAudioVideoFacade.test.ts +++ b/test/audiovideofacade/DefaultAudioVideoFacade.test.ts @@ -160,11 +160,18 @@ describe('DefaultAudioVideoFacade', () => { assert(spy.calledOnceWith(arg1, arg2)); }); - it('will call unbindVideoElement', () => { + it('will call unbindVideoElement with cleanUpVideoElement defaulting to true', () => { const spy = sinon.spy(controller.videoTileController, 'unbindVideoElement'); const arg1 = 0; facade.unbindVideoElement(arg1); - assert(spy.calledOnceWith(arg1)); + assert(spy.calledOnceWith(arg1, true)); + }); + + it('will call unbindVideoElement with cleanUpVideoElement as false', () => { + const spy = sinon.spy(controller.videoTileController, 'unbindVideoElement'); + const arg1 = 0; + facade.unbindVideoElement(arg1, false); + assert(spy.calledOnceWith(arg1, false)); }); it('will call startLocalVideoTile', () => { diff --git a/test/videotilecontroller/DefaultVideoTileController.test.ts b/test/videotilecontroller/DefaultVideoTileController.test.ts index 2bc54ffaff..8825e3feec 100644 --- a/test/videotilecontroller/DefaultVideoTileController.test.ts +++ b/test/videotilecontroller/DefaultVideoTileController.test.ts @@ -145,7 +145,7 @@ describe('DefaultVideoTileController', () => { it('ignores if no tile bound to a tileId', () => { const loggerSpy = sinon.spy(audioVideoController.logger, 'warn'); tileController.unbindVideoElement(0); - expect(loggerSpy.calledWith('Ignoring video element un-binding for unknown tile id 0')).to.be + expect(loggerSpy.calledWith('Ignoring video element unbinding for unknown tile id 0')).to.be .true; }); }); From 823bff582cdc8f82323a66304a612fc6314d334c Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Wed, 11 May 2022 15:34:51 -0700 Subject: [PATCH 6/7] Address feedback on documentation --- docs/classes/defaultvideotile.html | 8 ++++---- docs/interfaces/videotilecontroller.html | 12 ++++++------ docs/modules/apioverview.html | 2 +- guides/03_API_Overview.md | 2 +- src/videotile/DefaultVideoTile.ts | 8 ++++---- src/videotilecontroller/VideoTileController.ts | 12 ++++++------ 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/docs/classes/defaultvideotile.html b/docs/classes/defaultvideotile.html index 81be0e58ed..7a1b9f0e0a 100644 --- a/docs/classes/defaultvideotile.html +++ b/docs/classes/defaultvideotile.html @@ -485,7 +485,7 @@

    Static disconnectVideo
    -

    Disconnect a video stream from a video element by clearing the srcObject of the video element.

    +

    Disconnect a video stream from a video element by setting HTMLVideoElement.srcObject to null.

    Parameters

    @@ -500,14 +500,14 @@
    videoElement: HTMLVideoElementdueToPause: boolean

    A flag to indicate whether this function is called due to pausing video tile. - Based on keepLastFrameWhenPaused, it clears out the videoElement's srcObject.

    + Based on keepLastFrameWhenPaused, it sets HTMLVideoElement.srcObject to null.

  • Default value keepLastFrameWhenPaused: boolean | undefined = false
    -

    If true and dueToPause is also true, then we will not clear out the srcObject of the - video element when it is paused and therefore, the last frame of the stream will be shown.

    +

    If true and dueToPause is also true, then we will not set HTMLVideoElement.srcObject of the + video element to null when it is paused and therefore, the last frame of the stream will be shown.

    • diff --git a/docs/interfaces/videotilecontroller.html b/docs/interfaces/videotilecontroller.html index 9d4f2b2908..b5b49b61ac 100644 --- a/docs/interfaces/videotilecontroller.html +++ b/docs/interfaces/videotilecontroller.html @@ -678,8 +678,8 @@

    unbindVideoElement

    -

    Unbinds the video element from the tile if it exists for the provided tileId. - The video tile's bounded video element and that element's width and height are set to null.

    +

    Unbinds the video element from the tile if it exists for the provided tileId. + The video tile's bounded video element and that element's width and height are set to null.

    Parameters

    @@ -690,10 +690,10 @@
    tileId: number
  • Optional cleanUpVideoElement: boolean
    -

    By default, the bounded video element's srcObject is also set to null using disconnectVideoStreamFromVideoElement. - Pass false for cleanUpVideoElement, if you do not intend to clear the bounded video element's srcObject. - This does not remove the provided tileId mapping from the tile map in the DefaultVideoTileController. - To remove the mapping and destroy the tile for this tileId, you can use removeVideoTile.

    +

    By default, the bounded video element's srcObject is also set to null. + Pass false for cleanUpVideoElement, if you do not intend to set the bounded video element's srcObject to null. + This does not remove the provided tileId mapping from the tile map in the DefaultVideoTileController. + To remove the mapping and destroy the tile for this tileId, you can use removeVideoTile.

    • diff --git a/docs/modules/apioverview.html b/docs/modules/apioverview.html index 948904f48a..1142715004 100644 --- a/docs/modules/apioverview.html +++ b/docs/modules/apioverview.html @@ -318,7 +318,7 @@

    7a. Share local video

    7b. Display local and remote video

    You are responsible for maintaining HTMLVideoElement objects in the DOM and arranging their layout within the web page. To display a video, you must handle the videoTileDidUpdate and videoTileWasRemoved callbacks in an AudioVideoObserver. In the implementation of videoTileDidUpdate, bind the tile ID from the provided VideoTileState with the HTMLVideoElement in your DOM by calling meetingSession.audioVideo.bindVideoElement(tileId, videoElement).

    -

    To unbind a tile, call meetingSession.audioVideo.unbindVideoElement(tileId). Note that, this will also cleanup the HTML video element's srcObject by default. Call unbindVideoElement(tileId, false) to avoid the video element cleanup. Check this PR description for more details.

    +

    To unbind a tile, call meetingSession.audioVideo.unbindVideoElement(tileId). Note that this will also set HTMLVideoElement.srcObject to null. Call unbindVideoElement(tileId, false) to avoid the video element clean up. Check this PR description for more details.

    A tileId is a unique identifier representing a video stream. When you stop and start, it generates a new tileId. You can have tileIds exceeding 25; they merely identify a particular stream uniquely. When you start video it consumes a video publishing slot, when you stop video it releases that video publishing slot. Pausing does not affect video publishing slots; it allows a remote to choose to not receive a video stream (and thus not consume bandwidth and CPU for that stream).

    7c. Pause and unpause video (optional)

    diff --git a/guides/03_API_Overview.md b/guides/03_API_Overview.md index 46175d5cd9..8f72759db3 100644 --- a/guides/03_API_Overview.md +++ b/guides/03_API_Overview.md @@ -300,7 +300,7 @@ To stop sharing video with others, call meetingSession.audioVideo.[stopLocalVide You are responsible for maintaining HTMLVideoElement objects in the DOM and arranging their layout within the web page. To display a video, you must handle the [videoTileDidUpdate](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotiledidupdate) and [videoTileWasRemoved](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotilewasremoved) callbacks in an [AudioVideoObserver](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html). In the implementation of [videoTileDidUpdate](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotiledidupdate), bind the tile ID from the provided VideoTileState with the HTMLVideoElement in your DOM by calling meetingSession.audioVideo.[bindVideoElement(tileId, videoElement)](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#bindvideoelement). -To unbind a tile, call meetingSession.audioVideo.[unbindVideoElement(tileId)](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#unbindvideoelement). Note that, this will also cleanup the HTML video element's `srcObject` by default. Call `unbindVideoElement(tileId, false)` to avoid the video element cleanup. Check [this PR](https://github.com/aws/amazon-chime-sdk-js/pull/2217) description for more details. +To unbind a tile, call meetingSession.audioVideo.[unbindVideoElement(tileId)](https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#unbindvideoelement). Note that this will also set `HTMLVideoElement.srcObject` to `null`. Call `unbindVideoElement(tileId, false)` to avoid the video element clean up. Check [this PR](https://github.com/aws/amazon-chime-sdk-js/pull/2217) description for more details. A `tileId` is a unique identifier representing a video stream. When you stop and start, it generates a new `tileId`. You can have tileIds exceeding 25; they merely identify a particular stream uniquely. When you start video it consumes a video publishing slot, when you stop video it releases that video publishing slot. Pausing does not affect video publishing slots; it allows a remote to choose to not receive a video stream (and thus not consume bandwidth and CPU for that stream). diff --git a/src/videotile/DefaultVideoTile.ts b/src/videotile/DefaultVideoTile.ts index 6d118bbd01..099b18d299 100644 --- a/src/videotile/DefaultVideoTile.ts +++ b/src/videotile/DefaultVideoTile.ts @@ -62,12 +62,12 @@ export default class DefaultVideoTile implements DevicePixelRatioObserver, Video } /** - * Disconnect a video stream from a video element by clearing the srcObject of the video element. + * Disconnect a video stream from a video element by setting `HTMLVideoElement.srcObject` to `null`. * @param videoElement The video element input. * @param dueToPause A flag to indicate whether this function is called due to pausing video tile. - * Based on keepLastFrameWhenPaused, it clears out the videoElement's srcObject. - * @param keepLastFrameWhenPaused If true and dueToPause is also true, then we will not clear out the srcObject of the - * video element when it is paused and therefore, the last frame of the stream will be shown. + * Based on `keepLastFrameWhenPaused`, it sets `HTMLVideoElement.srcObject` to `null`. + * @param keepLastFrameWhenPaused If `true` and `dueToPause` is also `true`, then we will not set `HTMLVideoElement.srcObject` of the + * video element to `null` when it is paused and therefore, the last frame of the stream will be shown. */ static disconnectVideoStreamFromVideoElement( videoElement: HTMLVideoElement | null, diff --git a/src/videotilecontroller/VideoTileController.ts b/src/videotilecontroller/VideoTileController.ts index e6d07b63fe..77424ce74c 100644 --- a/src/videotilecontroller/VideoTileController.ts +++ b/src/videotilecontroller/VideoTileController.ts @@ -21,12 +21,12 @@ export default interface VideoTileController { bindVideoElement(tileId: number, videoElement: HTMLVideoElement): void; /** - * Unbinds the video element from the tile if it exists for the provided tileId. - * The video tile's bounded video element and that element's width and height are set to null. - * @param cleanUpVideoElement By default, the bounded video element's srcObject is also set to null using [[disconnectVideoStreamFromVideoElement]]. - * Pass false for cleanUpVideoElement, if you do not intend to clear the bounded video element's srcObject. - * This does not remove the provided tileId mapping from the tile map in the [[DefaultVideoTileController]]. - * To remove the mapping and destroy the tile for this tileId, you can use [[removeVideoTile]]. + * Unbinds the video element from the tile if it exists for the provided `tileId`. + * The video tile's bounded video element and that element's `width` and `height` are set to null. + * @param cleanUpVideoElement By default, the bounded video element's `srcObject` is also set to null. + * Pass `false` for `cleanUpVideoElement`, if you do not intend to set the bounded video element's `srcObject` to `null`. + * This does not remove the provided `tileId` mapping from the tile map in the [[DefaultVideoTileController]]. + * To remove the mapping and destroy the tile for this `tileId`, you can use [[removeVideoTile]]. */ unbindVideoElement(tileId: number, cleanUpVideoElement?: boolean): void; From 86dd08745949c3c52a112c3a74f858c34deec4e3 Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Wed, 11 May 2022 16:09:09 -0700 Subject: [PATCH 7/7] Address unbindVideoElement feedback --- docs/classes/defaultvideotilecontroller.html | 40 +++++++++---------- .../DefaultVideoTileController.ts | 10 ++--- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/docs/classes/defaultvideotilecontroller.html b/docs/classes/defaultvideotilecontroller.html index 05908b8e6c..4f006e7084 100644 --- a/docs/classes/defaultvideotilecontroller.html +++ b/docs/classes/defaultvideotilecontroller.html @@ -184,7 +184,7 @@

    addVideoTile

  • Parameters

    @@ -234,7 +234,7 @@

    captureVideoTile

    Parameters

    @@ -258,7 +258,7 @@

    getAllRemoteVideoTiles

    Returns VideoTile[]

    @@ -276,7 +276,7 @@

    getAllVideoTiles

    Returns VideoTile[]

    @@ -294,7 +294,7 @@

    getLocalVideoTile

    Returns VideoTile | null

    @@ -312,7 +312,7 @@

    getVideoTile

    Parameters

    @@ -336,7 +336,7 @@

    getVideoTileArea

    Parameters

    @@ -360,7 +360,7 @@

    getVideoTileForAttendeeId

    Parameters

    @@ -384,7 +384,7 @@

    hasStartedLocalVideoTile

    Returns boolean

    @@ -402,7 +402,7 @@

    haveVideoTileForAttendeeId

    Parameters

    @@ -426,7 +426,7 @@

    haveVideoTilesWithStreams

    Returns boolean

    @@ -444,7 +444,7 @@

    pauseVideoTile

    Parameters

    @@ -468,7 +468,7 @@

    removeAllVideoTiles

    Returns void

    @@ -486,7 +486,7 @@

    removeLocalVideoTile

    Returns void

    @@ -504,7 +504,7 @@

    removeVideoTile

    Parameters

    @@ -528,7 +528,7 @@

    removeVideoTilesByAttendeeId

    Parameters

    @@ -552,7 +552,7 @@

    sendTileStateUpdate

    Parameters

    @@ -576,7 +576,7 @@

    startLocalVideoTile

    Returns number

    @@ -594,7 +594,7 @@

    stopLocalVideoTile

    Returns void

    @@ -639,7 +639,7 @@

    unpauseVideoTile

    Parameters

    diff --git a/src/videotilecontroller/DefaultVideoTileController.ts b/src/videotilecontroller/DefaultVideoTileController.ts index 71fff64f4e..d88fa0927b 100644 --- a/src/videotilecontroller/DefaultVideoTileController.ts +++ b/src/videotilecontroller/DefaultVideoTileController.ts @@ -63,14 +63,12 @@ export default class DefaultVideoTileController implements VideoTileController { this.logger.warn(`Ignoring video element unbinding for unknown tile id ${tileId}`); return; } + this.logger.info('Unbinding the video element'); + const videoElement = tile.stateRef().boundVideoElement; + tile.bindVideoElement(null); if (cleanUpVideoElement) { - this.logger.info(`Unbinding and cleaning up the video element`); - const videoElement = tile.stateRef().boundVideoElement; - tile.bindVideoElement(null); + this.logger.info('Cleaning up the video element'); DefaultVideoTile.disconnectVideoStreamFromVideoElement(videoElement, false); - } else { - this.logger.info(`Unbinding the video element`); - tile.bindVideoElement(null); } }