1812293 - [Google Meet] The webcam video stream is not being seen by the other participant if one disables and re-enables it

Status: VERIFIED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Daniel Bodea [:danibodea]]

Note

• The video of Googe Meet is not being sent to the other participant if he disabled his own video and reactivates it.

Found in

• Beta v110.0b5

Affected versions

• Nightly v111.0a1
• Beta v110.b5

Tested platforms

• Affected platforms: Windows 10, Ubuntu 22, Mac OS 12
• Unaffected platforms: -

Steps to reproduce

1. Join the same meeting
2. One participant disabled his video.
3. He re-enables the video.

Expected result

• The other participant can see his video.

Actual result

• The other participant can NOT see his video. A single image may appear after a few minutes.

Regression range

• First bad revision: 4a8af015edd89e39350a0c2a38796b91a96eb0e4
• Last good revision: 5a2aac1feb784be71e029497969db1a25f581ec0
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5a2aac1feb784be71e029497969db1a25f581ec0&tochange=4a8af015edd89e39350a0c2a38796b91a96eb0e4
• Potentially regressed by: 1676855

Additional notes

• additional_note1

Comment 1

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

:bwc, since you are the author of the regressor, bug 1676855, could you take a look?

For more information, please visit auto_nag documentation.

Comment 2

[Byron Campen [:bwc]]

Pretty sure I see the problem here.

Comment 4

[Byron Campen [:bwc]]

Pretty sure this is a simple enough fix for an uplift.

Comment 5

[Byron Campen [:bwc]]

Attachment: Bug 1812293: Test-case for bug. r?jib

Comment 6

[Byron Campen [:bwc]]

Attachment: Bug 1812293: Fix bugs where replaceTrack didn't change mTransmitting appropriately. r?jib

Depends on D167981

Comment 7

[Byron Campen [:bwc]]

This is related to bug 1606875, and I think bug 1401592 just made it permafail.

Comment 8

[Byron Campen [:bwc]]

Try looks fine:

https://treeherder.mozilla.org/jobs?repo=try&revision=3055b3c51866608430be7bf8f9aaf39c2b4a6fe3
https://treeherder.mozilla.org/jobs?repo=try&revision=d717a513357a8e59190e0d3b77b2921ab948f7f0

Comment 9

[Byron Campen [:bwc]]

I've manually verified that the patch fixes the meet issue.

Comment 10

[Pascal Chevrel:pascalc]

This is marked as S2 and regressed in 110, can we get this landed on m-c and uplifted in our last betas this week or is the fix too risky for an uplift? Thanks

Comment 11

[Byron Campen [:bwc]]

This is a low-risk fix, so uplift is reasonable.

Comment 12

[Byron Campen [:bwc]]

Try looks fine for latest update:

https://treeherder.mozilla.org/jobs?repo=try&revision=df9d01b1594d03f535c83c208a86b89e7cfa1188
https://treeherder.mozilla.org/jobs?repo=try&revision=2c8c28c9b304cc82fa1bd9b5ed9b351087b870be

Comment 13

[Byron Campen [:bwc]]

Fix is now a tiny bit riskier, but still pretty low risk.

Comment 14

[Byron Campen [:bwc]]

An alternate approach based on reviews:

https://treeherder.mozilla.org/jobs?repo=try&revision=490972d6c945581b804ea6070a9f68bc0fff3acb
https://treeherder.mozilla.org/jobs?repo=try&revision=5dee58745e01610afc33a9ac11a3afc198c6f9d1

Comment 15

[Byron Campen [:bwc]]

Attachment: Bug 1812293: Remove this code that was breaking the replaceTrack(null->non-null) case. r?pehrsons

We don't need to do anything like this, since setting the nullptr on the pipeline
accomplishes what we want here.

Depends on D167981

Comment 16

[Byron Campen [:bwc]]

Try looks fine.

Comment 17

[Byron Campen [:bwc]]

Manual verification checks out.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e64d1a91860
Test-case for bug. r=jib
https://hg.mozilla.org/integration/autoland/rev/e4a3279a2c72
Remove this code that was breaking the replaceTrack(null->non-null) case. r=pehrsons

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/38288 for changes under testing/web-platform/tests

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/6e64d1a91860
https://hg.mozilla.org/mozilla-central/rev/e4a3279a2c72

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 22

[Pascal Chevrel:pascalc]

Byron, could you request uplift for our last beta? Thanks

Comment 23

[Byron Campen [:bwc]]

Comment on attachment 9315112 [details]
Bug 1812293: Remove this code that was breaking the replaceTrack(null->non-null) case. r?pehrsons

Beta/Release Uplift Approval Request

• User impact if declined: Google Meet (and probably other services) will break on release.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low risk, just removed some code that was not important to have.
• String changes made/needed: None.
• Is Android affected?: Yes

Comment 24

[Pascal Chevrel:pascalc]

Comment on attachment 9315112 [details]
Bug 1812293: Remove this code that was breaking the replaceTrack(null->non-null) case. r?pehrsons

Approved for our last 110 beta, thanks.

Comment 25

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/10454441f309
https://hg.mozilla.org/releases/mozilla-beta/rev/162bf264e7df

Comment 26

[Daniel Bodea [:danibodea]]

I have verified this fix with Nightly v111.0a1 from 2022-02-03 and Beta v110.0b9 on Windows 10, Mac OS 11 and Ubuntu 22.