Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2609863007:
Add new mic types (Closed)
DescriptionAdd new mic types
Chrome OS audio server is going to report more types of microphones,
implement the UI side corresponding code to parse these types from
node info.
BUG=672537
Review-Url: https://codereview.chromium.org/2609863007
Cr-Commit-Position: refs/heads/master@{#443271}
Committed: https://chromium.googlesource.com/chromium/src/+/1b273edcb44ef69ce19b93a73622460de70fe14d
Patch Set 1 #
Total comments: 3
Patch Set 2 : Add comment to explain why rear mic has lower priority #
Messages
Total messages: 19 (9 generated)
Description was changed from ========== Add new mic types Chrome OS audio server is going to report more types of microphones, implement the UI side corresponding code to parse these types from node info. BUG=None ========== to ========== Add new mic types Chrome OS audio server is going to report more types of microphones, implement the UI side corresponding code to parse these types from node info. BUG=None ==========
hychao@chromium.org changed reviewers: + hshi@chromium.org, jennyz@chromium.org
https://codereview.chromium.org/2609863007/diff/1/chromeos/audio/audio_device.cc File chromeos/audio/audio_device.cc (right): https://codereview.chromium.org/2609863007/diff/1/chromeos/audio/audio_device... chromeos/audio/audio_device.cc:39: case AUDIO_TYPE_REAR_MIC: why is the FRONT_MIC assigned priority "1" but REAR_MIC priority "0"? there must be a very good reason for this so if you could comment in the CL description that would be nice
https://codereview.chromium.org/2609863007/diff/1/chromeos/audio/audio_device.cc File chromeos/audio/audio_device.cc (right): https://codereview.chromium.org/2609863007/diff/1/chromeos/audio/audio_device... chromeos/audio/audio_device.cc:39: case AUDIO_TYPE_REAR_MIC: Yes, same question as hshi, please comment for the reason.
lgtm with nits.
https://codereview.chromium.org/2609863007/diff/1/chromeos/audio/audio_device.cc File chromeos/audio/audio_device.cc (right): https://codereview.chromium.org/2609863007/diff/1/chromeos/audio/audio_device... chromeos/audio/audio_device.cc:39: case AUDIO_TYPE_REAR_MIC: On 2017/01/06 18:41:23, hshi1 wrote: > why is the FRONT_MIC assigned priority "1" but REAR_MIC priority "0"? > > there must be a very good reason for this so if you could comment in the CL > description that would be nice Added comment for this. The idea is the same as setting BT input to priority=0 above. For those mics not for normal use case, we set priority to zero to avoid auto node selection jumps to a special mic and user doesn't notice that.
sorry I missed this earlier. LGTM nit: is there no bug filed for this? maybe you want to file a bug to track the related work
Description was changed from ========== Add new mic types Chrome OS audio server is going to report more types of microphones, implement the UI side corresponding code to parse these types from node info. BUG=None ========== to ========== Add new mic types Chrome OS audio server is going to report more types of microphones, implement the UI side corresponding code to parse these types from node info. BUG=672537 ==========
On 2017/01/11 18:43:41, hshi1 wrote: > sorry I missed this earlier. LGTM > > nit: is there no bug filed for this? maybe you want to file a bug to track the > related work updated the bug number. thanks!
The CQ bit was checked by hychao@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jennyz@chromium.org Link to the patchset: https://codereview.chromium.org/2609863007/#ps20001 (title: "Add comment to explain why rear mic has lower priority")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
The CQ bit was checked by hychao@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484237726250820,
"parent_rev": "4ef9895a701e744bce46c6a4bb6978e06fe67a0a", "commit_rev":
"1b273edcb44ef69ce19b93a73622460de70fe14d"}
Message was sent while issue was closed.
Description was changed from ========== Add new mic types Chrome OS audio server is going to report more types of microphones, implement the UI side corresponding code to parse these types from node info. BUG=672537 ========== to ========== Add new mic types Chrome OS audio server is going to report more types of microphones, implement the UI side corresponding code to parse these types from node info. BUG=672537 Review-Url: https://codereview.chromium.org/2609863007 Cr-Commit-Position: refs/heads/master@{#443271} Committed: https://chromium.googlesource.com/chromium/src/+/1b273edcb44ef69ce19b93a73622... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/1b273edcb44ef69ce19b93a73622... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
