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

Issue 890242 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

CRAS: setting audio input to BT headset from in-app settings causes record failure

Project Member Reported by hychao@chromium.org, Sep 28

Issue description

This is an issue happening on ToT. Discovered from an user report b/111882020

Repro steps:
 - Launch http://meet.google.com/_meet
 - Pair and connect BT headset HM1200
 - In Meet>Settings, microphone drop list. Select between "Default - HM1200" and "HM1200" options.

Result:
 - UI shows warning telling that your mic might have problem.

What happens to these steps is that changing microphone settings will create and close input streams repeatedly. And when close-then-reopen happens too soon, BT headset mic cannot handle that and will return device or resource busy error.

What is special in this test scenario is that Google Meet settings UI uses pinned stream to open input device.
And we didn't handle retry in the pinned stream path, like what we have for normal stream add/init:
https://chromium-review.googlesource.com/414854

To fix this, we should generalize the device init retry so it works for normal & pinned stream path.


 
Cc: kotah@chromium.org mpricone@chromium.org
Labels: Hotlist-Enterprise
Cc: cvintila@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/14327702091ee73622c2d54c70d8135233428465

commit 14327702091ee73622c2d54c70d8135233428465
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Wed Oct 10 16:44:56 2018

CRAS: iodev_list - Check if pinned device is open already

cras_iodev can be opened and accessed by audio_thread
in two ways:
(1) UI enables an input/output audio node.
(2) An application creates a stream that pins to specific audio node.

This change adds a function to ask if the given device is already
in use by the audio_thread. It can be used to avoid unecessary
calls and possible multi thread access to iodev.

BUG= chromium:890242 ,  chromium:863823 
TEST=unittest

Change-Id: I24713e15f1039599abd3387fbc1bbe2c490efff1
Reviewed-on: https://chromium-review.googlesource.com/1257487
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/14327702091ee73622c2d54c70d8135233428465/cras/src/server/audio_thread.h
[modify] https://crrev.com/14327702091ee73622c2d54c70d8135233428465/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/14327702091ee73622c2d54c70d8135233428465/cras/src/server/audio_thread.c
[modify] https://crrev.com/14327702091ee73622c2d54c70d8135233428465/cras/src/tests/iodev_list_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/0646381c7a9dd2bd52dea1e7b082f705681bf723

commit 0646381c7a9dd2bd52dea1e7b082f705681bf723
Author: Hsin-Yu Chao <hychao@google.com>
Date: Wed Oct 10 16:44:57 2018

CRAS: tests - Fix iodev_list_unittest

Fallback devices' state should be initialized to closed in each
test cases.
The incorrect iodev state misled a few test cases, fixing them.

BUG= chromium:890242 
TEST=Unittest

Change-Id: Ie3220e6d51432d9ec02b5fde0003d2e928e49074
Reviewed-on: https://chromium-review.googlesource.com/1257488
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/0646381c7a9dd2bd52dea1e7b082f705681bf723/cras/src/tests/iodev_list_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/380919c441afa207b0df11bb80e8fd2991881b6a

commit 380919c441afa207b0df11bb80e8fd2991881b6a
Author: Hsin-Yu Chao <hychao@google.com>
Date: Wed Oct 10 16:44:57 2018

CRAS: iodev_list - Generalize init retry for pinned stream

Iodev init retry is a mechanism important for hotplug headset types
like USB and BT. And it was implemented only for when audio device
is enabled from UI, not for when a stream pins to specific device.

Now Chrome browser provides API letting web app to select which audio
input/output nodes to use. This feature uses pinned stream, and is
suffering from device open issue when selecting to BT headset.
This change generalizes the iodev init retry to all paths that opens
an iodev, to stablize hotplug headset use cases.

BUG= chromium:890242 
TEST=Connect BT headset HM1100. Launch Google Meet and under
settings select input option to/from "Default - HM1100" and "HM1100"
verifiy UI doesn't show warning about mic.

Change-Id: Ic7289ffdc05d1f4cf4cca47488590c3dbf375247
Reviewed-on: https://chromium-review.googlesource.com/1257489
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/380919c441afa207b0df11bb80e8fd2991881b6a/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/380919c441afa207b0df11bb80e8fd2991881b6a/cras/src/tests/iodev_list_unittest.cc

