-
Notifications
You must be signed in to change notification settings - Fork 5.8k
trickle-ice: show url and relayProtocol if present #1561
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
Conversation
let candidateStats; | ||
if (['srflx', 'relay'].includes(candidate.type)) { | ||
stats.forEach(report => { | ||
if (!candidateStats && report.type === 'local-candidate' && report.address === candidate.address && report.port === candidate.port) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I /think/ this sufficient to match the candidate
@henbos @alvestrand PTAL. The API inconsistency bothers me, I don't see a reason not to expose the same properties in onicecandidates as we do in stats. |
note: this will work in M105+ once https://chromiumdash.appspot.com/commit/1fe14f2752a35932bea6d6ccff7433a6716c6391 is available
alternative implementation of webrtc#1561
const stats = await pc.getStats(); | ||
stats.forEach(report => { | ||
if (!candidateStats && report.type === 'local-candidate' && report.address === candidate.address && report.port === candidate.port) { | ||
candidateStats = report; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should there be a 'break' here, or is there some fear that we'll want to match the last one not the first one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can't break a .forEach :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this will be OBE once the URL is available in the candidate, but needed for 3 months while the next version rolls out.
Is the relay protocol available in stats and should be extracted the same way?
relayProtocol is polyfilled in Chrome/Safari in adapter-latest.js as of yesterday! |
changed to actually have a built-in expiry date so the getStats call no longer happens when url is there |
since some of the current information is not relevant for this use-case and revert the width of the table which was enlarged in webrtc#1561 since this is no longer necessary.
note: this will work in M105+ once
https://chromiumdash.appspot.com/commit/1fe14f2752a35932bea6d6ccff7433a6716c6391
is available