Closed Bug 1576771 Opened 1 year ago Closed 11 months ago

Replacing video track in Hubs fails to send data to SFU

Categories

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

68 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 70+ verified
firefox69 --- wontfix
firefox70 + verified
firefox71 + verified

People

(Reporter: gfodor, Assigned: pehrsons, NeedInfo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Hi there, we're seeing what seems to be a potential bug in Firefox when using the screen sharing functionality in Hubs, causing the video stream to not be sent over the wire.

Note that at the time of this writing screen sharing seems completely broken in Firefox Nightly, this bug seems manifest in Firefox 68.0.2.

Reproduction steps:

  1. Go to hubs.mozilla.com and create a room
  2. Open up a new private browser window at the same URL as your new room. You should now have two browser windows open to the new room.
  3. In the first window, click through the entry flow by clicking "Enter Room" at the bottom, accepting the default avatar, "Enter on Screen", granting microphone permission, and "Enter Room" on the audio page.
  4. In the second window, you should see the avatar of the first window (and hear their audio over the microphone if you selected one.)
  5. In the first window, click the screen sharing button -- at the top center of the screen there is a small HUD, it is the button farthest to the left that looks like a computer monitor.
  6. Select a window to share and confirm.
  7. In the both windoww, you should now see the shared screen appear in the room.
  8. In the first window, click the screen sharing button again to end sharing.
  9. In the second window, you should see the shared screen disappear.
  10. In the first window, click the screen sharing button again to begin sharing the screen again. Choose the same window.
  11. In the first window, you will see the screen appear as expected. In the second window, you will see a black square where the screen should be.

The underlying logging in our SFU, and the about: webrtc logging implies there is no data being sent over the WebRTC video stream after the second share. This bug occurs if you use either screen sharing or camera sharing. This bug does not happen in chrome, with either screen sharing or camera sharing. (At the time of this writing, screen sharing in chrome via getDisplayMedia is not currently released yet at hubs.mozilla.com, but local testing showed that this issue does not occur when doing screen sharing from Chrome.)

Under the hood, the way the tracks are managed in Hubs are as follows:

  • We have two RTCRtpSenders, one for the audio track and one for the video track going to our SFU. After the first share, both are enabled.

  • After you remove the first share, we set the video track's enabled bit to false on the corresponding sender.

  • When the second share is created, we call replaceTrack() on the sender, passing it the new video track.

It seems possible something is going wrong during replaceTrack, or something upstream is going wrong (or we are failing to do something properly) when tearing down the first track. Given that it is working properly in Chrome, it seemed possible that it was a browser bug so I opened this issue. Unfortunately I could not figure out how to get the debug logging working for webrtc (the about:webrtc debug logging button shows no log file name when I enable debug mode, and the environment variable in the wiki did not seem to help.)

Thanks for any help!

This could be reproduced on my site.

jib, would you mind taking a look?

Flags: needinfo?(jib)
Priority: -- → P2

Hi Greg,
If this is related to replaceTrack api, something weird I observed in my app while using replaceTrack. ReplaceTrack does not get effect unless un till I do the below :
window.setTimeout(function() {
newStream.getTracks()[0].enabled = false;
window.setTimeout(function() {
newStream.getTracks()[0].enabled = true;
}, 200);
}, 1000);
Not sure why. However, the replaceTrack works fine till FF67.
Would be good if you try the above work around and see if that fix the issue. Meanwhile, we need some investigation from Firefox team on it.

Wow, yes I can confirm this resolves the issue. We will put in a workaround for now.

Thank you for that info!

Hi Pradeep, thanks for providing a workaround! And thanks Greg for filing the bug. Sorry for the late reply. Catching up from summer.

Do either of you have a small reproducible piece of javascript that shows the replaceTrack problem? I'm having trouble narrowing down the problem using the hub code. A basic replaceTrack fiddle seems to work for me: https://jsfiddle.net/jib1/yukbn0ep/ - Does it only happen with screen sharing specifically?

Another angle would be if you know it works in 67 to run https://mozilla.github.io/mozregression/install.html on your system to locate the regression that way.

Flags: needinfo?(jib) → needinfo?(itpandey.88)

Unfortunately I don't have a simpler repro in this case. (Other Hubs related bugs I've had I've tracked down like this, but this one seems fairly involved to do so since I'm not super familiar with all the WebRTC negotiation etc.) The issue seems to happen with either camera sharing or screen sharing, and I can confirm that the workaround above fixes the issue.

I took a quick look at the example fiddle -- not positive but I don't think that captures the issue properly. In Hubs, the sender can still see the data if looking at the stream, its just not going out over the wire. So I think to properly repro you'd need to have a multi-user example properly doing P2P streaming. What I'd expect is the sender would still render the pixels if reading from the sender stream/track, but the receiver would just see a black quad, after the track is replaced the first time on the sender side.

