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

Preserve ICE candidate Extensions #3024

Merged
merged 1 commit into from
Jan 31, 2025
Merged

Conversation

JoeTurki
Copy link
Member

@JoeTurki JoeTurki commented Jan 30, 2025

Description

Ensure that ICE candidates are retained when parsed and stringified, and converted back to ICE.candidate.

Reference issue

required ICE PR pion/ice#758 and pion/ice#759

Partly solves #2993

Copy link
Member

@Sean-Der Sean-Der left a comment

Choose a reason for hiding this comment

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

These are all great across the board, thanks for taking on this project @JoeTurki :)

@JoeTurki JoeTurki force-pushed the impr/ice-candidate-extensions branch from 1f7e2dd to bacd6b7 Compare January 31, 2025 08:17
@JoeTurki JoeTurki marked this pull request as ready for review January 31, 2025 08:17
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

Attention: Patch coverage is 91.30435% with 4 lines in your changes missing coverage. Please review.

Project coverage is 78.57%. Comparing base (7c3b128) to head (a0d7d02).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
icecandidate.go 91.30% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3024      +/-   ##
==========================================
+ Coverage   78.40%   78.57%   +0.17%     
==========================================
  Files          89       89              
  Lines       11037    11078      +41     
==========================================
+ Hits         8654     8705      +51     
+ Misses       1894     1885       -9     
+ Partials      489      488       -1     
Flag Coverage Δ
go 80.12% <91.30%> (+0.17%) ⬆️
wasm 63.90% <91.30%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JoeTurki JoeTurki marked this pull request as draft January 31, 2025 08:21
@JoeTurki JoeTurki force-pushed the impr/ice-candidate-extensions branch 3 times, most recently from dea14d6 to eb6f46b Compare January 31, 2025 20:46
Ensure that ICE candidates are retained when parsed and stringified,
and converted to ICE.candidate.
@JoeTurki JoeTurki force-pushed the impr/ice-candidate-extensions branch from eb6f46b to a0d7d02 Compare January 31, 2025 20:49
@JoeTurki JoeTurki marked this pull request as ready for review January 31, 2025 20:49
@JoeTurki JoeTurki merged commit a0d7d02 into master Jan 31, 2025
18 checks passed
@JoeTurki JoeTurki deleted the impr/ice-candidate-extensions branch January 31, 2025 21:01
@JoeTurki
Copy link
Member Author

@Sean-Der I've updated the implementation so that the ICECandidate stores extensions as a string with minimal serialization instead of a slice. to ensure that it remain compatible (equality), All private implementations, hope that was ok.

@Sean-Der
Copy link
Member

@JoeTurki Sounds great to me! I have no idea what is really going on, but I trust your judgement :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants