1729455 - remote-outbound-rtp's remoteTimestamp field uses the time base of NTP (1 Jan 1900) instead of DOMHighResTimeStamp (1 Jan 1970)

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P3)

Description

[Andreas Pehrson [:pehrsons]]

There's a good 70 extra years added onto remoteTimestamp compared to what an application might expect.

Comment 1

[Andreas Pehrson [:pehrsons]]

The timestamps from libwebrtc are also not going through the same precision reduction logic that our own TimestampMaker has. Could fix that here too.

Comment 2

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

Good catch. I am not sure about degrading the timestamps, as they can be used to uniquely identify specific packets, though I am not sure how accurate the current conversion code is. We should at least put that behind a pref to start.

Comment 3

[Andreas Pehrson [:pehrsons]]

Gotcha. Well let me know in review and I'll make adjustments. The patches are already written.

Comment 5

[Andreas Pehrson [:pehrsons]]

I'll leave out the precision reduction on remote timestamps then, but put them on local receive timestamps, which I'll be taking from libwebrtc now.

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Test remoteTimestamp's age in mochitest. r?jib!

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Add to stats the local receive time for receiving video Sender Reports. r?ng!

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Fix timestamps for remote stats. r?ng!

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Rebase libwebrtc ntp timestamps to proper DOMHighResTimeStamp time. r?ng!

NTP time starts at "midnight on January first 1900" (RFC868).
DOMHighResTimeStamp starts at "midnight at the beginning of 1 January 1970 UTC"
(ECMA-262).

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Modularize RTCStatsTimestampMaker::GetNow(). r?ng!, r?bwc!

This is needed in order to provide precision reduction on timestamps from
libwebrtc.

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Reduce time precision on timestamps from libwebrtc. r?ng!

Comment 12

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Make GetRemoteSSRC return a Maybe. r?ng!

Comment 13

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Inject our own clock into libwebrtc. r?bwc!, r?ng!

This solves issues around translating timestamps from the libwebrtc clock to
DOMHighResTimeStamp which is using our timestamp maker. With the regular
upstream clock there's a risk those timestamps appear in the future, compared
to Performance.now, leading to test failures in stats.

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Cherry-pick libwebrtc patches that makes SystemTimeNanos overrideable. r?bwc!

This lets us define our own monotonic system clock for libwebrtc to use.