I figured out the STRs. It's when replacing a disabled track with an enabled one. This should result in data sent, but bug 1423253 broke this in 68:

  1. Open https://jsfiddle.net/jib1/x03wspuq/
  2. (Allow permission if necessary)
  3. Check the ☐ scramble box.

Expected results:

  • Whitenoise appars in the second element

Actual results:

  • The second element remains black.

Regression range:

15:38.77 INFO: Narrowed inbound regression window from [deda4112, e5fc0c88] (3 builds) to [fc6f661f, e5fc0c88] (2 builds) (~1 steps left)
15:38.77 INFO: No more inbound revisions, bisection finished.
15:38.77 INFO: Last good revision: fc6f661f4113fa38c6e44663c404edc7e3ceadec
15:38.77 INFO: First bad revision: e5fc0c881af02fe17288ea08f3f5c3a85a5070e8
15:38.78 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fc6f661f4113fa38c6e44663c404edc7e3ceadec&tochange=e5fc0c881af02fe17288ea08f3f5c3a85a5070e8

Workarounds:

  1. Either temporarily enable a track before replacing it:
const old = sender.track;
const tmp = old.enabled;
old.enabled = true;
await sender.replaceTrack(withTrack);
old.enabled = tmp;
  1. Or twiddle the enabled state afterwards.
await sender.replaceTrack(withTrack);
if (sender.track.enabled) {
  sender.track.enabled = false;
  sender.track.enabled = true;
}

Note you don't need setTimeout to make the workaround work.

[Tracking Requested - why for this release]: Regression causing WebRTC video/audio to not play when relying on replaceTrack() to replace a disabled track with an enabled one. Simple workaround exists.

Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(apehrson)
Keywords: regression
Regressed by: 1423253

That's unfortunate. I'll try to add a WPT together with a fix.

Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Flags: needinfo?(apehrson)

I'm hitting errors uploading my recording of this to pernosco. I'll debug locally.

That was a regression on pernosco's part that was backed out. A recording is available at https://pernos.co/debug/ZONuEm4BX_45rh34dQfJkg/index.html.

I have a fix for this, but seeing that there is no framework for checking video color in the WPT mediacapture suites, I am thinking of adding this as a mochitest for now.

Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6fb1164908f1
Add mochitest. r=jib
https://hg.mozilla.org/integration/autoland/rev/2d4f1f657c82
Reset VideoFrameConverter's track-enabled state when disconnecting video listeners. r=bwc
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Hi, do you want to request uplift to beta?

Flags: needinfo?(apehrson)

Comment on attachment 9090397 [details]
Bug 1576771 - Reset VideoFrameConverter's track-enabled state when disconnecting video listeners. r?dminor

Beta/Release Uplift Approval Request

  • User impact if declined: In a video call service a remote video stream may appear black when expected to contain real video.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 7.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Trivial change
  • String changes made/needed:

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: 68 regression affecting potentially any video calling/conferencing service. Ones allowing screensharing to be enabled on the fly seem more exposed to the path needed to trigger the bug. This path is a bit tricky so when the bug is triggered the STR is not necessarily understood.
  • User impact if declined: In a video call service a remote video stream may appear black when expected to contain real video.
  • Fix Landed on Version: 71
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Trivial change
  • String or UUID changes made by this patch:
Flags: needinfo?(apehrson)
Attachment #9090397 - Flags: approval-mozilla-esr68?
Attachment #9090397 - Flags: approval-mozilla-beta?
Attachment #9090396 - Flags: approval-mozilla-beta? approval-mozilla-esr68?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Reproduced the issue using comment 7 on affected build 71.0a1 2019-09-02 On Windows 10, then verified the fix also using comment 7 on:

Environment:

Windows 10 x64 Pro, Ubuntu 16.04 x64, Mac 10.14.6

Versions:

71.0a1 2019-09-09

Leaving qe+ for beta uplift verification.

Comment on attachment 9090397 [details]
Bug 1576771 - Reset VideoFrameConverter's track-enabled state when disconnecting video listeners. r?dminor

Fix for video regression from 68, looks ok for beta/esr uplift.

Attachment #9090397 - Flags: approval-mozilla-esr68?
Attachment #9090397 - Flags: approval-mozilla-esr68+
Attachment #9090397 - Flags: approval-mozilla-beta?
Attachment #9090397 - Flags: approval-mozilla-beta+
Attachment #9090396 - Flags: approval-mozilla-esr68?
Attachment #9090396 - Flags: approval-mozilla-esr68+
Attachment #9090396 - Flags: approval-mozilla-beta?
Attachment #9090396 - Flags: approval-mozilla-beta+

Verified fixed on Windows 10 x64, Ubuntu 18.04 and MacOS 10.14 by following the steps provided in Comment 7 in Beta 70.0b13 (64-bit).
Leaving qe+ flag for esr verification

Also verified on 68.2.0esr 2019-10-10 (taskcluster build) on Ubuntu 16.04/Mac 10.14.6 / Windows 10.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.