Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2144333002:
MuteSource Audio During Full Screen Cast (Closed)
DescriptionMuteSource Audio During Full Screen Cast
Previously, when casting a chrome tab, the source audio is
automatically muted; however, casting the full screen, the
system audio is still audible.
This CL makes the behavior consistent.
P.S. But the technique is different for muting a tab and muting system audio.
1. For muting tab, we divert the audio stream from speaker
to WebContentsAudioInputStream.
2. For muting system audio, we call the OS API to do so,
which is just as if the user click the "mute" button on
the task bar.
Visible difference is that during capturing, the user is
not able to unmute tab audio on the capture side, but
nothing can prevent the end user to unmute system audio.
BUG=629561
NOPRESUBMIT=true
Committed: https://crrev.com/a282a4569fe0786fe2bec8c9d0bfbcadff276330
Cr-Commit-Position: refs/heads/master@{#408672}
Patch Set 1 : Mute system audio for getUserMedia(audio:System) #
Total comments: 4
Patch Set 2 : Memorize Mute Status Before Capture #
Total comments: 14
Patch Set 3 : Fix Issue #Patch Set 4 : Run Git Cl Format #
Total comments: 10
Patch Set 5 : nit #Patch Set 6 : Run Git Cl Format #
Total comments: 2
Patch Set 7 : Patch 3 #Patch Set 8 : nit #Patch Set 9 : Move mute to start #Patch Set 10 : Rebase #Messages
Total messages: 100 (71 generated)
Description was changed from ========== Mute Unmute Source Audio During Desktop/Tab Capture BUG= ========== to ========== Mute Unmute Source Audio During Desktop/Tab Capture 1. Add auto mute functionality for full screen capture; 2. Add constraints to control mute or unmute. BUG=595428 (Focus on #23 as starting point of contraints) ==========
Patchset #1 (id:1) has been deleted
The CQ bit was checked by qiangchen@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...
qiangchen@chromium.org changed reviewers: + miu@chromium.org
Can you glance at this experimental CL?
This is the minimal change set I can work out, so that during Casting full
screen, the source audio is automatically muted by default.
P.S. Currently we have
- screen capture (getUserMedia({audio:System, video:Screen}))
- tab capture (tabCapture)
- desktop capture (chooseDesktopMedia)
They have overlapped functionality, and thus partially overlapped code paths.
Besides, we abused terminology "desktop capture" in the code, and some "if"
checks to distinguish which API initiates the requests are hacky. So I think I
will have to do a code clean up before we can support constraint controlled
mute/unmute behavior. Otherwise the code will look more complicated and
confusing.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
General approach seems fine to me. Ideally, you should loop in someone that's spent time working on the Windows audio code to review that. https://codereview.chromium.org/2144333002/diff/60001/chrome/browser/media/de... File chrome/browser/media/desktop_capture_access_handler.cc (right): https://codereview.chromium.org/2144333002/diff/60001/chrome/browser/media/de... chrome/browser/media/desktop_capture_access_handler.cc:143: bool mute_system_audio, Can this method be called with media_id set to kLoopbackWithMuteDeviceId, instead of adding this extra boolean to this already-huge argument list? https://codereview.chromium.org/2144333002/diff/60001/media/audio/cras/cras_i... File media/audio/cras/cras_input.cc (right): https://codereview.chromium.org/2144333002/diff/60001/media/audio/cras/cras_i... media/audio/cras/cras_input.cc:100: cras_client_set_system_mute(client_, 1); What if the system audio was already muted? Perhaps you should call cras_client_get_system_mute, stash that away, and then mute system audio. Then later, in Stop(), restore the original mute state. Same applies to the Win platform code changes...
The CQ bit was checked by qiangchen@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...
Description was changed from ========== Mute Unmute Source Audio During Desktop/Tab Capture 1. Add auto mute functionality for full screen capture; 2. Add constraints to control mute or unmute. BUG=595428 (Focus on #23 as starting point of contraints) ========== to ========== Mute Unmute Source Audio During Desktop/Tab Capture 1. Add auto mute functionality for full screen capture; 2. Add constraints to control mute or unmute. henrika@chromium.org BUG=595428 (Focus on #23 as starting point of contraints) ==========
The CQ bit was checked by qiangchen@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: This issue passed the CQ dry run.
Description was changed from ========== Mute Unmute Source Audio During Desktop/Tab Capture 1. Add auto mute functionality for full screen capture; 2. Add constraints to control mute or unmute. henrika@chromium.org BUG=595428 (Focus on #23 as starting point of contraints) ========== to ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 ==========
qiangchen@chromium.org changed reviewers: + henrika@chromium.org
Patchset #1 (id:20001) has been deleted
Patchset #3 (id:80001) has been deleted
Patchset #1 (id:40001) has been deleted
qiangchen@chromium.org changed reviewers: + dalecurtis@chromium.org
Implementation for muting system audio during full screen cast. henrika@: can you focus on the windows change? We are trying to call OS API to change the mute status of the speaker. dalecurtis@: miu@ does not own all the files, namely the files under content folder. Can you also take a look? Thanks. https://codereview.chromium.org/2144333002/diff/60001/chrome/browser/media/de... File chrome/browser/media/desktop_capture_access_handler.cc (right): https://codereview.chromium.org/2144333002/diff/60001/chrome/browser/media/de... chrome/browser/media/desktop_capture_access_handler.cc:143: bool mute_system_audio, On 2016/07/19 01:09:44, miu wrote: > Can this method be called with media_id set to kLoopbackWithMuteDeviceId, > instead of adding this extra boolean to this already-huge argument list? This function is shared between HandleRequest and ProcessScreenCaptureRequest. The former is for chooseDesktopMedia and the latter for getUserMedia({screen}), and thus we need to cater different use. https://codereview.chromium.org/2144333002/diff/60001/media/audio/cras/cras_i... File media/audio/cras/cras_input.cc (right): https://codereview.chromium.org/2144333002/diff/60001/media/audio/cras/cras_i... media/audio/cras/cras_input.cc:100: cras_client_set_system_mute(client_, 1); On 2016/07/19 01:09:44, miu wrote: > What if the system audio was already muted? Perhaps you should call > cras_client_get_system_mute, stash that away, and then mute system audio. Then > later, in Stop(), restore the original mute state. > > Same applies to the Win platform code changes... Done.
Why do this vs using the normal muting API in AudioSyncReader?
On 2016/07/20 22:54:02, DaleCurtis wrote: > Why do this vs using the normal muting API in AudioSyncReader? Because during full screen cast, we are sharing the whole screen, and the system audio. Thus we are muting the whole system audio. Not just the audio generated by Chrome.
I see, general approach sounds fine to me, but don't know enough about the Windows details. Defer to henrika@ for WASAPI changes.
Please extend the comments and try to avoid duplicating code in the Windows implementation. Also, make it more clear that these new parts are intended for a very specific case only. https://codereview.chromium.org/2144333002/diff/100001/media/audio/audio_devi... File media/audio/audio_device_description.h (right): https://codereview.chromium.org/2144333002/diff/100001/media/audio/audio_devi... media/audio/audio_device_description.h:33: static const char kLoopbackWithMuteDeviceId[]; Please add a comment for the new constant. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.cc (right): https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:170: if (device_id_ == AudioDeviceDescription::kLoopbackWithMuteDeviceId && Add a comment on 169 explaining what you do here. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:175: NULL, reinterpret_cast<void**>(&pVolume)); Can you use base::win::ScopedComPtr<IAudioEndpointVolume> here as well (your style is not in line with the rest of the file)? https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:494: IAudioEndpointVolume* pVolume = NULL; See comments above. Also, you duplicate code here. Can you break out a static helper to avoid that. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:495: HRESULT hr2 = Try to clean this up and avoid using a second HRESULT here. Makes it hare to follow the error handling IMHO. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:501: if (SUCCEEDED(hr2) && !muted) { Improve comments and explain why you always mute here. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.h (right): https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.h:210: // True, if we have muted the system audio for the stream capturing. Extend the comments here and make it clear that the member is only touched under special circumstances (device_id_ == AudioDeviceDescription::kLoopbackWithMuteDeviceId). Also, make it more clear that it is not set by any external API but to store a local state.
The CQ bit was checked by qiangchen@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: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Patchset #3 (id:120001) has been deleted
Patchset #3 (id:140001) has been deleted
The CQ bit was checked by qiangchen@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...
Patchset #3 (id:160001) has been deleted
The CQ bit was checked by qiangchen@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: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by qiangchen@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: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Patchset #4 (id:200001) has been deleted
The CQ bit was checked by qiangchen@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...
Fixed. https://codereview.chromium.org/2144333002/diff/100001/media/audio/audio_devi... File media/audio/audio_device_description.h (right): https://codereview.chromium.org/2144333002/diff/100001/media/audio/audio_devi... media/audio/audio_device_description.h:33: static const char kLoopbackWithMuteDeviceId[]; On 2016/07/25 08:38:19, henrika wrote: > Please add a comment for the new constant. Done. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.cc (right): https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:170: if (device_id_ == AudioDeviceDescription::kLoopbackWithMuteDeviceId && On 2016/07/25 08:38:19, henrika wrote: > Add a comment on 169 explaining what you do here. Done. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:175: NULL, reinterpret_cast<void**>(&pVolume)); On 2016/07/25 08:38:19, henrika wrote: > Can you use base::win::ScopedComPtr<IAudioEndpointVolume> here as well (your > style is not in line with the rest of the file)? Done. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:494: IAudioEndpointVolume* pVolume = NULL; On 2016/07/25 08:38:19, henrika wrote: > See comments above. Also, you duplicate code here. Can you break out a static > helper to avoid that. Done. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:495: HRESULT hr2 = On 2016/07/25 08:38:19, henrika wrote: > Try to clean this up and avoid using a second HRESULT here. Makes it hare to > follow the error handling IMHO. Done. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:501: if (SUCCEEDED(hr2) && !muted) { On 2016/07/25 08:38:19, henrika wrote: > Improve comments and explain why you always mute here. Done. https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.h (right): https://codereview.chromium.org/2144333002/diff/100001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.h:210: // True, if we have muted the system audio for the stream capturing. On 2016/07/25 08:38:19, henrika wrote: > Extend the comments here and make it clear that the member is only touched under > special circumstances (device_id_ == > AudioDeviceDescription::kLoopbackWithMuteDeviceId). Also, make it more clear > that it is not set by any external API but to store a local state. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
LGTM with nits https://codereview.chromium.org/2144333002/diff/220001/media/audio/audio_devi... File media/audio/audio_device_description.h (right): https://codereview.chromium.org/2144333002/diff/220001/media/audio/audio_devi... media/audio/audio_device_description.h:34: // Similar with |kLoopbackInputDeviceId|, with only difference that this ID nit, 'Similar to' instead of 'Similar with' https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.cc (right): https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:173: if (system_audio_volume_) { What is system_audio_volume_ is NULL? Add DCHECK perhaps just in case? Or a log message. https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:386: double audio_delay_frames = Indentation looks really odd here. Think I like the old version better. https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:492: // If the system audio is mute at the time of capturing, then no need to muted https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.h (right): https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.h:218: // indicate we need to unmute the system audio when stopping capturing. ..and indicates that we need to...
(focused on Windows parts)
Patchset #5 (id:240001) has been deleted
The CQ bit was checked by qiangchen@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: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by qiangchen@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...
dalecurtis@ and miu@: can you take a look at this CL? Henrik has finished his review on windows part. https://codereview.chromium.org/2144333002/diff/220001/media/audio/audio_devi... File media/audio/audio_device_description.h (right): https://codereview.chromium.org/2144333002/diff/220001/media/audio/audio_devi... media/audio/audio_device_description.h:34: // Similar with |kLoopbackInputDeviceId|, with only difference that this ID On 2016/07/26 10:04:25, henrika wrote: > nit, 'Similar to' instead of 'Similar with' Done. https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.cc (right): https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:173: if (system_audio_volume_) { On 2016/07/26 10:04:25, henrika wrote: > What is system_audio_volume_ is NULL? Add DCHECK perhaps just in case? Or a log > message. Done. https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:386: double audio_delay_frames = On 2016/07/26 10:04:25, henrika wrote: > Indentation looks really odd here. Think I like the old version better. git cl format changed it to this way. If I change it back, it cannot pass presubmit check. https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.cc:492: // If the system audio is mute at the time of capturing, then no need to On 2016/07/26 10:04:25, henrika wrote: > muted Done. https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... File media/audio/win/audio_low_latency_input_win.h (right): https://codereview.chromium.org/2144333002/diff/220001/media/audio/win/audio_... media/audio/win/audio_low_latency_input_win.h:218: // indicate we need to unmute the system audio when stopping capturing. On 2016/07/26 10:04:25, henrika wrote: > ..and indicates that we need to... Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Why have the media/audio/win files changed so much? I don't see what git cl format did that should have caused that. Can you revert that from this CL and submit it as a separate patch? It's impossible to review those now. https://codereview.chromium.org/2144333002/diff/280001/media/audio/cras/cras_... File media/audio/cras/cras_input.cc (right): https://codereview.chromium.org/2144333002/diff/280001/media/audio/cras/cras_... media/audio/cras/cras_input.cc:244: if (mute_system_audio_ && mute_done_) { Open is called once, but Start()/Stop() may be called multiple times. Is this the right place for this?
On 2016/07/26 23:42:58, DaleCurtis wrote: > Why have the media/audio/win files changed so much? I don't see what git cl > format did that should have caused that. Can you revert that from this CL and > submit it as a separate patch? It's impossible to review those now. > > https://codereview.chromium.org/2144333002/diff/280001/media/audio/cras/cras_... > File media/audio/cras/cras_input.cc (right): > > https://codereview.chromium.org/2144333002/diff/280001/media/audio/cras/cras_... > media/audio/cras/cras_input.cc:244: if (mute_system_audio_ && mute_done_) { > Open is called once, but Start()/Stop() may be called multiple times. Is this > the right place for this? That's from "git cl format", the automatical coding style fix. Without doing that, the trybot presubmit will fail.
Sure, but can you do a white space change to those files and cl format them before submitting this CL?>
You should also be able to update the hyper-blame ignore list to skip that during blame; though I don't know how to do that yet.
Yes, please avoid making "git cl format" changes in this CL. It blocks the view.
Patchset #7 (id:300001) has been deleted
Patchset #9 (id:360001) has been deleted
The CQ bit was checked by qiangchen@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...
Moved the mute to Start() Removed the git cl format. https://codereview.chromium.org/2144333002/diff/280001/media/audio/cras/cras_... File media/audio/cras/cras_input.cc (right): https://codereview.chromium.org/2144333002/diff/280001/media/audio/cras/cras_... media/audio/cras/cras_input.cc:244: if (mute_system_audio_ && mute_done_) { On 2016/07/26 23:42:58, DaleCurtis wrote: > Open is called once, but Start()/Stop() may be called multiple times. Is this > the right place for this? Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Doesn't look like cl format has been removed, still massive diffs.
On 2016/07/27 17:55:23, DaleCurtis wrote: > Doesn't look like cl format has been removed, still massive diffs. Mine looks good. You can view the diff betwen patch 9 and the master. You cannot view the diff between patch 9 and patch 4, 5 or 6, because patch 4,5,6 are post "git cl format".
? I'm just looking at the default view. I.e. what shows up directly on https://codereview.chromium.org/2144333002 without any comparison between patch sets. I.e. what's in the tree and what you're trying to submit.
On 2016/07/27 18:31:04, DaleCurtis wrote: > ? I'm just looking at the default view. I.e. what shows up directly on > https://codereview.chromium.org/2144333002 without any comparison between patch > sets. I.e. what's in the tree and what you're trying to submit. Ah, it looks like the git tool on my windows machine is problematic when generating change patch. It is using simplest way, as if you delete everything and insert everything. Do you mind to view the "side by side" diff, that looks good.
I think something is misconfigured on your machine. Even the side by side diff is wrong for me.
Patchset #9 (id:380001) has been deleted
On 2016/07/27 20:36:04, DaleCurtis wrote: > I think something is misconfigured on your machine. Even the side by side diff > is wrong for me. I download the patch to my linux machine, and re upload it again. Should look fine now. Thanks.
lgtm
The CQ bit was checked by qiangchen@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...
henrika@: Can you take a look again. I moved the mute process to Start().
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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-...) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by qiangchen@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: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 ========== to ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 NOPRESUBMIT=true ==========
LGTM (it looks better now, thanks)
The CQ bit was checked by qiangchen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/2144333002/#ps420001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 NOPRESUBMIT=true ========== to ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 NOPRESUBMIT=true ==========
Message was sent while issue was closed.
Committed patchset #10 (id:420001)
Message was sent while issue was closed.
Description was changed from ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 NOPRESUBMIT=true ========== to ========== MuteSource Audio During Full Screen Cast Previously, when casting a chrome tab, the source audio is automatically muted; however, casting the full screen, the system audio is still audible. This CL makes the behavior consistent. P.S. But the technique is different for muting a tab and muting system audio. 1. For muting tab, we divert the audio stream from speaker to WebContentsAudioInputStream. 2. For muting system audio, we call the OS API to do so, which is just as if the user click the "mute" button on the task bar. Visible difference is that during capturing, the user is not able to unmute tab audio on the capture side, but nothing can prevent the end user to unmute system audio. BUG=629561 NOPRESUBMIT=true Committed: https://crrev.com/a282a4569fe0786fe2bec8c9d0bfbcadff276330 Cr-Commit-Position: refs/heads/master@{#408672} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/a282a4569fe0786fe2bec8c9d0bfbcadff276330 Cr-Commit-Position: refs/heads/master@{#408672} |
