Fix video republishing issues

[pblazej]

Summary

republishAllTracks() routes every local track through unpublish(), which calls track.stop() and tears down the capturer. For iOS broadcast extension and ReplayKit screen capture this closes the IPC and the extension self-terminates — the track gets re-added to the new publisher but no frames flow.

Fixed by giving unpublish() an internal stopTrack: variant and taking that path for screen shares, so the capturer keeps running while the track is properly detached from the old publisher and reattached to the new one.

• unpublish(publication:notify:) stays as-is (public + ObjC selector unchanged) and forwards to a new internal _unpublish(publication:notify:stopTrack:). _unpublish is the previous body with the stop gated on the flag, so anything passing stopTrack: true behaves exactly as before.
• Screen shares are unpublished with stopTrack: false. Everything else unpublish() does still happens: sender + simulcast-sender removal from the publisher, publisherShouldNegotiate(), onUnpublish(), didUnpublishTrack.
• The detached publication then drops its track reference so it stops observing the capturer that is deliberately left running.
• rtpSenderForCodec.removeAll() in _unpublish, once the simulcast senders have been removed from the publisher. _publish re-attaches the main rtpSender but never the per-codec ones, so a stale entry made _set(subscribedCodec:) report a match against a sender of the destroyed peer connection and publish(additionalVideoCodec:) never re-added the backup codec.
• VideoCapturer.stopCapture() now nils the cached dimensions. It reset dimensionsCompleter but left _state.dimensions at its last value, and set(dimensions:) only resumes the completer when the value changes — so a source resuming at the same resolution never resolved dimensions again and _publish timed out in "Waiting for dimensions to resolve". CameraCapturer had its own set(dimensions: nil) on stop; BufferCapturer and BroadcastScreenCapturer did not, so no buffer-backed video track could be re-published after a full reconnect. This is the second half of the screen share failure.
• Muted screen shares take the normal path instead of being detached and then skipped.
• Per-track republish failures are logged individually with their source, and the first error is rethrown so both call sites' error handling stays reachable.
• Camera/mic republish is untouched: same publication list, same stopTrack: true teardown, same per-track error log as the previous unpublishAll().

Resolves #1004

Why keeping the capturer alive works

Track.start() short-circuits on trackState == .started, so skipping stop() leaves the capturer's startStopCounter at 1 and _publish's track.start() is a no-op. Nothing else in _publish touches the capture source.

On the WebRTC side, Transport.remove(track:) is _pc.removeTrack(sender) followed by stopInternal() on the video transceiver. So the old sender's track is nulled (sink detached, encoder stops) and the transceiver is stopped, making its m-section recyclable in unified plan; the debounced negotiation lets the removal and the following addTransceiver coalesce into a single offer. The LKRTCVideoSource is owned by the VideoCapturer and the LKRTCVideoTrack by LocalVideoTrack — neither is owned by the peer connection — so the same media track is handed to the new PC with capture uninterrupted.

The two republish paths differ in whether transports are live, and both are now correct:

	transport	behavior
Full reconnect (Room+EngineDelegate)	already nil (cleanUpRTC())	sender removal skipped, matching the "never removeTrack before close()" constraint in Transport.cleanUp
Room move (_republishLocalTracks)	live	sender removed and transceiver stopped, so _publish no longer adds a second transceiver for the same media track

E2EE also round-trips again: didUnpublishTrack disables and drops the old frame cryptor, didPublishTrack builds a fresh one bound to the new sender.

Test plan

• iOS: start a screen share via broadcast extension, force a network outage long enough to trigger a full reconnect, confirm frames resume without re-prompting the broadcast picker
• iOS: same scenario with in-app screen share (no extension)
• macOS: start screen share, trigger full reconnect, confirm frames resume
• Room move with an active screen share: confirm a single video m-line in the new offer and no duplicate upload
• Camera track with preferredBackupCodec survives a full reconnect with the backup codec re-published
• E2EE enabled: screen share stays decryptable for remote participants after a full reconnect
• Regression: camera + mic still survive a full reconnect
• Regression: stopping the broadcast from Control Center after a reconnect unpublishes exactly once

