New issue
Advanced search Search tips

Issue 704918 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task

Blocking:
issue 682245



Sign in to add a comment

getUserMedia() needs to return after track is initialized

Project Member Reported by hta@chromium.org, Mar 24 2017

Issue description

Some decisions on audio track configuration (echo canceller) are taken after getUserMedia() returns. This is an issue for getSettings().

The right solution is to wait until initialization is finished before returning from getUserMedia.

This is a blocker for getSettings() returning the correct value of echoCancellation (off or on).

 

Comment 1 by hta@chromium.org, Mar 24 2017

Components: Blink>WebRTC
Labels: -Type-Bug OS-All Type-Task

Comment 2 by hta@chromium.org, Mar 24 2017

Blocking: 682245

Comment 3 by hta@chromium.org, Apr 5 2017

Owner: guidou@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 6 2017

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

commit a52d69d1877b170684041bed04d28acced85e40d
Author: hta <hta@chromium.org>
Date: Thu Apr 06 17:18:29 2017

Move getUserMedia finish to "when audio track configured".

Previously, getUserMedia() would complete as soon as the sources
were initialized - which did not do all the initialization.
This CL will make getUserMedia() return when the track has been
told of its configuration through OnSetFormat() from upstream.

This will allow GetSettings() to read the audio configuration from
the track without worrying about whether the configuration is
propagated or not.

BUG= 704918 

Review-Url: https://codereview.chromium.org/2777583002
Cr-Commit-Position: refs/heads/master@{#462523}

[modify] https://crrev.com/a52d69d1877b170684041bed04d28acced85e40d/content/renderer/media/media_stream_audio_source.h
[modify] https://crrev.com/a52d69d1877b170684041bed04d28acced85e40d/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/a52d69d1877b170684041bed04d28acced85e40d/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/a52d69d1877b170684041bed04d28acced85e40d/content/renderer/media/media_stream_audio_unittest.cc
[modify] https://crrev.com/a52d69d1877b170684041bed04d28acced85e40d/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/a52d69d1877b170684041bed04d28acced85e40d/content/renderer/media/user_media_client_impl_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 7 2017

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

commit 35aeae1c6f63abb9a4cbb23f497a523349db7aa8
Author: foolip <foolip@chromium.org>
Date: Fri Apr 07 06:44:56 2017

Revert of Move getUserMedia finish to "when audio track configured". (patchset #7 id:120001 of https://codereview.chromium.org/2777583002/ )

Reason for revert:
The added DCHECK(format_is_set()) is failing in MediaStreamTrack-getSettings.html

BUG= 709188 

Original issue's description:
> Move getUserMedia finish to "when audio track configured".
>
> Previously, getUserMedia() would complete as soon as the sources
> were initialized - which did not do all the initialization.
> This CL will make getUserMedia() return when the track has been
> told of its configuration through OnSetFormat() from upstream.
>
> This will allow GetSettings() to read the audio configuration from
> the track without worrying about whether the configuration is
> propagated or not.
>
> BUG= 704918 
>
> Review-Url: https://codereview.chromium.org/2777583002
> Cr-Commit-Position: refs/heads/master@{#462523}
> Committed: https://chromium.googlesource.com/chromium/src/+/a52d69d1877b170684041bed04d28acced85e40d

TBR=guidou@chromium.org,hta@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 704918 

Review-Url: https://codereview.chromium.org/2803183002
Cr-Commit-Position: refs/heads/master@{#462797}

[modify] https://crrev.com/35aeae1c6f63abb9a4cbb23f497a523349db7aa8/content/renderer/media/media_stream_audio_source.h
[modify] https://crrev.com/35aeae1c6f63abb9a4cbb23f497a523349db7aa8/content/renderer/media/media_stream_audio_track.cc
[modify] https://crrev.com/35aeae1c6f63abb9a4cbb23f497a523349db7aa8/content/renderer/media/media_stream_audio_track.h
[modify] https://crrev.com/35aeae1c6f63abb9a4cbb23f497a523349db7aa8/content/renderer/media/media_stream_audio_unittest.cc
[modify] https://crrev.com/35aeae1c6f63abb9a4cbb23f497a523349db7aa8/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/35aeae1c6f63abb9a4cbb23f497a523349db7aa8/content/renderer/media/user_media_client_impl_unittest.cc

Comment 6 by guidou@chromium.org, Jun 16 2017

Status: WontFix (was: Started)
This was intended for getSettings, but getSettings can be implemented without this.

Sign in to add a comment