From d966cee6abb7f6c855a50dd276c71c26ebfd1ca2 Mon Sep 17 00:00:00 2001 From: IsaFxck Date: Wed, 9 Sep 2026 06:20:27 +0300 Subject: [PATCH] fix(subscription): avoid requests from subscription events --- include/livekit/remote_track_publication.h | 6 ++ src/room.cpp | 4 +- src/tests/unit/test_room.cpp | 72 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/include/livekit/remote_track_publication.h b/include/livekit/remote_track_publication.h index 1c22568..4b315c7 100644 --- a/include/livekit/remote_track_publication.h +++ b/include/livekit/remote_track_publication.h @@ -96,6 +96,12 @@ class LIVEKIT_API RemoteTrackPublication : public TrackPublication { bool setVideoDimensions(const std::uint32_t width, const std::uint32_t height); private: + friend class Room; + + // Room events report observed state. Sending a subscription request from an + // event would let an old completion overwrite newer application intent. + void updateSubscriptionState(bool subscribed) noexcept { subscribed_ = subscribed; } + enum class VideoPreference { DEFAULT, DIMENSIONS, diff --git a/src/room.cpp b/src/room.cpp index ca35754..0282486 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -742,7 +742,7 @@ void Room::onEvent(const FfiEvent& event) { } // Attach to publication, mark subscribed rpublication->setTrack(remote_track); - rpublication->setSubscribed(true); + rpublication->updateSubscriptionState(true); } // Emit remote track_subscribed-style callback @@ -787,7 +787,7 @@ void Room::onEvent(const FfiEvent& event) { unsub_source = publication->source(); auto track = publication->track(); publication->setTrack(nullptr); - publication->setSubscribed(false); + publication->updateSubscriptionState(false); ev.participant = rparticipant; ev.publication = publication; ev.track = track; diff --git a/src/tests/unit/test_room.cpp b/src/tests/unit/test_room.cpp index 7700267..94854c9 100644 --- a/src/tests/unit/test_room.cpp +++ b/src/tests/unit/test_room.cpp @@ -93,6 +93,25 @@ class UnitLifecycleTrackingDelegate : public RoomDelegate { DisconnectReason reason = DisconnectReason::Unknown; }; +class UnitSubscriptionTrackingDelegate : public RoomDelegate { +public: + void onTrackSubscribed(Room&, const TrackSubscribedEvent& event) override { + EXPECT_TRUE(event.publication->subscribed()); + EXPECT_EQ(event.publication->track(), event.track); + ++subscribed; + } + + void onTrackUnsubscribed(Room&, const TrackUnsubscribedEvent& event) override { + EXPECT_FALSE(event.publication->subscribed()); + EXPECT_EQ(event.publication->track(), nullptr); + EXPECT_NE(event.track, nullptr); + ++unsubscribed; + } + + unsigned subscribed{0}; + unsigned unsubscribed{0}; +}; + } // namespace TEST_F(RoomTest, PublicationSnapshotBoundsAndRetainsRemovedHandles) { @@ -457,4 +476,57 @@ TEST_F(RoomTest, DisconnectAfterServerDisconnectCleansUpWithoutDuplicateNotifica EXPECT_EQ(delegate.callbacks.size(), 1) << "onDisconnected must not fire twice"; } +TEST_F(RoomTest, SubscriptionEventsUpdateStateWithoutSendingSubscriptionRequests) { + Room room; + UnitSubscriptionTrackingDelegate delegate; + std::atomic listener_calls{0}; + room.setDelegate(&delegate); + RoomTestAccess::installConnectedListener(room, listener_calls); + + auto participant = std::make_shared(FfiHandle{}, "participant-sid", "name", "remote", "", + std::unordered_map{}, + ParticipantKind::Standard, DisconnectReason::Unknown); + proto::OwnedTrackPublication owned; + owned.mutable_info()->set_sid("video"); + owned.mutable_info()->set_kind(proto::TrackKind::KIND_VIDEO); + participant->mutableTrackPublications().emplace("video", std::make_shared(owned)); + RoomTestAccess::setRemoteParticipant(room, participant); + + // A zero FFI handle makes any outbound subscription request throw. Event + // delivery must only record the observed state and notify the delegate; + // reissuing a request here can overwrite a newer application intent. + proto::FfiEvent subscribed_event; + subscribed_event.mutable_room_event()->set_room_handle(0); + auto* subscribed = subscribed_event.mutable_room_event()->mutable_track_subscribed(); + subscribed->set_participant_identity("remote"); + subscribed->mutable_track()->mutable_handle()->set_id(0); + subscribed->mutable_track()->mutable_info()->set_sid("video"); + subscribed->mutable_track()->mutable_info()->set_name("video"); + subscribed->mutable_track()->mutable_info()->set_kind(proto::TrackKind::KIND_VIDEO); + subscribed->mutable_track()->mutable_info()->set_stream_state(proto::StreamState::STATE_ACTIVE); + subscribed->mutable_track()->mutable_info()->set_muted(false); + subscribed->mutable_track()->mutable_info()->set_remote(true); + + proto::FfiEvent unsubscribed_event; + unsubscribed_event.mutable_room_event()->set_room_handle(0); + auto* unsubscribed = unsubscribed_event.mutable_room_event()->mutable_track_unsubscribed(); + unsubscribed->set_participant_identity("remote"); + unsubscribed->set_track_sid("video"); + + for (unsigned iteration = 0; iteration < 100; ++iteration) { + emitFfiEvent(subscribed_event); + EXPECT_EQ(delegate.subscribed, iteration + 1); + emitFfiEvent(unsubscribed_event); + EXPECT_EQ(delegate.unsubscribed, iteration + 1); + if (delegate.subscribed != iteration + 1 || delegate.unsubscribed != iteration + 1) { + break; + } + } + + proto::FfiEvent eos_event; + eos_event.mutable_room_event()->set_room_handle(0); + eos_event.mutable_room_event()->mutable_eos(); + emitFfiEvent(eos_event); +} + } // namespace livekit::test