reflow comments in platform/audio

reflow comments in platform/audio

BUG=563793
Committed: https://crrev.com/4a8a7ad6155544b087fdedff4bcb4e4a5a35bbd1
Cr-Commit-Position: refs/heads/master@{#422366}

[Nico, 2016-10-03 00:11:29]

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-03 00:11:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2016-10-03 00:11:56]

thakis@chromium.org changed reviewers:
+ dcheng@chromium.org

[commit-bot: I haz the power, 2016-10-03 01:14:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-03 01:14:06]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-10-03 01:21:04]

LGTM with some nits

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioArray.h (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioArray.h:99: // first alignment
failure.
Nit: just move this comment to the previous line

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioBus.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioBus.cpp:78: // channel lengths
Nit: move to previous line

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioBus.cpp:535: // rid of this
DenormalDisabler::flushDenormalFloatToZero() call.
Nit: merge into previous line

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioUtilities.h (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioUtilities.h:37: // each node in
the
Nit: merge next line

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/Biquad.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/Biquad.cpp:56: // rendering quantum of
128
Nit: merge next line

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/DownSampler.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/DownSampler.cpp:60: // right in the
middle at halfSize, which is 0.5 and we'll handle specially
Nit: I think "right" can fit on the previous line

(Sorry, this is distracting to me :P)

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.h
(right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.h:126: //
portion of the curve
Nit: merge

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/HRTFDatabase.h (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/HRTFDatabase.h:56: // - 1
(corresponding to angles of 0 -> 360).
Nit: this expression is a bit hard to parse correctly split up like this, don't
have a good suggestion though

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/MultiChannelResampler.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/MultiChannelResampler.cpp:91: size_t
m_framesToProcess;  // Used to verify that all channels ask for the
Nit: put this comment on the previous line to avoid wrapping

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/ReverbConvolver.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/ReverbConvolver.cpp:71: // stages will
double in size each time
Nit: MIght be more readable if comments aren't at end of line? I'm not sure
quite sure what the original intent here was though.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/SincResampler.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/SincResampler.cpp:51: //               
                <-------------------------------------------->
Is the original alignment here intentional? r5 lines up with the end of r2 and
spans to the end of r4.

[Nico, 2016-10-03 02:15:24]

Thanks! Kind of scary how much of these I missed, even though I tried pretty
hard not to.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioArray.h (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioArray.h:99: // first alignment
failure.
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: just move this comment to the previous line

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioBus.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioBus.cpp:535: // rid of this
DenormalDisabler::flushDenormalFloatToZero() call.
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: merge into previous line

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/AudioUtilities.h (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/AudioUtilities.h:37: // each node in
the
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: merge next line

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/Biquad.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/Biquad.cpp:56: // rendering quantum of
128
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: merge next line

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/DownSampler.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/DownSampler.cpp:60: // right in the
middle at halfSize, which is 0.5 and we'll handle specially
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: I think "right" can fit on the previous line
> 
> (Sorry, this is distracting to me :P)

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.h
(right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.h:126: //
portion of the curve
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: merge

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/HRTFDatabase.h (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/HRTFDatabase.h:56: // - 1
(corresponding to angles of 0 -> 360).
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: this expression is a bit hard to parse correctly split up like this,
don't
> have a good suggestion though

Done.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/MultiChannelResampler.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/MultiChannelResampler.cpp:91: size_t
m_framesToProcess;  // Used to verify that all channels ask for the
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: put this comment on the previous line to avoid wrapping

Done (I'm not sure I agree with it though; it's 2 lines either way and this way
it's more clear what the comment refers to)

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/ReverbConvolver.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/ReverbConvolver.cpp:71: // stages will
double in size each time
On 2016/10/03 01:21:04, dcheng wrote:
> Nit: MIght be more readable if comments aren't at end of line? I'm not sure
> quite sure what the original intent here was though.

I think this is pretty good as-is.

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/audio/SincResampler.cpp (right):

https://codereview.chromium.org/2384073002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/audio/SincResampler.cpp:51: //               
                <-------------------------------------------->
On 2016/10/03 01:21:04, dcheng wrote:
> Is the original alignment here intentional? r5 lines up with the end of r2 and
> spans to the end of r4.

Done. (Yes: r0 is blockSize + kernelSize / 2 large, r5 is blockSize large, and
r2 is kernelSize / 2. So they better line up -- good catch, thanks!)

[Nico, 2016-10-03 02:15:30]

The CQ bit was checked by thakis@chromium.org

[Nico, 2016-10-03 02:15:32]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2384073002/#ps20001
(title: "comments (heh!)")

[commit-bot: I haz the power, 2016-10-03 02:15:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raymond Toy, 2016-10-03 02:36:39]

rtoy@chromium.org changed reviewers:
+ rtoy@chromium.org

[Raymond Toy, 2016-10-03 02:36:40]

Will this cause merge conflicts for CLs that are in progress?

[Nico, 2016-10-03 02:50:23]

On 2016/10/03 02:36:40, Raymond Toy wrote:
> Will this cause merge conflicts for CLs that are in progress?

If they touch the same comments, then yes.

[commit-bot: I haz the power, 2016-10-03 03:42:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-03 03:47:11]

Description was changed from

==========
reflow comments in platform/audio

BUG=563793
==========

to

==========
reflow comments in platform/audio

BUG=563793

Committed: https://crrev.com/4a8a7ad6155544b087fdedff4bcb4e4a5a35bbd1
Cr-Commit-Position: refs/heads/master@{#422366}
==========

[commit-bot: I haz the power, 2016-10-03 03:47:12]

Patchset 2 (id:??) landed as
https://crrev.com/4a8a7ad6155544b087fdedff4bcb4e4a5a35bbd1
Cr-Commit-Position: refs/heads/master@{#422366}