Closed Bug 1515716 Opened 6 years ago Closed 6 years ago

Refactor WebRTC RTP stats types

Categories

(Core :: WebRTC, enhancement, P2)

Product:
Core
Component:
WebRTC
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: ng, Assigned: ng)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 1 obsolete file)

Bug 1515716 - refactor the RTC RTP stats types to match spec
47 bytes, text/x-phabricator-request
Details | Review 
The WebRTC RTP stats types in our WebIDL[1] need to be refactored to match the new type structure.  This is largely not user visible, and shouldn't have an noticeable impact. This was uncovered while removing isRemote in Bug 1380555.

[0] https://w3c.github.io/webrtc-stats/#streamstats-dict*
[1] https://searchfox.org/mozilla-central/source/dom/webidl/RTCStatsReport.webidl#29
In the current version of the spec the local and remote RTP stats, are additionally differentiated types. We currently use the older scheme in which the RTP stats were only differentiated on the Inbound/Outbound axis.
Depends on: 1347070
Priority: P1 → P2

realigning the RTP stats types to match spec. This involves breaking out the remote dictionary types. This shouldn't create user space visible changes.

Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/621207c8ef88
refactor the RTC RTP stats types to match spec r=jib,smaug
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/475c7fe27919
Backed out changeset 621207c8ef88 for failing at /src/base/pickle.cc on a CLOSED TREE.

Backed out changeset 621207c8ef88 (bug 1515716) for failing at /src/base/pickle.cc on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/475c7fe27919b7e336fada5c9733dfb13c0aea53

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=621207c8ef880bcee9a263fbf029fafd6a8364ea&selectedJob=232109762

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=232109762&repo=autoland&lineNumber=1673

Log snippet:

