New BWE: Integrate SenderTransportCongestionController in Transport

node/src/DirectTransport.ts

@@ -17,7 +17,11 @@ import {
 	parseTransportTraceEventData,
 } from './Transport';
 import type { SctpParameters } from './sctpParametersTypes';
+import type { RtpParameters } from './rtpParametersTypes';
+import type { Producer, ProducerOptions } from './ProducerTypes';
 import type { AppData } from './types';
+import * as ortc from './ortc';
+import * as utils from './utils';
 import { UnsupportedError } from './errors';
 import { Event, Notification } from './fbs/notification';
 import * as FbsDirectTransport from './fbs/direct-transport';
@@ -154,6 +158,22 @@ export class DirectTransportImpl<
 		);
 	}
 
+	override async produce<ProducerAppData extends AppData = AppData>(
+		options: ProducerOptions<ProducerAppData>
+	): Promise<Producer<ProducerAppData>> {
+		logger.debug('produce()');
+
+		// Clone given RTP parameters since bandwidth estimation stuff is removed
+		// from them below and they belong to the application.
+		const rtpParameters = utils.clone<RtpParameters>(options.rtpParameters);
+
+		// A DirectTransport doesn't run bandwidth estimation, so whatever was
+		// negotiated for it is dropped rather than honoured.
+		ortc.disableBweInRtpParameters(rtpParameters);
+
+		return super.produce({ ...options, rtpParameters });
+	}
+
 	sendRtcp(rtcpPacket: Buffer): void {
 		if (!Buffer.isBuffer(rtcpPacket)) {
 			throw new TypeError('rtcpPacket must be a Buffer');

node/src/PipeTransport.ts

@@ -21,7 +21,7 @@ import {
 	parseTuple,
 	parseTransportTraceEventData,
 } from './Transport';
-import type { Producer } from './ProducerTypes';
+import type { Producer, ProducerOptions } from './ProducerTypes';
 import type { Consumer, ConsumerType } from './ConsumerTypes';
 import { ConsumerImpl } from './Consumer';
 import type { RtpParameters } from './rtpParametersTypes';
@@ -223,6 +223,22 @@ export class PipeTransportImpl<PipeTransportAppData extends AppData = AppData>
 		}
 	}
 
+	override async produce<ProducerAppData extends AppData = AppData>(
+		options: ProducerOptions<ProducerAppData>
+	): Promise<Producer<ProducerAppData>> {
+		logger.debug('produce()');
+
+		// Clone given RTP parameters since bandwidth estimation stuff is removed
+		// from them below and they belong to the application.
+		const rtpParameters = utils.clone<RtpParameters>(options.rtpParameters);
+
+		// A PipeTransport doesn't run bandwidth estimation, so whatever was
+		// negotiated for it is dropped rather than honoured.
+		ortc.disableBweInRtpParameters(rtpParameters);
+
+		return super.produce({ ...options, rtpParameters });
+	}
+
 	override async consume<ConsumerAppData extends AppData = AppData>({
 		producerId,
 		appData,

node/src/Transport.ts

@@ -649,6 +649,10 @@ export abstract class TransportImpl<
 			remoteRtpCapabilities: clonedRtpCapabilities,
 			pipe,
 			enableRtx,
+			// A DirectTransport doesn't run bandwidth estimation, so it must not
+			// negotiate it whatever the given RTP capabilities say. A PipeTransport
+			// doesn't need this since its consume() overrides this method.
+			disableBwe: this.type === 'direct',
 		});
 
 		// Set MID.

node/src/ortc.ts

