Do not no-op server side network adaptation pauses that happen immedi…

…ately after subscribe (#2917)
aws · Jul 5, 2024 · 706a3b5 · 706a3b5
1 parent bf22ce0
commit 706a3b5
Show file tree

Hide file tree

Showing 8 changed files with 1,546 additions and 1,310 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Fixed rare race conditions with simulcast + server side network adaptation on third attendee join.
 - Make redundant audio worker code generation script work on Windows
+- Do not drop server side network adaptation pauses that happen immediately after SDP negotiation, by pre-emptively creating a tile immediately when applications subscribes to one, and attaching the video track later.
 
 ## [3.22.0] - 2024-03-15
 

diff --git a/docs/assets/js/search.js b/docs/assets/js/search.js
diff --git a/docs/classes/defaultaudiovideocontroller.html b/docs/classes/defaultaudiovideocontroller.html
diff --git a/docs/classes/noopaudiovideocontroller.html b/docs/classes/noopaudiovideocontroller.html
diff --git a/docs/classes/receiveremotevideopauseresumetask.html b/docs/classes/receiveremotevideopauseresumetask.html
@@ -114,6 +114,7 @@ <h3>Methods</h3>
 								<li class="tsd-kind-method tsd-parent-kind-class"><a href="receiveremotevideopauseresumetask.html#removeobserver" class="tsd-kind-icon">remove<wbr>Observer</a></li>
 								<li class="tsd-kind-method tsd-parent-kind-class tsd-is-overwrite"><a href="receiveremotevideopauseresumetask.html#run" class="tsd-kind-icon">run</a></li>
 								<li class="tsd-kind-method tsd-parent-kind-class tsd-is-inherited"><a href="receiveremotevideopauseresumetask.html#setparent" class="tsd-kind-icon">set<wbr>Parent</a></li>
+								<li class="tsd-kind-method tsd-parent-kind-class"><a href="receiveremotevideopauseresumetask.html#updatesubscribedgroupdids" class="tsd-kind-icon">update<wbr>Subscribed<wbr>Groupd<wbr>Ids</a></li>
 							</ul>
 						</section>
 					</div>
@@ -132,7 +133,7 @@ <h3>constructor</h3>
 							<aside class="tsd-sources">
 								<p>Overrides <a href="basetask.html">BaseTask</a>.<a href="basetask.html#constructor">constructor</a></p>
 								<ul>
-									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L24">src/task/ReceiveRemoteVideoPauseResumeTask.ts:24</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L25">src/task/ReceiveRemoteVideoPauseResumeTask.ts:25</a></li>
 								</ul>
 							</aside>
 							<h4 class="tsd-parameters-title">Parameters</h4>
@@ -163,7 +164,7 @@ <h3><span class="tsd-flag ts-flagProtected">Protected</span> task<wbr>Name</h3>
 					<aside class="tsd-sources">
 						<p>Overrides <a href="basetask.html">BaseTask</a>.<a href="basetask.html#taskname">taskName</a></p>
 						<ul>
-							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L22">src/task/ReceiveRemoteVideoPauseResumeTask.ts:22</a></li>
+							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L23">src/task/ReceiveRemoteVideoPauseResumeTask.ts:23</a></li>
 						</ul>
 					</aside>
 				</section>
@@ -226,7 +227,7 @@ <h3>handle<wbr>Signaling<wbr>Client<wbr>Event</h3>
 							<aside class="tsd-sources">
 								<p>Implementation of <a href="../interfaces/signalingclientobserver.html">SignalingClientObserver</a>.<a href="../interfaces/signalingclientobserver.html#handlesignalingclientevent">handleSignalingClientEvent</a></p>
 								<ul>
-									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L39">src/task/ReceiveRemoteVideoPauseResumeTask.ts:39</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L66">src/task/ReceiveRemoteVideoPauseResumeTask.ts:66</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -326,7 +327,7 @@ <h3>remove<wbr>Observer</h3>
 							<aside class="tsd-sources">
 								<p>Implementation of <a href="../interfaces/removableobserver.html">RemovableObserver</a>.<a href="../interfaces/removableobserver.html#removeobserver">removeObserver</a></p>
 								<ul>
-									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L30">src/task/ReceiveRemoteVideoPauseResumeTask.ts:30</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L31">src/task/ReceiveRemoteVideoPauseResumeTask.ts:31</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -349,7 +350,7 @@ <h3>run</h3>
 							<aside class="tsd-sources">
 								<p>Overrides <a href="basetask.html">BaseTask</a>.<a href="basetask.html#run">run</a></p>
 								<ul>
-									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L34">src/task/ReceiveRemoteVideoPauseResumeTask.ts:34</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L35">src/task/ReceiveRemoteVideoPauseResumeTask.ts:35</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -393,6 +394,29 @@ <h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</spa
 						</li>
 					</ul>
 				</section>
+				<section class="tsd-panel tsd-member tsd-kind-method tsd-parent-kind-class">
+					<a name="updatesubscribedgroupdids" class="tsd-anchor"></a>
+					<h3>update<wbr>Subscribed<wbr>Groupd<wbr>Ids</h3>
+					<ul class="tsd-signatures tsd-kind-method tsd-parent-kind-class">
+						<li class="tsd-signature tsd-kind-icon">update<wbr>Subscribed<wbr>Groupd<wbr>Ids<span class="tsd-signature-symbol">(</span>groupIds<span class="tsd-signature-symbol">: </span><span class="tsd-signature-type">Set</span><span class="tsd-signature-symbol">&lt;</span><span class="tsd-signature-type">number</span><span class="tsd-signature-symbol">&gt;</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">
+							<aside class="tsd-sources">
+								<ul>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/task/ReceiveRemoteVideoPauseResumeTask.ts#L40">src/task/ReceiveRemoteVideoPauseResumeTask.ts:40</a></li>
+								</ul>
+							</aside>
+							<h4 class="tsd-parameters-title">Parameters</h4>
+							<ul class="tsd-parameters">
+								<li>
+									<h5>groupIds: <span class="tsd-signature-type">Set</span><span class="tsd-signature-symbol">&lt;</span><span class="tsd-signature-type">number</span><span class="tsd-signature-symbol">&gt;</span></h5>
+								</li>
+							</ul>
+							<h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</span></h4>
+						</li>
+					</ul>
+				</section>
 			</section>
 		</div>
 		<div class="col-4 col-menu menu-sticky-wrap menu-highlight">
@@ -446,6 +470,9 @@ <h4 class="tsd-returns-title">Returns <span class="tsd-signature-type">void</spa
 							<li class=" tsd-kind-method tsd-parent-kind-class tsd-is-inherited">
 								<a href="receiveremotevideopauseresumetask.html#setparent" class="tsd-kind-icon">set<wbr>Parent</a>
 							</li>
+							<li class=" tsd-kind-method tsd-parent-kind-class">
+								<a href="receiveremotevideopauseresumetask.html#updatesubscribedgroupdids" class="tsd-kind-icon">update<wbr>Subscribed<wbr>Groupd<wbr>Ids</a>
+							</li>
 						</ul>
 					</li>
 				</ul>

diff --git a/src/audiovideocontroller/DefaultAudioVideoController.ts b/src/audiovideocontroller/DefaultAudioVideoController.ts
@@ -1,7 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { MeetingSessionCredentials } from '..';
+import { MeetingSessionCredentials, ReceiveRemoteVideoPauseResumeTask } from '..';
 import ActiveSpeakerDetector from '../activespeakerdetector/ActiveSpeakerDetector';
 import DefaultActiveSpeakerDetector from '../activespeakerdetector/DefaultActiveSpeakerDetector';
 import AudioMixController from '../audiomixcontroller/AudioMixController';
@@ -145,6 +145,7 @@ export default class DefaultAudioVideoController
   // * `MonitorTask` which updates `videosToReceive`
   private receiveIndexTask: ReceiveVideoStreamIndexTask | undefined = undefined;
   private monitorTask: MonitorTask | undefined = undefined;
+  private receiveRemotePauseResumeTask: ReceiveRemoteVideoPauseResumeTask | undefined = undefined;
 
   destroyed = false;
 
@@ -406,7 +407,9 @@ export default class DefaultAudioVideoController
     const setLocalDescription = new SetLocalDescriptionTask(context).once(createSDP);
     const ice = new FinishGatheringICECandidatesTask(context).once(setLocalDescription);
     const subscribeAck = new SubscribeAndReceiveSubscribeAckTask(context).once(ice);
-    const receivePauseResume = new ReceiveRemoteVideoPauseResume(context).once(subscribeAck);
+
+    this.receiveRemotePauseResumeTask = new ReceiveRemoteVideoPauseResume(context);
+    this.receiveRemotePauseResumeTask.once(subscribeAck);
 
     // The ending is a delicate time: we need the connection as a whole to have a timeout,
     // and for the attendee presence timer to not start ticking until after the subscribe/ack.
@@ -418,7 +421,7 @@ export default class DefaultAudioVideoController
           // The order of these two matters. If canceled, the first one that's still running
           // will contribute any special rejection, and we don't want that to be "attendee not found"!
           subscribeAck,
-          receivePauseResume,
+          this.receiveRemotePauseResumeTask,
           needsToWaitForAttendeePresence
             ? new TimeoutTask(
                 this.logger,
@@ -1062,6 +1065,7 @@ export default class DefaultAudioVideoController
     const groupIdsToReceive = context.videosToReceive.array().map((streamId: number) => {
       return context.videoStreamIndex.groupIdForStreamId(streamId);
     });
+    this.receiveRemotePauseResumeTask.updateSubscribedGroupdIds(new Set(groupIdsToReceive));
 
     const currentConfigs = convertVideoPreferencesToSignalingClientVideoSubscriptionConfiguration(
       context,
@@ -1390,6 +1394,9 @@ export default class DefaultAudioVideoController
 
     this.meetingSessionContext.volumeIndicatorAdapter.onReconnect();
     this.connectionHealthData.reset();
+    this.receiveRemotePauseResumeTask = new ReceiveRemoteVideoPauseResume(
+      this.meetingSessionContext
+    );
     try {
       await new SerialGroupTask(this.logger, this.wrapTaskName('AudioVideoReconnect'), [
         new TimeoutTask(
@@ -1415,6 +1422,7 @@ export default class DefaultAudioVideoController
             new FinishGatheringICECandidatesTask(this.meetingSessionContext),
             new SubscribeAndReceiveSubscribeAckTask(this.meetingSessionContext),
             new SetRemoteDescriptionTask(this.meetingSessionContext),
+            this.receiveRemotePauseResumeTask,
           ]),
           this.configuration.connectionTimeoutMs
         ),

diff --git a/src/task/ReceiveRemoteVideoPauseResumeTask.ts b/src/task/ReceiveRemoteVideoPauseResumeTask.ts
@@ -15,6 +15,7 @@ import BaseTask from './BaseTask';
  * `VideoDownlinkBandwidthPolicy.getServerSideNetworkAdaption()` == `BandwidthProbingAndRemoteVideoQualityAdaption`)
  * which indicates remote video sources that have been paused or resumed and updates [[VideoDownlinkBandwidthPolicy]]
  * and [[VideoTileController]]
+ *
  */
 export default class ReceiveRemoteVideoPauseResume
   extends BaseTask
@@ -36,6 +37,32 @@ export default class ReceiveRemoteVideoPauseResume
     this.context.removableObservers.push(this);
   }
 
+  updateSubscribedGroupdIds(groupIds: Set<number>): void {
+    const existingVideoTileIds = new Set<number>();
+    for (const groupId of groupIds) {
+      const attendeeId = this.context.videoStreamIndex.attendeeIdForGroupId(groupId);
+      if (attendeeId.length === 0) {
+        this.logger.warn(`Could not find attendee ID for newly subscribed group ID ${groupId}`);
+        continue;
+      }
+      let videoTile = this.context.videoTileController.getVideoTileForAttendeeId(attendeeId);
+      if (videoTile === undefined) {
+        this.logger.info(
+          `No existing video tile for attendee ID ${attendeeId} with new group ID ${groupId}. Creating new one.`
+        );
+        videoTile = this.context.videoTileController.addVideoTile();
+        videoTile.bindVideoStream(attendeeId, false, null, 0, 0, 0, null);
+      }
+      existingVideoTileIds.add(videoTile.id());
+    }
+
+    this.serverPausedVideoTileIds.forEach(id => {
+      if (!existingVideoTileIds.has(id)) {
+        this.serverPausedVideoTileIds.delete(id);
+      }
+    });
+  }
+
   handleSignalingClientEvent(event: SignalingClientEvent): void {
     if (
       event.type !== SignalingClientEventType.ReceivedSignalFrame ||
@@ -48,8 +75,8 @@ export default class ReceiveRemoteVideoPauseResume
     // @ts-ignore: force cast to SdkPauseFrame
     const pauseResumeFrame: SdkPauseResumeFrame = event.message.pause;
     const messageType = event.message.type;
-    this.context.logger.info(
-      `received new ${
+    this.logger.info(
+      `Received new ${
         messageType === SdkSignalFrame.Type.PAUSE ? 'pause' : 'resume'
       } frame: ${JSON.stringify(pauseResumeFrame)}`
     );
@@ -65,11 +92,16 @@ export default class ReceiveRemoteVideoPauseResume
 
     const tiles = pauseResumeFrame.groupIds.map((groupId: number) => {
       const attendeeId = this.context.videoStreamIndex.attendeeIdForGroupId(groupId);
+      if (attendeeId.length === 0) {
+        this.logger.warn(`Could not find attendee ID for paused group ID ${groupId}`);
+        return undefined;
+      }
       return this.context.videoTileController.getVideoTileForAttendeeId(attendeeId);
     });
 
     for (const tile of tiles) {
       if (tile === undefined) {
+        // Warning is logged above
         continue;
       }
       if (messageType === SdkSignalFrame.Type.PAUSE) {
@@ -85,6 +117,7 @@ export default class ReceiveRemoteVideoPauseResume
           tile.pause();
         }
       } else {
+        // Don't resume user paused video tiles
         if (tile.state().paused && this.serverPausedVideoTileIds.has(tile.id())) {
           this.serverPausedVideoTileIds.delete(tile.id());
           this.context.videoDownlinkBandwidthPolicy.forEachObserver(