Closed Bug 1495446 Opened Last year Closed 10 months ago

In getStats(), RTCP timestamps have the wrong epoch

Categories

(Core :: WebRTC, defect, P2)

53 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 - wontfix
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: jib, Assigned: ng)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

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!
The RTCP stats timestamps are recorded in reference to "an arbitrary time", we need to report them in the current epoch time
Attachment #9013453 - Attachment description: Bug 1495446 - fix epoch of WebRTC RTCP stats → Bug 1495446 - fix timestamp of WebRTC RTCP stats
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)
Depends on: 1376873
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)
Blocks: 1496533
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.
With bug 1376873 now fixed, is this something you can come back to?
Flags: needinfo?(jib)
Jared, yes. I'll see about getting this landed today.
Flags: needinfo?(jib)
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
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/836f752852c5
fix timestamp of WebRTC RTCP stats r=jib
https://hg.mozilla.org/mozilla-central/rev/836f752852c5
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Is this something we should consider for Beta backport or can it ride the trains?
Flags: needinfo?(na-g)
Flags: in-testsuite+
Ryan, this should be considered for Beta.
Flags: needinfo?(na-g)
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 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+
You need to log in before you can comment on or make changes to this bug.