Eng work for getting pinned stream from headset micjack, actually getting from internal mic |
||||||||||||||
Issue descriptionVersion: tot OS: ChromeOS bug described in crbug.com/651512 On tot or M54 (54.0.2840.68), open chrome://flags, and turn on #enumerate-audio-devices flag. Then repeat the repro steps on 651512. Comment 10 and 11 in crbug.com/651512 has logs and some information. I agreed with dgreid@ that we should expose more device settings to the api. What about the last 32 bits of cras_node_id_t? Firstly assigned to hychao@.
,
Nov 1 2016
The api means https://cs.chromium.org/chromium/src/media/audio/cras/cras_input.cc?q=add_pinned&sq=package:chromium&l=224, cras_client_add_pinned_stream, which is defined here: https://cs.corp.google.com/chromeos_public/src/third_party/adhd/cras/src/libcras/cras_client.c?type=cs&q=cras_client_add_pinned_stream&l=2353, It takes uint32_t dev_idx. I don't see there is a way that I can pass the full node id to differentiate it.
,
Nov 1 2016
Correct, streams get pinned to devices not nodes. This is because nodes on the same device often can't play different streams simultaneously.
,
Nov 1 2016
Thanks for the hint, Dylan. Now I understand the problem. Pinned stream has limitation when system UI selects to the other node (on the same device) for playback/recording. Examples of pairs: speaker <-> headphone, mic jack <-> internal mic. Given this limitation, maybe we should enumerate only the devices but not detailed to nodes. Like expose one entry for speaker & headphone labeled by 'device_name' of AudioDevice.
,
Nov 2 2016
It's messy because the ui needs to be able to select nodes so the user can choose headphone or speaker, but the pinned streams will play to both if they are pinned to either.
,
Nov 16 2016
Back to this issue. We cannot fix the hardware limitation. And I personally think the best thing to do is modify naming to make this feature less confusing. Maybe for #enumerate-audio-devices feature we put INTERNAL_SPEAKER and INTERNAL_MIC behind one entry named "Internal". Since only the internal device could have more than one node. That results the messy behavior as Dylan described in #5. And user will eventually realize that whenever pinned audio to this 'internal' device, it routes to headphone or speaker depends on system UI's selection. Example: - Output nodes exposed to Chrome UI: [ speaker(3:0), headphone(3:1), USB1(4:0), BT-headset(5:0)] - Output nodes exposed to mediaDevices.enumerateDevices() layer: [ internal(3), USB1(4), BT-headset(5) ] Assign back to warx@ to see if my suggestion is feasible.
,
Nov 17 2016
Wait, I started to realize that the #enumerate-audio-devices feature is only half done. I was thinking we only needed to deal with audio input devices and forgot to test audio output devices. The current behavior I implemented still relies on CRAS default output routing even selecting output destination device from drop down list. That is to say we got pinning stream from input on the web page, but still 'default' routing to output. The work should start with passing device_id to https://cs.chromium.org/chromium/src/media/audio/cras/audio_manager_cras.cc?sq=package:chromium&l=292. My apologize, will open another bug to track.
,
Nov 17 2016
,
Nov 19 2016
I create a cl to fix this and 666202: https://codereview.chromium.org/2516813002/ So per comment 6, I am worried about user experience here. If we "Internal" input to represent internal_mic and 3.5mm micjack, and "Internal" output to represent internal_speaker and 3.5mm headphone. User will really get confused at first because routing depends on chromeos system UI's selection. On chromeos system tray UI: I can see Speaker(Internal), Microphone(Internal), but not Headphone (Internal) and Mic Jack (Internal). I don't think we can assume that user knows that headphone is using internal node, if user has multiple outputs plugged. User has to try each one to find this.
,
Nov 19 2016
I'd agree that it doesn't make too much sense to display "internal" Maybe a combined "HP/Speaker"?
,
Nov 19 2016
"HP/Speaker" sounds better to me than "internal", how about input? Is "Mic/Mic Jack" a professional way to do this?
,
Nov 21 2016
Maybe Headset/Front Mic. I'm not sure on that one and it's going to get really messy as we add new mic locations.
,
Nov 21 2016
Yes, that is because we allow playing back to internal speaker with headphone plugged. If we are going to make this change, how could we let user know that this choice depends on system UI's selection? Another option may be to call cras_client_select_node API to activate the node if selection is HP/Speaker or HeadSet/Front Mic? But it may need more work and probably is dangerous since javascript could change system settings. Btw, can we get a PM to confirm the behavior here?
,
Nov 22 2016
That approach sounds reasonable to me. 1) Headset/Speakers 2) What about "Internal Mic/External Mic" ? +Tom for his input too
,
Nov 30 2016
For the ease of the PM's input, I will make a summary here: For the device with internal mic, we will show the label like "Headset/Front Mic". The actual recording depends on: 1) if there is a mic jack plugged, recording from mic jack or internal mic depends on chromeos system UI selection. 2) if there is no mic jack plugged, recording is from internal mic. For the device with internal speaker, we will show the label like "HP/Speaker". The actual playing-back depends on: 1) if there is a headphone plugged, playing back to headphone or internal speaker depends on chromeos system UI selection. 2) if there is no headphone plugged, playing back is to internal speaker. We need to make agreement on the label names for this : )
,
Dec 2 2016
Btw, if device has 3.5mm headset plugged, usb-headset plugged, on webpage we select "HP/Speaker", but chromeos system UI we select usb-headset output. Audio on the webpage is playbacked to internal speaker. so, if chromeos system UI doesn't select one of the "slash selections", internal mic and internal speaker is the default choice for routing.
,
Feb 2 2017
At least on Windows PCs, there is only one "device" for anything hooked up to the internal sound card, so builtin speakers and headphones through the jack, or built-in mic and mic through jack are the same entry in device selection. I expect users to be used to this, and be fine with the sound to seamlessly switch between built-in mic/speakers and headset when the jack is inserted. What you're saying is it's possible on chrome os to use the internal mic/speakers even when a headset jack is inserted? Is there a reason to expose this to the user? In particular, I'm a little worried about #c16 While it's most likely an unlikely scenario for RTC, as a user if I had the configuration described I would expect audio to be routed to the jack, not to the internal mic/speaker. If the system is able to differentiate between internal mic/speaker and headphones, why exactly can't this be exposed to pages? IMO, you either need to make this distinction available to pages, or you need to remove the distinction from the system UI, automatically using the headset if a jack is inserted.
,
Mar 10 2017
,
Apr 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/188483ae0dc7746fd2bc8d8ebdc2a57b33769007 commit 188483ae0dc7746fd2bc8d8ebdc2a57b33769007 Author: warx <warx@chromium.org> Date: Tue Apr 04 21:51:08 2017 cros: Using virtual audio device label for internal device Changes: Now that cras could remember last active audio node for pinning stream. crbug.com/658048 could be properly handled if we use a virtual device name for internal device to represent the internal device with last active internal node. After discussion with UX writer and reviewer: Decided using "Built-in-mic" for input device, and "Built-in-speaker" for output device. BUG= 658048 TEST=Test on Chrome OS 9426.0.0. The bug in crbug.com/658048 is handled, if GetUserMedia selects internal device, the source/destination depends on last active internal node or current system activated internal node. 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/2795933002 Cr-Commit-Position: refs/heads/master@{#461854} [modify] https://crrev.com/188483ae0dc7746fd2bc8d8ebdc2a57b33769007/media/audio/cras/audio_manager_cras.cc
,
Apr 4 2017
,
Apr 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/934e8926ee432fe5f5e6b2b9a1f196455ede77df commit 934e8926ee432fe5f5e6b2b9a1f196455ede77df Author: warx <warx@chromium.org> Date: Sat Apr 29 20:13:43 2017 cros: Update virtual audio label name to Built-in mic/speaker Changes: As from discussion with UX writer, updating the label name here. BUG= 658048 TEST=none 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/2847173003 Cr-Commit-Position: refs/heads/master@{#468248} [modify] https://crrev.com/934e8926ee432fe5f5e6b2b9a1f196455ede77df/media/audio/cras/audio_manager_cras.cc
,
Apr 29 2017
#21 -> request merge to M59
,
Apr 30 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58a885b06af608a8dd427e81a490ef5fb8cc0e2d commit 58a885b06af608a8dd427e81a490ef5fb8cc0e2d Author: Qiang Xu <warx@chromium.org> Date: Mon May 01 00:32:35 2017 [Merge to M59] cros: Update virtual audio label name to Built-in mic/speaker Changes: As from discussion with UX writer, updating the label name here. TBR=tommi@chromium.org BUG= 658048 TEST=none 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/2847173003 Cr-Commit-Position: refs/heads/master@{#468248} (cherry picked from commit 934e8926ee432fe5f5e6b2b9a1f196455ede77df) Review-Url: https://codereview.chromium.org/2856493002 . Cr-Commit-Position: refs/branch-heads/3071@{#314} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/58a885b06af608a8dd427e81a490ef5fb8cc0e2d/media/audio/cras/audio_manager_cras.cc
,
May 1 2017
,
Aug 1 2017
,
Nov 7 2017
|
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by hychao@chromium.org
, Oct 27 2016