New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 778039 link

Starred by 1 user
Status: Fixed
Owner:
guidou@chromium.org
Closed: Nov 2017
Cc:
emir...@chromium.org
█ mylesj@google.com
█ saeedj@google.com
chfremer@chromium.org
█ saeedj@chromium.org
guidou@chromium.org
niklase@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Intermittent TrackStartError when opening camera

Project Member Reported by chfremer@chromium.org, Oct 24 2017 
Chrome Version: Tested on developer build of M64 on 10/24/2017, but the issue seems to be already present in M62 stable and potentially in older versions as well.

What steps will reproduce the problem?
(1) Navigate to https://jsfiddle.net/5z5c7vxm/3/
(2) Click start
(3) Observe the output. 

What is the expected result?
All 100 iterations showing "Success"

What happens instead?
Occasionally an iteration shows "FAILED: TrackStartError"

Comment 1 by chfremer@chromium.org, Oct 24 2017 

Running this on a build with debug checks enabled, it appears like the TrackStartError cases correspond to a DCHECK being hit, which produces the following stack trace:

[1:7:1024/135147.994608:FATAL:binding_state.cc(90)] Check failed: !router_. 
#0 0x00b3f23cca97 base::debug::StackTrace::StackTrace()
#1 0x00b3f23eab21 logging::LogMessage::~LogMessage()
#2 0x00b3f2fa604a mojo::internal::BindingStateBase::BindInternal()
#3 0x00b3f67ceae8 mojo::internal::BindingState<>::Bind()
#4 0x00b3f67cba6c content::VideoCaptureImpl::StartCaptureInternal()
#5 0x00b3f67cb919 content::VideoCaptureImpl::StartCapture()
#6 0x00b3f23cdc31 base::debug::TaskAnnotator::RunTask()
#7 0x00b3f23f3240 base::internal::IncomingTaskQueue::RunTask()
#8 0x00b3f23f186f base::MessageLoop::RunTask()
#9 0x00b3f23f1bb5 base::MessageLoop::DeferOrRunPendingTask()
#10 0x00b3f23f1e66 base::MessageLoop::DoWork()
#11 0x00b3f23f6c19 base::MessagePumpLibevent::Run()
#12 0x00b3f23f11ea base::MessageLoop::Run()
#13 0x00b3f241e8e6 base::RunLoop::Run()
#14 0x00b3f245531c base::Thread::Run()
#15 0x00b3f24558c2 base::Thread::ThreadMain()
#16 0x00b3f244df0c base::(anonymous namespace)::ThreadFunc()
#17 0x7f0183217184 start_thread
#18 0x7f017d22dffd clone

Comment 2 by chfremer@chromium.org, Oct 24 2017 

A quick extra note: I sometimes also get a TrackStartError that does NOT trigger the DCHECK condition.

Comment 3 by saeedj@google.com, Oct 25 2017

Cc: saeedj@google.com

Comment 4 by mylesj@google.com, Oct 25 2017

Cc: mylesj@google.com

Comment 5 by chfremer@chromium.org, Oct 27 2017 

Here is some results from my analysis:

First, the DCHECK that is being hit is an unrelated small bug. It is not the cause for the TrackStartErrors. I am probably going to file a separate bug entry for it.

Second, here is the sequence of events that triggers the TrackStartError event:
1. [Iteration N] Renderer asks Browser to open the device.
2. [Iteration N] Browser opens the device and assigns a session_id, then returns
   the session_id to the Renderer.
4. [Iteration N] Renderer asks Brpwser to start capturing for the session.
5. [Iteration N+1] Renderer asks Browser to open the same device again.
6. [Iteration N+1] Browser finds that device is already open and returns the 
   existing session_id to the Renderer.
7. [Iteration N] Renderer asks Browser to stop the session
8. [Iteration N] Browser stops the session and deletes it, because no more 
   client is connected to it. It sends a STOPPED event back to the Renderer.
9. [Iteration N+1] Renderer asks Browser to start capturing for the session.
10. [Iteration N+1] Browser finds that the session no longer exists and replies 
   with a FAILED event.

The issue here is that getUserMedia is not atomic, and in the bad sequence above, a stop event gets scheduled and executed in the middle of processing a getUserMedia request. I am not sure what the spec says about it, but it feels like the implementation should probably make sure that this does not happen.

Comment 6 by chfremer@chromium.org, Oct 27 2017 

With the understanding from #5, I was able to simplify the repro script to the following:

https://jsfiddle.net/1ogvzacv/4/

