1321235 - Can not remove a stopped media track using removeTrack on Firefox version 52 onwards

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

Description

[Prerak Jain]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.98 Safari/537.36

Steps to reproduce:

Stop a media tracks from a MediaStream and then remove the stopped track from the MediaStream using removeTrack.
use - https://jsfiddle.net/7octza05/3/ to reproduce the issue


Actual results:

From Firefox 52 and onwards, if the track is stopped before calling replaceTrack(), it is not removed from the MediaStream object. It is working fine on stable (v50) and beta(51).

If I try to remove a track without stopping it the media capture continues to happens, eg I try to remove a video track from the mediaStream without stopping it the camera light does not turn off and capture cont


Expected results:

ReplaceTrack() should remove from media stream even if is is stopped

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Andreas - perhaps track cloning fallout?

Comment 2

[Loic]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4135fed00f09d87196f3b1382285b900615faf17&tochange=8e63884e4c094714cb2413af2d262cc3658302a5

Comment 3

[Andreas Pehrson [:pehrsons]]

Cloning was earlier. But yeah, some sort of fallout.

Comment 4

[Marcia Knous [:marcia]]

Tracking 53+ for this video regression.

Comment 5

[Julien Cristau [:jcristau]]

tracking as new regression in 52

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1321235 - Add a mochitest.

Review commit: https://reviewboard.mozilla.org/r/96966/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96966/

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1321235 - Let ended MediaStreamTracks be removable.

Review commit: https://reviewboard.mozilla.org/r/96968/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96968/

Comment 8

[Andreas Pehrson [:pehrsons]]

So turned out we didn't have a test for this. :/

Anyhow - fallout from Bug 1208316 - MediaStream.active. And thanks for reporting Prerak!

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8816201 [details]
Bug 1321235 - Add a mochitest.

https://reviewboard.mozilla.org/r/96966/#review97216

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8816202 [details]
Bug 1321235 - Let ended MediaStreamTracks be removable.

https://reviewboard.mozilla.org/r/96968/#review97218

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f0eb04ee1d5
Add a mochitest. r=jesup
https://hg.mozilla.org/integration/autoland/rev/eea552a9e162
Let ended MediaStreamTracks be removable. r=jesup

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7f0eb04ee1d5
https://hg.mozilla.org/mozilla-central/rev/eea552a9e162

Comment 13

[Prerak Jain]

Andreas, Thanks for fixing it quickly. Will this is be available on Firefox 52 also, as it's there too.

Comment 14

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8816202 [details]
Bug 1321235 - Let ended MediaStreamTracks be removable.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1208316
[User impact if declined]: We'd regress a shipped, minor, feature which behavior is clearly speced.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's very simple, and has automated test coverage.
[String changes made/needed]: None

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8816202 [details]
Bug 1321235 - Let ended MediaStreamTracks be removable.

fix new regression in aurora52

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/256286003335
https://hg.mozilla.org/releases/mozilla-aurora/rev/d3c8c1a41bd7

Comment 17

[Andrei Vaida [:avaida]]

qe-verify- per Comment 14