New issue
Advanced search Search tips

Issue 770908 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression


Participants' hotlists:
WebRTC-1.0-Spec-Compliance


Sign in to add a comment

MediaStream.clone() is not copying audio tracks after default device is changed

Reported by rrowl...@twilio.com, Oct 2 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Steps to reproduce the problem:
1. Set a USB mic as your default input device.
2. Call getUserMedia specifying `default` deviceId, save the MediaStream.
3. Unplug the USB mic (as another variation, plug back in after this)
4. Call MediaStream.clone() on the saved stream
5. Call .getAudioTracks() or .getTracks() on the cloned stream, verify the array is empty.

This logic is set up in this fiddle: https://jsfiddle.net/7gq8uvn1/4/

What is the expected behavior?
The audio track should be copied over to the cloned stream.

What went wrong?
The audio track is not copied over.

Did this work before? Yes Chrome 60

Does this work in other browsers? Yes

Chrome version: 61.0.3163.100  Channel: stable
OS Version: OS X 10.12.6
Flash Version: 

Customer reports that the issue caused by this was not present prior to Chrome 61 -- Therefore this seems like a regression in 61.
 
Labels: Needs-Bisect Needs-Triage-M61
Cc: pnangunoori@chromium.org
Labels: Needs-Feedback
Tested on latest Chrome Stable #61.0.3163.100, Canary # 63.0.3231.0, previous Chrome #59.0.3071.115 on Mac 10.12.6 on the provided test link https://jsfiddle.net/7gq8uvn1/4/ and no results were observed on the results pane.

Steps followed:
1. Changed the default input type to USB.
2. Run the test URL https://jsfiddle.net/7gq8uvn1/4/.
3. Allow the Mic to access.
4. Recording seems to happen.
5. Even if the mic is unplugged or plugged during the voice record or after the voice record audio file seems not copied nor any results are observed on the results pane on provided test URL.

Note: Same behavior is observed on FireFox for the test URL provided.

@rrowland -- Could you please let us know if we have missed anything from the above steps and screencast. Or provide the expected output on the test URL https://jsfiddle.net/7gq8uvn1/4/.

Please refer the screencast in the link provided below:
https://drive.google.com/a/google.com/file/d/0B91MtOa4hcAbeTlTdm5MOU1MOUk/view?usp=sharing

Thanks in advance!
@pnangoori Sorry for the confusion, the output is pushed to console rather than the output window.

https://i.imgur.com/rBMBFDJ.png
The image above shows how the output should look. This is how it looks if the USB headset is never disconnected. Line by line:
- It iterates over each audio track on the stream and prints track info
- It allows a few seconds to remove/replace the USB microphone
- It iterates over each audio track on the stream again, printing track info
- It clones the stream, and iterates over each audio track on the fresh cloned stream

As you can see in the above screenshot, the same audio track is printed in all 3 cases.

Here's a screenshot of the console output in the case where the USB microphone is removed or replaced during the "OK, unplug your USB headset..." timeout:
https://i.imgur.com/UktnZ66.png

You'll notice that the audio track is present on the original stream before the USB headset is unplugged, on the original stream after the USB is unplugged, but not present on the freshly cloned stream.

Note that the USB mic/headset needs to be set as the system's default device before running the script. It seems this behavior is triggered by the default device changing.
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 3 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "pnangunoori@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: kkaluri@chromium.org
Labels: Needs-Feedback
Unable to reproduce this issue on Mac 10.12.6 with chrome Stable ##61.0.3163.100, Canary # 63.0.3231.0 on the provided test url.

Steps Followed:
1. Changed the default input type to USB.
2. Run the test URL https://jsfiddle.net/7gq8uvn1/4/.
3. Allow the Mic to access.
4. Recording seems to happen.

Observed the console output is similar to the https://i.imgur.com/rBMBFDJ.png (from comment #2)

Attaching the screen-cast for reference.

@rrowland -- Could you please look into it and let us know if we have missed anything from the above steps and screencast.
770908.mp4
1.8 MB View Download
@kkaluri

A couple things I noticed from the video and your steps followed:
1) It looks like you're setting your default output device to the USB headset. This issue involves the input device (the microphone) so you will need to ensure your default input device is the USB headset.
2) It doesn't seem like you ever removed the USB headset from the USB port.

Please look again at the repro steps in the original ticket:

"Steps to reproduce the problem:
1. Set a USB mic as your default input device.
2. Call getUserMedia specifying `default` deviceId, save the MediaStream.
3. Unplug the USB mic (as another variation, plug back in after this)
4. Call MediaStream.clone() on the saved stream
5. Call .getAudioTracks() or .getTracks() on the cloned stream, verify the array is empty."

To translate specifically for this jsfiddle:

1. Set a USB mic as your default input device.
2. Load the script at https://jsfiddle.net/7gq8uvn1/4/
3. Open chrome debugging tools to the console
4. Run the script
5. When the "OK, unplug your USB headset..." log appears, unplug your USB headset.
6. Verify that no tracks are printed to console when iterating over the cloned stream's audio tracks.

If the meaning of the console output is unclear, please refer to my previous response in which I have attached screenshots of the expected and actual outputs, accompanied by a detailed explanation of what's happening and what should be happening.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 4 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "kkaluri@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: guidou@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: TE-NeedsTriageHelp
Unable to reproduce this issue from TE-End, hence adding TE-NeedsTriageHelp for further triage from dev team.

