Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2475953003:
Support floating-point audio output for Windows7+ (Closed)
|
Created:
4 years, 1 month ago by Raymond Toy Modified:
4 years, 1 month ago CC:
chromium-reviews, feature-media-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSupport floating-point audio output for Windows7+
Instead of converting audio to 16-bit PCM, pass the audio data
as floating-point directly on the low-latency path (WASAPI)
BUG=659641
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
TEST=Run test url from bug and set gain to 0.00002 and hear output
Committed: https://crrev.com/1ffd6214d17f7228f307e67b32152866b67d6029
Cr-Commit-Position: refs/heads/master@{#431882}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address review comments #
Total comments: 2
Patch Set 3 : Address review comments #Messages
Total messages: 21 (9 generated)
Description was changed from ========== Support floating-point audio output for Windows7+ Instead of converting audio to 16-bit PCM, pass the audio data as floating-point directly on the low-latency path (WASAPI) BUG=659641 TEST=Run test url from bug and set gain to 0.00002 and hear output ========== to ========== Support floating-point audio output for Windows7+ Instead of converting audio to 16-bit PCM, pass the audio data as floating-point directly on the low-latency path (WASAPI) BUG=659641 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 TEST=Run test url from bug and set gain to 0.00002 and hear output ==========
The CQ bit was checked by rtoy@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.
rtoy@chromium.org changed reviewers: + dalecurtis@chromium.org, tommi@chromium.org
PTAL. This is an updated version of https://codereview.chromium.org/2475453003/ that corrects an error. Main change is that we create a new AudioParameters object that has the desired values and use that everywhere.
Ah, sorry I missed this in the original review. https://codereview.chromium.org/2475953003/diff/1/media/audio/win/audio_low_l... File media/audio/win/audio_low_latency_output_win.cc (right): https://codereview.chromium.org/2475953003/diff/1/media/audio/win/audio_low_l... media/audio/win/audio_low_latency_output_win.cc:95: AudioParameters floatParams = AudioParameters(AudioParameters( float_params also you're constructing three audio parameters in this statement :) Just AudioParameters float_params(params.format(), ... )
https://codereview.chromium.org/2475953003/diff/1/media/audio/win/audio_low_l... File media/audio/win/audio_low_latency_output_win.cc (right): https://codereview.chromium.org/2475953003/diff/1/media/audio/win/audio_low_l... media/audio/win/audio_low_latency_output_win.cc:95: AudioParameters floatParams = AudioParameters(AudioParameters( On 2016/11/07 18:54:00, DaleCurtis wrote: > float_params also you're constructing three audio parameters in this statement > :) Just AudioParameters float_params(params.format(), ... ) Whoa! Don't know how that happened!
https://codereview.chromium.org/2475953003/diff/20001/media/audio/win/audio_l... File media/audio/win/audio_low_latency_output_win.cc (right): https://codereview.chromium.org/2475953003/diff/20001/media/audio/win/audio_l... media/audio/win/audio_low_latency_output_win.cc:95: AudioParameters floatParams = AudioParameters( Still two :) Remove the " = AudioParameters" Also fix naming -- we don't use camelCase in Chromium code.
https://codereview.chromium.org/2475953003/diff/20001/media/audio/win/audio_l... File media/audio/win/audio_low_latency_output_win.cc (right): https://codereview.chromium.org/2475953003/diff/20001/media/audio/win/audio_l... media/audio/win/audio_low_latency_output_win.cc:95: AudioParameters floatParams = AudioParameters( On 2016/11/07 19:13:17, DaleCurtis wrote: > Still two :) Remove the " = AudioParameters" Also fix naming -- we don't use > camelCase in Chromium code. Sorry about that! Should be fixed now, and no camel case.
lgtm
tommi@ Friendly ping, in case you want to take a look as well.
On 2016/11/09 18:19:14, Raymond Toy wrote: > tommi@ Friendly ping, in case you want to take a look as well. tommi@ Friendly ping
lgtm
The CQ bit was checked by rtoy@chromium.org
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 ========== Support floating-point audio output for Windows7+ Instead of converting audio to 16-bit PCM, pass the audio data as floating-point directly on the low-latency path (WASAPI) BUG=659641 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 TEST=Run test url from bug and set gain to 0.00002 and hear output ========== to ========== Support floating-point audio output for Windows7+ Instead of converting audio to 16-bit PCM, pass the audio data as floating-point directly on the low-latency path (WASAPI) BUG=659641 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 TEST=Run test url from bug and set gain to 0.00002 and hear output ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Support floating-point audio output for Windows7+ Instead of converting audio to 16-bit PCM, pass the audio data as floating-point directly on the low-latency path (WASAPI) BUG=659641 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 TEST=Run test url from bug and set gain to 0.00002 and hear output ========== to ========== Support floating-point audio output for Windows7+ Instead of converting audio to 16-bit PCM, pass the audio data as floating-point directly on the low-latency path (WASAPI) BUG=659641 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 TEST=Run test url from bug and set gain to 0.00002 and hear output Committed: https://crrev.com/1ffd6214d17f7228f307e67b32152866b67d6029 Cr-Commit-Position: refs/heads/master@{#431882} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/1ffd6214d17f7228f307e67b32152866b67d6029 Cr-Commit-Position: refs/heads/master@{#431882} |
