Skip to content

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

Merged
merged 8 commits into from
Oct 18, 2022

Conversation

fippo
Copy link
Collaborator

@fippo fippo commented Jun 22, 2022

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) {
Copy link
Collaborator Author

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

@fippo
Copy link
Collaborator Author

fippo commented Jun 22, 2022

@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.

@fippo fippo force-pushed the props-from-stats branch from 4187587 to d94a6ee Compare June 27, 2022 11:56
@fippo fippo force-pushed the props-from-stats branch from d94a6ee to 649a41d Compare June 27, 2022 12:31
fippo added a commit to fippo/webrtc that referenced this pull request Aug 16, 2022
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;
Copy link
Contributor

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?

Copy link
Collaborator Author

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 :-(

Copy link
Contributor

@alvestrand alvestrand left a 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?

@fippo
Copy link
Collaborator Author

fippo commented Oct 17, 2022

relayProtocol is polyfilled in Chrome/Safari in adapter-latest.js as of yesterday!

@fippo
Copy link
Collaborator Author

fippo commented Oct 17, 2022

changed to actually have a built-in expiry date so the getStats call no longer happens when url is there

@fippo fippo merged commit 5990ba4 into webrtc:gh-pages Oct 18, 2022
@fippo fippo deleted the props-from-stats branch October 18, 2022 18:58
fippo added a commit to fippo/webrtc that referenced this pull request Oct 28, 2022
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.
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