Update AudioVideoFacade

- Update docs. - Tested in video_test app. - Updated unit tests.
aws · May 11, 2022 · 5b8f1a8 · 5b8f1a8
1 parent 2f402ce
commit 5b8f1a8
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 13 deletions.
diff --git 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
@@ -2236,7 +2236,7 @@ <h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</spa
 					<a name="unbindvideoelement" class="tsd-anchor"></a>
 					<h3>unbind<wbr>Video<wbr>Element</h3>
 					<ul class="tsd-signatures tsd-kind-method tsd-parent-kind-class">
-						<li class="tsd-signature tsd-kind-icon">unbind<wbr>Video<wbr>Element<span class="tsd-signature-symbol">(</span>tileId<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">number</span><span class="tsd-signature-symbol">)</span><span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">void</span></li>
+						<li class="tsd-signature tsd-kind-icon">unbind<wbr>Video<wbr>Element<span class="tsd-signature-symbol">(</span>tileId<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">number</span>, cleanUpVideoElement<span class="tsd-signature-symbol">?: </span><span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol">)</span><span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">void</span></li>
 					</ul>
 					<ul class="tsd-descriptions">
 						<li class="tsd-description">
@@ -2251,6 +2251,9 @@ <h4 class="tsd-parameters-title">Parameters</h4>
 								<li>
 									<h5>tileId: <span class="tsd-signature-type">number</span></h5>
 								</li>
+								<li>
+									<h5><span class="tsd-flag ts-flagDefault value">Default value</span> cleanUpVideoElement: <span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol"> = true</span></h5>
+								</li>
 							</ul>
 							<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
 						</li>

diff --git a/docs/interfaces/audiovideofacade.html b/docs/interfaces/audiovideofacade.html
@@ -2426,7 +2426,7 @@ <h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</spa
 					<a name="unbindvideoelement" class="tsd-anchor"></a>
 					<h3>unbind<wbr>Video<wbr>Element</h3>
 					<ul class="tsd-signatures tsd-kind-method tsd-parent-kind-interface tsd-is-inherited">
-						<li class="tsd-signature tsd-kind-icon">unbind<wbr>Video<wbr>Element<span class="tsd-signature-symbol">(</span>tileId<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">number</span><span class="tsd-signature-symbol">)</span><span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">void</span></li>
+						<li class="tsd-signature tsd-kind-icon">unbind<wbr>Video<wbr>Element<span class="tsd-signature-symbol">(</span>tileId<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">number</span>, cleanUpVideoElement<span class="tsd-signature-symbol">?: </span><span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol">)</span><span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">void</span></li>
 					</ul>
 					<ul class="tsd-descriptions">
 						<li class="tsd-description">
@@ -2441,6 +2441,9 @@ <h4 class="tsd-parameters-title">Parameters</h4>
 								<li>
 									<h5>tileId: <span class="tsd-signature-type">number</span></h5>
 								</li>
+								<li>
+									<h5><span class="tsd-flag ts-flagOptional">Optional</span> cleanUpVideoElement: <span class="tsd-signature-type">boolean</span></h5>
+								</li>
 							</ul>
 							<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
 						</li>

diff --git a/docs/interfaces/videotilecontrollerfacade.html b/docs/interfaces/videotilecontrollerfacade.html
@@ -408,7 +408,7 @@ <h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</spa
 					<a name="unbindvideoelement" class="tsd-anchor"></a>
 					<h3>unbind<wbr>Video<wbr>Element</h3>
 					<ul class="tsd-signatures tsd-kind-method tsd-parent-kind-interface">
-						<li class="tsd-signature tsd-kind-icon">unbind<wbr>Video<wbr>Element<span class="tsd-signature-symbol">(</span>tileId<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">number</span><span class="tsd-signature-symbol">)</span><span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">void</span></li>
+						<li class="tsd-signature tsd-kind-icon">unbind<wbr>Video<wbr>Element<span class="tsd-signature-symbol">(</span>tileId<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">number</span>, cleanUpVideoElement<span class="tsd-signature-symbol">?: </span><span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol">)</span><span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">void</span></li>
 					</ul>
 					<ul class="tsd-descriptions">
 						<li class="tsd-description">
@@ -422,6 +422,9 @@ <h4 class="tsd-parameters-title">Parameters</h4>
 								<li>
 									<h5>tileId: <span class="tsd-signature-type">number</span></h5>
 								</li>
+								<li>
+									<h5><span class="tsd-flag ts-flagOptional">Optional</span> cleanUpVideoElement: <span class="tsd-signature-type">boolean</span></h5>
+								</li>
 							</ul>
 							<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
 						</li>

diff --git a/docs/modules/apioverview.html b/docs/modules/apioverview.html
@@ -318,7 +318,7 @@ <h3>7a. Share local video</h3>
 						<h3>7b. Display local and remote video</h3>
 					</a>
 					<p>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 <a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotiledidupdate">videoTileDidUpdate</a> and <a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotilewasremoved">videoTileWasRemoved</a> callbacks in an <a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html">AudioVideoObserver</a>. In the implementation of <a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideoobserver.html#videotiledidupdate">videoTileDidUpdate</a>, bind the tile ID from the provided VideoTileState with the HTMLVideoElement in your DOM by calling meetingSession.audioVideo.<a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#bindvideoelement">bindVideoElement(tileId, videoElement)</a>.</p>
-					<p>To unbind a tile, call meetingSession.audioVideo.<a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#unbindvideoelement">unbindVideoElement(tileId)</a>.</p>
+					<p>To unbind a tile, call meetingSession.audioVideo.<a href="https://aws.github.io/amazon-chime-sdk-js/interfaces/audiovideofacade.html#unbindvideoelement">unbindVideoElement(tileId)</a>. Note that, this will also cleanup the HTML video element&#39;s <code>srcObject</code> by default. Call <code>unbindVideoElement(tileId, false)</code> to avoid the video element cleanup. Check <a href="https://github.com/aws/amazon-chime-sdk-js/pull/2217">this PR</a> description for more details.</p>
 					<p>A <code>tileId</code> is a unique identifier representing a video stream. When you stop and start, it generates a new <code>tileId</code>. 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).</p>
 					<a href="#7c-pause-and-unpause-video-optional" id="7c-pause-and-unpause-video-optional" style="color: inherit; text-decoration: none;">
 						<h3>7c. Pause and unpause video (optional)</h3>

diff --git 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
diff --git 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
@@ -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
@@ -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', () => {