Reformat existing code. There should be no functional effects.

Reformat existing code.  There should be no functional effects.

This includes changes like:
* Attempt to break lines at better positions
* Use "override" in more places, don't use "virtual" with it
* Use {} where the body is more than one line
* Make declaration and definition arg names match
* Eliminate unused code
* EXPECT_EQ(expected, actual) (but use (actual, expected) for e.g. _GT)
* Correct #include order
* Use anonymous namespaces in preference to "static" for file-scoping
* Eliminate unnecessary casts
* Update reference code in comments of ARM assembly sources to match actual current C code
* Fix indenting to be more style-guide compliant
* Use arraysize() in more places
* Use bool instead of int for "boolean" values (0/1)
* Shorten and simplify code
* Spaces around operators
* 80 column limit
* Use const more consistently
* Space goes after '*' in type name, not before
* Remove unnecessary return values
* Use "(var == const)", not "(const == var)"
* Spelling
* Prefer true, typed constants to "enum hack" constants
* Avoid "virtual" on non-overridden functions
* ASSERT(x == y) -> ASSERT_EQ(y, x)

BUG=none
R=andrew@webrtc.org, asapersson@webrtc.org, henrika@webrtc.org, juberti@webrtc.org, kjellander@webrtc.org, kwiberg@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/728d9037c016c01295177fa700fc7927f0bb80bb

[Peter Kasting, 2015-06-10 02:13:55]

pkasting@chromium.org changed reviewers:
+ andrew@webrtc.org, asapersson@webrtc.org, henrika@webrtc.org,
juberti@webrtc.org, kjellander@webrtc.org, kwiberg@webrtc.org, wez@chromium.org

[Peter Kasting, 2015-06-10 02:15:55]

This is basically a catchall change the covers all the non-functional bits not
in my other recent changes that I want to land before converting a variety of
things to size_t.

The individual files are mostly independent (other than a few cases where I
changed function prototypes, e.g. to eliminate return values).

juberti: talk/app/webrtc/ OWNERS
andrew: webrtc/common_audio/, webrtc/modules/audio_processing/,
webrtc/modules/interface/ OWNERS
kwiberg: webrtc/modules/audio_coding/ OWNERS
henrika: webrtc/modules/audio_device/, webrtc/system_wrappers/,
webrtc/voice_engine/ OWNERS
wez: webrtc/modules/desktop_capture/ OWNERS
asapersson: webrtc/modules/utility/ OWNERS
kjellander: webrtc/test/, webrtc/tools/ OWNERS

[juberti1, 2015-06-10 05:36:52]

lgtm

[kjellander_webrtc, 2015-06-10 06:12:24]

On 2015/06/10 at 05:36:52, juberti wrote:
> lgtm

webrtc/{test,tools}: lgtm

[Andrew MacDonald, 2015-06-10 06:23:56]

lgtm on my parts with a nit and one potentially legitimate concern.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
File webrtc/common_audio/resampler/sinc_resampler.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
webrtc/common_audio/resampler/sinc_resampler.cc:100: 
I prefer to drop the vertical line between nested namespaces (and we do the same
elsewhere) but don't insist.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
File webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.cc (left):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.cc:28: double
duration = static_cast<double>(total_samples_) / sample_rate_;
You have a pending CL which converts sample_rate_ to an int, in which case this
will be an integer divide and you'll need the cast.

[åsapersson, 2015-06-10 07:50:42]

lgtm: webrtc/modules/utility/

[henrika_webrtc, 2015-06-10 08:11:25]

LGTM: webrtc/modules/audio_device/, webrtc/system_wrappers/

[henrika_webrtc, 2015-06-10 08:11:35]

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_device/a...
File webrtc/modules/audio_device/audio_device_buffer.cc (left):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_device/a...
webrtc/modules/audio_device/audio_device_buffer.cc:409: if (nSamples !=
_recSamples)
Not sure why these lines are removed. Care to elaborate.

[kwiberg-webrtc, 2015-06-10 11:58:33]

lgtm for webrtc/modules/audio_coding/

with (mostly) optional nits

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g711/test/testG711.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g711/test/testG711.cc:32: return true;
Umm... this return true if it failed and false if it succeeded? Please add that
to the comment...