Requesting "MediaStream" dev team to look into this issue.
I can reproduce and was able to bisect the issue.
The MediaStream cloning code apparently never intended to clone ended tracks, and pre-61 versions were cloning them due to a bug (the cloned track incorrectly has live state in those versions).
The bug was fixed in 61 and now cloning works as originally intended. This is nevertheless a bug, because ended tracks should be cloned as well according to spec.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e2320cef7449a57ee0d62cd95318836bf65ecdd4

commit e2320cef7449a57ee0d62cd95318836bf65ecdd4
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Fri Oct 06 12:14:57 2017

Revert "Include ended tracks when cloning/constructing MediaStreams"

This reverts commit 5342a1c2607885ce35edff4c713cd51c197e87d1.

Speculative reason for revert: MediaStream-idl.https.html is failing [1] and this is the only CL in the regression range that changes MediaStream.

[1] https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20%28Nexus4%29/builds/70104

Original change's description:
> Include ended tracks when cloning/constructing MediaStreams
> 
> Bug:  770908 
> Change-Id: If9537579fd14b5fe88b7a084e7e941646d5f81f2
> Reviewed-on: https://chromium-review.googlesource.com/702265
> Commit-Queue: Harald Alvestrand <hta@chromium.org>
> Reviewed-by: Harald Alvestrand <hta@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#507011}

TBR=hta@chromium.org,guidou@chromium.org

Change-Id: Ic8b553744c9ac2bad62f425fb6ad558f3f003fb4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  770908 
Reviewed-on: https://chromium-review.googlesource.com/704894
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507036}
[add] https://crrev.com/e2320cef7449a57ee0d62cd95318836bf65ecdd4/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStream-idl.https-expected.txt
[modify] https://crrev.com/e2320cef7449a57ee0d62cd95318836bf65ecdd4/third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-clone-expected.txt
[modify] https://crrev.com/e2320cef7449a57ee0d62cd95318836bf65ecdd4/third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-clone.html
[modify] https://crrev.com/e2320cef7449a57ee0d62cd95318836bf65ecdd4/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamConstructor.html
[modify] https://crrev.com/e2320cef7449a57ee0d62cd95318836bf65ecdd4/third_party/WebKit/Source/modules/mediastream/MediaStream.cpp

Comment 13 by ajha@chromium.org, Nov 21 2017

Labels: -Needs-Bisect
Removing the Needs-Bisect as per C#10.
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cac679fd8583721a04323b18b2fe0772f002a23

commit 9cac679fd8583721a04323b18b2fe0772f002a23
Author: Guido Urdaneta <guidou@chromium.org>
Date: Wed Oct 03 15:00:44 2018

Remove incorrect DCHECK from MediaStreamVideoSource.

When the last track from a source is removed, the source is
asynchronously stopped. Between the request to stop and the reception of
a notification about actual stop it is possible that a new track is
added to the source (e.g., by cloning the track being removed).

There was a DCHECK asserting that there could be no tracks when the stop
notification is received, but as the example above shows, this is not
necessarily the case. No further changes are required since the newly
added track will start out ended due to being connected to a stopped source.

A test that fails with the old DCHECK is added to prevent similar
regressions.

Drive-by: Replace the |webkit_| prefix with |web_| on the test.

Bug:  770908 
Change-Id: I7ce01d3f0a2a5f77e161c88eb58c5334fad0aa3b
Reviewed-on: https://chromium-review.googlesource.com/c/1258002
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596212}
[modify] https://crrev.com/9cac679fd8583721a04323b18b2fe0772f002a23/content/renderer/media/stream/media_stream_video_source.cc
[modify] https://crrev.com/9cac679fd8583721a04323b18b2fe0772f002a23/content/renderer/media/stream/media_stream_video_source.h
[modify] https://crrev.com/9cac679fd8583721a04323b18b2fe0772f002a23/content/renderer/media/stream/media_stream_video_source_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b33058c7dc94ae14c629b1d8895782408cbf4301

commit b33058c7dc94ae14c629b1d8895782408cbf4301
Author: Guido Urdaneta <guidou@chromium.org>
Date: Wed Oct 03 16:17:43 2018

Include ended tracks when cloning MediaStreams.

This CL also covers the MediaStream constructor.
In addition, tests are moved to WPT.

Bug:  770908 
Change-Id: I6db9a86e9ee839efc2b04b00ae8fc57f07d852c3
Reviewed-on: https://chromium-review.googlesource.com/c/1256796
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596247}
[add] https://crrev.com/b33058c7dc94ae14c629b1d8895782408cbf4301/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStream-clone.https.html
[delete] https://crrev.com/1880a97b3a2021e3848e3d791b31836ed3c64f45/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStream-idl.https-expected.txt
[modify] https://crrev.com/b33058c7dc94ae14c629b1d8895782408cbf4301/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStream-idl.https.html
[delete] https://crrev.com/1880a97b3a2021e3848e3d791b31836ed3c64f45/third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-clone.html
[delete] https://crrev.com/1880a97b3a2021e3848e3d791b31836ed3c64f45/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamConstructor.html
[modify] https://crrev.com/b33058c7dc94ae14c629b1d8895782408cbf4301/third_party/blink/renderer/modules/mediastream/media_stream.cc

Status: Fixed (was: Assigned)
Cc: guidou@chromium.org
 Issue 768258  has been merged into this issue.

Sign in to add a comment