Skip to content

Conversation

@nbsp
Copy link
Contributor

@nbsp nbsp commented May 27, 2024

highly wip, went through the migration notes and the v1.15.7..v2.0.0 commit log for client-sdk-js and made some API changes. doesn't include any non-API-breaking change from client-sdk-js, nor any API additions from versions later than 2.0.0.

builds fine, but untested, couldn't get any of the examples to work even on main (perhaps my NixOS/Wayland system is confusing)

  • DataPacket now uses reliable: bool and not kind: DataPacketKind
  • get_trackget_track_publication etc
  • participantsremote_participants
  • VideoQuality::OFF removal couldn't find any mentions of VideoQuality::OFF here in the realtime SDK
  • async sid()
  • update Cargo.toml versions in affected crates

@lukasIO lukasIO requested review from lukasIO and theomonnom May 27, 2024 11:24
@nbsp
Copy link
Contributor Author

nbsp commented May 27, 2024

looking into pub fn sid(&self) further i don't think there is much reason for having it asynchronous, as Room::connect already awaits the join response that includes the SID

@lukasIO
Copy link
Contributor

lukasIO commented May 27, 2024

The async Sid for room is due to the fact that the sid returned in the join response could still be empty

edit: this is how we handle it for the js-client today

@nbsp
Copy link
Contributor Author

nbsp commented May 27, 2024

pushed an async sid() proof of concept, if you could review the general idea while i work out the kinks

  • added sid_rx and sid_tx oneshot channel to RoomSession
  • sid_tx is wrapped in an Option because it. works. and it didn't work without it ¯\_(ツ)_/¯
  • impl Debug for Room and impl From<&FfiRoom> for proto::RoomInfo are both functions dependent on sid() that can't be async, dunno how to figure that out fixed with #[tokio::main]

@nbsp nbsp force-pushed the sdk-v2 branch 2 times, most recently from 710e8c0 to 804ae50 Compare May 28, 2024 04:27
@nbsp nbsp marked this pull request as ready for review May 28, 2024 04:27
@nbsp nbsp requested a review from lukasIO May 28, 2024 05:05
@nbsp nbsp changed the title livekit: Update to v2 APIs livekit, livekit-ffi: Update to v2 APIs May 28, 2024
@nbsp nbsp requested a review from theomonnom May 31, 2024 13:13
@theomonnom
Copy link
Member

Nice, lgtm, have you been able to test the changes?

@nbsp
Copy link
Contributor Author

nbsp commented May 31, 2024

unfortunately i wasn't able to run any of the sample code in the examples directories, even under the main branch. might have something to do with my linux distro + display server combo, but that's outside the scope of this PR

@theomonnom
Copy link
Member

Can you try an example that doesn't use any display env? Like compiling livekit_ffi and trying it on Python/Node

@nbsp
Copy link
Contributor Author

nbsp commented Jun 1, 2024

Can you try an example that doesn't use any display env? Like compiling livekit_ffi and trying it on Python/Node

confirmed working with python-sdks/examples/basic_room.py, with minor changes to the Participant class (kind → reliable)

Copy link
Member

@theomonnom theomonnom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok nice lgtm!

@theomonnom theomonnom merged commit f65b3fa into livekit:main Jun 3, 2024
@nbsp nbsp deleted the sdk-v2 branch June 3, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants