Skip to content

Conversation

@lukasIO
Copy link
Contributor

@lukasIO lukasIO commented May 5, 2025

Main motivation is to get the new pre connect buffer AudioTrackFeature added for usage in livekit/agents#2171

UserRejected = 12,
/// SIP protocol failure or unexpected response
SipTrunkFailure = 13,
ConnectionTimeout = 14,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we simply add this without breaking users' code?

}

async fn write(&self, bytes: &[u8]) -> StreamResult<()> {
async fn write(&self, bytes: &'a [u8]) -> StreamResult<()> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the lifetime annotation here was needed to make it compile locally on my setup (v1.83) - not sure why this wasn't failing on CI, any drawbacks around adding this?

@lukasIO lukasIO requested review from ladvoc and theomonnom May 5, 2025 08:02
Comment on lines 222 to 223
// TODO implement webhooks
webhooks: Default::default(),
Copy link
Member

Choose a reason for hiding this comment

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

The reason why we have to do this everytime is because we're using the wrong syntax

Suggested change
// TODO implement webhooks
webhooks: Default::default(),
..Default::default(),

@theomonnom
Copy link
Member

Is the AudioTrackFeature already done? I don't see it anywhere in the code

@lukasIO
Copy link
Contributor Author

lukasIO commented May 5, 2025

Ah, I assumed trackInfo just gets passed as is, but it doesn't. Will add handling for audio track features

@lukasIO lukasIO changed the title Bump protocol to v1.37.1 Bump protocol to v1.37.1 and add get_audio_features May 5, 2025
@lukasIO lukasIO requested a review from theomonnom May 5, 2025 14:41
@lukasIO lukasIO changed the title Bump protocol to v1.37.1 and add get_audio_features Bump protocol to v1.37.1 and add audio_features handling May 5, 2025
@lukasIO
Copy link
Contributor Author

lukasIO commented May 5, 2025

@theomonnom I added handling for audio_features both on the rust side and FFI

DisconnectReason::UserUnavailable => Self::UserUnavailable,
DisconnectReason::UserRejected => Self::UserRejected,
DisconnectReason::SipTrunkFailure => Self::SipTrunkFailure,
DisconnectReason::ConnectionTimeout => Self::ConnectionTimeout,
Copy link
Member

Choose a reason for hiding this comment

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

Can you create a similar conversion for the TrackFeature?
impl From< AudioTrackFeature > for proto:: AudioTrackFeature {

We're currently only relying on the integer, which is unsafe and will panic as soon as the proto adds a new one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theomonnom I added conversions in both directions and added AudioTrackFeature to the rtc prelude

@lukasIO lukasIO merged commit de3a3e1 into main May 7, 2025
15 of 20 checks passed
@lukasIO lukasIO deleted the lukas/update-protocol branch May 7, 2025 09:37
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