Skip to content

Conversation

dennwc
Copy link
Contributor

@dennwc dennwc commented Sep 29, 2025

No description provided.

@dennwc dennwc self-assigned this Sep 29, 2025
@dennwc dennwc requested a review from a team as a code owner September 29, 2025 20:52
@nishadmusthafa
Copy link
Contributor

Logic change looks good to me. How do we test and make sure that this doesn't cause major issues during rollout ?

@dennwc
Copy link
Contributor Author

dennwc commented Sep 30, 2025

E2E passes, it specifically tests if audio conversion works as expected. I'll also test manually, just in case. This change won't be included in the current release until then.

Copy link
Contributor

@nishadmusthafa nishadmusthafa left a comment

Choose a reason for hiding this comment

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

Probably good to have @alexlivekit also give it a pass.

Copy link
Contributor

@alexlivekit alexlivekit left a comment

Choose a reason for hiding this comment

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

lgtm. Don't know if we want to get defensive to the point of making sure w.SampleRate() / rtp.DefFramesPerSec results in an integer, but that's only a minor nitpick.

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