Closed
Bug 1495446
Opened 6 years ago
Closed 6 years ago
In getStats(), RTCP timestamps have the wrong epoch
Categories
(Core :: WebRTC, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla66
People
(Reporter: jib, Assigned: ng)
References
(Depends on 1 open bug)
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
Discovered in bug 979649 comment 12 by re-enabled test.
STRs:
1. Open https://jsfiddle.net/jib1/jpmL6eb5/ and share cam+mic
Expected results:
- RTP and RTCP timestamps are very close.
Actual results:
- RTCP timestamps are from 1970 instead of 2018:
outbound-rtp audio Mon Oct 01 2018 11:02:34 GMT-0400 (Eastern Daylight Time)
SSRC: 2820930439 Sent: 15415 packets (1.82 MB)
Frames encoded: undefined Dropped frames: undefined
RTCP inbound-rtp audio Thu Jan 08 1970 05:07:39 GMT-0500 (Eastern Standard Time)
SSRC: 2820930439 Received: 15262 packets (1.50 MB) Lost: 0
Discarded packets: undefined Jitter: 0.005
outbound-rtp video Mon Oct 01 2018 11:02:34 GMT-0400 (Eastern Daylight Time)
SSRC: 936741363 Sent: 66825 packets (73.15 MB)
Frames encoded: 8891 Dropped frames: 3857049062
RTCP inbound-rtp video Thu Jan 08 1970 05:07:42 GMT-0500 (Eastern Standard Time)
SSRC: 936741363 Received: 66746 packets (71.23 MB) Lost: 0
Discarded packets: undefined Jitter: 0.005
Regression range:
17:55.25 INFO: Got as far as we can go bisecting nightlies...
17:55.25 INFO: Last good revision: 8dd496fd015a2b6e99573070279d9d1593836ea9 (2017-03-15)
17:55.25 INFO: First bad revision: ff04d410e74b69acfab17ef7e73e7397602d5a68 (2017-03-16)
17:55.25 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8dd496fd015a2b6e99573070279d9d1593836ea9&tochange=ff04d410e74b69acfab17ef7e73e7397602d5a68
Mozregression was not able to narrow it any further (it could not find builds, see errors below), but I suspect bug 1343691.
> 17:55.25 INFO: Switching bisection method to taskcluster
> 17:55.25 INFO: Getting mozilla-central builds between 8dd496fd015a2b6e99573070279d9d1593836ea9 and ff04d410e74b69acfab17ef7e73e7397602d5a68
> 18:00.75 WARNING: Skipping build ff04d410e74b: Unable to find build info using the taskcluster route 'gecko.v2.mozilla-central.revision.ff04d410e74b69acfab17ef7e73e7397602d5a68.firefox.macosx64-opt'
> ...
> 18:34.56 CRITICAL: First build 8dd496fd015a is missing, but mozregression can't find a build before - so it is excluded, but it could contain the regression!
> 19:08.50 CRITICAL: Last build ff04d410e74b is missing, but mozregression can't find a build after - so it is excluded, but it could contain the regression!
Reporter | ||
Updated•6 years ago
|
status-firefox62:
--- → affected
status-firefox63:
--- → affected
status-firefox64:
--- → affected
status-firefox-esr60:
--- → affected
Reporter | ||
Updated•6 years ago
|
tracking-firefox64:
--- → ?
Assignee | ||
Comment 1•6 years ago
|
||
The RTCP stats timestamps are recorded in reference to "an arbitrary time", we need to report them in the current epoch time
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
Updated•6 years ago
|
Attachment #9013453 -
Attachment description: Bug 1495446 - fix epoch of WebRTC RTCP stats → Bug 1495446 - fix timestamp of WebRTC RTCP stats
Comment 4•6 years ago
|
||
Jan-Ivar, my changeset that uncommented this failing test was backed out (bug 979649 comment 9). Would you like to reland that test when you land this fix? Or would you like me to reland the test before (with the one failing test case temporarily disabled) or after you land this fix?
Flags: needinfo?(jib)
Updated•6 years ago
|
Keywords: regression
Reporter | ||
Comment 5•6 years ago
|
||
Chris, since this fix is blocking on bug 1376873 it might be a good idea to land your test now with a todo comment mentioning this bug, so we don't forget. Thanks!
Flags: needinfo?(jib)
Updated•6 years ago
|
Comment 6•6 years ago
|
||
If this depends on bug 1376873, which AIUI is planned for 65, it seems quite unlikely we'd uplift to branches. Though feel free to reset status flags if I misunderstood.
Comment 7•6 years ago
|
||
When this epoch bug is fixed, `if (false)` check [1] (added in changeset [2]) should be removed so as to re-enable the ok(res.timestamp >= minimum) test [3].
[1] https://hg.mozilla.org/integration/mozilla-inbound/file/f4c2c10af632/dom/media/tests/mochitest/pc.js#l1797
[2] https://hg.mozilla.org/integration/mozilla-inbound/rev/f4c2c10af632
[3] https://hg.mozilla.org/integration/mozilla-inbound/file/f4c2c10af632/dom/media/tests/mochitest/pc.js#l1798
status-firefox65:
--- → affected
Comment 8•6 years ago
|
||
With bug 1376873 now fixed, is this something you can come back to?
Flags: needinfo?(jib)
Assignee | ||
Comment 9•6 years ago
|
||
Jared, yes. I'll see about getting this landed today.
Flags: needinfo?(jib)
Assignee | ||
Comment 10•6 years ago
|
||
Assignee | ||
Comment 11•6 years ago
|
||
Comment 12•6 years ago
|
||
Nico, FYI: when you rebase your patch onto mozilla-central's head, you will hit a merge conflict in dom/media/tests/mochitest/pc.js because bug 1225729 re-enabled the tests by removing the `if (res.timestamp != 2085978496000)` check. You should test after rebasing to ensure that your epoch fix works on Android.
https://hg.mozilla.org/mozilla-central/rev/69d881a78667
Depends on: 1225729
Assignee | ||
Comment 13•6 years ago
|
||
Assignee | ||
Comment 14•6 years ago
|
||
Assignee | ||
Comment 15•6 years ago
|
||
Comment 16•6 years ago
|
||
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/836f752852c5
fix timestamp of WebRTC RTCP stats r=jib
Comment 17•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Comment 18•6 years ago
|
||
Is this something we should consider for Beta backport or can it ride the trains?
Flags: needinfo?(na-g)
Flags: in-testsuite+
Assignee | ||
Comment 20•6 years ago
|
||
Comment on attachment 9013453 [details]
Bug 1495446 - fix timestamp of WebRTC RTCP stats
[Beta/Release Uplift Approval Request]
Feature/Bug causing the regression: Bug 979649
User impact if declined: RTCP stats will have an incorrect timestamp, making it difficult for websites that use WebRTC to make intelligent decisions about the quality of service of calls.
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): This patch is low complexity, but spread over a number of files.
String changes made/needed: none
Attachment #9013453 -
Flags: approval-mozilla-beta?
Comment 21•6 years ago
|
||
Comment on attachment 9013453 [details]
Bug 1495446 - fix timestamp of WebRTC RTCP stats
[Triage Comment]
Improves websites' ability to determine the quality of WebRTC calls being made. Approved for 65.0b5.
Attachment #9013453 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 22•6 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•