Note the timeout of 4 milliseconds in Line 11. This value may have to be slightly tuned depending on the machine. If set too low, the stop events will always be sent before the subsequent getUserMedia call is processed and things will work fine. If set too high, it will almost always reuse the existing session and quickly sweep through all 100 iterations.

Comment 7 by chfremer@chromium.org, Oct 27 2017 

With the understanding from #5, I was able to simplify the repro script to the following:

https://jsfiddle.net/1ogvzacv/4/

Note the timeout of 4 milliseconds in Line 11. This value may have to be slightly tuned depending on the machine. If set too low, the stop events will always be sent before the subsequent getUserMedia call is processed and things will work fine. If set too high, it will almost always reuse the existing session and quickly sweep through all 100 iterations.

Comment 8 by chfremer@chromium.org, Oct 27 2017

Cc: guidou@chromium.org
With the improved repro, I did a bisect to see where this symptom started happening.

You are probably looking for a change made after 458553 (known good), but no later than 458561 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/0116ce334c021757db311c867d96bb56faeda1b7..203431000fbc3145c7732a85cadd348378bc7d24

CL where this repro starts triggering:
https://chromium.googlesource.com/chromium/src/+/d727ed27c3dde3c85195625b97e79472eda73508

First landed in 59.0.3048.0

Please note that the fact that this CL starts triggering this particular repro  does not necessarily mean that the same issue did not already exist before. It could have existed before but in different form requiring a different repro. More analysis would be needed to understand this better.

guidou@: Please let me know if you have any ideas or insights.

Comment 9 by guidou@chromium.org, Oct 27 2017 

I'll take a look. I can reproduce semi-reliably.

Comment 10 by guidou@chromium.org, Oct 30 2017

Cc: -guidou@chromium.org chfremer@chromium.org
Owner: guidou@chromium.org
The issue is that stop requests should be serialized together with getUserMedia() and applyConstraints() requests, and possibly others.
I'll try to prepare a patch supporting that.

Comment 11 by guidou@chromium.org, Oct 30 2017 

After closer inspection, I think serializing gUM, applyConstraints and stop() is enough, because they are the only ones that can result in the track's source being stopped started or restarted.

Comment 12 by guidou@chromium.org, Nov 2 2017 

Actually, a better solution is add use counts to the implementations of MediaStreamProvider (AudioInputStreamManager and VideoCaptureManager) in the browser, such that when a device is requested to be opened twice, it gets closed only after two close requests.

Comment 13 by guidou@chromium.org, Nov 2 2017 

I tried to implement the use count, but it looks like its more involved than I thought. Serializing gUM and stop looks like the way to go.

Comment 14 by guidou@chromium.org, Nov 2 2017

Status: Assigned (was: Available)
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 3 2017 

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

commit cb4b12350cf62a4934c010591f3ae51be723d23a
Author: Guido Urdaneta <guidou@chromium.org>
Date: Fri Nov 03 20:35:01 2017

Remove dead code supporting MediaStream.stop().

This method was removed long ago from the Web platform, but the
supporting code was still there.

Bug:  778039 
Change-Id: I6c51597acf70ac30f310880f83545f03afbc8939
Reviewed-on: https://chromium-review.googlesource.com/743601
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513899}
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/content/renderer/media/media_stream_center.cc
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/content/renderer/media/media_stream_center.h
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/content/shell/test_runner/mock_web_media_stream_center.cc
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/content/shell/test_runner/mock_web_media_stream_center.h
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.cpp
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.h
[modify] https://crrev.com/cb4b12350cf62a4934c010591f3ae51be723d23a/third_party/WebKit/public/platform/WebMediaStreamCenter.h

Comment 16 by guidou@chromium.org, Nov 7 2017

Labels: -Pri-2 Pri-1

Comment 17 by guidou@chromium.org, Nov 7 2017

Labels: M-64
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 10 2017 

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

commit a58c8bd608d040102d374322b48aa9055d8fcdbe
Author: Guido Urdaneta <guidou@chromium.org>
Date: Fri Nov 10 07:14:19 2017

Serialize MediaStreamTrack.stop() with getUserMedia()

When multiple getUserMedia and stop() requests are active concurrently, 
getUserMedia() sometimes fails with TrackStartError.
The reason is that the underlying video capture subsystem does not keep
a count of how many sources have opened a video device.
It can happen that a stop request stops video capture while a 
getUserMedia() request is being processed, with the result being that
the video device is closed before getUserMedia() starts the new video
track, resulting in an error.

One solution to this problem is to make sure that stop requests and
getUserMedia() requests cannot run concurrently, which is the approach
followed by this CL.

This CL basically moves handling of MediaStramTrack.stop() from 
MediaStreamCenter to UserMediaClient, and puts stop() requests in the
same queue used for getUserMedia() and applyConstraints() requests.


