Closed
Bug 1498253
Opened 6 years ago
Closed 6 years ago
Remove _current_sync_offset from channel.h
Categories
(Core :: WebRTC: Audio/Video, enhancement, P3)
Tracking
()
RESOLVED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: dminor, Assigned: dminor)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
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.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → dminor
Assignee | ||
Comment 1•6 years ago
|
||
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
Flags: needinfo?(na-g)
Comment 2•6 years ago
|
||
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.
Flags: needinfo?(na-g)
Assignee | ||
Comment 3•6 years ago
|
||
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.
Updated•6 years ago
|
Attachment #9031188 -
Attachment description: Bug 1498253 - Remove mozAvSyncDelay and mozJitterBufferDelay from webidl → Bug 1498253 - Remove mozAvSyncDelay and mozJitterBufferDelay from webidl; r=baku
Pushed by dminor@mozilla.com: 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•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/80e86c169d20 https://hg.mozilla.org/mozilla-central/rev/9d28fd352175
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in
before you can comment on or make changes to this bug.
Description
•