Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use rtc::ToString instead of std::to_string in SocketAddress::PortAsString() #156

Conversation

mgsloan
Copy link
Contributor

@mgsloan mgsloan commented Nov 26, 2024

Justification for this change is that std::to_string should be avoided as it uses the user's locale and calls to it get serialized, which is bad for concurrency.

My actual motivation for this is quite bizarre. Before this change, with Zed's use of the LiveKit Rust SDK, I was getting connection strings that were not valid utf-8, instead having a port of 3\u0000\u0000\u001c\u0000. I have not figured out how that could happen or why this change fixes it.

…rtAsString()`

Justification for this change is that `std::to_string` should be
avoided as it uses the user's locale and calls to it get serialized,
which is bad for concurrency.

My actual motivation for this is quite bizarre. Before this change,
with Zed's use of the LiveKit Rust SDK, I was getting connection
strings that were not valid utf-8, instead having a port of
`3\u0000\u0000\u001c\u0000`. I have not figured out how that could
happen or why this change fixes it.
mgsloan added a commit to zed-industries/zed that referenced this pull request Nov 29, 2024
Copy link
Member

@cloudwebrtc cloudwebrtc left a comment

Choose a reason for hiding this comment

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

lg!

@cloudwebrtc cloudwebrtc merged commit b99fd2c into webrtc-sdk:m125_release Dec 3, 2024
hiroshihorie added a commit that referenced this pull request Dec 9, 2024
commit 102940838f859941a41b2f3062d4eb8a61be6414
Author: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
Date:   Thu Dec 5 15:27:34 2024 +0700

    audio engine 1

commit d31187b
Author: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
Date:   Thu Dec 5 15:26:38 2024 +0700

    Connect voice engine mute to adm

commit 3df68d4
Author: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
Date:   Fri Oct 11 20:41:44 2024 +0900

    Revert "Stop recording on mute (turn off mic indicator) (#55)"

    This reverts commit c0209ef.

commit b99fd2c
Author: Michael Sloan <mgsloan@gmail.com>
Date:   Mon Dec 2 18:59:22 2024 -0700

    Use `rtc::ToString` instead of `std::to_string` in `SocketAddress::PortAsString()` (#156)

    Justification for this change is that `std::to_string` should be avoided
    as it uses the user's locale and calls to it get serialized, which is
    bad for concurrency.

    My actual motivation for this is quite bizarre. Before this change, with
    Zed's use of the LiveKit Rust SDK, I was getting connection strings that
    were not valid utf-8, instead having a port of
    `3\u0000\u0000\u001c\u0000`. I have not figured out how that could
    happen or why this change fixes it.

commit 543121b
Author: davidliu <davidliu@deviange.net>
Date:   Wed Oct 30 20:33:46 2024 -0700

    Custom audio input for Android (#154)
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