From 9339f92646ca63ae24122388a1d5008ce9cd7e77 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Fri, 1 Sep 2023 19:56:00 -0700 Subject: [PATCH] Bugfixes with multicodec simulcast (#349) * Attempt to simplify example app * * Fix crashes when backupCodec isn't set * Avoid using default settings, instead use the track's publication options * Correctly set SVC codec layers * fix import_sorter. * fix flutter analyzer. --------- Co-authored-by: cloudwebrtc --- example/android/build.gradle | 2 +- example/ios/Runner.xcodeproj/project.pbxproj | 1 + example/lib/pages/connect.dart | 73 ++++++---------- lib/src/core/room.dart | 6 +- lib/src/options.dart | 7 +- lib/src/participant/local.dart | 87 ++++++++++---------- lib/src/publication/local.dart | 3 + lib/src/track/local/video.dart | 9 +- lib/src/utils.dart | 28 +++++-- pubspec.lock | 30 +++---- 10 files changed, 122 insertions(+), 124 deletions(-) diff --git a/example/android/build.gradle b/example/android/build.gradle index 53f4dbec..e16e578d 100644 --- a/example/android/build.gradle +++ b/example/android/build.gradle @@ -24,6 +24,6 @@ subprojects { project.evaluationDependsOn(':app') } -task clean(type: Delete) { +tasks.register("clean", Delete) { delete rootProject.buildDir } diff --git a/example/ios/Runner.xcodeproj/project.pbxproj b/example/ios/Runner.xcodeproj/project.pbxproj index f96cd128..414cd407 100644 --- a/example/ios/Runner.xcodeproj/project.pbxproj +++ b/example/ios/Runner.xcodeproj/project.pbxproj @@ -300,6 +300,7 @@ files = ( ); inputPaths = ( + "${TARGET_BUILD_DIR}/${INFOPLIST_PATH}", ); name = "Thin Binary"; outputPaths = ( diff --git a/example/lib/pages/connect.dart b/example/lib/pages/connect.dart index e985d960..b13b7113 100644 --- a/example/lib/pages/connect.dart +++ b/example/lib/pages/connect.dart @@ -130,11 +130,6 @@ class _ConnectPageState extends State { print('Connecting with url: ${_uriCtrl.text}, ' 'token: ${_tokenCtrl.text}...'); - //create new room - final room = Room(); - - // Create a Listener before connecting - final listener = room.createListener(); E2EEOptions? e2eeOptions; if (_e2ee) { final keyProvider = await BaseKeyProvider.create(); @@ -143,55 +138,41 @@ class _ConnectPageState extends State { await keyProvider.setKey(sharedKey); } - BackupVideoCodec? backupVideoCodec; - String preferredCodec = 'H264'; - if (_multiCodec && _preferredCodec != 'Preferred Codec') { - if (['av1', 'vp9'].contains(_preferredCodec.toLowerCase())) { - backupVideoCodec = BackupVideoCodec( - simulcast: true, - codec: _backupCodec, - encoding: const VideoEncoding( - maxBitrate: 2 * 1000 * 1000, - maxFramerate: 30, - )); - } - + String preferredCodec = 'VP8'; + if (_preferredCodec != 'Preferred Codec') { preferredCodec = _preferredCodec; } + // create new room + final room = Room( + roomOptions: RoomOptions( + adaptiveStream: _adaptiveStream, + dynacast: _dynacast, + defaultAudioPublishOptions: const AudioPublishOptions( + dtx: true, + ), + defaultVideoPublishOptions: VideoPublishOptions( + simulcast: _simulcast, + videoCodec: preferredCodec, + ), + defaultScreenShareCaptureOptions: const ScreenShareCaptureOptions( + useiOSBroadcastExtension: true, + params: VideoParametersPresets.screenShareH1080FPS30), + e2eeOptions: e2eeOptions, + defaultCameraCaptureOptions: const CameraCaptureOptions( + maxFrameRate: 30, + params: VideoParametersPresets.h720_169, + ), + )); + + // Create a Listener before connecting + final listener = room.createListener(); + // Try to connect to the room // This will throw an Exception if it fails for any reason. await room.connect( _uriCtrl.text, _tokenCtrl.text, - roomOptions: RoomOptions( - adaptiveStream: _adaptiveStream, - dynacast: _dynacast, - defaultAudioPublishOptions: - const AudioPublishOptions(name: 'custom_audio_track_name'), - defaultVideoPublishOptions: VideoPublishOptions( - simulcast: _simulcast, - videoCodec: preferredCodec, - backupCodec: backupVideoCodec, - ), - defaultScreenShareCaptureOptions: const ScreenShareCaptureOptions( - useiOSBroadcastExtension: true, - params: VideoParameters( - dimensions: VideoDimensionsPresets.h1080_169, - encoding: VideoEncoding( - maxBitrate: 3 * 1000 * 1000, - maxFramerate: 15, - ))), - e2eeOptions: e2eeOptions, - defaultCameraCaptureOptions: const CameraCaptureOptions( - maxFrameRate: 30, - params: VideoParameters( - dimensions: VideoDimensionsPresets.h720_169, - encoding: VideoEncoding( - maxBitrate: 2 * 1000 * 1000, - maxFramerate: 30, - ))), - ), fastConnectOptions: _fastConnect ? FastConnectOptions( microphone: const TrackOption(enabled: true), diff --git a/lib/src/core/room.dart b/lib/src/core/room.dart index adf29fef..9e8f96c8 100644 --- a/lib/src/core/room.dart +++ b/lib/src/core/room.dart @@ -284,13 +284,13 @@ class Room extends DisposableChangeNotifier with EventsEmittable { } var videoTrack = publication.track as LocalVideoTrack; final newCodecs = await videoTrack.setPublishingCodecs( - event.subscribedCodecs, publication); + event.subscribedCodecs, videoTrack); for (var codec in newCodecs) { if (isBackupCodec(codec)) { logger.info( 'publishing backup codec ${codec} for ${publication.track?.sid}'); - await localParticipant?.publishAdditionalCodecForTrack( - videoTrack, codec, roomOptions.defaultVideoPublishOptions); + await localParticipant?.publishAdditionalCodecForPublication( + publication, codec); } } } else if (event.subscribedQualities.isNotEmpty) { diff --git a/lib/src/options.dart b/lib/src/options.dart index abbd9f06..33ea800e 100644 --- a/lib/src/options.dart +++ b/lib/src/options.dart @@ -163,12 +163,13 @@ class RoomOptions { class BackupVideoCodec { BackupVideoCodec({ - required this.codec, - required this.encoding, + this.codec = 'vp8', + this.encoding, this.simulcast = true, }); String codec; - VideoEncoding encoding; + // optional, when unset, it'll be computed based on dimensions and codec + VideoEncoding? encoding; bool simulcast; } diff --git a/lib/src/participant/local.dart b/lib/src/participant/local.dart index 727e8f91..ab6975f0 100644 --- a/lib/src/participant/local.dart +++ b/lib/src/participant/local.dart @@ -137,6 +137,24 @@ class LocalParticipant extends Participant { ); } + // handle SVC publishing + final isSVC = isSVCCodec(publishOptions.videoCodec); + if (isSVC) { + if (!room.roomOptions.dynacast) { + room.engine.roomOptions = room.roomOptions.copyWith(dynacast: true); + } + if (publishOptions.backupCodec == null) { + publishOptions = publishOptions.copyWith( + backupCodec: BackupVideoCodec(), + ); + } + if (publishOptions.scalabilityMode == null) { + publishOptions = publishOptions.copyWith( + scalabilityMode: 'L3T3_KEY', + ); + } + } + // use constraints passed to getUserMedia by default VideoDimensions dimensions = track.currentOptions.params.dimensions; @@ -159,14 +177,6 @@ class LocalParticipant extends Participant { logger.fine( 'Compute encodings with resolution: ${dimensions}, options: ${publishOptions}'); - if (isSVCCodec(publishOptions.videoCodec) && - publishOptions.scalabilityMode == null) { - // set scalabilityMode to 'L3T3_KEY' by default - publishOptions = publishOptions.copyWith( - scalabilityMode: 'L3T3_KEY', - ); - } - // Video encodings and simulcasts final encodings = Utils.computeVideoEncodings( isScreenShare: track.source == TrackSource.screenShareVideo, @@ -177,17 +187,17 @@ class LocalParticipant extends Participant { logger.fine('Using encodings: ${encodings?.map((e) => e.toMap())}'); - final layers = Utils.computeVideoLayers(dimensions, encodings); + final layers = Utils.computeVideoLayers( + dimensions, + encodings, + isSVC, + ); logger.fine('Video layers: ${layers.map((e) => e)}'); var simulcastCodecs = []; if (publishOptions.backupCodec != null && publishOptions.backupCodec!.codec != publishOptions.videoCodec) { - if (!room.roomOptions.dynacast) { - room.engine.roomOptions = room.roomOptions.copyWith(dynacast: true); - } - simulcastCodecs = [ lk_rtc.SimulcastCodec( codec: publishOptions.videoCodec.toLowerCase(), @@ -263,6 +273,7 @@ class LocalParticipant extends Participant { track: track, ); addTrackPublication(pub); + pub.backupVideoCodec = publishOptions.backupCodec; // did publish await track.onPublish(); @@ -545,44 +556,35 @@ class LocalParticipant extends Participant { return oldValue; } - Future publishAdditionalCodecForTrack( - LocalVideoTrack track, + Future publishAdditionalCodecForPublication( + LocalTrackPublication publication, String backupCodec, - VideoPublishOptions? options, ) async { - // is it not published? if so skip - LocalTrackPublication? existingPublication; - for (var publication in videoTracks) { - if (publication.track == null) { - continue; - } - if (publication.track == track) { - existingPublication = publication; - } - } - if (existingPublication == null) { - throw Exception('track is not published'); + if (publication.track is! LocalVideoTrack) { + throw Exception('multi-codec simulcast is supported only for video'); } + var track = publication.track as LocalVideoTrack; - options ??= room.roomOptions.defaultVideoPublishOptions; + final backupCodecOpts = publication.backupVideoCodec; + if (backupCodecOpts == null) { + throw Exception('backupCodec settings not specified'); + } - options = options.copyWith(simulcast: options.backupCodec!.simulcast); + var options = room.roomOptions.defaultVideoPublishOptions; + options = options.copyWith(simulcast: backupCodecOpts.simulcast); - if (options.backupCodec == null || - options.backupCodec?.codec.toLowerCase() == - options.videoCodec.toLowerCase()) { - // backup codec publishing is disabled + if (backupCodec.toLowerCase() == publication.track?.codec?.toLowerCase()) { + // not needed, same codec already published return; } - if (backupCodec != options.backupCodec?.codec.toLowerCase()) { + if (backupCodec != backupCodecOpts.codec.toLowerCase()) { logger.warning( - 'requested a different codec than specified as backup serverRequested: ${backupCodec}, backup: ${options.backupCodec?.codec}', + 'requested a different codec than specified as backup serverRequested: ${backupCodec}, backup: ${backupCodecOpts.codec}', ); } - var encodings = Utils.computeTrackBackupEncodings(track, options); - + var encodings = Utils.computeTrackBackupEncodings(track, backupCodecOpts); if (encodings == null) { logger.fine( 'backup codec has been disabled, ignoring request to add additional codec for track'); @@ -592,13 +594,14 @@ class LocalParticipant extends Participant { var simulcastTrack = track.addSimulcastTrack(backupCodec, encodings); var dimensions = track.currentOptions.params.dimensions; - var layers = Utils.computeVideoLayers(dimensions, encodings); + var layers = Utils.computeVideoLayers( + dimensions, encodings, isSVCCodec(backupCodec)); simulcastTrack.sender = await room.engine.createSimulcastTransceiverSender( track, simulcastTrack, encodings, - existingPublication, + publication, backupCodec, ); @@ -614,12 +617,12 @@ class LocalParticipant extends Participant { source: track.source.toPBType(), dimensions: dimensions, videoLayers: layers, - sid: existingPublication.sid, + sid: publication.sid, simulcastCodecs: [ lk_rtc.SimulcastCodec( codec: backupCodec.toLowerCase(), cid: cid, - enableSimulcastLayers: options.backupCodec!.simulcast), + enableSimulcastLayers: backupCodecOpts.simulcast), ]); await room.engine.negotiate(); diff --git a/lib/src/publication/local.dart b/lib/src/publication/local.dart index d2535a1b..e08ec9e6 100644 --- a/lib/src/publication/local.dart +++ b/lib/src/publication/local.dart @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import '../options.dart'; import '../participant/local.dart'; import '../proto/livekit_models.pb.dart' as lk_models; import '../proto/livekit_rtc.pb.dart' as lk_rtc; @@ -24,6 +25,8 @@ class LocalTrackPublication extends TrackPublication { @override final LocalParticipant participant; + BackupVideoCodec? backupVideoCodec; + LocalTrackPublication({ required this.participant, required lk_models.TrackInfo info, diff --git a/lib/src/track/local/video.dart b/lib/src/track/local/video.dart index 4c3437c5..af17c98c 100644 --- a/lib/src/track/local/video.dart +++ b/lib/src/track/local/video.dart @@ -21,7 +21,6 @@ import '../../events.dart'; import '../../logger.dart'; import '../../proto/livekit_models.pb.dart' as lk_models; import '../../proto/livekit_rtc.pb.dart' as lk_rtc; -import '../../publication/local.dart'; import '../../support/platform.dart'; import '../../types/other.dart'; import '../options.dart'; @@ -277,13 +276,13 @@ extension LocalVideoTrackExt on LocalVideoTrack { }); } - Future> setPublishingCodecs(List codecs, - LocalTrackPublication publication) async { + Future> setPublishingCodecs( + List codecs, LocalTrack track) async { logger.fine('setPublishingCodecs $codecs'); // only enable simulcast codec for preference codec setted if (codec == null && codecs.isNotEmpty) { - await updatePublishingLayers(publication.track, codecs[0].qualities); + await updatePublishingLayers(track, codecs[0].qualities); return []; } @@ -293,7 +292,7 @@ extension LocalVideoTrackExt on LocalVideoTrack { for (var codec in codecs) { if (this.codec?.toLowerCase() == codec.codec.toLowerCase()) { - await updatePublishingLayers(publication.track, codec.qualities); + await updatePublishingLayers(track, codec.qualities); } else { final simulcastCodecInfo = simulcastCodecs[codec.codec]; logger.fine('setPublishingCodecs $codecs'); diff --git a/lib/src/utils.dart b/lib/src/utils.dart index dc6f6459..751457fa 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -489,12 +489,13 @@ class Utils { @internal static List? computeTrackBackupEncodings( LocalVideoTrack track, - VideoPublishOptions opts, + BackupVideoCodec backupOpts, ) { - opts = opts.copyWith( - videoCodec: opts.backupCodec!.codec, - videoEncoding: opts.backupCodec!.encoding); - + final opts = VideoPublishOptions( + videoCodec: backupOpts.codec, + videoEncoding: backupOpts.encoding, + simulcast: backupOpts.simulcast, + ); var encodings = computeVideoEncodings( isScreenShare: track.source == TrackSource.screenShareVideo, dimensions: track.currentOptions.params.dimensions, @@ -507,6 +508,7 @@ class Utils { static List computeVideoLayers( VideoDimensions dimensions, List? encodings, + bool isSVC, ) { // default to a single layer, HQ if (encodings == null) { @@ -520,6 +522,22 @@ class Utils { ]; } + if (isSVC) { + final sm = ScalabilityMode(encodings[0].scalabilityMode ?? 'L3T3_KEY'); + final List layers = []; + final maxBitrate = encodings[0].maxBitrate ?? 0; + for (var i = 0; i < sm.spatial; i++) { + layers.add(lk_models.VideoLayer( + quality: lk_models.VideoQuality.valueOf( + lk_models.VideoQuality.HIGH.value - i), + width: (dimensions.width / math.pow(2, i)).floor(), + height: (dimensions.height / math.pow(2, i)).floor(), + bitrate: (maxBitrate / math.pow(3, i)).ceil(), + )); + } + return layers; + } + return encodings.map((e) { final scale = e.scaleResolutionDownBy ?? 1; var quality = videoQualityForRid(e.rid); diff --git a/pubspec.lock b/pubspec.lock index 68be44ff..5e7caf53 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -93,10 +93,10 @@ packages: dependency: "direct main" description: name: collection - sha256: f092b211a4319e98e5ff58223576de6c2803db36221657b46c82574721240687 + sha256: "4a07be6cb69c84d677a6c3096fcf960cc3285a8330b4603e0d463d15d9bd934c" url: "https://pub.dev" source: hosted - version: "1.17.2" + version: "1.17.1" connectivity_plus: dependency: "direct main" description: @@ -308,18 +308,18 @@ packages: dependency: transitive description: name: matcher - sha256: "1803e76e6653768d64ed8ff2e1e67bea3ad4b923eb5c56a295c3e634bad5960e" + sha256: "6501fbd55da300384b768785b83e5ce66991266cec21af89ab9ae7f5ce1c4cbb" url: "https://pub.dev" source: hosted - version: "0.12.16" + version: "0.12.15" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: "9528f2f296073ff54cb9fee677df673ace1218163c3bc7628093e7eed5203d41" + sha256: d92141dc6fe1dad30722f9aa826c7fbc896d021d792f80678280601aff8cf724 url: "https://pub.dev" source: hosted - version: "0.5.0" + version: "0.2.0" meta: dependency: "direct main" description: @@ -473,10 +473,10 @@ packages: dependency: transitive description: name: source_span - sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" + sha256: dd904f795d4b4f3b870833847c461801f6750a9fa8e61ea5ac53f9422b31f250 url: "https://pub.dev" source: hosted - version: "1.10.0" + version: "1.9.1" stack_trace: dependency: transitive description: @@ -521,10 +521,10 @@ packages: dependency: transitive description: name: test_api - sha256: "75760ffd7786fffdfb9597c35c5b27eaeec82be8edfb6d71d32651128ed7aab8" + sha256: eb6ac1540b26de412b3403a163d919ba86f6a973fe6cc50ae3541b80092fdcfb url: "https://pub.dev" source: hosted - version: "0.6.0" + version: "0.5.1" tint: dependency: transitive description: @@ -565,14 +565,6 @@ packages: url: "https://pub.dev" source: hosted version: "1.1.0" - web: - dependency: transitive - description: - name: web - sha256: dc8ccd225a2005c1be616fe02951e2e342092edf968cf0844220383757ef8f10 - url: "https://pub.dev" - source: hosted - version: "0.1.4-beta" webrtc_interface: dependency: transitive description: @@ -622,5 +614,5 @@ packages: source: hosted version: "3.1.2" sdks: - dart: ">=3.1.0-185.0.dev <4.0.0" + dart: ">=3.0.0 <4.0.0" flutter: ">=3.3.0"