1636185 - Setting signaled remote video ssrcs fails

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Byron Campen [:bwc]]

We're failing here on the initial negotiation every time: https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#993

On the bright side, this means we've really been exercising the ssrc-switching code.

Not sure if we're seeing a similar failure for audio.

Comment 1

[Byron Campen [:bwc]]

Attachment: Bug 1636185: Don't use GetRemoteSSRCLocked when setting/unsetting ssrcs, because that is intended to be used for stats, not configuration. r?dminor

Comment 2

[Byron Campen [:bwc]]

Attachment: Bug 1636185: Inline this since nothing else was using it. r?dminor

Depends on D74267

Comment 3

[Byron Campen [:bwc]]

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

Comment 4

[Byron Campen [:bwc]]

Interesting. Seeing failures on dom/media/tests/mochitest/test_peerConnection_simulcastOddResolution.html, but only on Linux 18.04 x64 QuantumRender opt, and not on any of the other simulcast tests...

Comment 5

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

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:bwc, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Byron Campen [:bwc]]

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

Comment 7

[Byron Campen [:bwc]]

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

Comment 8

[Byron Campen [:bwc]]

Seems like running the failing test in isolation works fine. Still trying to figure out what is going on here.

Comment 9

[Byron Campen [:bwc]]

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

Comment 10

[Byron Campen [:bwc]]

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

Comment 11

[Byron Campen [:bwc]]

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

Comment 12

[Byron Campen [:bwc]]

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

Comment 13

[Byron Campen [:bwc]]

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

Comment 14

[Byron Campen [:bwc]]

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

Comment 15

[Byron Campen [:bwc]]

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

Comment 16

[Byron Campen [:bwc]]

Attachment: Bug 1636185: Don't start emitting frames until we can set our initial RID filter. r?jib

This test was flaky because until we set a rid filter, all ssrcs were being
received. This meant that the resolution could have been consistent with any of
foo, bar, or baz when we started looking for resize events. In some cases, that
meant that we would never see the resize event for the transition to bar,
because the video element was already at that resolution.

Depends on D74279

Comment 17

[Byron Campen [:bwc]]

Try looks good.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a50daa9ee769
Don't use GetRemoteSSRCLocked when setting/unsetting ssrcs, because that is intended to be used for stats, not configuration. r=dminor
https://hg.mozilla.org/integration/autoland/rev/c0aab9a91e21
Inline this since nothing else was using it. r=dminor
https://hg.mozilla.org/integration/autoland/rev/b38d112a2d6a
Don't start emitting frames until we can set our initial RID filter. r=jib

Comment 19

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/a50daa9ee769
https://hg.mozilla.org/mozilla-central/rev/c0aab9a91e21
https://hg.mozilla.org/mozilla-central/rev/b38d112a2d6a

Comment 20

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

The patch landed in nightly and beta is affected.
:bwc, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 21

[Byron Campen [:bwc]]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #20)

The patch landed in nightly and beta is affected.
:bwc, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

We aren't even sure if this was causing problems in practice, so I don't think we want to uplift this right now. The test case fix might be worth uplifting if that test misbehaves on beta though.