[task 2019-03-06T09:32:05.543Z] 09:32:05 INFO - GECKO(1069) | (registry/INFO) insert 'ice.tcp.so_sock_count' (INT4) succeeded: 0
[task 2019-03-06T09:32:05.544Z] 09:32:05 INFO - GECKO(1069) | (registry/INFO) insert 'ice.tcp.listen_backlog' (INT4) succeeded: 10
[task 2019-03-06T09:32:05.545Z] 09:32:05 INFO - GECKO(1069) | (registry/INFO) insert 'ice.tcp.disable' (char) succeeded: \000
[task 2019-03-06T09:32:05.566Z] 09:32:05 INFO - GECKO(1069) | ++DOCSHELL 0xdd68fc00 == 9 [pid = 1069] [id = {ecdc4b36-3f22-4a2c-904b-e54271318475}]
[task 2019-03-06T09:32:05.566Z] 09:32:05 INFO - GECKO(1069) | ++DOMWINDOW == 17 (0xe1075780) [pid = 1069] [serial = 19] [outer = (nil)]
[task 2019-03-06T09:32:05.567Z] 09:32:05 INFO - GECKO(1069) | ++DOMWINDOW == 18 (0xdd699800) [pid = 1069] [serial = 20] [outer = 0xe1075780]
[task 2019-03-06T09:32:05.684Z] 09:32:05 INFO - GECKO(1069) | [Child 1179: Main Thread]: I/signaling [main|PeerConnectionMedia] PeerConnectionMedia.cpp:51: OnProxyAvailable: Proxy Available: 0
[task 2019-03-06T09:32:05.685Z] 09:32:05 INFO - GECKO(1069) | [Child 1179: Main Thread]: I/signaling [main|PeerConnectionMedia] PeerConnectionMedia.cpp:66: SetProxyOnPcm: Had proxyinfo
[task 2019-03-06T09:32:05.686Z] 09:32:05 INFO - GECKO(1069) | [Child 1179: Main Thread]: I/signaling [main|PeerConnectionMedia] PeerConnectionMedia.cpp:51: OnProxyAvailable: Proxy Available: 0
[task 2019-03-06T09:32:05.687Z] 09:32:05 INFO - GECKO(1069) | [Child 1179: Main Thread]: I/signaling [main|PeerConnectionMedia] PeerConnectionMedia.cpp:66: SetProxyOnPcm: Had proxyinfo
[task 2019-03-06T09:32:05.688Z] 09:32:05 INFO - GECKO(1069) | [Parent 1069, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805D0021: file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 993
[task 2019-03-06T09:32:05.744Z] 09:32:05 INFO - GECKO(1069) | [Parent 1069, Main Thread] WARNING: NS_ENSURE_TRUE(root) failed: file /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp, line 3193
[task 2019-03-06T09:32:06.429Z] 09:32:06 INFO - GECKO(1069) | [Child 1179: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2730: GetStats: Found no pipelines matching selector.
[task 2019-03-06T09:32:06.429Z] 09:32:06 INFO - GECKO(1069) | [Child 1179: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2730: GetStats: Found no pipelines matching selector.
[task 2019-03-06T09:32:06.512Z] 09:32:06 INFO - GECKO(1069) | --DOMWINDOW == 17 (0xdc58a120) [pid = 1069] [serial = 11] [outer = (nil)] [url = about:blank]
[task 2019-03-06T09:32:06.513Z] 09:32:06 INFO - GECKO(1069) | --DOMWINDOW == 16 (0xdc58a450) [pid = 1069] [serial = 13] [outer = (nil)] [url = chrome://browser/content/webrtcIndicator.xul]
[task 2019-03-06T09:32:06.514Z] 09:32:06 INFO - GECKO(1069) | --DOMWINDOW == 15 (0xdc58a780) [pid = 1069] [serial = 15] [outer = (nil)] [url = chrome://browser/content/webrtcIndicator.xul]
[task 2019-03-06T09:32:06.514Z] 09:32:06 INFO - GECKO(1069) | --DOCSHELL 0xdda1e000 == 8 [pid = 1069] [id = {2c20f661-8d7f-454d-b7ad-44f3c7e6b38b}] [url = chrome://browser/content/webrtcIndicator.xul]
[task 2019-03-06T09:32:06.515Z] 09:32:06 INFO - GECKO(1069) | --DOCSHELL 0xdd699c00 == 7 [pid = 1069] [id = {48a6dfd2-6a6f-4533-9838-950803460a0c}] [url = about:blank]
[task 2019-03-06T09:32:06.515Z] 09:32:06 INFO - GECKO(1069) | --DOCSHELL 0xdd698800 == 6 [pid = 1069] [id = {646d58b7-9649-429f-8166-ffca5f384395}] [url = chrome://browser/content/webrtcIndicator.xul]
[task 2019-03-06T09:32:07.423Z] 09:32:07 INFO - GECKO(1069) | [Child 1179: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2730: GetStats: Found no pipelines matching selector.
[task 2019-03-06T09:32:07.424Z] 09:32:07 INFO - GECKO(1069) | [Child 1179: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2730: GetStats: Found no pipelines matching selector.
[task 2019-03-06T09:32:07.627Z] 09:32:07 INFO - GECKO(1069) | TEST DEVICES: Got loopback audio: Monitor of Null Output
[task 2019-03-06T09:32:07.631Z] 09:32:07 INFO - GECKO(1069) | TEST DEVICES: Got loopback video: Dummy video device (0x0000)
[task 2019-03-06T09:32:08.420Z] 09:32:08 INFO - GECKO(1069) | [Child 1179: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2730: GetStats: Found no pipelines matching selector.
[task 2019-03-06T09:32:08.425Z] 09:32:08 INFO - GECKO(1069) | [Socket 1094, Main Thread] ###!!! ABORT: file /builds/worker/workspace/build/src/ipc/chromium/src/base/pickle.cc, line 187
[task 2019-03-06T09:34:47.102Z] 09:34:47 INFO - GECKO(1069) | #01: mozilla::Logger::~Logger() [ipc/chromium/src/base/logging.cc:48]
[task 2019-03-06T09:34:47.102Z] 09:34:47 INFO -
[task 2019-03-06T09:34:47.102Z] 09:34:47 INFO - GECKO(1069) | #02: Pickle::ReadBool(PickleIterator*, bool*) const [ipc/chromium/src/base/pickle.cc:188]
[task 2019-03-06T09:34:47.102Z] 09:34:47 INFO -
[task 2019-03-06T09:34:47.103Z] 09:34:47 INFO - GECKO(1069) | #03: IPC::ParamTraits<mozilla::dom::Optional<unsigned int> >::Read(IPC::Message const*, PickleIterator*, mozilla::dom::Optional<unsigned int>*) [ipc/glue/IPCMessageUtils.h:1061]
[task 2019-03-06T09:34:47.104Z] 09:34:47 INFO -

Flags: needinfo?(na-g)
Blocks: 1533020

realigning the RTP stats types to match spec. This involves breaking out the remote dictionary types. This shouldn't create user space visible changes.

Attachment #9050543 - Attachment is obsolete: true
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/a60750794c2e
refactor the RTC RTP stats types to match spec r=jib,smaug

https://hg.mozilla.org/mozilla-central/rev/a60750794c2e

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Would it be possible that this change caused an abrupt and sustained change in the reported values of WEBRTC_VIDEO_QUALITY_OUTBOUND_JITTER and WEBRTC_VIDEO_QUALITY_INBOUND_JITTER on 03-26?

Alert: http://alerts.telemetry.mozilla.org/index.html#/detectors/1/metrics/917/alerts/?from=2019-03-26&to=2019-03-26
Values over time: https://mzl.la/2TNHlHN

Flags: needinfo?(na-g)

(In reply to Chris H-C :chutten from comment #10)

Would it be possible that this change caused an abrupt and sustained change in the reported values of WEBRTC_VIDEO_QUALITY_OUTBOUND_JITTER and WEBRTC_VIDEO_QUALITY_INBOUND_JITTER on 03-26?

Alert: http://alerts.telemetry.mozilla.org/index.html#/detectors/1/metrics/917/alerts/?from=2019-03-26&to=2019-03-26
Values over time: https://mzl.la/2TNHlHN

Yes, I will look into this.

Chris, just to make sure that I am interpreting the graphs correctly (rel https://mzl.la/2TNHlHN).

It looks like we were reporting a median jitter of 0ms (which I think was almost certainly incorrect) before a change that occurred around March 26th, after which we began reporting a value of about 10 - 12 ms. Does this seem correct to you?

If so, I think this patch fixed a bug with our jitter stats reporting.

Flags: needinfo?(na-g) → needinfo?(chutten)

Before the change on 03-26 the median jitter was definitely being reported as 0. 99.95% of all jitter reported was 0 (see, for example, nightly 67's histogram: https://mzl.la/2TKi2Gt).

Your thought agrees with my guess: that, suddenly, jitter is being reported properly. It is still reporting mostly 0, but if you ignore that there's a distribution around a mode of ~7ms which is clearly visible: https://mzl.la/2TQCl5f

These WEBRTC probes have been in place for some years now and the mostly-0 nature of them wasn't noticed. Should these probes be removed as unused?

Flags: needinfo?(chutten) → needinfo?(na-g)

I think they need to be monitored if they are going to be used to catch WebRTC call quality regressions, which is probably a larger discussion with the WebRTC team. Nils, thoughts?

Flags: needinfo?(na-g) → needinfo?(drno)
Flags: needinfo?(drno)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: