Seed the publisher's video start bitrate through setBitrate on native

.changes/video-start-bitrate-set-bitrate

@@ -0,0 +1 @@
+patch type="changed" "On iOS, macOS and Android the video start bitrate is now set through the peer connection's bitrate API instead of an SDP edit, while web, Windows and Linux keep writing it into the offer"

lib/src/core/transport.dart

@@ -171,6 +171,22 @@ void applyVideoStartBitrate(Map<String, dynamic> media, int codecPayload, int st
   media['fmtp'] = fmtpList;
 }
 
+/// How the connection-level video start bitrate reaches libwebrtc.
+enum VideoStartBitrateMethod {
+  /// `RTCPeerConnection.setBitrate` once the offer is sent.
+  setBitrate,
+
+  /// `x-google-start-bitrate` written into the offer.
+  sdp,
+}
+
+/// `setBitrate` is implemented natively only on iOS, macOS and Android. Host unit tests use
+/// the SDP method unless a test picks one.
+VideoStartBitrateMethod _defaultVideoStartBitrateMethod() =>
+    !lkPlatformIsTest() && [PlatformType.iOS, PlatformType.macOS, PlatformType.android].contains(lkPlatform())
+    ? VideoStartBitrateMethod.setBitrate
+    : VideoStartBitrateMethod.sdp;
+
 typedef TransportOnOffer = void Function(rtc.RTCSessionDescription offer);
 typedef TransportOnNegotiationError = void Function(Object error);
 typedef PeerConnectionCreate =
