Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Nov 2, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 13 additions & 13 deletions docs/classes/redundantaudioencoder.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand All @@ -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">
Expand Down
2 changes: 1 addition & 1 deletion docs/globals.html
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/modules.html
View file
Open in desktop

Large diffs are not rendered by default.

39 changes: 28 additions & 11 deletions src/redundantaudioencoder/RedundantAudioEncoder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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;
}

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand All @@ -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--) {
Expand All @@ -642,7 +659,7 @@ export default class RedundantAudioEncoder {
encodings[encodings.length - 1].timestamp,
frameMetadata.synchronizationSource
);
controller.enqueue(frame);
this.enqueueAudioFrameIfPayloadSizeIsValid(frame, controller);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/redundantaudioencoderworkercode/RedundantAudioEncoderWorkerCode.ts
View file
Open in desktop

Large diffs are not rendered by default.

96 changes: 65 additions & 31 deletions test/redundantaudioencoder/RedundantAudioEncoder.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
);
});
Expand Down Expand Up @@ -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);
Expand Down
Loading