1534466 - implement getSynchronizationSources for Video

Status: RESOLVED FIXED

(Core :: WebRTC, enhancement, P4)

Description

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

getSynchronizationSources is not implemented for video, only audio where it's primary purpose is to get the audio levels of packets. This probably isn't too hard, however there hasn't been anyone asking for it yet either.

Comment 2

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

I also filed: https://github.com/web-platform-tests/wpt/issues/15828, which is a bug against the test suite requiring the optional field 'voiceActivityFlag'.

Comment 3

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=725d138ffb6be6174230d6dc64a5e89501b57001

Comment 4

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

Attachment: Bug 1534466 - implement getContributingSources and getSynchronizationSources for video

Bug 1534466 - P1 refactor RtpSourceObserver to accept RTPHeader

Bug 1534466 - P2 Add RtpSourceObserver to VideoConduit

Bug 1534466 - P3 Update WPT ini to reflect newly passing tests

Comment 5

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

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

Comment 6

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c2468cb9389b14ad806cb4a3759548e7b09029f

Comment 7

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

Reminder to self, land Monday 3/18.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:ng, could you have a look please?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f641f92edb2
implement getContributingSources and getSynchronizationSources for video r=jib

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/4f641f92edb2

Comment 11

[Chris Mills [:cmills]]

We've got these documented, but we should probably update the compat data.

Comment 12

[Eric Shepherd [:sheppy]]

Did we previously have these in IDL and implemented to do nothing? I see no WebIDL changes here, which you normally do when API support is added for methods... according to the docs we already have in place, we have had these since Firefox 59. Was that audio only?

Comment 13

[Eric Shepherd [:sheppy]]

I've performed the following updates:

• Submitted BCD PR 4340 to add data about audio and video track support to getContributingSources() and getSynchronizationSources()
• Added a bullet point to the WebRTC section on Firefox 68 for developers, under APIs

Comment 14

[Byron Campen [:bwc]]

I'm not seeing any modifications to VideoConduit in this landing, and the wpt for video is still timing out. Did some part of this patchset get lost somehow?

Comment 15

[Byron Campen [:bwc]]

^

Comment 16

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

Yes, looking at the patch is history in fabricator the changes to VideoConduit were dropped. I will see about this today.

Comment 17

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

Attachment: Bug 1534466 - P2 - add missing video impl for GetRtpSources;r?jib

Comment 18

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=283a2a6032e02e63018f39d0716efea33a9daf7e

Comment 19

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

Some of the changes were lost during a rebase before landing it. I have restored those changes, which had been reviewed previously, but a refactoring of the timestamp code means that some new changes needed to be made. This patch includes turning on the relavent WPTs which provide coverage.

Comment 20

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6580e35975d5341adf769f50ca1b7dc247b38494

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e102ee3e495a
P2 - add missing video impl for GetRtpSources;r=jib

Comment 22

[Narcis Beleuzu [:NarcisB]]

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