Skip to content

Commit

Permalink
Update unbindVideoElement (#2217)
Browse files Browse the repository at this point in the history 
* Clear srcObject in unbindVideoElement API

* Cover POSTLogger unload event code path in unit test

* Update unbindVideoElement documentation

* 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.

* Update AudioVideoFacade

- Update docs.
- Tested in video_test app.
- Updated unit tests.

* Address feedback on documentation

* Address unbindVideoElement feedback
  • Loading branch information
@devalevenkatesh
devalevenkatesh committed May 12, 2022
1 parent c9d8954 commit b7d2c31
Show file tree
Hide file tree
Showing 19 changed files with 174 additions and 100 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- 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.

Expand Down
5 changes: 4 additions & 1 deletion docs/classes/defaultaudiovideofacade.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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>
Expand Down
39 changes: 19 additions & 20 deletions docs/classes/defaultvideotile.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ <h3>constructor</h3>
<li class="tsd-description">
<aside class="tsd-sources">
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L99">src/videotile/DefaultVideoTile.ts:99</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L98">src/videotile/DefaultVideoTile.ts:98</a></li>
</ul>
</aside>
<h4 class="tsd-parameters-title">Parameters</h4>
Expand Down Expand Up @@ -162,7 +162,7 @@ <h3>bind<wbr>Video<wbr>Element</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#bindvideoelement">bindVideoElement</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L189">src/videotile/DefaultVideoTile.ts:189</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L188">src/videotile/DefaultVideoTile.ts:188</a></li>
</ul>
</aside>
<h4 class="tsd-parameters-title">Parameters</h4>
Expand All @@ -186,7 +186,7 @@ <h3>bind<wbr>Video<wbr>Stream</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#bindvideostream">bindVideoStream</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L143">src/videotile/DefaultVideoTile.ts:143</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L142">src/videotile/DefaultVideoTile.ts:142</a></li>
</ul>
</aside>
<h4 class="tsd-parameters-title">Parameters</h4>
Expand Down Expand Up @@ -228,7 +228,7 @@ <h3>capture</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#capture">capture</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L245">src/videotile/DefaultVideoTile.ts:245</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L244">src/videotile/DefaultVideoTile.ts:244</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">ImageData</span><span class="tsd-signature-symbol"> | </span><span class="tsd-signature-type">null</span></h4>
Expand All @@ -246,7 +246,7 @@ <h3>destroy</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#destroy">destroy</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L112">src/videotile/DefaultVideoTile.ts:112</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L111">src/videotile/DefaultVideoTile.ts:111</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
Expand All @@ -264,7 +264,7 @@ <h3>device<wbr>Pixel<wbr>Ratio<wbr>Changed</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/devicepixelratioobserver.html">DevicePixelRatioObserver</a>.<a href="../interfaces/devicepixelratioobserver.html#devicepixelratiochanged">devicePixelRatioChanged</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L126">src/videotile/DefaultVideoTile.ts:126</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L125">src/videotile/DefaultVideoTile.ts:125</a></li>
</ul>
</aside>
<h4 class="tsd-parameters-title">Parameters</h4>
Expand All @@ -288,7 +288,7 @@ <h3>id</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#id">id</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L131">src/videotile/DefaultVideoTile.ts:131</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L130">src/videotile/DefaultVideoTile.ts:130</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">number</span></h4>
Expand All @@ -306,7 +306,7 @@ <h3>mark<wbr>Poor<wbr>Connection</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#markpoorconnection">markPoorConnection</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L227">src/videotile/DefaultVideoTile.ts:227</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L226">src/videotile/DefaultVideoTile.ts:226</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">boolean</span></h4>
Expand All @@ -324,7 +324,7 @@ <h3>pause</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#pause">pause</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L213">src/videotile/DefaultVideoTile.ts:213</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L212">src/videotile/DefaultVideoTile.ts:212</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
Expand All @@ -342,7 +342,7 @@ <h3>set<wbr>Stream<wbr>Id</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#setstreamid">setStreamId</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L258">src/videotile/DefaultVideoTile.ts:258</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L257">src/videotile/DefaultVideoTile.ts:257</a></li>
</ul>
</aside>
<h4 class="tsd-parameters-title">Parameters</h4>
Expand All @@ -366,7 +366,7 @@ <h3>state</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#state">state</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L135">src/videotile/DefaultVideoTile.ts:135</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L134">src/videotile/DefaultVideoTile.ts:134</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <a href="videotilestate.html" class="tsd-signature-type">VideoTileState</a></h4>
Expand All @@ -384,7 +384,7 @@ <h3>state<wbr>Ref</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#stateref">stateRef</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L139">src/videotile/DefaultVideoTile.ts:139</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L138">src/videotile/DefaultVideoTile.ts:138</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <a href="videotilestate.html" class="tsd-signature-type">VideoTileState</a></h4>
Expand All @@ -402,7 +402,7 @@ <h3>unmark<wbr>Poor<wbr>Connection</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#unmarkpoorconnection">unmarkPoorConnection</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L236">src/videotile/DefaultVideoTile.ts:236</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L235">src/videotile/DefaultVideoTile.ts:235</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">boolean</span></h4>
Expand All @@ -420,7 +420,7 @@ <h3>unpause</h3>
<aside class="tsd-sources">
<p>Implementation of <a href="../interfaces/videotile.html">VideoTile</a>.<a href="../interfaces/videotile.html#unpause">unpause</a></p>
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L220">src/videotile/DefaultVideoTile.ts:220</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L219">src/videotile/DefaultVideoTile.ts:219</a></li>
</ul>
</aside>
<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
Expand Down Expand Up @@ -480,13 +480,12 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> disconnect<wbr>Video<wbr>
<li class="tsd-description">
<aside class="tsd-sources">
<ul>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L73">src/videotile/DefaultVideoTile.ts:73</a></li>
<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/videotile/DefaultVideoTile.ts#L72">src/videotile/DefaultVideoTile.ts:72</a></li>
</ul>
</aside>
<div class="tsd-comment tsd-typography">
<div class="lead">
<p>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.</p>
<p>Disconnect a video stream from a video element by setting <code>HTMLVideoElement.srcObject</code> to <code>null</code>.</p>
</div>
</div>
<h4 class="tsd-parameters-title">Parameters</h4>
Expand All @@ -501,14 +500,14 @@ <h5>videoElement: <span class="tsd-signature-type">HTMLVideoElement</span><span
<h5>dueToPause: <span class="tsd-signature-type">boolean</span></h5>
<div class="tsd-comment tsd-typography">
<p>A flag to indicate whether this function is called due to pausing video tile.
If true, then we will not stop the stream&#39;s tracks and just clearing out the srcObject.</p>
Based on <code>keepLastFrameWhenPaused</code>, it sets <code>HTMLVideoElement.srcObject</code> to <code>null</code>.</p>
</div>
</li>
<li>
<h5><span class="tsd-flag ts-flagDefault value">Default value</span> keepLastFrameWhenPaused: <span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol"> | </span><span class="tsd-signature-type">undefined</span><span class="tsd-signature-symbol"> = false</span></h5>
<div class="tsd-comment tsd-typography">
<p>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.</p>
<p>If <code>true</code> and <code>dueToPause</code> is also <code>true</code>, then we will not set <code>HTMLVideoElement.srcObject</code> of the
video element to <code>null</code> when it is paused and therefore, the last frame of the stream will be shown.</p>
</div>
</li>
</ul>
Expand Down

0 comments on commit b7d2c31

Please sign in to comment.