1495446 - In getStats(), RTCP timestamps have the wrong epoch

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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!

Comment 1

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1495446 - fix timestamp of WebRTC RTCP stats

The RTCP stats timestamps are recorded in reference to "an arbitrary time", we need to report them in the current epoch time

Comment 2

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70516e478bfe2d183580774c0f89ff5721413b5e

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26e0e6ba3757decd9b04937530d0e71e762504a2

Comment 4

[Chris Peterson [:cpeterson]]

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?

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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!

Comment 6

[Julien Cristau [:jcristau]]

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

[Chris Peterson [:cpeterson]]

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

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

With bug 1376873 now fixed, is this something you can come back to?

Comment 9

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Jared, yes. I'll see about getting this landed today.

Comment 10

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66b65ab547d3d0c4e758dd7564ef249108d8bbc7

Comment 11

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7fb67f15896d425b350d6eee91674cddbcbdff0a

Comment 12

[Chris Peterson [:cpeterson]]

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

Comment 13

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=61245399499cff0ad2770f45bec98adc6aed3a42

Comment 14

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=42ab1a24ecaac67fbaa75d74b987629c61b1b93a

Comment 15

[Nico Grunbaum [:ng, @chew:mozilla.org]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94b5117c7bef4bdd3ed01ff26350893024a20e78

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/836f752852c5
fix timestamp of WebRTC RTCP stats r=jib

Comment 17

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/836f752852c5

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider for Beta backport or can it ride the trains?

Comment 19

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Ryan, this should be considered for Beta.

Comment 20

[Nico Grunbaum [:ng, @chew:mozilla.org]]

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

Comment 21

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/97d55c66f8ee