Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2771133002:
[chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. (Closed)
Description[chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume.
This allows external process to control Cast media volume.
Bug: internal b/36101895
Test: assistantdefault
Change-Id: I6816f629d9a26a5756139ea2bb6db2789e8264df
Review-Url: https://codereview.chromium.org/2771133002
Cr-Commit-Position: refs/heads/master@{#459597}
Committed: https://chromium.googlesource.com/chromium/src/+/699c4507d3b4fea0660627db4bef5e500f28545c
Patch Set 1 #
Total comments: 4
Patch Set 2 : [chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. #
Messages
Total messages: 24 (16 generated)
Description was changed from ========== [chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. This allows external process to control Cast media volume. Bug: internal b/36101895 Test: assistantdefault Change-Id: I6816f629d9a26a5756139ea2bb6db2789e8264df ========== to ========== [chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. This allows external process to control Cast media volume. Bug: internal b/36101895 Test: assistantdefault Change-Id: I6816f629d9a26a5756139ea2bb6db2789e8264df ==========
gfhuang@chromium.org changed reviewers: + halliwell@chromium.org, kmackay@chromium.org
lgtm https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... File chromecast/media/cma/backend/alsa/volume_control.cc (right): https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... chromecast/media/cma/backend/alsa/volume_control.cc:137: base::Unretained(this), type, level, false)); false /* from_alsa */ https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... chromecast/media/cma/backend/alsa/volume_control.cc:148: base::Unretained(this), type, muted, false)); /* from_alsa */
The CQ bit was checked by gfhuang@chromium.org to run a CQ dry run
Dry run: 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
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... File chromecast/media/cma/backend/alsa/volume_control.cc (right): https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... chromecast/media/cma/backend/alsa/volume_control.cc:137: base::Unretained(this), type, level, false)); On 2017/03/24 04:02:50, kmackay wrote: > false /* from_alsa */ Done. https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... chromecast/media/cma/backend/alsa/volume_control.cc:148: base::Unretained(this), type, muted, false)); On 2017/03/24 04:02:50, kmackay wrote: > /* from_alsa */ Done.
The CQ bit was checked by gfhuang@chromium.org to run a CQ dry run
Dry run: 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
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
On 2017/03/24 06:30:00, gfhuang wrote: > https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... > File chromecast/media/cma/backend/alsa/volume_control.cc (right): > > https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... > chromecast/media/cma/backend/alsa/volume_control.cc:137: base::Unretained(this), > type, level, false)); > On 2017/03/24 04:02:50, kmackay wrote: > > false /* from_alsa */ > > Done. > > https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backen... > chromecast/media/cma/backend/alsa/volume_control.cc:148: base::Unretained(this), > type, muted, false)); > On 2017/03/24 04:02:50, kmackay wrote: > > /* from_alsa */ > > Done. lgtm
The CQ bit was checked by gfhuang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kmackay@chromium.org Link to the patchset: https://codereview.chromium.org/2771133002/#ps20001 (title: "[chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume.")
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: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by gfhuang@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": 1490391323593470,
"parent_rev": "f8473e4cc3f29a43b8fcc5e26ab84f3b825d6e36", "commit_rev":
"699c4507d3b4fea0660627db4bef5e500f28545c"}
Message was sent while issue was closed.
Description was changed from ========== [chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. This allows external process to control Cast media volume. Bug: internal b/36101895 Test: assistantdefault Change-Id: I6816f629d9a26a5756139ea2bb6db2789e8264df ========== to ========== [chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. This allows external process to control Cast media volume. Bug: internal b/36101895 Test: assistantdefault Change-Id: I6816f629d9a26a5756139ea2bb6db2789e8264df Review-Url: https://codereview.chromium.org/2771133002 Cr-Commit-Position: refs/heads/master@{#459597} Committed: https://chromium.googlesource.com/chromium/src/+/699c4507d3b4fea0660627db4bef... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/699c4507d3b4fea0660627db4bef... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