Cc: geohsu@chromium.org
Labels: Merge-Request-70
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 11

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
note: this has been verified on R71-11145.0.0.
Labels: -Merge-Review-70 Merge-Approved-70
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 12

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/83addda994d654f37a3b9bfea2a6a149442eb8ae

commit 83addda994d654f37a3b9bfea2a6a149442eb8ae
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Fri Oct 12 07:31:19 2018

CRAS: iodev_list - Check if pinned device is open already

cras_iodev can be opened and accessed by audio_thread
in two ways:
(1) UI enables an input/output audio node.
(2) An application creates a stream that pins to specific audio node.

This change adds a function to ask if the given device is already
in use by the audio_thread. It can be used to avoid unecessary
calls and possible multi thread access to iodev.

BUG= chromium:890242 ,  chromium:863823 
TEST=unittest

Change-Id: I24713e15f1039599abd3387fbc1bbe2c490efff1
Reviewed-on: https://chromium-review.googlesource.com/1257487
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 14327702091ee73622c2d54c70d8135233428465)
Reviewed-on: https://chromium-review.googlesource.com/c/1278606
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/83addda994d654f37a3b9bfea2a6a149442eb8ae/cras/src/server/audio_thread.h
[modify] https://crrev.com/83addda994d654f37a3b9bfea2a6a149442eb8ae/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/83addda994d654f37a3b9bfea2a6a149442eb8ae/cras/src/server/audio_thread.c
[modify] https://crrev.com/83addda994d654f37a3b9bfea2a6a149442eb8ae/cras/src/tests/iodev_list_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/66f631ef779a4bbdf0daa61a29155768248283b8

commit 66f631ef779a4bbdf0daa61a29155768248283b8
Author: Hsin-Yu Chao <hychao@google.com>
Date: Fri Oct 12 07:34:38 2018

CRAS: tests - Fix iodev_list_unittest

Fallback devices' state should be initialized to closed in each
test cases.
The incorrect iodev state misled a few test cases, fixing them.

BUG= chromium:890242 
TEST=Unittest

Change-Id: Ie3220e6d51432d9ec02b5fde0003d2e928e49074
Reviewed-on: https://chromium-review.googlesource.com/1257488
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 0646381c7a9dd2bd52dea1e7b082f705681bf723)
Reviewed-on: https://chromium-review.googlesource.com/c/1278607
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/66f631ef779a4bbdf0daa61a29155768248283b8/cras/src/tests/iodev_list_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/871297cce17b83d6e16a0ce5ec173ee34f5ec33a

commit 871297cce17b83d6e16a0ce5ec173ee34f5ec33a
Author: Hsin-Yu Chao <hychao@google.com>
Date: Fri Oct 12 07:59:09 2018

CRAS: iodev_list - Generalize init retry for pinned stream

Iodev init retry is a mechanism important for hotplug headset types
like USB and BT. And it was implemented only for when audio device
is enabled from UI, not for when a stream pins to specific device.

Now Chrome browser provides API letting web app to select which audio
input/output nodes to use. This feature uses pinned stream, and is
suffering from device open issue when selecting to BT headset.
This change generalizes the iodev init retry to all paths that opens
an iodev, to stablize hotplug headset use cases.

BUG= chromium:890242 
TEST=Connect BT headset HM1100. Launch Google Meet and under
settings select input option to/from "Default - HM1100" and "HM1100"
verifiy UI doesn't show warning about mic.

Change-Id: Ic7289ffdc05d1f4cf4cca47488590c3dbf375247
Reviewed-on: https://chromium-review.googlesource.com/1257489
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 380919c441afa207b0df11bb80e8fd2991881b6a)
Reviewed-on: https://chromium-review.googlesource.com/c/1278608
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/871297cce17b83d6e16a0ce5ec173ee34f5ec33a/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/871297cce17b83d6e16a0ce5ec173ee34f5ec33a/cras/src/tests/iodev_list_unittest.cc

Labels: -Merge-Approved-70
Status: Fixed (was: Started)

Comment 14 by hychao@chromium.org, Today (108 minutes ago)

Cc: msnoxell@chromium.org cychiang@chromium.org dbbrooks@chromium.org
 Issue 868635  has been merged into this issue.

Sign in to add a comment