Ducking fix #2: Removed the aec state as an input parameter to the FilterFar function.

Removed the aec state as an input parameter to the FilterFar function.

BUG=webrtc:5201
Committed: https://crrev.com/54eb5e2e9ae91e9ad6dcb297de7b918ebe706d5f
Cr-Commit-Position: refs/heads/master@{#10787}

[peah-webrtc, 2015-11-19 10:49:51]

Description was changed from

==========
Removed the aec state as an input parameter to the FilterFar function.



BUG=webrtc:5201
==========

to

==========
Removed the aec state as an input parameter to the FilterFar function.



BUG=webrtc:5201
==========

[peah-webrtc, 2015-11-19 10:50:29]

peah@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, ivoc@webrtc.org

[peah-webrtc, 2015-11-19 10:50:30]

Pure refactoring of the function parameter list. The changes are bitexact.

[hlundin-webrtc, 2015-11-20 10:59:40]

Nits.

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_internal.h (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:175: int xfBufBlockPos,
Let the new parameters be named according to our style guide. You are already
poking on every line that would be affected by this anyway.

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_neon.c (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_neon.c:43: const int
num_partitions_local = num_partitions;
You don't need the local variant, do you?

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_sse2.c:39: const int
num_partitions_local = num_partitions;
Same here: unnecessary?

[ivoc, 2015-11-20 12:59:52]

I have no further comments to add to Henrik's. So LGTM, assuming you address
those.

[peah-webrtc, 2015-11-24 06:48:56]

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_internal.h (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:175: int xfBufBlockPos,
On 2015/11/20 10:59:40, hlundin-webrtc wrote:
> Let the new parameters be named according to our style guide. You are already
> poking on every line that would be affected by this anyway.

Nice! I did that, and also changed the function variables according to the style
guide. I also took the liberty of renaming the parameters be more verbose and
descriptive. They no longer fully match what the state variables are called but
at least I think they are more easy to understand as they are now named. Please
let me know if that is fine!

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_neon.c (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_neon.c:43: const int
num_partitions_local = num_partitions;
On 2015/11/20 10:59:40, hlundin-webrtc wrote:
> You don't need the local variant, do you?

I'm not 100% sure. It is const-declared, and this is code that is supposed to be
optimized for Neon, so in theory it could have an impact.

I agree with you, however, and don't think that it will matter, so I'll remove
the local variable. But please let me know if you think (in light of the above)
that is not the correct thing to do!

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_sse2.c:39: const int
num_partitions_local = num_partitions;
On 2015/11/20 10:59:40, hlundin-webrtc wrote:
> Same here: unnecessary?

Same comment applies as for Neon. Could you please verify that you think it is
ok also here.

[peah-webrtc, 2015-11-24 11:49:21]

According to kwiberg@ it seems like removing the local constant copy of the
input should be fine from an optimization perspective.

[hlundin-webrtc, 2015-11-25 12:49:40]

lgtm

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right):

https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_sse2.c:39: const int
num_partitions_local = num_partitions;
On 2015/11/24 06:48:56, peah-webrtc wrote:
> On 2015/11/20 10:59:40, hlundin-webrtc wrote:
> > Same here: unnecessary?
> 
> Same comment applies as for Neon. Could you please verify that you think it is
> ok also here.

Acknowledged.

https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec/aec_core_mips.c (right):

https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_mips.c:730: "lwc1       %[f0],    
0(%[x_pow])                  \n\t"
Maintain alignment of \n\t.

https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_mips.c:758: "addiu      %[x_pow],  
%[x_pow],     4              \n\t"
Maintain alignment of \n\t.

[peah-webrtc, 2015-11-25 13:20:41]

https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec/aec_core_mips.c (right):

https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_mips.c:730: "lwc1       %[f0],    
0(%[x_pow])                  \n\t"
On 2015/11/25 12:49:40, hlundin-webrtc wrote:
> Maintain alignment of \n\t.

Done.

https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_mips.c:758: "addiu      %[x_pow],  
%[x_pow],     4              \n\t"
On 2015/11/25 12:49:40, hlundin-webrtc wrote:
> Maintain alignment of \n\t.

Done.

[peah-webrtc, 2015-11-25 13:20:48]

The CQ bit was checked by peah@webrtc.org

[peah-webrtc, 2015-11-25 13:20:48]

The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org,
henrik.lundin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1454983006/#ps40001 (title:
"Corrected line alignment")

[commit-bot: I haz the power, 2015-11-25 13:20:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1454983006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1454983006/40001

[commit-bot: I haz the power, 2015-11-25 15:43:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-25 15:43:29]

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