Bug:  778039 
Change-Id: Id7e0ee38e40265fd599dc6dd0712f0b667d2e1a5
Reviewed-on: https://chromium-review.googlesource.com/751981
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515494}
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/browser/webrtc/webrtc_getusermedia_browsertest.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_center.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_center.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_source.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_source.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_track.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_video_capturer_source_unittest.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_video_source.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_video_source_unittest.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/media_stream_video_track.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/content/test/data/media/getusermedia.html
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediastream/UserMediaClient.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/modules/mediastream/UserMediaController.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.cpp
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/public/platform/WebMediaStreamCenter.h
[modify] https://crrev.com/a58c8bd608d040102d374322b48aa9055d8fcdbe/third_party/WebKit/public/web/WebUserMediaClient.h
Project Member

Comment 19 by bugdroid1@chromium.org, Nov 10 2017 

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

commit 5d611033318997f8c55b954d32ba335000fe8a3b
Author: Per Kjellander <perkj@chromium.org>
Date: Fri Nov 10 11:55:49 2017

Revert "Serialize MediaStreamTrack.stop() with getUserMedia()"

This reverts commit a58c8bd608d040102d374322b48aa9055d8fcdbe.

Reason for revert: Tentative rollback to see if this caused problems with WebRtcAudioDebugRecordingsBrowserTest.CallWithAudioDebugRecordings on Windows 8 and 10.
https://build.chromium.org/p/chromium.webrtc/builders/Win10%20Tester/builds/21971
 

Original change's description:
> Serialize MediaStreamTrack.stop() with getUserMedia()
> 
> When multiple getUserMedia and stop() requests are active concurrently, 
> getUserMedia() sometimes fails with TrackStartError.
> The reason is that the underlying video capture subsystem does not keep
> a count of how many sources have opened a video device.
> It can happen that a stop request stops video capture while a 
> getUserMedia() request is being processed, with the result being that
> the video device is closed before getUserMedia() starts the new video
> track, resulting in an error.
> 
> One solution to this problem is to make sure that stop requests and
> getUserMedia() requests cannot run concurrently, which is the approach
> followed by this CL.
> 
> This CL basically moves handling of MediaStramTrack.stop() from 
> MediaStreamCenter to UserMediaClient, and puts stop() requests in the
> same queue used for getUserMedia() and applyConstraints() requests.
> 
> 
> Bug:  778039 
> Change-Id: Id7e0ee38e40265fd599dc6dd0712f0b667d2e1a5
> Reviewed-on: https://chromium-review.googlesource.com/751981
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515494}

TBR=haraken@chromium.org,hbos@chromium.org,guidou@chromium.org

Change-Id: I93abb79d9cd3abfe08d9c0473e8bcf0207db7b3a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  778039 
Reviewed-on: https://chromium-review.googlesource.com/763487
Reviewed-by: Per Kjellander <perkj@chromium.org>
Commit-Queue: Per Kjellander <perkj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515529}
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/browser/webrtc/webrtc_getusermedia_browsertest.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_center.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_center.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_source.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_source.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_track.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_video_capturer_source_unittest.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_video_source.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_video_source_unittest.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/media_stream_video_track.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/content/test/data/media/getusermedia.html
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediastream/UserMediaClient.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/modules/mediastream/UserMediaController.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.cpp
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/public/platform/WebMediaStreamCenter.h
[modify] https://crrev.com/5d611033318997f8c55b954d32ba335000fe8a3b/third_party/WebKit/public/web/WebUserMediaClient.h
Project Member

Comment 20 by bugdroid1@chromium.org, Nov 10 2017 

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

commit a7dc3ad68cf2fb0c565e69d0f6ed1a82fac6a170
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Nov 10 12:44:51 2017

Revert "Serialize MediaStreamTrack.stop() with getUserMedia()"

This reverts commit a58c8bd608d040102d374322b48aa9055d8fcdbe.

Reason for revert: I suspect that this CL is the cause of flake in the imagecapture/MediaStreamTrack* layout tests. Examples:

https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/64556
https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/64555