This patch corresponds to the libwebrtc part of chromium bug 516700
(https://crbug.com/516700).

It consists of the following libwebrtc patches squashed:
https://webrtc.googlesource.com/src/+/b73c9f0bc3c0331c177fcbd54224b350153aebcd
https://webrtc.googlesource.com/src/+/da20c739a8607e9f43e6ac04024ea622167ec694
https://webrtc.googlesource.com/src/+/373bb7bec4d2df7378d3dae965313548e6817246
https://webrtc.googlesource.com/src/+/bb52bdf09516ca548c4aff50526eda561f239bc0

Comment 15

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Cherry-pick libwebrtc ntp clock consolidation patches. r?ng!, r?bwc!

This lets us inject our own clock to a Call and control the wall-clock time.

This patch corresponds to upstream bug 11327
(https://bugs.chromium.org/p/webrtc/issues/detail?id=11327) and is a squash of
the following upstream commits:

• https://webrtc.googlesource.com/src/+/314b78d467c224159fb28c8dddf1d6fd78be7faa
• https://webrtc.googlesource.com/src/+/e9dad5f053c6d206f1ebab3d99945f81f597ff70
• https://webrtc.googlesource.com/src/+/704d6e518aa789a5e76378b7b758d6a77b60312e
• https://webrtc.googlesource.com/src/+/6817809e26f7631b1caa3013ee45075b9fef58d3
• https://webrtc.googlesource.com/src/+/0de1ed0244a8da395811ad8952e5f5f8a2643e2a
• https://webrtc.googlesource.com/src/+/b59e9045bf215aaec869412b16d0fd39bf0df4ec
• https://webrtc.googlesource.com/src/+/2491dbdf0b59cc080ea7218c32c109f4c8b304fe
• https://webrtc.googlesource.com/src/+/95f1e5192c95903d0fb7ed14f0e3acf0b510a440
• https://webrtc.googlesource.com/src/+/47ed99872d8b6392d2eb2e3c890b0eb00209fa20
• https://webrtc.googlesource.com/src/+/00f6e75671b7c73af5b3c88d2b554ee7873275f9
• https://webrtc.googlesource.com/src/+/fe3dd51f326b8d418a2ce055b313a099af58a815
• https://webrtc.googlesource.com/src/+/a6b0d53dc250aaebc14315a44515971da72980cc
• https://webrtc.googlesource.com/src/+/46fbefa3023ca7d1c1967e92b68c343173a00db2
• https://webrtc.googlesource.com/src/+/cab90db24ac2e4ddbb9e7619f65bf5dc582783c0

Comment 16

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Update generated gn-config files. r?mjf!

Comment 17

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Define a gecko-specific rtc::SystemTimeNanos. r?bwc!

Comment 18

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - libwebrtc: Don't use wall clock for stats. r?bwc!

Comment 19

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Reduce time precision on timestamps from libwebrtc. r?ng!

https://phabricator.services.mozilla.com/D125716

Comment 20

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Simplify RtpSources timestamp conversion. r?bwc!

Comment 21

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Sort RTCStatsReport includes. r?bwc!

Comment 22

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Define our own webrtc::Clock based on RTCStatsTimestampMaker. r?bwc!

Comment 23

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Inject RTCStatsTimestampMakerRealtimeClock into Call instances. r?bwc!

This patch makes libwebrtc use our clock for timestamps.
It also makes sure future libwebrtc updates don't introduce unaudited use of the
libwebrtc realtime clock.

Comment 24

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Update gn-configs README with rust aarch64 stdlib instructions for Windows. r?mjf

Comment 25

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Simplify gn-configs generation instructions for mac. r?mjf

Comment 26

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Let configure figure out the mac os sdk itself. r?mjf

Comment 27

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Ensure the libwebrtc system clock is not used. r?bwc!

Comment 28

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Test that synchronization sources have a timestamp with the correct base. r?ng!

Comment 29

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Remove unused arg in DesktopCaptureImpl::DeliverCapturedFrame and simplify. r?bwc!

Comment 30

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1729455 - Plumb video frame timestamps properly through the GMP encoder. r?bwc!

This avoid triggering an assertion in RTPSenderVideo::SendVideo that doesn't
like negative timestamps.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2cee9ad6c6c
Test remoteTimestamp's age in mochitest. r=jib,ng
https://hg.mozilla.org/integration/autoland/rev/99267b6d193f
Add to stats the local receive time for receiving video Sender Reports. r=ng
https://hg.mozilla.org/integration/autoland/rev/cfcd4d853484
Fix timestamps for remote stats. r=ng
https://hg.mozilla.org/integration/autoland/rev/8e036b0931e3
Make GetRemoteSSRC return a Maybe. r=ng
https://hg.mozilla.org/integration/autoland/rev/907da0f267fd
Rebase libwebrtc ntp timestamps to proper DOMHighResTimeStamp time. r=ng
https://hg.mozilla.org/integration/autoland/rev/f058fbb8662e
Cherry-pick libwebrtc patches that makes SystemTimeNanos overrideable. r=bwc
https://hg.mozilla.org/integration/autoland/rev/080188fa01cf
Cherry-pick libwebrtc ntp clock consolidation patches. r=bwc,ng
https://hg.mozilla.org/integration/autoland/rev/7f7c9f002087
Define a gecko-specific rtc::SystemTimeNanos. r=bwc
https://hg.mozilla.org/integration/autoland/rev/11637120b0cc
Update generated gn-config files. r=mjf
https://hg.mozilla.org/integration/autoland/rev/eb07a028bc43
Update gn-configs README with rust aarch64 stdlib instructions for Windows. r=mjf
https://hg.mozilla.org/integration/autoland/rev/e8d24be16e22
Simplify gn-configs generation instructions for mac. r=mjf
https://hg.mozilla.org/integration/autoland/rev/0cfae6f33c35
Let configure figure out the mac os sdk itself. r=mjf
https://hg.mozilla.org/integration/autoland/rev/3e8ac168ee3d
Ensure the libwebrtc system clock is not used. r=bwc
https://hg.mozilla.org/integration/autoland/rev/5a3ecc96a699
libwebrtc: Don't use wall clock for stats. r=bwc
https://hg.mozilla.org/integration/autoland/rev/40008e4f1c1f
Modularize RTCStatsTimestampMaker::GetNow(). r=ng,bwc
https://hg.mozilla.org/integration/autoland/rev/d112b90b7c05
Reduce time precision on timestamps from libwebrtc. r=ng
https://hg.mozilla.org/integration/autoland/rev/b04e243f4ab5
Simplify RtpSources timestamp conversion. r=bwc
https://hg.mozilla.org/integration/autoland/rev/9b44714d7fce
Test that synchronization sources have a timestamp with the correct base. r=ng
https://hg.mozilla.org/integration/autoland/rev/0473c7cfd344
Sort RTCStatsReport includes. r=bwc
https://hg.mozilla.org/integration/autoland/rev/6911243d9ae0
Define our own webrtc::Clock based on RTCStatsTimestampMaker. r=bwc
https://hg.mozilla.org/integration/autoland/rev/052a33acc2e4
Remove unused arg in DesktopCaptureImpl::DeliverCapturedFrame and simplify. r=bwc,ng
https://hg.mozilla.org/integration/autoland/rev/0744d68b8c94
Inject RTCStatsTimestampMakerRealtimeClock into Call instances. r=bwc
https://hg.mozilla.org/integration/autoland/rev/eb27a22d5419
Plumb video frame timestamps properly through the GMP encoder. r=bwc

Comment 32

[Iulian Moraru]

Backed out for causing multiple build bustages.

Push with failures

Failure log for Windows 2012 x64 debug
Failure log for OS X Cross Compiled plain
Failure log for Linux x64 plain
and also this tier 2 builds
Failure log for Windows MinGW all MinGW-Clang builds for Windows 64-bits
Failure log for Windows MinGW all MinGW-Clang builds for Windows 32-bits

Backout link

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a55783edb50
Test remoteTimestamp's age in mochitest. r=jib,ng
https://hg.mozilla.org/integration/autoland/rev/75b2e5b22dd8
Add to stats the local receive time for receiving video Sender Reports. r=ng
https://hg.mozilla.org/integration/autoland/rev/2bee95385d95
Fix timestamps for remote stats. r=ng
https://hg.mozilla.org/integration/autoland/rev/ff2adec471a1
Make GetRemoteSSRC return a Maybe. r=ng
https://hg.mozilla.org/integration/autoland/rev/404b8c40b078
Rebase libwebrtc ntp timestamps to proper DOMHighResTimeStamp time. r=ng
https://hg.mozilla.org/integration/autoland/rev/89f79325e50a
Cherry-pick libwebrtc patches that makes SystemTimeNanos overrideable. r=bwc
https://hg.mozilla.org/integration/autoland/rev/d954e42b9fc3
Cherry-pick libwebrtc ntp clock consolidation patches. r=bwc,ng
https://hg.mozilla.org/integration/autoland/rev/fdeabccf0376
Define a gecko-specific rtc::SystemTimeNanos. r=bwc
https://hg.mozilla.org/integration/autoland/rev/064f668b8497
Update generated gn-config files. r=mjf
https://hg.mozilla.org/integration/autoland/rev/b4b99d2a0e4b
Update gn-configs README with rust aarch64 stdlib instructions for Windows. r=mjf
https://hg.mozilla.org/integration/autoland/rev/9c79b8f388ac
Simplify gn-configs generation instructions for mac. r=mjf
https://hg.mozilla.org/integration/autoland/rev/bad692b90c78
Let configure figure out the mac os sdk itself. r=mjf
https://hg.mozilla.org/integration/autoland/rev/ba5966905e63
Ensure the libwebrtc system clock is not used. r=bwc
https://hg.mozilla.org/integration/autoland/rev/88281da6942e
libwebrtc: Don't use wall clock for stats. r=bwc
https://hg.mozilla.org/integration/autoland/rev/f5f9031ad36d
Modularize RTCStatsTimestampMaker::GetNow(). r=ng,bwc
https://hg.mozilla.org/integration/autoland/rev/2311db30725c
Reduce time precision on timestamps from libwebrtc. r=ng
https://hg.mozilla.org/integration/autoland/rev/fe6f8e1d360c
Simplify RtpSources timestamp conversion. r=bwc
https://hg.mozilla.org/integration/autoland/rev/6ff38fcddbbc
Test that synchronization sources have a timestamp with the correct base. r=ng
https://hg.mozilla.org/integration/autoland/rev/8f90f559af34
Sort RTCStatsReport includes. r=bwc
https://hg.mozilla.org/integration/autoland/rev/f8e7943414f2
Define our own webrtc::Clock based on RTCStatsTimestampMaker. r=bwc
https://hg.mozilla.org/integration/autoland/rev/facac6bb25c3
Remove unused arg in DesktopCaptureImpl::DeliverCapturedFrame and simplify. r=bwc,ng
https://hg.mozilla.org/integration/autoland/rev/5388fb363fd2
Inject RTCStatsTimestampMakerRealtimeClock into Call instances. r=bwc
https://hg.mozilla.org/integration/autoland/rev/7a84736b96c0
Plumb video frame timestamps properly through the GMP encoder. r=bwc

Comment 34

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1a55783edb50
https://hg.mozilla.org/mozilla-central/rev/75b2e5b22dd8
https://hg.mozilla.org/mozilla-central/rev/2bee95385d95
https://hg.mozilla.org/mozilla-central/rev/ff2adec471a1
https://hg.mozilla.org/mozilla-central/rev/404b8c40b078
https://hg.mozilla.org/mozilla-central/rev/89f79325e50a
https://hg.mozilla.org/mozilla-central/rev/d954e42b9fc3
https://hg.mozilla.org/mozilla-central/rev/fdeabccf0376
https://hg.mozilla.org/mozilla-central/rev/064f668b8497
https://hg.mozilla.org/mozilla-central/rev/b4b99d2a0e4b
https://hg.mozilla.org/mozilla-central/rev/9c79b8f388ac
https://hg.mozilla.org/mozilla-central/rev/bad692b90c78
https://hg.mozilla.org/mozilla-central/rev/ba5966905e63
https://hg.mozilla.org/mozilla-central/rev/88281da6942e
https://hg.mozilla.org/mozilla-central/rev/f5f9031ad36d
https://hg.mozilla.org/mozilla-central/rev/2311db30725c
https://hg.mozilla.org/mozilla-central/rev/fe6f8e1d360c
https://hg.mozilla.org/mozilla-central/rev/6ff38fcddbbc
https://hg.mozilla.org/mozilla-central/rev/8f90f559af34
https://hg.mozilla.org/mozilla-central/rev/f8e7943414f2
https://hg.mozilla.org/mozilla-central/rev/facac6bb25c3
https://hg.mozilla.org/mozilla-central/rev/5388fb363fd2
https://hg.mozilla.org/mozilla-central/rev/7a84736b96c0