Oh, and put it in an anonymous namespace while you're at it.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g711/test/testG711.cc:41: bool endfile;
Consider moving declarations to first use while you're at it.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/test/testG722.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/test/testG722.cc:38: return true;
Comment return value. Namespace.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/test/testG722.cc:47: bool endfile;
Move to first use?

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.cc:30: (kSampleRateHz
/ 100 * (frame_size_ms / 10)) <= kMaxSamplesPerPacket;
I preferred the old way---it's easier to parse piecemeal. The new expression is
quite a mouthful.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/ilbc/cb_mem_energy_calc.c (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/ilbc/cb_mem_energy_calc.c:32: int16_t
base_size  /* (i) Index to where energy values should be stored */
Definite article removal FW!

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c:284: {
Consider moving opening brace to line 283 while you're at it.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/expand.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/expand.cc:851: assert(num_noise_samples <=
static_cast<size_t>(kMaxSampleRate / 8000 * 125));
DCHECK_LE?

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/n...
File webrtc/modules/audio_coding/neteq/merge.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_coding/n...
webrtc/modules/audio_coding/neteq/merge.cc:369: ((best_correlation_index +
input_length) < start_position)) {
Why the extra parentheses? There were too many to begin with.

To get rid of the repetition, maybe

  best_correlation_index + input_length
    < std::max<int>(timestamps_per_call_ + expand_->overlap_length(),
                    start_position)

but to make it truly readable one would probably have to extract at least one
named subexpression.

[Wez, 2015-06-10 17:39:31]

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/window_capture_utils.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/window_capture_utils.cc:32: if
(window_placement.showCmd == SW_SHOWMAXIMIZED) {
This changes behaviour in the case of showCmd=SW_SHOWNORMAL (=1) - AFAICT it
fixes things to behave as intended, but we should test that.

[Peter Kasting, 2015-06-11 04:31:41]

On 2015/06/10 08:11:25, henrika_webrtc wrote:
> LGTM: webrtc/modules/audio_device/, webrtc/system_wrappers/

Please also look at
webrtc/voice_engine/test/auto_test/standard/external_media_test.cc.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
File webrtc/common_audio/resampler/sinc_resampler.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
webrtc/common_audio/resampler/sinc_resampler.cc:100: 
On 2015/06/10 06:23:56, andrew wrote:
> I prefer to drop the vertical line between nested namespaces (and we do the
same
> elsewhere) but don't insist.

Done.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
File webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.cc (left):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/common_audio/resampler...
webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.cc:28: double
duration = static_cast<double>(total_samples_) / sample_rate_;
On 2015/06/10 06:23:56, andrew wrote:
> You have a pending CL which converts sample_rate_ to an int, in which case
this
> will be an integer divide and you'll need the cast.

You're right.  Reverted this.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_device/a...
File webrtc/modules/audio_device/audio_device_buffer.cc (left):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/audio_device/a...
webrtc/modules/audio_device/audio_device_buffer.cc:409: if (nSamples !=
_recSamples)
On 2015/06/10 08:11:35, henrika_webrtc wrote:
> Not sure why these lines are removed. Care to elaborate.

On line 401 above we unconditionally set |_recSamples| equal to |nSamples|. 
Then we make no change to either variable before reaching here.  Therefore, this
conditional can never be true.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/window_capture_utils.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/window_capture_utils.cc:32: if
(window_placement.showCmd == SW_SHOWMAXIMIZED) {
On 2015/06/10 17:39:31, Wez wrote:
> This changes behaviour in the case of showCmd=SW_SHOWNORMAL (=1) - AFAICT it
> fixes things to behave as intended, but we should test that.

Right, I goofed up in my mental arithmetic thinking this made no functional
change when including it in this CL.

IIRC, I changed this because I ran into a testcase locally that erroneously
broke here, and making this change fixed the test.  But I don't remember what
the test was.  I do know this change passes all Windows trybots and, as you
said, looks more correct.

Are there specific other tests you'd like me to do here?  Would you like me to
split this out to a separate CL to land?  Let me know how to proceed.

[Wez, 2015-06-11 17:00:47]

wez@chromium.org changed reviewers:
+ jiayl@chromium.org

[Wez, 2015-06-11 17:00:48]

+jiayl@, who I believe wrote the GetCroppedWindowRect() method, to confirm its
use/test procedure.

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/desktop_captur...
File webrtc/modules/desktop_capture/win/window_capture_utils.cc (right):

https://codereview.webrtc.org/1172163004/diff/1/webrtc/modules/desktop_captur...
webrtc/modules/desktop_capture/win/window_capture_utils.cc:32: if
(window_placement.showCmd == SW_SHOWMAXIMIZED) {
On 2015/06/11 04:31:41, Peter Kasting wrote:
> On 2015/06/10 17:39:31, Wez wrote:
> > This changes behaviour in the case of showCmd=SW_SHOWNORMAL (=1) - AFAICT it
> > fixes things to behave as intended, but we should test that.
> 
> Right, I goofed up in my mental arithmetic thinking this made no functional
> change when including it in this CL.
> 
> IIRC, I changed this because I ran into a testcase locally that erroneously
> broke here, and making this change fixed the test.  But I don't remember what
> the test was.  I do know this change passes all Windows trybots and, as you
> said, looks more correct.
> 
> Are there specific other tests you'd like me to do here?  Would you like me to
> split this out to a separate CL to land?  Let me know how to proceed.

Ugh... seems a waste to break this out to a separate CL but it'd let you keep
this one "clean" from a semantics PoV - up to you.

Wherever it lands, the CL description should include a line like:

"TEST=Verify that Chrome's WebRTC window-capture feature can capture an
unoccluded, non-maximized window correctly under Windows 8 or above."

[Peter Kasting, 2015-06-11 20:08:17]

On 2015/06/11 17:00:48, Wez wrote:
> Ugh... seems a waste to break this out to a separate CL but it'd let you keep
> this one "clean" from a semantics PoV - up to you.

I'll split it out, that way I won't have to wait for any further LGTMs on this
change.

[Peter Kasting, 2015-06-11 20:08:29]

pkasting@chromium.org changed reviewers:
- wez@chromium.org

[commit-bot: I haz the power, 2015-06-11 21:31:57]

Patchset 3 (id:??) landed as
https://crrev.com/728d9037c016c01295177fa700fc7927f0bb80bb
Cr-Commit-Position: refs/heads/master@{#9420}

[Peter Kasting, 2015-06-11 21:32:31]

Committed patchset #3 (id:40001) manually as
728d9037c016c01295177fa700fc7927f0bb80bb (presubmit successful).

[Ben Chan, 2015-06-17 14:48:55]

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

[Ben Chan, 2015-06-17 14:48:56]

https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
File webrtc/system_wrappers/interface/aligned_array.h (left):

https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
webrtc/system_wrappers/interface/aligned_array.h:71: int rows() const {
As a helper class, the rows() and cols() getters seem quite useful. This class
doesn't provide any iterator interface, which means a user of this class likely
needs to memorize the number of rows and columns separately. It seems the
benefit of keeping these two getters outweighs the cost, if any.

[Peter Kasting, 2015-06-17 19:47:29]

https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
File webrtc/system_wrappers/interface/aligned_array.h (left):

https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
webrtc/system_wrappers/interface/aligned_array.h:71: int rows() const {
On 2015/06/17 14:48:56, Ben Chan wrote:
> As a helper class, the rows() and cols() getters seem quite useful. This class
> doesn't provide any iterator interface, which means a user of this class
likely
> needs to memorize the number of rows and columns separately. It seems the
> benefit of keeping these two getters outweighs the cost, if any.

No one called these.  I have no opposition to adding them in the future if
someone wants to call them, but if they're simply unused I don't think they
should be publicly visible.

[Ben Chan, 2015-06-17 20:05:42]

On 2015/06/17 19:47:29, Peter Kasting wrote:
>
https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
> File webrtc/system_wrappers/interface/aligned_array.h (left):
> 
>
https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
> webrtc/system_wrappers/interface/aligned_array.h:71: int rows() const {
> On 2015/06/17 14:48:56, Ben Chan wrote:
> > As a helper class, the rows() and cols() getters seem quite useful. This
class
> > doesn't provide any iterator interface, which means a user of this class
> likely
> > needs to memorize the number of rows and columns separately. It seems the
> > benefit of keeping these two getters outweighs the cost, if any.
> 
> No one called these.  I have no opposition to adding them in the future if
> someone wants to call them, but if they're simply unused I don't think they
> should be publicly visible.

see cl/96202716

[kjellander_webrtc, 2015-06-18 08:22:46]

https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
File webrtc/system_wrappers/interface/aligned_array.h (left):

https://codereview.webrtc.org/1172163004/diff/40001/webrtc/system_wrappers/in...
webrtc/system_wrappers/interface/aligned_array.h:71: int rows() const {
On 2015/06/17 19:47:29, Peter Kasting wrote:
> On 2015/06/17 14:48:56, Ben Chan wrote:
> > As a helper class, the rows() and cols() getters seem quite useful. This
class
> > doesn't provide any iterator interface, which means a user of this class
> likely
> > needs to memorize the number of rows and columns separately. It seems the
> > benefit of keeping these two getters outweighs the cost, if any.
> 
> No one called these.  I have no opposition to adding them in the future if
> someone wants to call them, but if they're simply unused I don't think they
> should be publicly visible.

Since it seems there's code that uses them outside the WebRTC repo, I think we
should add them back. I made https://codereview.webrtc.org/1178043005/ for that.