1981040 - New fields in RTCIceCandidatePairStats do not match WebRTC spec

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[mattieruth]

Steps to reproduce:

Hosted a WebRTC call using Beta 142.0b7 and checked the candidate pair stats, specifically looking at currentRoundTripTime. (side note: thank you for implementing these!)

Actual results:

I got a currentRoundTripTime in milliseconds

Expected results:

The times should be reported in terms of seconds, per the spec: https://www.w3.org/TR/webrtc-stats/#candidatepair-dict*

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:mjf, since you are the author of the regressor, bug 1371391, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Michael Froman [:mjf]]

(In reply to mattieruth from comment #0)

Steps to reproduce:

Hosted a WebRTC call using Beta 142.0b7 and checked the candidate pair stats, specifically looking at currentRoundTripTime. (side note: thank you for implementing these!)

Actual results:

I got a currentRoundTripTime in milliseconds

Expected results:

The times should be reported in terms of seconds, per the spec: https://www.w3.org/TR/webrtc-stats/#candidatepair-dict*

Thank you for the report! I looked right past that unit issue.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1371391

Comment 5

[Michael Froman [:mjf]]

Attachment: Bug 1981040 - spec says RTCIceCandidatePairStats rtts should be in seconds not ms. r?bwc!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8cf488e06ee0
https://hg.mozilla.org/integration/autoland/rev/1ee33e211b59
spec says RTCIceCandidatePairStats rtts should be in seconds not ms. r=bwc

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/54510 for changes under testing/web-platform/tests

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/1ee33e211b59

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:mjf, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox143 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[mattieruth]

I know I'm biased, but I vote for an uplift

Comment 12

[Michael Froman [:mjf]]

Comment on attachment 9508863 [details]
Bug 1981040 - spec says RTCIceCandidatePairStats rtts should be in seconds not ms. r?bwc!

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Candidate pair rtt stats will be incorrect and could affect video quality.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very simple change to convert milliseconds to seconds.
• String changes made/needed: n/a
• Is Android affected?: Yes

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9508863 [details]
Bug 1981040 - spec says RTCIceCandidatePairStats rtts should be in seconds not ms. r?bwc!

Approved for 143.0b5.

Comment 14

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/2089054ef2b8
https://hg.mozilla.org/releases/mozilla-beta/rev/5035df3ed0e7

Comment 15

[mattieruth]

Thank you!