Limit max Redundant Audio Encoding packet size to 1000 bytes. (#2763)

In Chromium-based browsers, writing audio frames larger than 1000 bytes will cause an error to be thrown, so we limit the max Redundant Audio Encoding packet size to 1000 bytes. See https://crbug.com/1248479.
aws · Sep 26, 2023 · 2d41bcd · 2d41bcd
1 parent 9720e61
commit 2d41bcd
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 ### Fixed
+- Fixed audio send failing for the rest of the meeting when writing frames larger than 1000 bytes in Chrome, which could be caused by sending redundant audio
 
 ## [3.17.0] - 2023-08-15
 

diff --git a/docs/classes/redundantaudioencoder.html b/docs/classes/redundantaudioencoder.html
@@ -120,7 +120,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/redundantaudioencoder/RedundantAudioEncoder.ts#L73">src/redundantaudioencoder/RedundantAudioEncoder.ts:73</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L74">src/redundantaudioencoder/RedundantAudioEncoder.ts:74</a></li>
 								</ul>
 							</aside>
 							<h4 class="tsd-returns-title">Returns <a href="redundantaudioencoder.html" class="tsd-signature-type" data-tsd-kind="Class">RedundantAudioEncoder</a></h4>
@@ -136,7 +136,7 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> should<wbr>Log<wbr>Debug<
 					<div class="tsd-signature tsd-kind-icon">should<wbr>Log<wbr>Debug<span class="tsd-signature-symbol">:</span> <span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol"> = false</span></div>
 					<aside class="tsd-sources">
 						<ul>
-							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L68">src/redundantaudioencoder/RedundantAudioEncoder.ts:68</a></li>
+							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L69">src/redundantaudioencoder/RedundantAudioEncoder.ts:69</a></li>
 						</ul>
 					</aside>
 				</section>
@@ -146,7 +146,7 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> should<wbr>Report<wbr>Sta
 					<div class="tsd-signature tsd-kind-icon">should<wbr>Report<wbr>Stats<span class="tsd-signature-symbol">:</span> <span class="tsd-signature-type">boolean</span><span class="tsd-signature-symbol"> = false</span></div>
 					<aside class="tsd-sources">
 						<ul>
-							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L73">src/redundantaudioencoder/RedundantAudioEncoder.ts:73</a></li>
+							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L74">src/redundantaudioencoder/RedundantAudioEncoder.ts:74</a></li>
 						</ul>
 					</aside>
 				</section>
@@ -163,7 +163,7 @@ <h3>set<wbr>Num<wbr>Redundant<wbr>Encodings</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/redundantaudioencoder/RedundantAudioEncoder.ts#L212">src/redundantaudioencoder/RedundantAudioEncoder.ts:212</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L213">src/redundantaudioencoder/RedundantAudioEncoder.ts:213</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -191,7 +191,7 @@ <h3>set<wbr>Opus<wbr>Payload<wbr>Type</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/redundantaudioencoder/RedundantAudioEncoder.ts#L204">src/redundantaudioencoder/RedundantAudioEncoder.ts:204</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L205">src/redundantaudioencoder/RedundantAudioEncoder.ts:205</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -219,7 +219,7 @@ <h3>set<wbr>Red<wbr>Payload<wbr>Type</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/redundantaudioencoder/RedundantAudioEncoder.ts#L196">src/redundantaudioencoder/RedundantAudioEncoder.ts:196</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L197">src/redundantaudioencoder/RedundantAudioEncoder.ts:197</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -247,7 +247,7 @@ <h3>set<wbr>Redundancy<wbr>Enabled</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/redundantaudioencoder/RedundantAudioEncoder.ts#L226">src/redundantaudioencoder/RedundantAudioEncoder.ts:226</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L227">src/redundantaudioencoder/RedundantAudioEncoder.ts:227</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -276,7 +276,7 @@ <h3>setup<wbr>Passthrough<wbr>Transform</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/redundantaudioencoder/RedundantAudioEncoder.ts#L164">src/redundantaudioencoder/RedundantAudioEncoder.ts:164</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L165">src/redundantaudioencoder/RedundantAudioEncoder.ts:165</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -307,7 +307,7 @@ <h3>setup<wbr>Receiver<wbr>Transform</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/redundantaudioencoder/RedundantAudioEncoder.ts#L184">src/redundantaudioencoder/RedundantAudioEncoder.ts:184</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L185">src/redundantaudioencoder/RedundantAudioEncoder.ts:185</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -338,7 +338,7 @@ <h3>setup<wbr>Sender<wbr>Transform</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/redundantaudioencoder/RedundantAudioEncoder.ts#L172">src/redundantaudioencoder/RedundantAudioEncoder.ts:172</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L173">src/redundantaudioencoder/RedundantAudioEncoder.ts:173</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -369,7 +369,7 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> debug<wbr>Log</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/redundantaudioencoder/RedundantAudioEncoder.ts#L130">src/redundantaudioencoder/RedundantAudioEncoder.ts:130</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L131">src/redundantaudioencoder/RedundantAudioEncoder.ts:131</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -397,7 +397,7 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> get<wbr>Num<wbr>Redundant
 						<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/redundantaudioencoder/RedundantAudioEncoder.ts#L145">src/redundantaudioencoder/RedundantAudioEncoder.ts:145</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L146">src/redundantaudioencoder/RedundantAudioEncoder.ts:146</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -427,7 +427,7 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> initialize<wbr>Worker</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/redundantaudioencoder/RedundantAudioEncoder.ts#L85">src/redundantaudioencoder/RedundantAudioEncoder.ts:85</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L86">src/redundantaudioencoder/RedundantAudioEncoder.ts:86</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">

diff --git a/src/redundantaudioencoder/RedundantAudioEncoder.ts b/src/redundantaudioencoder/RedundantAudioEncoder.ts
@@ -5,8 +5,9 @@ export default class RedundantAudioEncoder {
   // Each payload must be less than 1024 bytes to fit the 10 bit block length
   private readonly maxRedPacketSizeBytes = 1 << 10;
 
-  // Limit payload to 1100 bytes to handle small MTU
-  private readonly maxAudioRtpPacketSizeBytes = 1100;
+  // Limit payload to 1000 bytes to handle small MTU. 1000 is chosen because in Chromium-based browsers, writing audio
+  // frames larger than 1000 bytes will cause an error to be thrown. See https://crbug.com/1248479.
+  private readonly maxAudioRtpPacketSizeBytes = 1000;
 
   // Each payload can encode a timestamp delta of 14 bits
   private readonly maxRedTimestampOffset = 1 << 14;
@@ -236,12 +237,12 @@ export default class RedundantAudioEncoder {
     frame: RTCEncodedAudioFrame,
     controller: TransformStreamDefaultController
   ): void {
-    // const frameMetadata = frame.getMetadata();
-    // // @ts-ignore
-    // if (frameMetadata.payloadType !== this.redPayloadType) {
-    //   controller.enqueue(frame);
-    //   return;
-    // }
+    const frameMetadata = frame.getMetadata();
+    // @ts-ignore
+    if (frameMetadata.payloadType !== this.redPayloadType) {
+      controller.enqueue(frame);
+      return;
+    }
     const primaryPayloadBuffer = this.getPrimaryPayload(frame.timestamp, frame.data);
     if (!primaryPayloadBuffer) {
       controller.enqueue(frame);

diff --git a/test/redundantaudioencoder/RedundantAudioEncoder.test.ts b/test/redundantaudioencoder/RedundantAudioEncoder.test.ts
@@ -574,20 +574,51 @@ describe('RedundantAudioEncoder', () => {
     });
 
     it('forwards non-RED frames', () => {
+      const header = createRedHeader(
+        true,
+        encoder['opusPayloadType'],
+        0,
+        importantPayload.byteLength
+      );
+      let i = 0;
+      for (; i < 2; ++i) {
+        frame = createRTCEncodedAudioFrame(packetizationTime * i, redPayloadType, 0);
+        frame.data = createRedPayload([header], [importantPayload]);
+        encoder['senderTransform'](frame, controller);
+      }
+
+      // The next outgoing packet should not contain the previous important payloads as redundancy.
+      encoder.setNumRedundantEncodings(1);
       encoder.setRedPayloadType(redPayloadType + 1);
+      frame = createRTCEncodedAudioFrame(packetizationTime * i, redPayloadType, 0);
+      frame.data = createRedPayload([header], [importantPayload]);
       encoder['senderTransform'](frame, controller);
-      expect(frame.data.byteLength).to.equal(0);
+      expect(frame.data.byteLength).to.equal(header.byteLength + importantPayload.byteLength);
     });
 
     it('forwards frames when the primary payload cannot be determined', () => {
+      const header = createRedHeader(
+        true,
+        encoder['opusPayloadType'],
+        0,
+        importantPayload.byteLength
+      );
+      let i = 0;
+      for (; i < 2; ++i) {
+        frame = createRTCEncodedAudioFrame(packetizationTime * i, redPayloadType, 0);
+        frame.data = createRedPayload([header], [importantPayload]);
+        encoder['senderTransform'](frame, controller);
+      }
+
+      // The next outgoing packet should not contain the previous important payloads as redundancy.
       const redundantHeader = createRedHeader(
         false,
         encoder['opusPayloadType'],
         0,
         redundantBlock.byteLength
       );
+      frame = createRTCEncodedAudioFrame(packetizationTime * i, redPayloadType, 0);
       frame.data = createRedPayload([redundantHeader, redundantHeader.slice(0, 3)], []);
-
       encoder['senderTransform'](frame, controller);
       expect(frame.data.byteLength).to.equal(
         redundantHeader.byteLength + redundantHeader.slice(0, 3).byteLength
@@ -1275,10 +1306,11 @@ describe('RedundantAudioEncoder', () => {
 
       // The RED payload should not include the previously added important payload as that would cause the packet size
       // limit to be exceeded.
-      const payloadLenBytes = encoder['maxRedPacketSizeBytes'] - 1;
+      const payloadLenBytes =
+        encoder['maxAudioRtpPacketSizeBytes'] - encoder['redLastHeaderSizeBytes'];
       expect(
         encoder['encode'](packetizationTime * 2, new ArrayBuffer(payloadLenBytes)).byteLength
-      ).to.equal(1 + payloadLenBytes);
+      ).to.equal(encoder['redLastHeaderSizeBytes'] + payloadLenBytes);
     });
   });