1606875 - replaceTrack of an enabled track with a disabled one (and vice versa) doesn't work reliably

Status: NEW

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

Description

[Byron Campen [:bwc]]

Note this try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60b488b977d69701ae9ad59dc59dfb75f6ef847b&selectedJob=283371382

I've dug into this a bit, and there seem to be multiple problems, but I have not figured how to fix them all.

Firstly, this code only forces black frames if the listener has been disabled due to privacy rules:

https://searchfox.org/mozilla-central/rev/053826b10f838f77c27507e5efecc96e34718541/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp#1248

It needs to take into account whether mFrame.GetForceBlack() is true also, since that is how frames are "blacked out" for direct listeners:

https://searchfox.org/mozilla-central/rev/053826b10f838f77c27507e5efecc96e34718541/dom/media/MediaTrackListener.cpp#28-30

Additionally, the way we are updating the track does not seem to result in the direct listeners learning anything about whether the new track is enabled or not.

Right now I am classifying this as a webrtc bug, but it might be a bug in MTG.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Is this a regression? Bug 1608118 was regressed by bug 1601034, so if they're dupes their regression ranges should match.

Comment 2

[Daniel Bodea [:danibodea]]

Can you provide me with a method to reproduce this issue of yours? I am attempting to find when it was introduced.
Otherwise, can you do that yourself?
https://mozilla.github.io/mozregression/

Thank you for your contribution!

Comment 3

[Byron Campen [:bwc]]

I think this is likely a dupe of bug 1608118. I'll check to see once I'm back in the office.

Comment 4

[Byron Campen [:bwc]]

Ok, bug 1608118 did not fix this. However, it may have been part of the necessary work. I will investigate.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db97255ce15bb05077623c99491d73b76f02a520&selectedJob=286723897

Comment 5

[Pradeep Kumar]

Hi Byron Campen,

I'm facing the same issue that sometimes replaceTrack is not working. As of now, I have identified one scenario where reproducibility is higher:
For first time user of Firefox, meaning fresh installation of Firefox, for some feature, I've to create the fake audio stream and which is made fakeStream.getTracks()[0].enabled=false as it is fake. And then later in the call, we need to replace the fakeStream's track with whatever we get from microphone. And here, we are facing issue that some times audio track is not being replaced properly and Firefox does not send the audio to farend.
Further, we attached the new audio stream to video element manually to test if new stream is proper or not, we found that audio plays.

For me, it looks like that by replaceTrack, somehow, RTPSender is not able to pick up the audio packets from new streams.
I'm not sure why I'm not facing issue with video fake stream as doing same for video as of audio too.
Attaching logs as NotWorkingFF

Comment 6

[Pradeep Kumar]

Attachment: NotWorkingFF.log

Comment 7

[Pradeep Kumar]

Comment on attachment 9123649 [details]
NotWorkingFF.log

For testing I usually run "/Applications/Firefox.app/Contents/MacOS/firefox --MOZ_LOG=timestamp,signaling:5,mtransport:5,MediaManager:5,GetUserMedia:4,jsep:5,RtpLogger:5,webrtc_trace:5,VP8TrackEncoder:5,VP8TrackDecoder:5 --MOZ_LOG_FILE=/tmp/ff71.log" , And attached log have the output of this one. If you need anything else I can do that as I can reproduce the issue more often.

Comment 8

[Byron Campen [:bwc]]

Does your use case get any better with the binaries on this try push?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=afc8baa66ecbda84701ab1d369abe79c96336abf&selectedJob=286871092

(OS X build here: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/X-ZSApoXRi6cPFdn763RFg/runs/0/artifacts/public/build/target.dmg)

Comment 9

[Pradeep Kumar]

Sorry to disappoint you that still I'm facing the issue. I used OS X build.

Comment 10

[Daniel Bodea [:danibodea]]

I am setting this NI to make sure that the issue is addressed. Thank you.

Comment 11

[Byron Campen [:bwc]]

Bug is confirmed, and I'm assigned, so no need for a needinfo.

Comment 12

[Byron Campen [:bwc]]

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

Comment 13

[Daniel Bodea [:danibodea]]

Setting the [qa-regression-triage] flag, considering that this bug is assigned and being addressed.
Please provide some sort of steps to reproduce and NI me if this bug needs verification.
Thank you for your contribution!

Comment 14

[Byron Campen [:bwc]]

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

Comment 15

[Byron Campen [:bwc]]

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

Comment 16

[Byron Campen [:bwc]]

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