Original change's description:
> Serialize MediaStreamTrack.stop() with getUserMedia()
> 
> When multiple getUserMedia and stop() requests are active concurrently, 
> getUserMedia() sometimes fails with TrackStartError.
> The reason is that the underlying video capture subsystem does not keep
> a count of how many sources have opened a video device.
> It can happen that a stop request stops video capture while a 
> getUserMedia() request is being processed, with the result being that
> the video device is closed before getUserMedia() starts the new video
> track, resulting in an error.
> 
> One solution to this problem is to make sure that stop requests and
> getUserMedia() requests cannot run concurrently, which is the approach
> followed by this CL.
> 
> This CL basically moves handling of MediaStramTrack.stop() from 
> MediaStreamCenter to UserMediaClient, and puts stop() requests in the
> same queue used for getUserMedia() and applyConstraints() requests.
> 
> 
> Bug:  778039 
> Change-Id: Id7e0ee38e40265fd599dc6dd0712f0b667d2e1a5
> Reviewed-on: https://chromium-review.googlesource.com/751981
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515494}

TBR=haraken@chromium.org,hbos@chromium.org,guidou@chromium.org

Change-Id: Ib586c26923e635b420732caf6515d404e54b66f2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  778039 
Reviewed-on: https://chromium-review.googlesource.com/763310
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515536}
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 11 2017 

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

commit 7c8e8e13ce6cc2d04469a2503244151b8842775e
Author: Guido Urdaneta <guidou@chromium.org>
Date: Sat Nov 11 08:48:59 2017

Reland "Serialize MediaStreamTrack.stop() with getUserMedia()"

This is a reland of a58c8bd608d040102d374322b48aa9055d8fcdbe
Original change's description:
> Serialize MediaStreamTrack.stop() with getUserMedia()
>
> When multiple getUserMedia and stop() requests are active concurrently,
> getUserMedia() sometimes fails with TrackStartError.
> The reason is that the underlying video capture subsystem does not keep
> a count of how many sources have opened a video device.
> It can happen that a stop request stops video capture while a
> getUserMedia() request is being processed, with the result being that
> the video device is closed before getUserMedia() starts the new video
> track, resulting in an error.
>
> One solution to this problem is to make sure that stop requests and
> getUserMedia() requests cannot run concurrently, which is the approach
> followed by this CL.
>
> This CL basically moves handling of MediaStramTrack.stop() from
> MediaStreamCenter to UserMediaClient, and puts stop() requests in the
> same queue used for getUserMedia() and applyConstraints() requests.
>
>
> Bug:  778039 
> Change-Id: Id7e0ee38e40265fd599dc6dd0712f0b667d2e1a5
> Reviewed-on: https://chromium-review.googlesource.com/751981
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515494}

TBR=haraken@chromium.org

Bug:  778039 
Change-Id: I461e3c98f3d6471cc218d48501d22c6c0a8b03e9
Reviewed-on: https://chromium-review.googlesource.com/763636
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515836}
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/browser/webrtc/webrtc_getusermedia_browsertest.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_center.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_center.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_source.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_source.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_track.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_video_capturer_source_unittest.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_video_source.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_video_source_unittest.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/media_stream_video_track.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/content/test/data/media/getusermedia.html
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediastream/UserMediaClient.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/modules/mediastream/UserMediaController.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.cpp
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/public/platform/WebMediaStreamCenter.h
[modify] https://crrev.com/7c8e8e13ce6cc2d04469a2503244151b8842775e/third_party/WebKit/public/web/WebUserMediaClient.h
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 11 2017 

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

commit b5107cb6171db8988059305f642b28565d7518c0
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Sat Nov 11 18:13:58 2017

Revert "Reland "Serialize MediaStreamTrack.stop() with getUserMedia()""

This reverts commit 7c8e8e13ce6cc2d04469a2503244151b8842775e.

Reason for revert: makes TabCaptureApiTest.ApiTests consistently fail on Linux MSan. See https://build.chromium.org/p/chromium.memory/builders/Linux%20MSan%20Tests/builds/5790

Original change's description:
> Reland "Serialize MediaStreamTrack.stop() with getUserMedia()"
> 
> This is a reland of a58c8bd608d040102d374322b48aa9055d8fcdbe
> Original change's description:
> > Serialize MediaStreamTrack.stop() with getUserMedia()
> >
> > When multiple getUserMedia and stop() requests are active concurrently,
> > getUserMedia() sometimes fails with TrackStartError.
> > The reason is that the underlying video capture subsystem does not keep
> > a count of how many sources have opened a video device.
> > It can happen that a stop request stops video capture while a
> > getUserMedia() request is being processed, with the result being that
> > the video device is closed before getUserMedia() starts the new video
> > track, resulting in an error.
> >
> > One solution to this problem is to make sure that stop requests and
> > getUserMedia() requests cannot run concurrently, which is the approach
> > followed by this CL.
> >
> > This CL basically moves handling of MediaStramTrack.stop() from
> > MediaStreamCenter to UserMediaClient, and puts stop() requests in the
> > same queue used for getUserMedia() and applyConstraints() requests.
> >
> >
> > Bug:  778039 
> > Change-Id: Id7e0ee38e40265fd599dc6dd0712f0b667d2e1a5
> > Reviewed-on: https://chromium-review.googlesource.com/751981
> > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > Reviewed-by: Henrik Boström <hbos@chromium.org>
> > Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#515494}
> 
> TBR=haraken@chromium.org
> 
> Bug:  778039 
> Change-Id: I461e3c98f3d6471cc218d48501d22c6c0a8b03e9
> Reviewed-on: https://chromium-review.googlesource.com/763636
> Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515836}

