Webrtc uses pinned stream while it should not |
|||||||||
Issue descriptionI tried on latest image 9975.0. Steps: 1. Plug USB headset, select USB as output if it is not. 2. Go to appr.tc and join a room. 3. Plug 3.5mm Headphone. 4. Select 3.5mm Headphone as output. 5. Play youtube. 6. Hear the WebRTC audio at USB, and youtube audio at 3.5mm Headphone. This suggests that WebRTC uses pinned stream on USB. From the test result of audio_AudioWebRTCLoopback, https://wmatrix.googleplex.com/matrix/unfiltered?tests=audio_AudioWebRTCLoopback&days_back=20&hide_missing=True 9942.0 is good and 9945.0 is bad. The blamelist of Chrome CLs: https://chromium.googlesource.com/chromium/src/+log/63.0.3212.0..63.0.3216.0?pretty=fuller&n=10000 I would guess it is this one: https://chromium-review.googlesource.com/662761
,
Sep 26 2017
Hmm, I'm getting more convinced it's a conceptual error to associate devices/assign group ids to the "default" devices, since they don't correspond to physical devices. They should probably just be treated as different from any other device.
,
Sep 26 2017
I don't have a working Chromium checkout right now (I'm travelling, and somehow the checkout on my laptop got corrupted), but I https://cs.chromium.org/chromium/src/media/audio/cras/audio_manager_cras.cc?type=cs&q=audiomanagercras&sq=package:chromium&l=207 should be "AudioDeviceDescription::kDefaultDeviceId" and AudioManagerCras::GetDefaultOutputDeviceID() should return "".
,
Sep 26 2017
Sorry, line 207 should be "return AudioDeviceDescription::kDefaultDeviceId".
,
Sep 27 2017
Great! Your suggestion fixed the problem.
,
Sep 27 2017
Is it possible for you to make the change, or should I take it ? Thanks!
,
Sep 28 2017
I posted the CL: https://chromium-review.googlesource.com/#/c/chromium/src/+/688381/ Please take a look, thanks!
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb5a4358523b8874ea6484796bfd31b73f83d075 commit eb5a4358523b8874ea6484796bfd31b73f83d075 Author: Cheng-Yi Chiang <cychiang@chromium.org> Date: Thu Sep 28 10:07:01 2017 Fix audio_manager_cras GetAssociatedOutputDeviceID for default device Fix the bug in crrev.com/c/662761 :Fix group id for default devices on Chrome OS. WebRTC incorrectly uses pinned stream on ChromeOS. Default device does not correspond to physical device, so there is no associated device for it. BUG= 768792 TEST=On ChromeOS, check audio output from WebRTC can be routed to different output device. 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 Change-Id: I295aa4072b44d5d124d3e27c1894bce3f3085860 Reviewed-on: https://chromium-review.googlesource.com/688381 Commit-Queue: Cheng-Yi Chiang <cychiang@chromium.org> Reviewed-by: Max Morin <maxmorin@chromium.org> Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org> Cr-Commit-Position: refs/heads/master@{#504956} [modify] https://crrev.com/eb5a4358523b8874ea6484796bfd31b73f83d075/media/audio/cras/audio_manager_cras.cc
,
Oct 12 2017
Add merge request to R62 for CL in #8. Thanks!
,
Oct 12 2017
This bug requires manual review: We are only 4 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 12 2017
Hi Bernie, I am sorry for the late request. The CL https://chromium-review.googlesource.com/#/c/688381/ was simple so it is safe to get merged. I have verified WebRTC works on 10024.0. Thanks!
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ceb41ed3cea56d876f40c720c78f4f40a79bfe86 commit ceb41ed3cea56d876f40c720c78f4f40a79bfe86 Author: Max Morin <maxmorin@chromium.org> Date: Thu Oct 12 11:18:18 2017 (cros) Associate "default" input to "default" output. The current implementation, associating "default" input to output based on hardware, has unwanted consequences. Thus, we associate "default" to "" instead. We also override "Get{In|Out}putGroupId" in AudioManagerCras to make sure it's still based on the underlying hardware device. BUG= 768792 TBR=cychiang 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 Change-Id: I8c7ba3b0c2ab16645352b8e32b499f71e219fcc8 Reviewed-on: https://chromium-review.googlesource.com/688797 Reviewed-by: Max Morin <maxmorin@chromium.org> Commit-Queue: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/heads/master@{#508311} [modify] https://crrev.com/ceb41ed3cea56d876f40c720c78f4f40a79bfe86/media/audio/audio_manager_base.h [modify] https://crrev.com/ceb41ed3cea56d876f40c720c78f4f40a79bfe86/media/audio/cras/audio_manager_cras.cc [modify] https://crrev.com/ceb41ed3cea56d876f40c720c78f4f40a79bfe86/media/audio/cras/audio_manager_cras.h
,
Oct 12 2017
Approved for 62.
,
Oct 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f6d967ea2c9c65fa2787463c07e4fa0031110ce commit 1f6d967ea2c9c65fa2787463c07e4fa0031110ce Author: Cheng-Yi Chiang <cychiang@chromium.org> Date: Fri Oct 13 06:16:40 2017 Fix audio_manager_cras GetAssociatedOutputDeviceID for default device Fix the bug in crrev.com/c/662761 :Fix group id for default devices on Chrome OS. WebRTC incorrectly uses pinned stream on ChromeOS. Default device does not correspond to physical device, so there is no associated device for it. BUG= 768792 TEST=On ChromeOS, check audio output from WebRTC can be routed to different output device. 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 Change-Id: I295aa4072b44d5d124d3e27c1894bce3f3085860 Reviewed-on: https://chromium-review.googlesource.com/688381 Commit-Queue: Cheng-Yi Chiang <cychiang@chromium.org> Reviewed-by: Max Morin <maxmorin@chromium.org> Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#504956}(cherry picked from commit eb5a4358523b8874ea6484796bfd31b73f83d075) Reviewed-on: https://chromium-review.googlesource.com/718256 Cr-Commit-Position: refs/branch-heads/3202@{#678} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/1f6d967ea2c9c65fa2787463c07e4fa0031110ce/media/audio/cras/audio_manager_cras.cc
,
Oct 13 2017
Merged the fix in #8 to m62. https://chromium-review.googlesource.com/c/chromium/src/+/718256
,
Oct 13 2017
Thanks Bernie and Wucheng!
,
Oct 16 2017
Issue 766782 has been merged into this issue.
,
Jan 22 2018
,
Jan 23 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by cychiang@chromium.org
, Sep 26 2017Owner: maxmorin@chromium.org