Tests

Tests/LiveKitCoreTests/Room/RepublishTracksTests.swift, parameterized over microphone / camera / screen share. Each case publishes the track, runs debug_simulate(scenario: .fullReconnect), and waits on a RoomDelegate-backed AsyncCompleter for the replacement publication. Screen share must return with its capturer untouched (startStopCounter still 1, no .stopped transition); every other source is expected to be torn down and started again. Frames come from a synthetic CVPixelBuffer feed, so the test needs no network and runs in ~3.5s.

Verified it fails without the fix: with keepCapturing forced to false, the screen share case fails on stopSpy.stops == 0 while the other two still pass.

Known gaps

• If re-publishing the screen share itself fails, _publish's catch still calls track.stop() and kills the capture source. The error is at least surfaced now.

🤖 Generated with Claude Code

[pblazej]

WebRTC semantics check

Verified against the libwebrtc tree that reusing the same LKRTCMediaStreamTrack across the old (closed) and new publisher peer connection is sound:

• PeerConnection::Close() does not end attached tracks. pc/peer_connection.cc:1906-1912 only iterates transceivers and calls StopInternal(), which routes through RtpSenderBase::Stop() (pc/rtp_sender.cc:562-580). Stop() detaches the track and unregisters the sender as an observer, but never mutates track state.
• Track state is driven by the source, not by the PC. pc/video_track.cc:147-152 is the only set_state(kEnded) call site for video, and it fires only when the underlying MediaSourceInterface::SourceState becomes kEnded. Closing the PC doesn't end the source, so the track stays kLive.
• The video source is independent of any PC. pc/video_track.cc:60-67 forwards sink registration to video_source_->internal()->AddOrUpdateSink(...). The source outlives the PC; frames keep flowing.
• Reattaching is supported. AddTransceiver(track, ...) on the new PC creates a fresh RtpSenderBase that registers as a new observer on the still-live track and adds its sink to the same source. No API or invariant prohibits reusing a track across PCs — only per-sender state is reset.
• Dropping old RtpSender refs is safe. After Stop() the sender is inert (stopped_ = true, media_channel_ = nullptr); destruction via the Swift refcount drop has nothing further to clean up.

Ordering caveat that this fix relies on: cleanUpRTC() awaits _state.transport?.close() before republishAllTracks() runs, so the old PC is fully closed (all old senders Stop()'d, detached from the track) before AddTransceiver registers a new sender on the new PC. No double-sink, no race.

Net effect for screen share: LKRTCVideoSource keeps producing frames into the same LKRTCVideoTrack; closing the old PC detached the old sender but left the track kLive; re-adding the track to the new PC registers a new sender that picks up the same source — frames resume.

[indenpendman]

This problem still exists

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

[pblazej]

Looked at whether the keep-alive should extend to camera tracks: the mechanism generalizes for free — widening the predicate to any unmuted video source is one line, and the parameterized test passes with the camera case flipped to uninterrupted — and the WebRTC side is cheap, since with no sender there are no sinks and AdaptedVideoTrackSource::AdaptFrame bails on !broadcaster_.frame_wanted() before any buffer wrapping, crop/scale or encode. For camera it would only be an optimization rather than a correctness fix, though: AVCaptureSession restarts fine, so the win is the ~few hundred ms of device enumeration + format matching + dimensionsCompleter.wait() on every republish, plus avoiding reacquisition failures like a device that went away mid-reconnect. The cost is that cleanUpParticipants(isFullReconnect: true) skips the local participant, so a reconnect that keeps failing leaves the camera live and its indicator lit for the whole retry cycle (10 attempts easing 0.3s → 7s), and nothing in the SDK observes AVCaptureSession interruption notifications today — a stop/start currently repairs an interrupted session by accident, and keep-alive would remove that, giving you a live sender with no frames. So this PR stays screen-share-only; if we want the camera latency win, the order should be interruption observation first, then an opt-in RoomOptions flag on top.