CRAS: USB device should use ID_SERIAL to make stable id more robust |
|||||||||
Issue descriptionCurrently CRAS makes the stable id from UID and VID of USB audio device, so that Chrome/webapp cannot tell between two headsets of the same brand & type. There is usage in Kiosk app to mark specific USB hardware as defect unit and ban it persistently, so we need to find a solution for that. udev info has two properties: ID_SERIAL and ID_SERIAL_SHORT which are supposed to be unique on different hardware unites of the same kind. cras_udev.c is a place to extract this serial string, pack it into struct cras_alsa_card_info and pass all the way to alsa_io. Since we've exposed the stable ID to Chrome for storing device preference for a few releases. We have to revise the stable ID while not breaking current usage. The plan is to use two properties for each node: id and id_old so that Chrome side can do something like: if not pref.has_id(node.id) and perf.has_id(node.id_old): pref[node.id] = pref[node.id_old] pref[node.id_old] = NULL
,
Nov 8 2016
Moja is very kind to take this.
,
Nov 16 2016
The CRAS side change is at https://chromium-review.googlesource.com/#/c/407723/ to add new node attribute "StableDeviceIdOld". Chrome UI should modify its code to take care backward compatibility before we merge this change. Something like: if node has StableDeviceIdOld, check preference for this key, and replace preference entry with new key: StableDeviceId. tbarzic@ can you help with that?
,
Nov 16 2016
Should we invert the backwards compatibility? If we make the new field StableDeviceIdNew (or something a little better) then chrome will keep working and be bisect-able across a larger range of platform versions.
,
Nov 16 2016
I slightly prefer approach in comment 4: adding separate stable_device_id_v2 property, but it shouldn't make that much difference. And, yes, I can help with Chrome side :)
,
Nov 17 2016
Do we really need backward compatibility? Because I saw a CL that changes the StableDeviceId too. https://chromium-review.googlesource.com/401146 In cras_alsa_io.c it changed the parameter of hash calculate. From sizeof(name) to strlen(name). I discussed StableDeviceIdNew with hychao@ before. If we choose to use StableDeviceIdNew, there might be some effort when we decide to remove it. The content of the StableDeviceID will be different after we deprecated StableDeviceIdNew. StableDeviceId will keep old value during the backward compatible stage and keep new value after it.
,
Nov 17 2016
I'll defer backward compatibility question to jennyz, since she is who owns audio stack in Chrome, but my guess is that we do need it (otherwise we lose info from Chrome preferences). I'm fine with both StableDeviceIdNew and StableDeviceIdOld, Chrome will have to handle change from old value to new value either way, it would be only the question of when.
,
Nov 21 2016
What I mean is we've already lose info once when this CL https://chromium-review.googlesource.com/401146 merged. But we can still keep backward compatibility this time. I want to use StableDeviceIdOld this time for keeping the StableId consistent.
,
Nov 22 2016
After discuss with hychao@. We decided to use StableDeviceIdNew. It will make cras merge code easily for adding and removing the StableDeviceIdNew.
,
Dec 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/c84eb15a0fbd4a6b336d11438836b644fefc2679 commit c84eb15a0fbd4a6b336d11438836b644fefc2679 Author: Moja Hsu <mojahsu@google.com> Date: Tue Nov 08 07:25:25 2016 CRAS: dbus_control - Add serial number for USB device for stable id Get additional sys attribute for "serial" which is serial number in udev. Add it into the stable_id_new calcuation and keep stable_id for backward compitable. BUG= chromium:661861 TEST=Plug 2 same type USB speakers into chromebook. execute "dbus-send --system --type=method_call --print-reply --dest=org.chromium.cras /org/chromium/cras org.chromium.cras.Control.GetNodes" Check if we have the same stable_id but different stable_id_new for the 2 speakers. pass make check for unit test. Change-Id: Iefefe383fd6143dd7c28fdba3cf3f3872c2dd726 Reviewed-on: https://chromium-review.googlesource.com/407723 Commit-Ready: Hsu Wei-Cheng <mojahsu@chromium.org> Tested-by: Hsu Wei-Cheng <mojahsu@chromium.org> Reviewed-by: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_a2dp_iodev.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_iodev_list.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_dbus_control.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/common/cras_types.h [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/tests/alsa_io_unittest.cc [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/tests/alsa_card_unittest.cc [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_udev.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_iodev.h [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_alsa_io.h [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_loopback_iodev.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_alsa_card.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/common/cras_iodev_info.h [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_bt_io.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_hfp_iodev.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/src/server/cras_alsa_io.c [modify] https://crrev.com/c84eb15a0fbd4a6b336d11438836b644fefc2679/cras/README.dbus-api
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/system_api/+/4489d28f1ec210c840d4f4827f8210ab27141f2d commit 4489d28f1ec210c840d4f4827f8210ab27141f2d Author: Toni Barzic <tbarzic@google.com> Date: Wed Nov 16 23:46:54 2016 system_api: Update dbus service constants for cras nod properties Adds const for new property exposed by cras node struct: StableDeviceIdOld - this will expose soon to be deprecated stable audio device ID value. BUG= chromium:661861 TEST=None Change-Id: I90715e6669b7520c2d68979ab2c1c301854b3dbf Reviewed-on: https://chromium-review.googlesource.com/411987 Commit-Ready: Toni Barzic <tbarzic@chromium.org> Tested-by: Toni Barzic <tbarzic@chromium.org> Reviewed-by: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/4489d28f1ec210c840d4f4827f8210ab27141f2d/dbus/service_constants.h
,
Dec 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff0f2ca2e95f208655869178c32cf666c8f6e478 commit ff0f2ca2e95f208655869178c32cf666c8f6e478 Author: tbarzic <tbarzic@chromium.org> Date: Fri Dec 09 02:33:16 2016 Roll cros_system_api to pick up new cras constant Rolling cros_system_api up to: https://chromium.googlesource.com/chromiumos/platform/system_api/+/4489d28f1ec210c840d4f4827f8210ab27141f2d BUG= 661861 , 659732 Review-Url: https://codereview.chromium.org/2556203002 Cr-Commit-Position: refs/heads/master@{#437439} [modify] https://crrev.com/ff0f2ca2e95f208655869178c32cf666c8f6e478/DEPS
,
Dec 12 2016
,
Dec 15 2016
The code in cras is ready and merged. Feel free to reuse or close this issue.
,
Dec 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3ab01a28f2ccd9abc62a3833d72959205ddce98b commit 3ab01a28f2ccd9abc62a3833d72959205ddce98b Author: tbarzic <tbarzic@chromium.org> Date: Sat Dec 17 03:12:53 2016 Handle audio node stable device ID change Chrome side change for updating the way stable device ID is calculated for USB audio device nodes in order to make them more usefull when distinguishing among USB audio devices with same USB vendor and product IDs. Cras will start providing new property - StableDeviceIdOld that will contain old, now deprecated stable device ID - this will be used to migrate audio device prefs to new stable ID (to provide some backward compatibility). This property is expected to eventually go away. This does not expose new value to audio extension API, but cl will follow - for now, keep providing old ID version through API. Huge line diff is mostly due to refactoring test to account for new stable device ID version :) Parametrizes cras audio client unit tests by the version od stable device ID provided by audio device - to ensure it keeps working as expcted with both stable device ID version (does not test for mixing devices with different stable device ID versions, that is never expected to happen). Added audio devices pref handler for migrating audio device records when audio device stable device ID changes. BUG= 661861 CQ_INCLUDE_TRYBOTS=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/2510093003 Cr-Commit-Position: refs/heads/master@{#439304} [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/audio_device.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/audio_device.h [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/audio_devices_pref_handler_impl.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/audio_devices_pref_handler_impl.h [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/audio_devices_pref_handler_impl_unittest.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/cras_audio_handler.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/audio/cras_audio_handler_unittest.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/dbus/audio_node.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/dbus/audio_node.h [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/dbus/cras_audio_client.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/dbus/cras_audio_client_unittest.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/chromeos/dbus/fake_cras_audio_client.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/extensions/browser/api/audio/audio_apitest.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/extensions/browser/api/audio/audio_service_chromeos.cc [modify] https://crrev.com/3ab01a28f2ccd9abc62a3833d72959205ddce98b/media/audio/audio_manager_unittest.cc
,
Dec 28 2016
,
Jan 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e3e93214f404cd6f3da984000a447d8284cf828 commit 4e3e93214f404cd6f3da984000a447d8284cf828 Author: tbarzic <tbarzic@chromium.org> Date: Wed Jan 18 01:18:23 2017 Update audio api to use v2 stable device ID Algorithm used to calculate stable audio device ID has been updated in cras to better distinguish instances of identical USB devcies. This exposes the new stable device ID to audio API. The existing stableDeviceId does not to be currently used, so it should be OK just to overwrite it. BUG= 661861 Review-Url: https://codereview.chromium.org/2585413002 Cr-Commit-Position: refs/heads/master@{#444221} [modify] https://crrev.com/4e3e93214f404cd6f3da984000a447d8284cf828/extensions/browser/api/audio/audio_apitest.cc [modify] https://crrev.com/4e3e93214f404cd6f3da984000a447d8284cf828/extensions/browser/api/audio/audio_service_chromeos.cc [modify] https://crrev.com/4e3e93214f404cd6f3da984000a447d8284cf828/extensions/test/data/api_test/audio/add_nodes/background.js [modify] https://crrev.com/4e3e93214f404cd6f3da984000a447d8284cf828/extensions/test/data/api_test/audio/remove_nodes/background.js
,
Mar 3 2017
,
Mar 3 2017
,
Aug 1 2017
,
Aug 2 2017
Verified in Chrome OS 9801.0.0, 62.0.3174.0. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by tbarzic@chromium.org
, Nov 3 2016Components: UI>Shell>Kiosk
Labels: -Pri-3 M-57 OS-Chrome Pri-2