Skip to content

Commit 7c232d9

Browse files
hiroshihoriehiroshihorie
authored
Clear simulcast codec state on unpublish and full reconnect (#1159)
Stacked on #1155, review that first. Only the last commit (`34aa23b`) is new here. Follow-up to the #1155 review. `LocalVideoTrack.simulcastCodecs` and `encodingBackups` were never cleared, so the backup codec senders they hold outlived the peer connection they belonged to: - After a full reconnect, `rePublishAllTracks` reuses the same track object, so later operations (like the degradation preference fan out from #1155) acted on senders from the torn down connection. #1155 guards those calls with try/catch, this PR removes the stale state itself. - If the server requested the backup codec again after a reconnect, `addSimulcastTrack` threw `'<codec> already added'` and the backup codec was never republished. - Unpublish removed the simulcast senders inside a fire and forget `forEach` (nothing awaited it) and force unwrapped a nullable sender. The fix adds `LocalVideoTrack.clearSimulcastState()` and calls it from the two places senders become invalid: `removePublishedTrack` (after removing them from the peer connection, now awaited via a snapshot loop) and `rePublishAllTracks` (before republishing on the new connection). ## Tests `test/track/simulcast_state_test.dart` pins the invariant: re-adding a backup codec after clearing succeeds where it previously threw, and encoding backups are cleared too. Full suite passes (392 tests), analyze, format, and import sorter clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent f24c1ea commit 7c232d9

4 files changed

Lines changed: 210 additions & 10 deletions

File tree

‎.changes/simulcast-codecs-lifecycle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
patch type="fixed" "Backup codec state is cleared on unpublish and full reconnect, so republishing no longer acts on senders from a torn down connection"

‎lib/src/participant/local.dart‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -552,23 +552,41 @@ class LocalParticipant extends Participant<LocalTrackPublication> {
552552
}
553553

554554
final sender = track.transceiver?.sender;
555+
var didRemoveSender = false;
555556
if (sender != null) {
556557
try {
557558
await room.engine.publisher?.pc.removeTrack(sender);
558-
if (track is LocalVideoTrack) {
559-
track.simulcastCodecs.forEach((key, simulcastTrack) async {
560-
await room.engine.publisher?.pc.removeTrack(simulcastTrack.sender!);
561-
});
562-
}
563559
} catch (e) {
564560
logger.warning('[$objectId] rtc.removeTrack() did throw $e');
565561
}
562+
didRemoveSender = true;
563+
}
566564

567-
// doesn't make sense to negotiate if already disposed
568-
if (!isDisposed) {
569-
// manual negotiation since track changed
570-
await room.engine.negotiate();
565+
// not gated on the primary sender, stale backup codec state must not
566+
// survive unpublish even when the track never got a live sender
567+
if (track is LocalVideoTrack) {
568+
// remove each backup sender on its own, one failure should not
569+
// prevent removal of the others
570+
for (final simulcastTrack in track.simulcastCodecs.values.toList()) {
571+
final simulcastSender = simulcastTrack.sender;
572+
if (simulcastSender == null) {
573+
continue;
574+
}
575+
try {
576+
await room.engine.publisher?.pc.removeTrack(simulcastSender);
577+
} catch (e) {
578+
logger.warning('[$objectId] rtc.removeTrack() did throw $e');
579+
}
580+
simulcastTrack.sender = null;
581+
didRemoveSender = true;
571582
}
583+
track.clearSimulcastState();
584+
}
585+
586+
// doesn't make sense to negotiate if already disposed
587+
if (didRemoveSender && !isDisposed) {
588+
// manual negotiation since track changed
589+
await room.engine.negotiate();
572590
}
573591

574592
// did unpublish
@@ -605,7 +623,11 @@ class LocalParticipant extends Participant<LocalTrackPublication> {
605623
if (track.track is LocalAudioTrack) {
606624
await publishAudioTrack(track.track as LocalAudioTrack);
607625
} else if (track.track is LocalVideoTrack) {
608-
await publishVideoTrack(track.track as LocalVideoTrack);
626+
final videoTrack = track.track as LocalVideoTrack;
627+
// a full reconnect replaced the peer connection, so any simulcast
628+
// codec senders the track still holds belong to the old one
629+
videoTrack.clearSimulcastState();
630+
await publishVideoTrack(videoTrack);
609631
}
610632
}
611633
}

‎lib/src/track/local/video.dart‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,18 @@ extension LocalVideoTrackExt on LocalVideoTrack {
504504
return simulcastCodecInfo;
505505
}
506506

507+
/// Drops all simulcast codec state tied to the current publish session.
508+
///
509+
/// Must be called when the track's senders are no longer valid, on unpublish
510+
/// and before republishing after a full reconnect. Otherwise later publishes
511+
/// see stale senders and [addSimulcastTrack] rejects the codec as a duplicate
512+
/// when the server requests the backup codec again.
513+
@internal
514+
void clearSimulcastState() {
515+
simulcastCodecs.clear();
516+
encodingBackups.clear();
517+
}
518+
507519
Future<void> setDegradationPreference(DegradationPreference preference) async {
508520
_degradationPreference = preference;
509521
await applyDegradationPreference(sender);

‎test/track/simulcast_state_test.dart‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
// Copyright 2026 LiveKit, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import 'dart:typed_data';
16+
17+
import 'package:flutter_test/flutter_test.dart';
18+
import 'package:flutter_webrtc/flutter_webrtc.dart' as rtc;
19+
20+
import 'package:livekit_client/src/track/local/video.dart';
21+
import 'package:livekit_client/src/track/options.dart';
22+
import 'package:livekit_client/src/types/other.dart';
23+
24+
void main() {
25+
TestWidgetsFlutterBinding.ensureInitialized();
26+
27+
LocalVideoTrack createTrack() {
28+
final mediaTrack = _FakeMediaStreamTrack(id: 'video-1', kind: 'video');
29+
final stream = _FakeMediaStream('stream-1');
30+
return LocalVideoTrack(
31+
TrackSource.camera,
32+
stream,
33+
mediaTrack,
34+
const CameraCaptureOptions(),
35+
);
36+
}
37+
38+
group('clearSimulcastState', () {
39+
test('allows re-adding a backup codec after clearing', () {
40+
final track = createTrack();
41+
42+
track.addSimulcastTrack('vp8', []);
43+
// simulates the server requesting the same backup codec again after a
44+
// reconnect, which previously threw because the map was never cleared
45+
expect(() => track.addSimulcastTrack('vp8', []), throwsException);
46+
47+
track.clearSimulcastState();
48+
expect(track.simulcastCodecs, isEmpty);
49+
expect(() => track.addSimulcastTrack('vp8', []), returnsNormally);
50+
});
51+
52+
test('clears encoding backups as well', () {
53+
final track = createTrack();
54+
track.encodingBackups[('sender-1', 0)] = rtc.RTCRtpEncoding();
55+
56+
track.clearSimulcastState();
57+
58+
expect(track.encodingBackups, isEmpty);
59+
});
60+
});
61+
}
62+
63+
class _FakeMediaStream extends rtc.MediaStream {
64+
final List<rtc.MediaStreamTrack> _tracks = [];
65+
66+
_FakeMediaStream(String id) : super(id, 'fake-owner');
67+
68+
@override
69+
bool? get active => true;
70+
71+
@override
72+
Future<void> addTrack(rtc.MediaStreamTrack track, {bool addToNative = true}) async {
73+
_tracks.add(track);
74+
}
75+
76+
@override
77+
Future<rtc.MediaStream> clone() async => _FakeMediaStream('${id}_clone');
78+
79+
@override
80+
List<rtc.MediaStreamTrack> getAudioTracks() => _tracks.where((t) => t.kind == 'audio').toList();
81+
82+
@override
83+
Future<void> getMediaTracks() async {}
84+
85+
@override
86+
List<rtc.MediaStreamTrack> getTracks() => List<rtc.MediaStreamTrack>.from(_tracks);
87+
88+
@override
89+
List<rtc.MediaStreamTrack> getVideoTracks() => _tracks.where((t) => t.kind == 'video').toList();
90+
91+
@override
92+
Future<void> removeTrack(rtc.MediaStreamTrack track, {bool removeFromNative = true}) async {
93+
_tracks.remove(track);
94+
}
95+
}
96+
97+
class _FakeMediaStreamTrack implements rtc.MediaStreamTrack {
98+
@override
99+
rtc.StreamTrackCallback? onEnded;
100+
101+
@override
102+
rtc.StreamTrackCallback? onMute;
103+
104+
@override
105+
rtc.StreamTrackCallback? onUnMute;
106+
107+
@override
108+
bool enabled;
109+
110+
@override
111+
final String id;
112+
113+
@override
114+
final String kind;
115+
116+
@override
117+
String? get label => '$kind-track';
118+
119+
@override
120+
bool? get muted => false;
121+
122+
_FakeMediaStreamTrack({
123+
required this.id,
124+
required this.kind,
125+
this.enabled = true,
126+
});
127+
128+
@override
129+
Future<void> adaptRes(int width, int height) async {}
130+
131+
@override
132+
Future<void> applyConstraints([Map<String, dynamic>? constraints]) async {}
133+
134+
@override
135+
Future<ByteBuffer> captureFrame() {
136+
throw UnimplementedError();
137+
}
138+
139+
@override
140+
Future<rtc.MediaStreamTrack> clone() async => _FakeMediaStreamTrack(id: id, kind: kind, enabled: enabled);
141+
142+
@override
143+
Future<void> dispose() async {}
144+
145+
@override
146+
Map<String, dynamic> getConstraints() => const {};
147+
148+
@override
149+
Map<String, dynamic> getSettings() => const {};
150+
151+
@override
152+
Future<bool> hasTorch() async => false;
153+
154+
@override
155+
void enableSpeakerphone(bool enable) {}
156+
157+
@override
158+
Future<void> setTorch(bool torch) async {}
159+
160+
@override
161+
Future<void> stop() async {}
162+
163+
@override
164+
Future<bool> switchCamera() async => false;
165+
}

0 commit comments

Comments
 (0)