Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2646423005:
Remove the wave based audio capture implementation for Windows (Closed)
|
Created:
3 years, 11 months ago by tommi (sloooow) - chröme Modified:
3 years, 11 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove the wave based audio capture implementation for Windows
BUG=684741
CQ_INCLUDE_TRYBOTS=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/2646423005
Cr-Commit-Position: refs/heads/master@{#446459}
Committed: https://chromium.googlesource.com/chromium/src/+/a589a11cc8809c88d62b32a5810d6994fad45aaa
Patch Set 1 #Patch Set 2 : Remove wave input code #
Total comments: 24
Patch Set 3 : Update histograms and call IsSupported in ctor #
Total comments: 4
Patch Set 4 : Simplify error handling #Patch Set 5 : Address comments #Patch Set 6 : Minor cleanup + documentation #
Messages
Total messages: 79 (47 generated)
Description was changed from ========== Test disabling high latency audio input on Windows BUG= ========== to ========== Test disabling high latency audio input on Windows BUG= CQ_INCLUDE_TRYBOTS=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 ==========
The CQ bit was checked by tommi@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...
Remove wave input code
The CQ bit was checked by tommi@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: mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)
Description was changed from ========== Test disabling high latency audio input on Windows BUG= CQ_INCLUDE_TRYBOTS=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 ========== to ========== Remove the wave based audio capture implementation for Windows BUG=684741 CQ_INCLUDE_TRYBOTS=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 ==========
tommi@chromium.org changed reviewers: + dalecurtis@chromium.org, henrika@chromium.org
(fixing subject and sending review request out again)
The CQ bit was checked by tommi@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...
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (left): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:142: enumeration_type_(CoreAudioUtil::IsSupported() ? kMMDeviceEnumeration This may be a race now in the setup of the internals for CoreAudioUtil::IsSupported(). Possibly it doesn't matter, but we may want to change that to a lazy_instance. https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:277: CoreAudioUtil::GetPreferredAudioParameters(device_id, false, ¶meters); This method will DCHECK() IsSupported(). I don't fully understand all the failure cases there, but if the DLLs or anything are missing we might get crashes instead of graceful failure. See l.219 in the CoreAudioUtil code. https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:379: if (cmd_line->HasSwitch(switches::kEnableExclusiveAudio)) { Hmm, I wonder if we should remove the exclusive audio support too. Is anyone using it? https://codereview.chromium.org/2646423005/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2646423005/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:26696: -<histogram name="Media.WindowsCoreAudioInput" enum="BooleanSuccess"> This should be left and marked as deprecated.
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:46: DEFINE_GUID(AM_KSCATEGORY_AUDIO, some changes, like this one, are due to git cl format. https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:282: // elsewhere if we weren't able to get the preferred audio parameters. Should we remove this block and return invalid audio parameters if this happens? Maybe add a UMA stat? https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:407: use_input_params = true; this is another thing that I think we may have unnecessary complexity. I could remove this part and the use_input_params variable.
Update histograms and call IsSupported in ctor
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (left): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:142: enumeration_type_(CoreAudioUtil::IsSupported() ? kMMDeviceEnumeration On 2017/01/24 21:09:30, DaleCurtis wrote: > This may be a race now in the setup of the internals for > CoreAudioUtil::IsSupported(). Possibly it doesn't matter, but we may want to > change that to a lazy_instance. Ah, thanks for pointing that out. I had forgotten that Win7 actually had issues there as well (wonder the status of that now, almost 4 years later). What do you think about keeping this comment, move it into the ctor body and call IsSupported directly? Henrik has a good point about the wave API being built on top of CoreAudio so I think that if CoreAudio itself doesn't work, there may not be much we can do and the methods for creating audio streams, will fail, but at least we can attempt to make them. https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:277: CoreAudioUtil::GetPreferredAudioParameters(device_id, false, ¶meters); On 2017/01/24 21:09:30, DaleCurtis wrote: > This method will DCHECK() IsSupported(). I don't fully understand all the > failure cases there, but if the DLLs or anything are missing we might get > crashes instead of graceful failure. See l.219 in the CoreAudioUtil code. I read over the bug, refreshing my memory. In the cases of missing DLLs, we'll get an error reported back (The specified module could not be found) https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:379: if (cmd_line->HasSwitch(switches::kEnableExclusiveAudio)) { On 2017/01/24 21:09:30, DaleCurtis wrote: > Hmm, I wonder if we should remove the exclusive audio support too. Is anyone > using it? Good question. Perhaps we could add a counter for that. I doubt its being used much but the reason for it existing, is to be able to lower latency. https://codereview.chromium.org/2646423005/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/2646423005/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:26696: -<histogram name="Media.WindowsCoreAudioInput" enum="BooleanSuccess"> On 2017/01/24 21:09:30, DaleCurtis wrote: > This should be left and marked as deprecated. Done.
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (left): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:142: enumeration_type_(CoreAudioUtil::IsSupported() ? kMMDeviceEnumeration On 2017/01/24 at 21:34:57, tommi (krómíum) wrote: > On 2017/01/24 21:09:30, DaleCurtis wrote: > > This may be a race now in the setup of the internals for > > CoreAudioUtil::IsSupported(). Possibly it doesn't matter, but we may want to > > change that to a lazy_instance. > > Ah, thanks for pointing that out. I had forgotten that Win7 actually had issues there as well (wonder the status of that now, almost 4 years later). > What do you think about keeping this comment, move it into the ctor body and call IsSupported directly? Henrik has a good point about the wave API being built on top of CoreAudio so I think that if CoreAudio itself doesn't work, there may not be much we can do and the methods for creating audio streams, will fail, but at least we can attempt to make them. I think that's the safest change for now. It'd be nice if we could leave this on the audio thread, but I think you had to move it here to fix races with other code (possibly the device enumeration code now being removed, hopefully) https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:277: CoreAudioUtil::GetPreferredAudioParameters(device_id, false, ¶meters); On 2017/01/24 at 21:34:56, tommi (krómíum) wrote: > On 2017/01/24 21:09:30, DaleCurtis wrote: > > This method will DCHECK() IsSupported(). I don't fully understand all the > > failure cases there, but if the DLLs or anything are missing we might get > > crashes instead of graceful failure. See l.219 in the CoreAudioUtil code. > > I read over the bug, refreshing my memory. In the cases of missing DLLs, we'll get an error reported back (The specified module could not be found) sgtm https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:282: // elsewhere if we weren't able to get the preferred audio parameters. On 2017/01/24 at 21:10:51, tommi (krómíum) wrote: > Should we remove this block and return invalid audio parameters if this happens? Maybe add a UMA stat? That sgtm, there's not point anymore. https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:379: if (cmd_line->HasSwitch(switches::kEnableExclusiveAudio)) { On 2017/01/24 at 21:34:56, tommi (krómíum) wrote: > On 2017/01/24 21:09:30, DaleCurtis wrote: > > Hmm, I wonder if we should remove the exclusive audio support too. Is anyone > > using it? > > Good question. Perhaps we could add a counter for that. I doubt its being used much but the reason for it existing, is to be able to lower latency. If crashes are any indication, there are 9 reports for all time :p https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20OMIT%20R... https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:407: use_input_params = true; On 2017/01/24 at 21:10:51, tommi (krómíum) wrote: > this is another thing that I think we may have unnecessary complexity. I could remove this part and the use_input_params variable. Since we're keeping WaveOut I think we still need this. Possibly this whole thing could be vastly simplified though to just always return input params if failure.
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:175: base::string16 AudioManagerWin::GetAudioInputDeviceModel() { Used at https://cs.chromium.org/chromium/src/content/browser/speech/speech_recognitio... but not sure if it is actually needed? https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:282: // elsewhere if we weren't able to get the preferred audio parameters. sgtm https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:379: if (cmd_line->HasSwitch(switches::kEnableExclusiveAudio)) { Smart to check crashes Dale ;-) I added the support a long time ago but since it was not possible to do the same on input, the actual usage seems very, very limited. Perhaps remove in a separate CL to make it more clear. https://codereview.chromium.org/2646423005/diff/40001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/40001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:288: // elsewhere if we weren't able to get the preferred audio parameters. Possibly add a LOG(WARNING) just in case. https://codereview.chromium.org/2646423005/diff/40001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:366: // Used for both AUDIO_PCM_LOW_LATENCY and AUDIO_PCM_LINEAR. Just FYI, I can only find one client (Pepper) that used LINEAR for input (https://cs.chromium.org/chromium/src/content/renderer/pepper/pepper_platform_...). Might be worth paying extra attention when landing this CL.
Simplify error handling
The CQ bit was checked by tommi@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...
tommi@chromium.org changed reviewers: + haraken@chromium.org
+ haraken for histograms.xml https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:282: // elsewhere if we weren't able to get the preferred audio parameters. On 2017/01/24 21:47:25, DaleCurtis wrote: > On 2017/01/24 at 21:10:51, tommi (krómíum) wrote: > > Should we remove this block and return invalid audio parameters if this > happens? Maybe add a UMA stat? > > That sgtm, there's not point anymore. Done. (just return if we failed or params aren't valid) https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:379: if (cmd_line->HasSwitch(switches::kEnableExclusiveAudio)) { On 2017/01/24 21:47:26, DaleCurtis wrote: > On 2017/01/24 at 21:34:56, tommi (krómíum) wrote: > > On 2017/01/24 21:09:30, DaleCurtis wrote: > > > Hmm, I wonder if we should remove the exclusive audio support too. Is anyone > > > using it? > > > > Good question. Perhaps we could add a counter for that. I doubt its being > used much but the reason for it existing, is to be able to lower latency. > > If crashes are any indication, there are 9 reports for all time :p > > https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20OMIT%20R... Added TODO for henrik to remove the switch and associated code. https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:407: use_input_params = true; On 2017/01/24 21:47:26, DaleCurtis wrote: > On 2017/01/24 at 21:10:51, tommi (krómíum) wrote: > > this is another thing that I think we may have unnecessary complexity. I > could remove this part and the use_input_params variable. > > Since we're keeping WaveOut I think we still need this. Possibly this whole > thing could be vastly simplified though to just always return input params if > failure. Makes sense. Looking at the case where input_params are invalid and we fail to get the preferred audio params, we may actually return valid audio parameters back. Looks like a bug to me and I wonder if we're doing ourselves a favour or creating problems by initializing the variables above, such as sample_rate, with a valid sample rate, without knowing anything about the device...
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:407: use_input_params = true; I kind of have the same feeling. It would probably be better to receive reports where a clear failure happened rather than figuring out that some default setting was used instead under very special circumstances. As you say, we don't know anything about the device.
haraken@chromium.org changed reviewers: + isherman@chromium.org
+isherman (I'm eligible to review UseCounter-only changes.)
histograms.xml lgtm
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_...)
Address comments
https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/20001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:175: base::string16 AudioManagerWin::GetAudioInputDeviceModel() { On 2017/01/25 12:43:33, henrika wrote: > Used at > https://cs.chromium.org/chromium/src/content/browser/speech/speech_recognitio... > but not sure if it is actually needed? It's not needed now that Olga has landed her patch to remove the dependency in content. I'll remove it in a separate cl. https://codereview.chromium.org/2646423005/diff/40001/media/audio/win/audio_m... File media/audio/win/audio_manager_win.cc (right): https://codereview.chromium.org/2646423005/diff/40001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:288: // elsewhere if we weren't able to get the preferred audio parameters. On 2017/01/25 12:43:33, henrika wrote: > Possibly add a LOG(WARNING) just in case. Done. https://codereview.chromium.org/2646423005/diff/40001/media/audio/win/audio_m... media/audio/win/audio_manager_win.cc:366: // Used for both AUDIO_PCM_LOW_LATENCY and AUDIO_PCM_LINEAR. On 2017/01/25 12:43:33, henrika wrote: > Just FYI, I can only find one client (Pepper) that used LINEAR for input > (https://cs.chromium.org/chromium/src/content/renderer/pepper/pepper_platform_...). > Might be worth paying extra attention when landing this CL. Yes, Dale knows a bit about that code. I think it would be good to move that over to low latency in general as there have been some problems with how things are now (see e.g. crbug.com/157613).
The CQ bit was checked by tommi@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...
lgtm thanks for cleaning this up!
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...)
lgtm
The CQ bit was checked by tommi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from isherman@chromium.org Link to the patchset: https://codereview.chromium.org/2646423005/#ps80001 (title: "Address comments")
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: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by tommi@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Remove CHECK that caused webkit tests to fail on win_chromium_rel_ng
The CQ bit was checked by tommi@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...
Remove [D]CHECKs that currently trigger on the Windows 2008 Server bot
The CQ bit was checked by tommi@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: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
Add a check to MakeLowLatencyOutputStream for IsSupported to work around bot issue
Make GetPreferredOutputStreamParameters consistently return invalid params on error
The CQ bit was checked by tommi@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.
Minor cleanup + documentation
The CQ bit was checked by tommi@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 #6 (id:100001) has been deleted
Patchset #6 (id:120001) has been deleted
Patchset #6 (id:140001) has been deleted
Patchset #6 (id:160001) has been deleted
Dale and Henrik - Please take at the difference between patch sets 5 and 6 (3 files changed). It turns out that "Windows Server 2008 R2" can be configured such that "desktop features" aren't available, such as parts of Core Audio. So, in that case, Core Audio methods now do not DCHECK on IsSupported. Error handling was being done correctly anyway. AudioDeviceListenerWin does no longer CHECK() on IsSupported either. In the case where support isn't there, it becomes a noop (which is fine). GetPreferredOutputStreamParameters now returns invalid audio parameters if they can't be gotten from the OS. Interestingly, this is exactly what we talked about a couple of patch sets ago :) The failure on the bot was that we would fail to get the preferred params and still create valid params based on the input params. Those returned params would then be used later to create an audio stream and then we failed miserably due to a missing component. So the right thing to do is to not try to make up stuff if the OS can't give us the correct data ;) Another good thing is that the behavior of the "GetPreferred" methods is now consistent between input and output.
still lgtm with the assumption that wave input wasn't working on 2008R2 in that case or that we don't think this is a use case worth continuing.
Nice catch Tommi! Still LGTM. I wonder what happened on this bot before your patch?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/01/26 19:19:14, DaleCurtis wrote: > still lgtm with the assumption that wave input wasn't working on 2008R2 in that > case or that we don't think this is a use case worth continuing. On 2017/01/26 19:31:48, henrika wrote: > Nice catch Tommi! > > Still LGTM. > > I wonder what happened on this bot before your patch? Thanks both. Yes, audio wasn't working before either. Here's an example printout from that bot before this change: 12:14:24.637 916 [5532:5060:0126/121424.011:2603297:ERROR:audio_output_resampler.cc(310)] Unable to open audio device in low latency mode. Falling back to high latency audio output. 12:14:24.637 916 [5532:5060:0126/121424.012:2603297:ERROR:audio_output_resampler.cc(320)] Unable to open audio device in high latency mode. Falling back to fake audio output. Here's a printout with the change: 11:28:10.756 2640 [4476:4292:0126/112810.652:3647974:ERROR:audio_manager_win.cc(407)] GetPreferredAudioParameters failed: 88890004 11:28:10.756 2640 [4476:4292:0126/112810.652:3647974:ERROR:audio_manager_base.cc(257)] Invalid audio output parameters received; using fake audio path. Channels: 0, Sample Rate: 0, Bits Per Sample: 0, Frames Per Buffer: 0 Now, we essentially hit the below code path and fail earlier. https://cs.chromium.org/chromium/src/media/audio/audio_manager_base.cc?rcl=0&...
The CQ bit was checked by tommi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from isherman@chromium.org Link to the patchset: https://codereview.chromium.org/2646423005/#ps180001 (title: "Minor cleanup + documentation")
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": 180001, "attempt_start_ts": 1485466912847660,
"parent_rev": "a8f908ca6ab0abe86af75c688099d8c9dee1685c", "commit_rev":
"a589a11cc8809c88d62b32a5810d6994fad45aaa"}
Message was sent while issue was closed.
Description was changed from ========== Remove the wave based audio capture implementation for Windows BUG=684741 CQ_INCLUDE_TRYBOTS=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 ========== to ========== Remove the wave based audio capture implementation for Windows BUG=684741 CQ_INCLUDE_TRYBOTS=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/2646423005 Cr-Commit-Position: refs/heads/master@{#446459} Committed: https://chromium.googlesource.com/chromium/src/+/a589a11cc8809c88d62b32a5810d... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:180001) as https://chromium.googlesource.com/chromium/src/+/a589a11cc8809c88d62b32a5810d... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
