Chromium Code Reviews
Help | Chromium Project | Sign in
(9)

Issue 2771133002: [chromcast] Still need to call StreamMixerAlsa::SetVolume, even when ALSA doesn't own the volume. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month ago by gfhuang
Modified:
1 month ago
Reviewers:
halliwell, kmackay
CC:
chromium-reviews, alokp+watch_chromium.org, lcwu+watch_chromium.org, halliwell+watch_chromium.org, feature-media-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

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)
gfhuang
1 month ago (2017-03-24 03:24:52 UTC) #3
kmackay
lgtm https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backend/alsa/volume_control.cc File chromecast/media/cma/backend/alsa/volume_control.cc (right): https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backend/alsa/volume_control.cc#newcode137 chromecast/media/cma/backend/alsa/volume_control.cc:137: base::Unretained(this), type, level, false)); false /* from_alsa */ ...
1 month ago (2017-03-24 04:02:50 UTC) #4
gfhuang
https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backend/alsa/volume_control.cc File chromecast/media/cma/backend/alsa/volume_control.cc (right): https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backend/alsa/volume_control.cc#newcode137 chromecast/media/cma/backend/alsa/volume_control.cc:137: base::Unretained(this), type, level, false)); On 2017/03/24 04:02:50, kmackay wrote: ...
1 month ago (2017-03-24 06:30:00 UTC) #9
halliwell
On 2017/03/24 06:30:00, gfhuang wrote: > https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backend/alsa/volume_control.cc > File chromecast/media/cma/backend/alsa/volume_control.cc (right): > > https://codereview.chromium.org/2771133002/diff/1/chromecast/media/cma/backend/alsa/volume_control.cc#newcode137 > ...
1 month ago (2017-03-24 12:38:45 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2771133002/20001
1 month ago (2017-03-24 14:13:30 UTC) #17
commit-bot: I haz the power
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/builds/179477)
1 month ago (2017-03-24 17:52:25 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2771133002/20001
1 month ago (2017-03-24 21:36:08 UTC) #21
commit-bot: I haz the power
1 month ago (2017-03-24 23:34:29 UTC) #24
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/699c4507d3b4fea0660627db4bef...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46