@@ -182,11 +198,16 @@ class Transport extends Disposable {
   final List<rtc.RTCIceCandidate> _pendingCandidates = [];
   final List<TrackBitrateInfo> _bitrateTrackers = [];
 
-  /// Whether an offer carrying the connection-level `x-google-start-bitrate` has been
-  /// accepted locally. The hint is written once per peer connection: libwebrtc retains
-  /// `start_bitrate_bps` in `RtpBitrateConfigurator` and re-applies it on network route
-  /// changes, so a later rewrite is at best a no-op and at worst restarts a converged
-  /// bandwidth estimator. A new peer connection (full reconnect) seeds a new estimator.
+  final VideoStartBitrateMethod _videoStartBitrateMethod;
+
+  /// Whether the connection-level video start bitrate has been applied. With
+  /// [VideoStartBitrateMethod.sdp] that is an accepted offer carrying `x-google-start-bitrate`.
+  /// With [VideoStartBitrateMethod.setBitrate] it is set while the call is in flight and
+  /// cleared again if the call fails. The value is applied once per peer connection:
+  /// libwebrtc retains `start_bitrate_bps` in `RtpBitrateConfigurator` and re-applies it on
+  /// network route changes, so a later rewrite is at best a no-op and at worst restarts a
+  /// converged bandwidth estimator. A new peer connection (full reconnect) seeds a new
+  /// estimator.
   bool _hasAppliedVideoStartBitrate = false;
 
   bool restartingIce = false;
@@ -197,7 +218,7 @@ class Transport extends Disposable {
   ConnectOptions connectOptions;
 
   // private constructor
-  Transport._(this.pc, this.connectOptions) {
+  Transport._(this.pc, this.connectOptions, this._videoStartBitrateMethod) {
     //
     onDispose(() async {
       _cancelDebounce?.call();
@@ -235,11 +256,16 @@ class Transport extends Disposable {
     PeerConnectionCreate peerConnectionCreate, {
     RTCConfiguration? rtcConfig,
     required ConnectOptions connectOptions,
+    @visibleForTesting VideoStartBitrateMethod? videoStartBitrateMethod,
   }) async {
     rtcConfig ??= const RTCConfiguration();
     logger.fine('[PCTransport] creating ${rtcConfig.toMap()}');
     final pc = await peerConnectionCreate(rtcConfig.toMap());
-    return Transport._(pc, connectOptions);
+    return Transport._(
+      pc,
+      connectOptions,
+      videoStartBitrateMethod ?? _defaultVideoStartBitrateMethod(),
+    );
   }
 
   late final negotiate = Utils.createDebounceFunc(
@@ -335,13 +361,16 @@ class Transport extends Disposable {
     }
 
     final sdpParsed = sdp_transform.parse(offer.sdp ?? '');
-    // One value for every video m-section, written only on the first offer that carries local
-    // video: the hint is connection-level in libwebrtc, so differing per-section values would
-    // be last-writer-wins on m-section order. Offers before any video is published (data
-    // channel or audio only) find no target and leave the latch unset.
+    // One connection-level value, applied only for the first offer that carries local video:
+    // libwebrtc keeps a single start bitrate per connection, so differing per-section values
+    // would be last-writer-wins on m-section order. The sdp method writes it into every video
+    // m-section. The setBitrate method passes it to the peer connection once the offer is sent.
+    // Offers before any video is published (data channel or audio only) find no target and
+    // leave the latch unset.
     final connectionStartBitrate = _hasAppliedVideoStartBitrate
         ? null
         : computeConnectionStartBitrate(sdpParsed['media'] ?? const [], _bitrateTrackers);
+    final sdpStartBitrate = _videoStartBitrateMethod == VideoStartBitrateMethod.sdp ? connectionStartBitrate : null;
     var appliedVideoStartBitrate = false;
     sdpParsed['media']?.forEach((media) {
       if (media['type'] == 'video') {
@@ -357,8 +386,8 @@ class Transport extends Disposable {
           if (codecPayload == null) {
             continue;
           }
-          if (codecPayload > 0 && connectionStartBitrate != null) {
-            applyVideoStartBitrate(media, codecPayload, connectionStartBitrate);
+          if (codecPayload > 0 && sdpStartBitrate != null) {
+            applyVideoStartBitrate(media, codecPayload, sdpStartBitrate);
             appliedVideoStartBitrate = true;
           }
           break;
@@ -378,6 +407,29 @@ class Transport extends Disposable {
       _hasAppliedVideoStartBitrate = true;
     }
     onOffer?.call(offer);
+
+    if (_videoStartBitrateMethod == VideoStartBitrateMethod.setBitrate &&
+        connectionStartBitrate != null &&
+        !_hasAppliedVideoStartBitrate) {
+      await _setVideoStartBitrate(connectionStartBitrate);
+    }
+  }
+
+  /// Never throws: a failure here would be reported as a failed negotiation although the offer
+  /// was already sent.
+  Future<void> _setVideoStartBitrate(int startBitrateKbps) async {
+    // Latch before awaiting so an offer created meanwhile does not seed again.
+    _hasAppliedVideoStartBitrate = true;
+    try {
+      if (await pc.setBitrate(startBitrate: startBitrateKbps * 1000)) {
+        logger.fine('[$objectId] video start bitrate set to $startBitrateKbps kbps');
+        return;
+      }
+      logger.warning('[$objectId] setBitrate() rejected video start bitrate $startBitrateKbps kbps');
+    } catch (error) {
+      logger.warning('[$objectId] setBitrate() failed with error: $error');
+    }
+    _hasAppliedVideoStartBitrate = false;
   }
 
   Future<void> addIceCandidate(rtc.RTCIceCandidate candidate) async {

test/core/start_bitrate_test.dart

@@ -15,6 +15,8 @@
 @Timeout(Duration(seconds: 5))
 library;
 
+import 'dart:async';
+
 import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter_webrtc/flutter_webrtc.dart' as rtc;
 import 'package:sdp_transform/sdp_transform.dart' as sdp_transform;
@@ -27,6 +29,7 @@ import 'package:livekit_client/src/core/transport.dart'
     show
         TrackBitrateInfo,
         Transport,
+        VideoStartBitrateMethod,
         applyVideoStartBitrate,
         computeConnectionStartBitrate,
         computeTrackStartBitrate,
@@ -102,8 +105,58 @@ a=mid:1
 a=msid:stream other-track
 a=rtpmap:96 VP8/90000''';
 
+// An offer before any video is published: only the data channel.
+const dataChannelOnly = '''v=0
+o=- 0 0 IN IP4 127.0.0.1
+s=-
+t=0 0
+a=group:BUNDLE 0
+m=application 9 UDP/DTLS/SCTP webrtc-datachannel
+c=IN IP4 0.0.0.0
+a=mid:0
+a=sctp-port:5000''';
+
 List<dynamic> mediaOf(String sdp) => sdp_transform.parse(sdp)['media'] as List<dynamic>;
 
+// [twoVideoSections] as a real offer lists it, with more than one payload per video section.
+// `createAndSendOffer` reads the payload list as a string, and a single payload parses as an int.
+final videoOffer = twoVideoSections.replaceAll('SAVPF 96', 'SAVPF 96 97');
+
+/// Offers [offerSdp] and can hold [setBitrate] open until [setBitrateGate] completes.
+class _OfferingPeerConnection extends MockPeerConnection {
+  String offerSdp = videoOffer;
+  Completer<void>? setBitrateGate;
+
+  @override
+  Future<rtc.RTCSessionDescription> createOffer([Map<String, dynamic>? constraints]) async =>
+      rtc.RTCSessionDescription(offerSdp, 'offer');
+
+  @override
+  Future<bool> setBitrate({int? minBitrate, int? startBitrate, int? maxBitrate}) async {
+    final result = await super.setBitrate(minBitrate: minBitrate, startBitrate: startBitrate, maxBitrate: maxBitrate);
+    await setBitrateGate?.future;
+    return result;
+  }
+
+  /// Answers the pending offer so the next one is sent rather than deferred.
+  Future<void> answer() => setRemoteDescription(rtc.RTCSessionDescription('v=0', 'answer'));
+}
+
+Future<(Transport, _OfferingPeerConnection)> createTransport(VideoStartBitrateMethod method) async {
+  final pc = _OfferingPeerConnection();
+  final transport = await Transport.create(
+    (Map<String, dynamic> configuration, [Map<String, dynamic>? constraints]) async => pc,
+    connectOptions: const ConnectOptions(),
+    videoStartBitrateMethod: method,
+  );
+  addTearDown(transport.dispose);
+  transport.onNegotiationError = (error) => fail('unexpected negotiation error: $error');
+  return (transport, pc);
+}
+
+// The camera on mid 1 of [videoOffer] targets 1000 kbps, so the connection value is 900.
+TrackBitrateInfo cameraTracker() => TrackBitrateInfo(cid: 'camera-cid', transceiver: null, codec: 'VP8', maxbr: 1000);
+
 void main() {
   group('video start bitrate', () {
     test('matches only the section whose msid track ID matches the cid', () {
@@ -237,6 +290,138 @@ void main() {
     });
   });
 
+  group('video start bitrate through setBitrate', () {
+    test('the first offer with video calls setBitrate once, after onOffer, without the SDP hint', () async {
+      final (transport, pc) = await createTransport(VideoStartBitrateMethod.setBitrate);
+      transport.setTrackBitrateInfo(cameraTracker());
+      final callsAtOffer = <int>[];
+      final offers = <rtc.RTCSessionDescription>[];
+      transport.onOffer = (offer) {
+        callsAtOffer.add(pc.setBitrateCalls.length);
+        offers.add(offer);
+      };
+
+      await transport.createAndSendOffer();
+
+      expect(callsAtOffer, [0]);
+      expect(pc.setBitrateCalls, [(minBitrate: null, startBitrate: 900000, maxBitrate: null)]);
+      expect(offers.single.sdp, isNot(contains('x-google-start-bitrate')));
+      expect((await pc.getLocalDescription())!.sdp, isNot(contains('x-google-start-bitrate')));
+    });
+
+    test('a later offer does not call setBitrate again', () async {
+      final (transport, pc) = await createTransport(VideoStartBitrateMethod.setBitrate);
+      transport.setTrackBitrateInfo(cameraTracker());
+      transport.onOffer = (_) {};
+
+      await transport.createAndSendOffer();
+      await pc.answer();
+      await transport.createAndSendOffer();
+
+      expect(pc.setBitrateCalls, hasLength(1));
+    });
+
+    test('an offer created while setBitrate is pending does not call it again', () async {
+      final (transport, pc) = await createTransport(VideoStartBitrateMethod.setBitrate);
+      transport.setTrackBitrateInfo(cameraTracker());
+      transport.onOffer = (_) {};
+      final gate = pc.setBitrateGate = Completer<void>();
+
+      final first = transport.createAndSendOffer();
+      while (pc.setBitrateCalls.isEmpty) {
+        await Future<void>.delayed(Duration.zero);
+      }
+      await pc.answer();
+      await transport.createAndSendOffer();
+      gate.complete();
+      await first;
+
+      expect(pc.setBitrateCalls, hasLength(1));
+    });
+
+    test('an offer before any video does not call it, the first offer with video does', () async {
+      final (transport, pc) = await createTransport(VideoStartBitrateMethod.setBitrate);
+      transport.onOffer = (_) {};
+
+      pc.offerSdp = dataChannelOnly;
+      await transport.createAndSendOffer();
+      expect(pc.setBitrateCalls, isEmpty);
+
+      await pc.answer();
+      pc.offerSdp = videoOffer;
+      transport.setTrackBitrateInfo(cameraTracker());
+      await transport.createAndSendOffer();
+      expect(pc.setBitrateCalls.map((c) => c.startBitrate), [900000]);
+    });
+
+    for (final (name, fail) in <(String, void Function(_OfferingPeerConnection))>[
+      ('a false result', (pc) => pc.setBitrateResult = false),
+      ('a thrown error', (pc) => pc.setBitrateError = Exception('setBitrate failed')),
+    ]) {
+      test('$name does not consume it, and the next offer retries', () async {
+        final (transport, pc) = await createTransport(VideoStartBitrateMethod.setBitrate);
+        transport.setTrackBitrateInfo(cameraTracker());
+        final offers = <rtc.RTCSessionDescription>[];
+        transport.onOffer = offers.add;
+        final reported = <Object>[];
+        transport.onNegotiationError = reported.add;
+        fail(pc);
+
+        // The debounced path reports anything createAndSendOffer throws as a negotiation error.
+        transport.negotiate(null);
+        await Future<void>.delayed(const Duration(milliseconds: 200));
+        expect(offers, hasLength(1));
+        expect(reported, isEmpty);
+        expect(pc.setBitrateCalls, hasLength(1));
+
+        pc
+          ..setBitrateResult = true
+          ..setBitrateError = null;
+        await pc.answer();
+        await transport.createAndSendOffer();
+        expect(pc.setBitrateCalls.map((c) => c.startBitrate), [900000, 900000]);
+
+        await pc.answer();
+        await transport.createAndSendOffer();
+        expect(pc.setBitrateCalls, hasLength(2));
+      });
+    }
+  });
+
+  group('video start bitrate through the SDP', () {
+    test('the first offer with video carries the hint and setBitrate is never called', () async {
+      final (transport, pc) = await createTransport(VideoStartBitrateMethod.sdp);
+      transport.setTrackBitrateInfo(cameraTracker());
+      final offers = <rtc.RTCSessionDescription>[];
+      transport.onOffer = offers.add;
+
+      await transport.createAndSendOffer();
+      await pc.answer();
+      await transport.createAndSendOffer();
+
+      expect(paramSet(fmtpOf(mediaOf(offers[0].sdp!), '1', 96)!), contains('x-google-start-bitrate=900'));
+      expect(offers[1].sdp, isNot(contains('x-google-start-bitrate')));
+      expect(pc.setBitrateCalls, isEmpty);
+    });
+
+    test('is the default in host tests', () async {
+      final pc = _OfferingPeerConnection();
+      final transport = await Transport.create(
+        (Map<String, dynamic> configuration, [Map<String, dynamic>? constraints]) async => pc,
+        connectOptions: const ConnectOptions(),
+      );
+      addTearDown(transport.dispose);
+      transport.setTrackBitrateInfo(cameraTracker());
+      final offers = <rtc.RTCSessionDescription>[];
+      transport.onOffer = offers.add;
+
+      await transport.createAndSendOffer();
+
+      expect(offers.single.sdp, contains('x-google-start-bitrate=900'));
+      expect(pc.setBitrateCalls, isEmpty);
+    });
+  });
+
   group('computeStartTargetBitrate', () {
     final simulcast = [
       rtc.RTCRtpEncoding(rid: 'q', maxBitrate: 150000),

test/mock/peerconnection_mock.dart

@@ -294,6 +294,23 @@ a=rtpmap:32 MPV/90000
     appliedConfiguration = configuration;
   }
 
+  /// Arguments of every [setBitrate] call, in order.
+  final List<({int? minBitrate, int? startBitrate, int? maxBitrate})> setBitrateCalls = [];
+
+  /// What [setBitrate] returns, unless [setBitrateError] is set, in which case it throws that.
+  bool setBitrateResult = true;
+  Object? setBitrateError;
+
+  @override
+  Future<bool> setBitrate({int? minBitrate, int? startBitrate, int? maxBitrate}) async {
+    setBitrateCalls.add((minBitrate: minBitrate, startBitrate: startBitrate, maxBitrate: maxBitrate));
+    final error = setBitrateError;
+    if (error != null) {
+      throw error;
+    }
+    return setBitrateResult;
+  }
+
   static Future<RTCPeerConnection> create(
     Map<String, dynamic> configuration, [
     Map<String, dynamic>? constraints,