Issue metadata
Sign in to add a comment
|
Default device lookup broken on Chrome OS |
||||||||||||||||||||||
Issue descriptionThe new function GetPrimaryActiveOutputNodeOnMainThread from https://chromium-review.googlesource.com/c/chromium/src/+/637512 doesn't actually assign to the output pointer. This causes computations of the group id property to fail for the default output device.
,
Sep 12 2017
Connect some device (like a headset), make sure it is selected as default in the system UI, and issue a device enumeration (for example on https://guidou.github.io/enumdemo3.html). In this case, the "default" device should have the same group id as the headset devices which doesn't have the "default" device id (on the test page, the group id is the long hex string in the last column). Instead, the "default" device now gets a different group id, since the audio manager doesn't identify which is the default device correctly.
,
Sep 12 2017
Does audio_manager_unittest.cc have a test for this? Even if there's a test, there is no ChromeOS bot in chrome CQ.
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a00d6fa0a60f520958721cf380dedfe7981e353c commit a00d6fa0a60f520958721cf380dedfe7981e353c Author: Max Morin <maxmorin@chromium.org> Date: Tue Sep 12 12:01:56 2017 Fix group id for default devices on Chrome OS. This CL addresses a couple of bugs related to group id computations. First, the default output device isn't correctly identified as GetPrimaryActiveOutputNodeOnMainThread function doesn't actually set the output value. Second, default input device id was assumed to correspond to the built-in microphone in earlier versions of the code. This isn't true (and wasn't true before either). Now, we actually look up what device is the default input device. BUG=636300, 764228 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: Ief22bfa5dffe29279dd906dd252c4d2489f30f1b Reviewed-on: https://chromium-review.googlesource.com/662761 Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org> Commit-Queue: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/heads/master@{#501240} [modify] https://crrev.com/a00d6fa0a60f520958721cf380dedfe7981e353c/media/audio/cras/audio_manager_cras.cc [modify] https://crrev.com/a00d6fa0a60f520958721cf380dedfe7981e353c/media/audio/cras/audio_manager_cras.h
,
Sep 12 2017
I filed issue 764291 about adding tests, but as you said tests couldn't currently run on the CQ anyways. Requesting merge to M62: Not sure when the next beta release is, I'll let this bake in canary for a couple of days if release owner prefers.
,
Sep 12 2017
Consider this merge approved, but please ensure a build of Chrome with this change makes it through the PFQ on ToT first, as the branch builders have no PFQ protection.
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5947ab6d4e7e3e9f5775208b9b569b1904479b9c commit 5947ab6d4e7e3e9f5775208b9b569b1904479b9c Author: Max Morin <maxmorin@chromium.org> Date: Fri Sep 15 08:24:51 2017 [M62] Fix group id for default devices on Chrome OS. This CL addresses a couple of bugs related to group id computations. First, the default output device isn't correctly identified as GetPrimaryActiveOutputNodeOnMainThread function doesn't actually set the output value. Second, default input device id was assumed to correspond to the built-in microphone in earlier versions of the code. This isn't true (and wasn't true before either). Now, we actually look up what device is the default input device. BUG=636300, 764228 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: Ief22bfa5dffe29279dd906dd252c4d2489f30f1b Reviewed-on: https://chromium-review.googlesource.com/662761 Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org> Commit-Queue: Max Morin <maxmorin@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#501240}(cherry picked from commit a00d6fa0a60f520958721cf380dedfe7981e353c) Reviewed-on: https://chromium-review.googlesource.com/667149 Reviewed-by: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#246} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/5947ab6d4e7e3e9f5775208b9b569b1904479b9c/media/audio/cras/audio_manager_cras.cc [modify] https://crrev.com/5947ab6d4e7e3e9f5775208b9b569b1904479b9c/media/audio/cras/audio_manager_cras.h
,
Sep 18 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by wuchengli@chromium.org
, Sep 12 2017