TBR=haraken@chromium.org,hbos@chromium.org,guidou@chromium.org

Change-Id: I308ecf1a9a3b031bfa0d2b0f4f22967e7758ad43
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  778039 
Reviewed-on: https://chromium-review.googlesource.com/764632
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515848}
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/browser/webrtc/webrtc_getusermedia_browsertest.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_center.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_center.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_source.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_source.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_track.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_video_capturer_source_unittest.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_video_source.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_video_source_unittest.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/media_stream_video_track.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/content/test/data/media/getusermedia.html
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediastream/UserMediaClient.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/modules/mediastream/UserMediaController.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.cpp
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/public/platform/WebMediaStreamCenter.h
[modify] https://crrev.com/b5107cb6171db8988059305f642b28565d7518c0/third_party/WebKit/public/web/WebUserMediaClient.h
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 13 2017 

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

commit 26387bf1c7f788ab6617d00569a538637bf33deb
Author: Guido Urdaneta <guidou@chromium.org>
Date: Mon Nov 13 12:25:07 2017

Reland "Reland "Serialize MediaStreamTrack.stop() with getUserMedia()""

This CL runs requests on the same task where they are issued if possible,
which is more similar to the way MST.stop() used to run.
If this is not enough to make the failing racy test pass, the only solution
is to disable the test on the bots where it fails.

Reusing haraken@ lgtm for the Blink part.

TBR=haraken@chromium.org

This is a reland of 7c8e8e13ce6cc2d04469a2503244151b8842775e
Original change's description:
> Reland "Serialize MediaStreamTrack.stop() with getUserMedia()"
>
> This is a reland of a58c8bd608d040102d374322b48aa9055d8fcdbe
> Original change's description:
> > Serialize MediaStreamTrack.stop() with getUserMedia()
> >
> > When multiple getUserMedia and stop() requests are active concurrently,
> > getUserMedia() sometimes fails with TrackStartError.
> > The reason is that the underlying video capture subsystem does not keep
> > a count of how many sources have opened a video device.
> > It can happen that a stop request stops video capture while a
> > getUserMedia() request is being processed, with the result being that
> > the video device is closed before getUserMedia() starts the new video
> > track, resulting in an error.
> >
> > One solution to this problem is to make sure that stop requests and
> > getUserMedia() requests cannot run concurrently, which is the approach
> > followed by this CL.
> >
> > This CL basically moves handling of MediaStramTrack.stop() from
> > MediaStreamCenter to UserMediaClient, and puts stop() requests in the
> > same queue used for getUserMedia() and applyConstraints() requests.
> >
> >
> > Bug:  778039 
> > Change-Id: Id7e0ee38e40265fd599dc6dd0712f0b667d2e1a5
> > Reviewed-on: https://chromium-review.googlesource.com/751981
> > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > Reviewed-by: Henrik Boström <hbos@chromium.org>
> > Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#515494}
>
> TBR=haraken@chromium.org
>
> Bug:  778039 
> Change-Id: I461e3c98f3d6471cc218d48501d22c6c0a8b03e9
> Reviewed-on: https://chromium-review.googlesource.com/763636
> Commit-Queue: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515836}

Bug:  778039 
Change-Id: Idd2e1c95952b9c924037beaca987344093751009
Reviewed-on: https://chromium-review.googlesource.com/765657
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515932}
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/browser/webrtc/webrtc_getusermedia_browsertest.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_center.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_center.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_source.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_source.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_track.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_video_capturer_source_unittest.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_video_source.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_video_source_unittest.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/media_stream_video_track.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/content/test/data/media/getusermedia.html
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.cpp
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediastream/MediaStreamTrack.idl
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediastream/UserMediaClient.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/modules/mediastream/UserMediaController.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.cpp
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/Source/platform/mediastream/MediaStreamCenter.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/public/platform/WebMediaStreamCenter.h
[modify] https://crrev.com/26387bf1c7f788ab6617d00569a538637bf33deb/third_party/WebKit/public/web/WebUserMediaClient.h

Comment 24 by guidou@chromium.org, Nov 14 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment