From 5b8f1a8d0be615ce578dcc79a245ddda6979dc7b Mon Sep 17 00:00:00 2001 From: Venkatesh Devale Date: Wed, 11 May 2022 14:16:53 -0700 Subject: [PATCH] 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/VideoTileControllerFacade.ts | 2 +- test/audiovideofacade/DefaultAudioVideoFacade.test.ts | 11 +++++++++-- 10 files changed, 29 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff1cd64798..7edf7aadd0 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/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', () => {