Do no enqueue any audio payloads larger than 1000 bytes (#2791)

In Chromium-based browsers, writing audio payloads larger than 1000 bytes using the WebRTC Insertable Streams API (which is used to enable dynamic audio redundancy) will cause an error to be thrown and cause audio flow to permanently stop. See https://crbug.com/1248479.
aws · Nov 2, 2023 · dc32d47 · dc32d47
1 parent 9c54b6f
commit dc32d47
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 ### Fixed
+- Do not allow redundant audio worker to enqueue any audio payloads larger than 1000 bytes to avoid permanently stopping the audio flow
 
 ## [3.18.2] - 2023-10-09
 

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#L74">src/redundantaudioencoder/RedundantAudioEncoder.ts:74</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L76">src/redundantaudioencoder/RedundantAudioEncoder.ts:76</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</h3>
 					<div class="tsd-signature tsd-kind-icon">should<wbr>Log<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#L69">src/redundantaudioencoder/RedundantAudioEncoder.ts:69</a></li>
+							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L71">src/redundantaudioencoder/RedundantAudioEncoder.ts:71</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#L74">src/redundantaudioencoder/RedundantAudioEncoder.ts:74</a></li>
+							<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L76">src/redundantaudioencoder/RedundantAudioEncoder.ts:76</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#L213">src/redundantaudioencoder/RedundantAudioEncoder.ts:213</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L215">src/redundantaudioencoder/RedundantAudioEncoder.ts:215</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#L205">src/redundantaudioencoder/RedundantAudioEncoder.ts:205</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L207">src/redundantaudioencoder/RedundantAudioEncoder.ts:207</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#L197">src/redundantaudioencoder/RedundantAudioEncoder.ts:197</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L199">src/redundantaudioencoder/RedundantAudioEncoder.ts:199</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#L225">src/redundantaudioencoder/RedundantAudioEncoder.ts:225</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#L165">src/redundantaudioencoder/RedundantAudioEncoder.ts:165</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L167">src/redundantaudioencoder/RedundantAudioEncoder.ts:167</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#L185">src/redundantaudioencoder/RedundantAudioEncoder.ts:185</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L187">src/redundantaudioencoder/RedundantAudioEncoder.ts:187</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#L173">src/redundantaudioencoder/RedundantAudioEncoder.ts:173</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L175">src/redundantaudioencoder/RedundantAudioEncoder.ts:175</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -369,7 +369,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#L146">src/redundantaudioencoder/RedundantAudioEncoder.ts:146</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L148">src/redundantaudioencoder/RedundantAudioEncoder.ts:148</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -399,7 +399,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#L86">src/redundantaudioencoder/RedundantAudioEncoder.ts:86</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L88">src/redundantaudioencoder/RedundantAudioEncoder.ts:88</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">
@@ -421,7 +421,7 @@ <h3><span class="tsd-flag ts-flagStatic">Static</span> 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#L131">src/redundantaudioencoder/RedundantAudioEncoder.ts:131</a></li>
+									<li>Defined in <a href="https://github.com/aws/amazon-chime-sdk-js/blob/main/src/redundantaudioencoder/RedundantAudioEncoder.ts#L133">src/redundantaudioencoder/RedundantAudioEncoder.ts:133</a></li>
 								</ul>
 							</aside>
 							<div class="tsd-comment tsd-typography">

diff --git a/docs/globals.html b/docs/globals.html
diff --git a/docs/modules.html b/docs/modules.html
diff --git a/src/redundantaudioencoder/RedundantAudioEncoder.ts b/src/redundantaudioencoder/RedundantAudioEncoder.ts
@@ -6,8 +6,10 @@ export default class RedundantAudioEncoder {
   private readonly maxRedPacketSizeBytes = 1 << 10;
 
   // 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;
+  // payloads larger than 1000 bytes using the WebRTC Insertable Streams API (which is used to enable dynamic audio
+  // redundancy) will cause an error to be thrown and cause audio flow to permanently stop. See
+  // https://crbug.com/1248479.
+  private readonly maxAudioPayloadSizeBytes = 1000;
 
   // Each payload can encode a timestamp delta of 14 bits
   private readonly maxRedTimestampOffset = 1 << 14;
@@ -227,6 +229,21 @@ export default class RedundantAudioEncoder {
     RedundantAudioEncoder.log(`redundancy ${this.redundancyEnabled ? 'enabled' : 'disabled'}`);
   }
 
+  /**
+   * Helper function to only enqueue audio frames if they do not exceed the audio payload byte limit imposed by
+   * Chromium-based browsers. Chromium will throw an error (https://crbug.com/1248479) if an audio payload larger than
+   * 1000 bytes is enqueued. Any controller that attempts to enqueue an audio payload larger than 1000 bytes will
+   * encounter this error and will permanently stop sending or receiving audio.
+   */
+  private enqueueAudioFrameIfPayloadSizeIsValid(
+    // @ts-ignore
+    frame: RTCEncodedAudioFrame,
+    controller: TransformStreamDefaultController
+  ): void {
+    if (frame.data.byteLength > this.maxAudioPayloadSizeBytes) return;
+    controller.enqueue(frame);
+  }
+
   /**
    * Receives encoded frames and modifies as needed before sending to transport.
    */
@@ -238,22 +255,22 @@ export default class RedundantAudioEncoder {
     const frameMetadata = frame.getMetadata();
     // @ts-ignore
     if (frameMetadata.payloadType !== this.redPayloadType) {
-      controller.enqueue(frame);
+      this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
       return;
     }
     const primaryPayloadBuffer = this.getPrimaryPayload(frame.timestamp, frame.data);
     if (!primaryPayloadBuffer) {
-      controller.enqueue(frame);
+      this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
       return;
     }
     const encodedBuffer = this.encode(frame.timestamp, primaryPayloadBuffer);
     /* istanbul ignore next */
     if (!encodedBuffer) {
-      controller.enqueue(frame);
+      this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
       return;
     }
     frame.data = encodedBuffer;
-    controller.enqueue(frame);
+    this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
     return;
   }
 
@@ -403,15 +420,15 @@ export default class RedundantAudioEncoder {
     if (
       primaryPayloadSize === 0 ||
       primaryPayloadSize >= this.maxRedPacketSizeBytes ||
-      primaryPayloadSize >= this.maxAudioRtpPacketSizeBytes
+      primaryPayloadSize >= this.maxAudioPayloadSizeBytes
     ) {
       return null;
     }
 
     const numRedundantEncodings = this.numRedundantEncodings;
     let headerSizeBytes = this.redLastHeaderSizeBytes;
     let payloadSizeBytes = primaryPayloadSize;
-    let bytesAvailable = this.maxAudioRtpPacketSizeBytes - primaryPayloadSize - headerSizeBytes;
+    let bytesAvailable = this.maxAudioPayloadSizeBytes - primaryPayloadSize - headerSizeBytes;
     const redundantEncodingTimestamps: Array<number> = new Array();
     const redundantEncodingPayloads: Array<ArrayBuffer> = new Array();
 
@@ -618,7 +635,7 @@ export default class RedundantAudioEncoder {
     const frameMetadata = frame.getMetadata();
     // @ts-ignore
     if (frameMetadata.payloadType !== this.redPayloadType) {
-      controller.enqueue(frame);
+      this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
       return;
     }
     // @ts-ignore
@@ -629,7 +646,7 @@ export default class RedundantAudioEncoder {
       frameMetadata.sequenceNumber
     );
     if (!encodings) {
-      controller.enqueue(frame);
+      this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
       return;
     }
     for (let i = encodings.length - 1; i >= 0; i--) {
@@ -642,7 +659,7 @@ export default class RedundantAudioEncoder {
       encodings[encodings.length - 1].timestamp,
       frameMetadata.synchronizationSource
     );
-    controller.enqueue(frame);
+    this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
   }
 
   /**

diff --git a/src/redundantaudioencoderworkercode/RedundantAudioEncoderWorkerCode.ts b/src/redundantaudioencoderworkercode/RedundantAudioEncoderWorkerCode.ts
diff --git a/test/redundantaudioencoder/RedundantAudioEncoder.test.ts b/test/redundantaudioencoder/RedundantAudioEncoder.test.ts
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import * as chai from 'chai';
+import * as sinon from 'sinon';
 
 import { NoOpDebugLogger } from '../../src';
 import RedundantAudioEncoder from '../../src/redundantaudioencoder/RedundantAudioEncoder';
@@ -500,7 +501,7 @@ describe('RedundantAudioEncoder', () => {
     });
   });
 
-  describe('setupReceivedTransform', () => {
+  describe('setupReceiverTransform', () => {
     it('creates the transform pipe', () => {
       const readable: ReadableStream = new ReadableStream();
       const writable: WritableStream = new WritableStream();
@@ -554,16 +555,7 @@ describe('RedundantAudioEncoder', () => {
 
     beforeEach(() => {
       redPayloadType = encoder['redPayloadType'];
-
-      frame = {
-        data: new ArrayBuffer(0),
-        timestamp: 0,
-        // @ts-ignore
-        getMetadata: () => {
-          // @ts-ignore
-          return { payloadType: redPayloadType } as RTCEncodedAudioFrameMetadata;
-        },
-      };
+      frame = createRTCEncodedAudioFrame(0, redPayloadType, 0);
 
       controller = {
         desiredSize: 0,
@@ -573,6 +565,34 @@ describe('RedundantAudioEncoder', () => {
       };
     });
 
+    it('enqueues audio payloads that are <= the size limit', () => {
+      frame = createRTCEncodedAudioFrame(0, undefined, 0);
+      const enqueueSpy = sinon.spy(controller, 'enqueue');
+
+      frame.data = new ArrayBuffer(500);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      encoder['senderTransform'](frame, controller);
+      expect(enqueueSpy.calledOnce).to.be.true;
+      enqueueSpy.resetHistory();
+
+      frame.data = new ArrayBuffer(1000);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      encoder['senderTransform'](frame, controller);
+      expect(enqueueSpy.calledOnce).to.be.true;
+      enqueueSpy.restore();
+    });
+
+    it('does not enqueue audio payloads that are > the size limit', () => {
+      frame = createRTCEncodedAudioFrame(0, undefined, 0);
+      const enqueueSpy = sinon.spy(controller, 'enqueue');
+
+      frame.data = new ArrayBuffer(1100);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      encoder['senderTransform'](frame, controller);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      enqueueSpy.restore();
+    });
+
     it('forwards non-RED frames', () => {
       const header = createRedHeader(
         true,
@@ -638,15 +658,8 @@ describe('RedundantAudioEncoder', () => {
           0,
           importantPayload.byteLength
         );
-        frame = {
-          data: createRedPayload([header], [importantPayload]),
-          timestamp: packetizationTime * i,
-          // @ts-ignore
-          getMetadata: () => {
-            // @ts-ignore
-            return { payloadType: redPayloadType } as RTCEncodedAudioFrameMetadata;
-          },
-        };
+        frame = createRTCEncodedAudioFrame(packetizationTime * i, redPayloadType, 0);
+        frame.data = createRedPayload([header], [importantPayload]);
 
         encoder['senderTransform'](frame, controller);
         if (i < 2) {
@@ -674,15 +687,8 @@ describe('RedundantAudioEncoder', () => {
           0,
           importantPayload.byteLength
         );
-        frame = {
-          data: createRedPayload([header], [importantPayload]),
-          timestamp: packetizationTime * i,
-          // @ts-ignore
-          getMetadata: () => {
-            // @ts-ignore
-            return { payloadType: redPayloadType } as RTCEncodedAudioFrameMetadata;
-          },
-        };
+        frame = createRTCEncodedAudioFrame(packetizationTime * i, redPayloadType, 0);
+        frame.data = createRedPayload([header], [importantPayload]);
 
         encoder['senderTransform'](frame, controller);
         expect(frame.data.byteLength).to.equal(primaryHeaderLenBytes + importantPayload.byteLength);
@@ -714,6 +720,34 @@ describe('RedundantAudioEncoder', () => {
       };
     });
 
+    it('enqueues audio payloads that are <= the size limit', () => {
+      const frame = createRTCEncodedAudioFrame(0, undefined, 0);
+      const enqueueSpy = sinon.spy(controller, 'enqueue');
+
+      frame.data = new ArrayBuffer(500);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      encoder['senderTransform'](frame, controller);
+      expect(enqueueSpy.calledOnce).to.be.true;
+      enqueueSpy.resetHistory();
+
+      frame.data = new ArrayBuffer(1000);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      encoder['senderTransform'](frame, controller);
+      expect(enqueueSpy.calledOnce).to.be.true;
+      enqueueSpy.restore();
+    });
+
+    it('does not enqueue audio payloads that are > the size limit', () => {
+      const frame = createRTCEncodedAudioFrame(0, undefined, 0);
+      const enqueueSpy = sinon.spy(controller, 'enqueue');
+
+      frame.data = new ArrayBuffer(1100);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      encoder['senderTransform'](frame, controller);
+      expect(enqueueSpy.calledOnce).to.be.false;
+      enqueueSpy.restore();
+    });
+
     it('forwards non-RED frames', () => {
       const frame = createRTCEncodedAudioFrame(960, encoder['redPayloadType'], 10);
       encoder.setRedPayloadType(redPayloadType + 1);
@@ -1236,7 +1270,7 @@ describe('RedundantAudioEncoder', () => {
       expect(encoder['encode'](0, new ArrayBuffer(encoder['maxRedPacketSizeBytes']))).to.equal(
         null
       );
-      expect(encoder['encode'](0, new ArrayBuffer(encoder['maxAudioRtpPacketSizeBytes']))).to.equal(
+      expect(encoder['encode'](0, new ArrayBuffer(encoder['maxAudioPayloadSizeBytes']))).to.equal(
         null
       );
     });
@@ -1307,7 +1341,7 @@ 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['maxAudioRtpPacketSizeBytes'] - encoder['redLastHeaderSizeBytes'];
+        encoder['maxAudioPayloadSizeBytes'] - encoder['redLastHeaderSizeBytes'];
       expect(
         encoder['encode'](packetizationTime * 2, new ArrayBuffer(payloadLenBytes)).byteLength
       ).to.equal(encoder['redLastHeaderSizeBytes'] + payloadLenBytes);