1498253 - Remove _current_sync_offset from channel.h

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement, P3)

Description

[Dan Minor [:dminor]]

From Andreas:

Bug 964127. Do we still need this? Seems to be used for about:webrtc's "A/V Sync" which I can no longer find; and two telemetry scalars, only one of which show up on the telemetry dashboard, but it's always 0 [1].

[1] https://mzl.la/2IHiMZ6

Seems like a good candidate for removal.

Comment 1

[Dan Minor [:dminor]]

This whole GetDelayEstimates function is our code. We should probably be getting our stats from the AudioReceiveStream in this case rather than modifying Channel to add this method. Unfortunately the AudioReceiveStream stats do not provide everything we have here. As mentioned above, what we have here doesn't seem to be heavily used. It does look like we expose this through some moz prefixed stats at [1].

Nico, do you have any thoughts on what to do with this method, the associated telemetry and the moz prefixed stats? Thanks!

[1] https://searchfox.org/mozilla-central/rev/ff46b36ac2ebb243ec95fdab01340391963d62e5/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp#2946

Comment 2

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

Dan, I agree that this is a good candidate for removal. Now is a good time to remove the prefixed stats. That said, I find the telemetry curious.

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1498253 - Remove mozAvSyncDelay and mozJitterBufferDelay; r=ng!

The value for mozAvSyncDelay has been broken since the branch 57 update
(Bug 1341285). We added SetCurrentSyncOffset() but never called it from
anywhere.

In the future we should be getting stats from AudioReceiveStream rather than
modifying the channel code, the delay_estimate_ms field provides almost the
same information.

Since we're attempting to get rid of moz prefixed stats, it makes sense to just
remove this code rather than fix it. The associated telemetry code has been
broken since Bug 1341285 as well so I think it is safe to remove.

Comment 4

[Dan Minor [:dminor]]

Attachment: Bug 1498253 - Remove mozAvSyncDelay and mozJitterBufferDelay from webidl; r=baku

Depends on D14462

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80e86c169d20
Remove mozAvSyncDelay and mozJitterBufferDelay; r=ng
https://hg.mozilla.org/integration/autoland/rev/9d28fd352175
Remove mozAvSyncDelay and mozJitterBufferDelay from webidl; r=baku

Comment 7

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/80e86c169d20
https://hg.mozilla.org/mozilla-central/rev/9d28fd352175