@@ -616,11 +616,13 @@ export function getConsumerRtpParameters({
 	remoteRtpCapabilities,
 	pipe,
 	enableRtx,
+	disableBwe = false,
 }: {
 	consumableRtpParameters: RtpParameters;
 	remoteRtpCapabilities: RtpCapabilities;
 	pipe: boolean;
 	enableRtx: boolean;
+	disableBwe?: boolean;
 }): RtpParameters {
 	const consumerParams: RtpParameters = {
 		codecs: [],
@@ -694,6 +696,19 @@ export function getConsumerRtpParameters({
 			)
 		);
 
+	// A transport that doesn't run bandwidth estimation must not negotiate it.
+	// Dropping its header extensions is enough, since the RTCP feedback below is
+	// reduced out of them.
+	if (disableBwe) {
+		consumerParams.headerExtensions = consumerParams.headerExtensions.filter(
+			ext =>
+				ext.uri !==
+					'http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time' &&
+				ext.uri !==
+					'http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01'
+		);
+	}
+
 	// Reduce codecs' RTCP feedback. Use Transport-CC if available, REMB otherwise.
 	if (
 		consumerParams.headerExtensions.some(
@@ -831,11 +846,13 @@ export function getPipeConsumerRtpParameters({
 			continue;
 		}
 
+		// Drop the bandwidth estimation feedback, which a PipeTransport doesn't run,
+		// and the generic NACK when there is no RTX to retransmit with.
 		codec.rtcpFeedback = codec.rtcpFeedback!.filter(
 			fb =>
-				(fb.type === 'nack' && fb.parameter === 'pli') ||
-				(fb.type === 'ccm' && fb.parameter === 'fir') ||
-				(enableRtx && fb.type === 'nack' && !fb.parameter)
+				fb.type !== 'goog-remb' &&
+				fb.type !== 'transport-cc' &&
+				(enableRtx || fb.type !== 'nack' || Boolean(fb.parameter))
 		);
 
 		consumerParams.codecs.push(codec);
@@ -890,6 +907,29 @@ export function getPipeConsumerRtpParameters({
 	return consumerParams;
 }
 
+/**
+ * Remove from the given RTP parameters everything related to bandwidth
+ * estimation, which a transport that doesn't run it must not negotiate.
+ *
+ * It mutates the given RTP parameters, so the caller must not pass the ones the
+ * application gave.
+ */
+export function disableBweInRtpParameters(rtpParameters: RtpParameters): void {
+	rtpParameters.headerExtensions = rtpParameters.headerExtensions?.filter(
+		ext =>
+			ext.uri !==
+				'http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time' &&
+			ext.uri !==
+				'http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01'
+	);
+
+	for (const codec of rtpParameters.codecs) {
+		codec.rtcpFeedback = codec.rtcpFeedback?.filter(
+			fb => fb.type !== 'transport-cc' && fb.type !== 'goog-remb'
+		);
+	}
+}
+
 function isRtxCodec(codec: RtpCodecCapability | RtpCodecParameters): boolean {
 	return /.+\/rtx$/i.test(codec.mimeType);
 }

node/src/supportedRtpCapabilities.ts

@@ -232,27 +232,19 @@ const supportedRtpCapabilities: RouterRtpCapabilities = {
 			preferredEncrypt: false,
 			direction: 'recvonly',
 		},
-		{
-			kind: 'audio',
-			uri: 'http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time',
-			preferredId: 4,
-			preferredEncrypt: false,
-			direction: 'sendrecv',
-		},
 		{
 			kind: 'video',
 			uri: 'http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time',
 			preferredId: 4,
 			preferredEncrypt: false,
 			direction: 'sendrecv',
 		},
-		// NOTE: For audio we just enable transport-wide-cc-01 when receiving media.
 		{
 			kind: 'audio',
 			uri: 'http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01',
 			preferredId: 5,
 			preferredEncrypt: false,
-			direction: 'recvonly',
+			direction: 'sendrecv',
 		},
 		{
 			kind: 'video',

node/src/test/test-Consumer.ts

@@ -744,20 +744,14 @@ test('consumer.dump() succeeds', async () => {
 	});
 	expect(dump1.rtpParameters.codecs[0]!.rtcpFeedback).toEqual([]);
 	expect(Array.isArray(dump1.rtpParameters.headerExtensions)).toBe(true);
-	expect(dump1.rtpParameters.headerExtensions!.length).toBe(3);
+	expect(dump1.rtpParameters.headerExtensions!.length).toBe(2);
 	expect(dump1.rtpParameters.headerExtensions).toEqual([
 		{
 			uri: 'urn:ietf:params:rtp-hdrext:sdes:mid',
 			id: 1,
 			encrypt: false,
 			parameters: {},
 		},
-		{
-			uri: 'http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time',
-			id: 4,
-			parameters: {},
-			encrypt: false,
-		},
 		{
 			uri: 'urn:ietf:params:rtp-hdrext:ssrc-audio-level',
 			id: 6,

node/src/test/test-DirectTransport.ts

@@ -370,6 +370,125 @@ test('dataProducer.send() with subchannels succeeds', async () => {
 	}
 }, 5000);
 
+test('directTransport.produce() removes BWE stuff from RTP parameters', async () => {
+	const router = await ctx.worker!.createRouter({
+		mediaCodecs: [
+			{
+				kind: 'video',
+				mimeType: 'video/VP8',
+				clockRate: 90000,
+			},
+		],
+	});
+
+	const directTransport = await router.createDirectTransport();
+
+	const producer = await directTransport.produce({
+		kind: 'video',
+		rtpParameters: {
+			mid: 'VIDEO',
+			codecs: [
+				{
+					mimeType: 'video/VP8',
+					payloadType: 112,
+					clockRate: 90000,
+					rtcpFeedback: [
+						{ type: 'nack', parameter: 'pli' },
+						{ type: 'goog-remb' },
+						{ type: 'transport-cc' },
+					],
+				},
+			],
+			headerExtensions: [
+				{
+					uri: 'http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time',
+					id: 4,
+				},
+				{
+					uri: 'http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01',
+					id: 5,
+				},
+				{
+					uri: 'urn:3gpp:video-orientation',
+					id: 13,
+				},
+			],
+			encodings: [{ ssrc: 22222222 }],
+			rtcp: { cname: 'video-1' },
+		},
+	});
+
+	expect(producer.rtpParameters.headerExtensions).toEqual([
+		{
+			uri: 'urn:3gpp:video-orientation',
+			id: 13,
+			encrypt: false,
+			parameters: {},
+		},
+	]);
+	expect(producer.rtpParameters.codecs[0]!.rtcpFeedback).toEqual([
+		{ type: 'nack', parameter: 'pli' },
+	]);
+
+	router.close();
+}, 2000);
+
+test('directTransport.consume() removes BWE stuff from RTP parameters', async () => {
+	const router = await ctx.worker!.createRouter({
+		mediaCodecs: [
+			{
+				kind: 'video',
+				mimeType: 'video/VP8',
+				clockRate: 90000,
+			},
+		],
+	});
+
+	const directTransport = await router.createDirectTransport();
+
+	const producer = await directTransport.produce({
+		kind: 'video',
+		rtpParameters: {
+			mid: 'VIDEO',
+			codecs: [
+				{
+					mimeType: 'video/VP8',
+					payloadType: 112,
+					clockRate: 90000,
+					rtcpFeedback: [{ type: 'nack', parameter: 'pli' }],
+				},
+			],
+			encodings: [{ ssrc: 22222222 }],
+			rtcp: { cname: 'video-1' },
+		},
+	});
+
+	// Consume with the full Router RTP capabilities, which do include the BWE
+	// header extensions and RTCP feedback.
+	const consumer = await directTransport.consume({
+		producerId: producer.id,
+		rtpCapabilities: router.rtpCapabilities,
+	});
+
+	expect(
+		consumer.rtpParameters.headerExtensions!.map(ext => ext.uri)
+	).not.toContain('http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time');
+	expect(
+		consumer.rtpParameters.headerExtensions!.map(ext => ext.uri)
+	).not.toContain(
+		'http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01'
+	);
+
+	for (const codec of consumer.rtpParameters.codecs) {
+		expect(codec.rtcpFeedback!.map(fb => fb.type)).not.toContain('goog-remb');
+		expect(codec.rtcpFeedback!.map(fb => fb.type)).not.toContain(
+			'transport-cc'
+		);
+	}
+
+	router.close();
+}, 2000);
+
 test('DirectTransport methods reject if closed', async () => {
 	const directTransport = await ctx.router!.createDirectTransport();
 	const onObserverClose = jest.fn();

rust/src/ortc.rs

@@ -759,6 +759,7 @@ pub(crate) fn get_consumer_rtp_parameters(
     remote_rtp_capabilities: &RtpCapabilities,
     pipe: bool,
     enable_rtx: bool,
+    disable_bwe: bool,
 ) -> Result<RtpParameters, ConsumerRtpParametersError> {
     let mut consumer_params = RtpParameters {
         rtcp: consumable_rtp_parameters.rtcp.clone(),
@@ -835,6 +836,17 @@ pub(crate) fn get_consumer_rtp_parameters(
         .cloned()
         .collect();
 
+    // A transport that doesn't run bandwidth estimation must not negotiate it. Dropping its header
+    // extensions is enough, since the RTCP feedback below is reduced out of them.
+    if disable_bwe {
+        consumer_params.header_extensions.retain(|ext| {
+            !matches!(
+                ext.uri,
+                RtpHeaderExtensionUri::AbsSendTime | RtpHeaderExtensionUri::TransportWideCcDraft01
+            )
+        });
+    }
+
     // Reduce codecs' RTCP feedback. Use Transport-CC if available, REMB otherwise.
     if consumer_params
         .header_extensions
@@ -953,9 +965,11 @@ pub(crate) fn get_pipe_consumer_rtp_parameters(
 
         let mut codec = codec.clone();
 
+        // Drop the bandwidth estimation feedback, which a PipeTransport doesn't run, and the
+        // generic NACK when there is no RTX to retransmit with.
         codec.rtcp_feedback_mut().retain(|fb| {
-            matches!(fb, RtcpFeedback::NackPli | RtcpFeedback::CcmFir)
-                || (enable_rtx && fb == &RtcpFeedback::Nack)
+            !matches!(fb, RtcpFeedback::GoogRemb | RtcpFeedback::TransportCc)
+                && (enable_rtx || fb != &RtcpFeedback::Nack)
         });
 
         consumer_params.codecs.push(codec);
@@ -1013,6 +1027,23 @@ pub(crate) fn get_pipe_consumer_rtp_parameters(
     consumer_params
 }
 
+/// Removes from the given RTP parameters everything related to bandwidth estimation, which a
+/// transport that doesn't run it must not negotiate.
+pub(crate) fn disable_bwe_in_rtp_parameters(rtp_parameters: &mut RtpParameters) {
+    rtp_parameters.header_extensions.retain(|ext| {
+        !matches!(
+            ext.uri,
+            RtpHeaderExtensionUri::AbsSendTime | RtpHeaderExtensionUri::TransportWideCcDraft01
+        )
+    });
+
+    for codec in &mut rtp_parameters.codecs {
+        codec
+            .rtcp_feedback_mut()
+            .retain(|fb| !matches!(fb, RtcpFeedback::GoogRemb | RtcpFeedback::TransportCc));
+    }
+}
+
 struct CodecToMatch<'a> {
     channels: Option<NonZeroU8>,
     clock_rate: NonZeroU32,