From 128a8ff0b91dfd4db2657ffa0a780d87d33eb216 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 9 Jan 2024 12:02:29 -0800 Subject: [PATCH 1/3] Remove unnecessary mutexes from livekit client types Co-authored-by: Conrad --- crates/live_kit_client/src/prod.rs | 60 ++++++++++++++---------------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/crates/live_kit_client/src/prod.rs b/crates/live_kit_client/src/prod.rs index b2b83e95fcf33543aed6d548b21a5f07d6f6b400..74cf8423bc57f638d5dc3602f765e4f656fe9df0 100644 --- a/crates/live_kit_client/src/prod.rs +++ b/crates/live_kit_client/src/prod.rs @@ -164,29 +164,26 @@ pub enum ConnectionState { } pub struct Room { - native_room: Mutex, + native_room: swift::Room, connection: Mutex<( watch::Sender, watch::Receiver, )>, remote_audio_track_subscribers: Mutex>>, remote_video_track_subscribers: Mutex>>, - _delegate: Mutex, + _delegate: RoomDelegate, } -trait AssertSendSync: Send {} -impl AssertSendSync for Room {} - impl Room { pub fn new() -> Arc { Arc::new_cyclic(|weak_room| { let delegate = RoomDelegate::new(weak_room.clone()); Self { - native_room: Mutex::new(unsafe { LKRoomCreate(delegate.native_delegate) }), + native_room: unsafe { LKRoomCreate(delegate.native_delegate) }, connection: Mutex::new(watch::channel_with(ConnectionState::Disconnected)), remote_audio_track_subscribers: Default::default(), remote_video_track_subscribers: Default::default(), - _delegate: Mutex::new(delegate), + _delegate: delegate, } }) } @@ -201,7 +198,7 @@ impl Room { let (did_connect, tx, rx) = Self::build_done_callback(); unsafe { LKRoomConnect( - *self.native_room.lock(), + self.native_room, url.as_concrete_TypeRef(), token.as_concrete_TypeRef(), did_connect, @@ -271,7 +268,7 @@ impl Room { } unsafe { LKRoomPublishVideoTrack( - *self.native_room.lock(), + self.native_room, track.0, callback, Box::into_raw(Box::new(tx)) as *mut c_void, @@ -301,7 +298,7 @@ impl Room { } unsafe { LKRoomPublishAudioTrack( - *self.native_room.lock(), + self.native_room, track.0, callback, Box::into_raw(Box::new(tx)) as *mut c_void, @@ -312,14 +309,14 @@ impl Room { pub fn unpublish_track(&self, publication: LocalTrackPublication) { unsafe { - LKRoomUnpublishTrack(*self.native_room.lock(), publication.0); + LKRoomUnpublishTrack(self.native_room, publication.0); } } pub fn remote_video_tracks(&self, participant_id: &str) -> Vec> { unsafe { let tracks = LKRoomVideoTracksForRemoteParticipant( - *self.native_room.lock(), + self.native_room, CFString::new(participant_id).as_concrete_TypeRef(), ); @@ -348,7 +345,7 @@ impl Room { pub fn remote_audio_tracks(&self, participant_id: &str) -> Vec> { unsafe { let tracks = LKRoomAudioTracksForRemoteParticipant( - *self.native_room.lock(), + self.native_room, CFString::new(participant_id).as_concrete_TypeRef(), ); @@ -380,7 +377,7 @@ impl Room { ) -> Vec> { unsafe { let tracks = LKRoomAudioTrackPublicationsForRemoteParticipant( - *self.native_room.lock(), + self.native_room, CFString::new(participant_id).as_concrete_TypeRef(), ); @@ -508,9 +505,8 @@ impl Room { impl Drop for Room { fn drop(&mut self) { unsafe { - let native_room = &*self.native_room.lock(); - LKRoomDisconnect(*native_room); - CFRelease(native_room.0); + LKRoomDisconnect(self.native_room); + CFRelease(self.native_room.0); } } } @@ -726,7 +722,7 @@ impl Drop for LocalTrackPublication { } pub struct RemoteTrackPublication { - native_publication: Mutex, + native_publication: swift::RemoteTrackPublication, } impl RemoteTrackPublication { @@ -735,21 +731,19 @@ impl RemoteTrackPublication { CFRetain(native_track_publication.0); } Self { - native_publication: Mutex::new(native_track_publication), + native_publication: native_track_publication, } } pub fn sid(&self) -> String { unsafe { - CFString::wrap_under_get_rule(LKRemoteTrackPublicationGetSid( - *self.native_publication.lock(), - )) - .to_string() + CFString::wrap_under_get_rule(LKRemoteTrackPublicationGetSid(self.native_publication)) + .to_string() } } pub fn is_muted(&self) -> bool { - unsafe { LKRemoteTrackPublicationIsMuted(*self.native_publication.lock()) } + unsafe { LKRemoteTrackPublicationIsMuted(self.native_publication) } } pub fn set_enabled(&self, enabled: bool) -> impl Future> { @@ -767,7 +761,7 @@ impl RemoteTrackPublication { unsafe { LKRemoteTrackPublicationSetEnabled( - *self.native_publication.lock(), + self.native_publication, enabled, complete_callback, Box::into_raw(Box::new(tx)) as *mut c_void, @@ -780,13 +774,13 @@ impl RemoteTrackPublication { impl Drop for RemoteTrackPublication { fn drop(&mut self) { - unsafe { CFRelease((*self.native_publication.lock()).0) } + unsafe { CFRelease(self.native_publication.0) } } } #[derive(Debug)] pub struct RemoteAudioTrack { - native_track: Mutex, + native_track: swift::RemoteAudioTrack, sid: Sid, publisher_id: String, } @@ -797,7 +791,7 @@ impl RemoteAudioTrack { CFRetain(native_track.0); } Self { - native_track: Mutex::new(native_track), + native_track, sid, publisher_id, } @@ -822,13 +816,13 @@ impl RemoteAudioTrack { impl Drop for RemoteAudioTrack { fn drop(&mut self) { - unsafe { CFRelease(self.native_track.lock().0) } + unsafe { CFRelease(self.native_track.0) } } } #[derive(Debug)] pub struct RemoteVideoTrack { - native_track: Mutex, + native_track: swift::RemoteVideoTrack, sid: Sid, publisher_id: String, } @@ -839,7 +833,7 @@ impl RemoteVideoTrack { CFRetain(native_track.0); } Self { - native_track: Mutex::new(native_track), + native_track, sid, publisher_id, } @@ -888,7 +882,7 @@ impl RemoteVideoTrack { on_frame, on_drop, ); - LKVideoTrackAddRenderer(*self.native_track.lock(), renderer); + LKVideoTrackAddRenderer(self.native_track, renderer); rx } } @@ -896,7 +890,7 @@ impl RemoteVideoTrack { impl Drop for RemoteVideoTrack { fn drop(&mut self) { - unsafe { CFRelease(self.native_track.lock().0) } + unsafe { CFRelease(self.native_track.0) } } } From 356f9fc3b64a375d0af98c33ebe53bad24bcd539 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 9 Jan 2024 12:50:00 -0800 Subject: [PATCH 2/3] Store a raw Room pointer on RoomDelegate --- crates/live_kit_client/src/prod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/live_kit_client/src/prod.rs b/crates/live_kit_client/src/prod.rs index 74cf8423bc57f638d5dc3602f765e4f656fe9df0..df33cf72f0239ef4e2464c664ddf641cd299305f 100644 --- a/crates/live_kit_client/src/prod.rs +++ b/crates/live_kit_client/src/prod.rs @@ -513,14 +513,15 @@ impl Drop for Room { struct RoomDelegate { native_delegate: swift::RoomDelegate, - _weak_room: Weak, + weak_room: *mut c_void, } impl RoomDelegate { fn new(weak_room: Weak) -> Self { + let weak_room = weak_room.into_raw() as *mut c_void; let native_delegate = unsafe { LKRoomDelegateCreate( - weak_room.as_ptr() as *mut c_void, + weak_room, Self::on_did_disconnect, Self::on_did_subscribe_to_remote_audio_track, Self::on_did_unsubscribe_from_remote_audio_track, @@ -532,7 +533,7 @@ impl RoomDelegate { }; Self { native_delegate, - _weak_room: weak_room, + weak_room, } } @@ -647,6 +648,7 @@ impl Drop for RoomDelegate { fn drop(&mut self) { unsafe { CFRelease(self.native_delegate.0); + let _ = Weak::from_raw(self.weak_room); } } } From e3c603f41bc04eb364d2822e7dbc7c3178be347b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 9 Jan 2024 12:54:51 -0800 Subject: [PATCH 3/3] Make RemoteTrackPublication a tuple struct again --- crates/live_kit_client/src/prod.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/crates/live_kit_client/src/prod.rs b/crates/live_kit_client/src/prod.rs index df33cf72f0239ef4e2464c664ddf641cd299305f..5d8ef9bf134a61111247d3fac5b94fd87a50caf8 100644 --- a/crates/live_kit_client/src/prod.rs +++ b/crates/live_kit_client/src/prod.rs @@ -723,29 +723,22 @@ impl Drop for LocalTrackPublication { } } -pub struct RemoteTrackPublication { - native_publication: swift::RemoteTrackPublication, -} +pub struct RemoteTrackPublication(swift::RemoteTrackPublication); impl RemoteTrackPublication { pub fn new(native_track_publication: swift::RemoteTrackPublication) -> Self { unsafe { CFRetain(native_track_publication.0); } - Self { - native_publication: native_track_publication, - } + Self(native_track_publication) } pub fn sid(&self) -> String { - unsafe { - CFString::wrap_under_get_rule(LKRemoteTrackPublicationGetSid(self.native_publication)) - .to_string() - } + unsafe { CFString::wrap_under_get_rule(LKRemoteTrackPublicationGetSid(self.0)).to_string() } } pub fn is_muted(&self) -> bool { - unsafe { LKRemoteTrackPublicationIsMuted(self.native_publication) } + unsafe { LKRemoteTrackPublicationIsMuted(self.0) } } pub fn set_enabled(&self, enabled: bool) -> impl Future> { @@ -763,7 +756,7 @@ impl RemoteTrackPublication { unsafe { LKRemoteTrackPublicationSetEnabled( - self.native_publication, + self.0, enabled, complete_callback, Box::into_raw(Box::new(tx)) as *mut c_void, @@ -776,7 +769,7 @@ impl RemoteTrackPublication { impl Drop for RemoteTrackPublication { fn drop(&mut self) { - unsafe { CFRelease(self.native_publication.0) } + unsafe { CFRelease(self.0 .0) } } }