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

Issue 722335 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocked on:
issue 725819



Sign in to add a comment

Fire "ended" event when mic is receiving no signal

Project Member Reported by solenberg@chromium.org, May 15 2017

Issue description

If we get no callbacks with input audio for a certain time, we should close the mic and fire the "ended" event so that applications can know what goes on and possibly inform users the input isn't working correctly.
 
Labels: -Pri-3 M-60 OS-All Pri-2
Status: Assigned (was: Untriaged)
Cc: tlegrand@chromium.org

Comment 3 Deleted

Proof of concept that works: https://codereview.chromium.org/2888383002/

The detection is in ProcessedLocalAudioSource on the renderer side, and StopSourceOnError() is called which will set the readyState to ended and fire the event.

The location for detection hasn't been decided yet, but the renderer side is likely a good choice. For WebRTC, also LocalMediaStreamAudioSource can be used if no processing is required (based on constraints). That class is also used by tab and screen capture were we don't want this. Detection could be done further down the stack, e.g. AudioInputDevice.
Cc: dalecur...@chromium.org m...@chromium.org
The CL is now changed to do the detection in the AudioInputDevice; more on that in the CL description. It's under review.
I picked the detection hangover time to 20 seconds. That's based on that it will take 18 seconds for the Mac implementation[1] to detect, restart, detect again, and then log and report stats. We want to make sure we don't stop before that. Those times can maybe be shorter.

Does 20 seconds before notifying make sense in a user perspective? My hunch is that it could be a bit too long. How many users would give up and close a call quicker than that?

[1] https://cs.chromium.org/chromium/src/media/audio/mac/audio_low_latency_input_mac.cc

Comment 7 Deleted

Blockedon: 725819
Labels: -Type-Bug Type-Feature
 Issue 549021  is the related bug for Mac input audio.
Project Member

Comment 11 by bugdroid1@chromium.org, May 24 2017

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

commit dc44616c932d390c6ee11c2eb5931d51e7df91a7
Author: grunell <grunell@chromium.org>
Date: Wed May 24 19:12:05 2017

Stop source and fire MediaStreamTrack ended event if missing audio input callbacks are detected.

This way the javascript application is informed when there's a problem with the input device. Since the detection is in the renderer process we would also catch any other issues in the pipeline, including on the process border (i.e. the socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so that other components down the stack have a chance to defer start if needed and report startup success stats. Specifically the Mac implementation. See also code comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with https://codereview.chromium.org/2891303002/ and using a WebRTC test page with the MediaStreamTrack.onended set to log that the event was fired.

BUG= 722335 
TEST=See above.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/dc44616c932d390c6ee11c2eb5931d51e7df91a7/content/renderer/media/local_media_stream_audio_source.cc
[modify] https://crrev.com/dc44616c932d390c6ee11c2eb5931d51e7df91a7/media/audio/audio_input_device.cc
[modify] https://crrev.com/dc44616c932d390c6ee11c2eb5931d51e7df91a7/media/audio/audio_input_device.h
[modify] https://crrev.com/dc44616c932d390c6ee11c2eb5931d51e7df91a7/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/dc44616c932d390c6ee11c2eb5931d51e7df91a7/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
Status: Started (was: Fixed)
CL was reverted. Re-land CL (with small fix) is in CQ.
Project Member

Comment 14 by bugdroid1@chromium.org, May 25 2017

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

commit 021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c
Author: grunell <grunell@chromium.org>
Date: Thu May 25 11:30:27 2017

Re-land: Stop source and fire MediaStreamTrack ended event if missing audio input callbacks are detected.

This way the javascript application is informed when there's a problem with the input device. Since the detection is in the renderer process we would also catch any other issues in the pipeline, including on the process border (i.e. the socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so that other components down the stack have a chance to defer start if needed and report startup success stats. Specifically the Mac implementation. See also code comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with https://codereview.chromium.org/2891303002/ and using a WebRTC test page with the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001 (http://crrev.com/2888383002#ps300001)

BUG= 722335 
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c/content/renderer/media/local_media_stream_audio_source.cc
[modify] https://crrev.com/021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c/media/audio/audio_input_device.cc
[modify] https://crrev.com/021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c/media/audio/audio_input_device.h
[modify] https://crrev.com/021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)
The reland has now passed all Chromium WebRTC bots, except the Mac Tester which is offline (filed issue 726311). The change has been tested by the FYI Mac Tester though.
See also b/32775860.
Labels: -Restrict-View-Google

Sign in to add a comment