Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

R=henrik.lundin@webrtc.org, peah@webrtc.org
TBR=tina.legrand@webrtc.org

Committed: https://crrev.com/40cbec54154da73b9d10195799894b861796f965
Cr-Commit-Position: refs/heads/master@{#12249}

[aluebs-webrtc, 2016-04-05 02:29:57]

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

[hlundin-webrtc, 2016-04-05 06:17:04]

lgtm

No unit test catching this bug? Consider adding one.

[peah-webrtc, 2016-04-05 08:56:05]

On 2016/04/05 06:17:04, hlundin-webrtc wrote:
> lgtm
> 
> No unit test catching this bug? Consider adding one.

lgtm but could you please also give a comment with your view on what problems
have been caused by this. 
Afaics the proc_rate is chosen as min(input and output rates) which should mean
that this never caused a buffer overrun, right? 
But this should also mean that if the output of APM was multichannel, only the
first part of the APM output would have been interlieved, and the second part
would have been deinterlieved, right?

[turaj, 2016-04-05 15:07:44]

I have no further comments, as I'm not familiar with this part of the code.

[aluebs-webrtc, 2016-04-05 22:10:26]

On 2016/04/05 08:56:05, peah-webrtc wrote:
> On 2016/04/05 06:17:04, hlundin-webrtc wrote:
> > lgtm
> > 
> > No unit test catching this bug? Consider adding one.
> 
> lgtm but could you please also give a comment with your view on what problems
> have been caused by this. 
> Afaics the proc_rate is chosen as min(input and output rates) which should
mean
> that this never caused a buffer overrun, right? 
> But this should also mean that if the output of APM was multichannel, only the
> first part of the APM output would have been interlieved, and the second part
> would have been deinterlieved, right?

This bug was only present in the AudioFrame interface, since it was in
InterleaveTo. And in this interface, because the input and output sample rate is
the same (it uses the same AudioFrame), the proc sample rate is chosen to be the
same. But when removing the 48kHz sample rate support for ARM builds this was
not true anymore, since a 48kHz AudioFrame would be processed at 32kHz. So this
bug was only a problem for ARM builds, using the AudioFrame interface and using
48kHz sample rate. It didn't manifest as a buffer overrun, more like a underrun,
since in that case it was only copying the first 320 samples from the 480
buffer. Apparently this was caught by our tests, once I removed the constraint
for 16kHz for the AECM, but I dismissed it as the ARM platforms didn't support
48kHz and that would generate some actual differences, so I only tested up to
32kHz on fixed point. But now I added 48kHz tests for the fixed point case that
would have caught this.

[aluebs-webrtc, 2016-04-05 22:10:37]

The CQ bit was checked by aluebs@webrtc.org

[aluebs-webrtc, 2016-04-05 22:10:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, peah@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1862553002/#ps20001 (title:
"Add 48kHz tests")

[commit-bot: I haz the power, 2016-04-05 22:10:49]

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

[commit-bot: I haz the power, 2016-04-05 22:19:40]

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

[commit-bot: I haz the power, 2016-04-05 22:19:41]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/4620)

[aluebs-webrtc, 2016-04-05 22:21:36]

Description was changed from

==========
Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()
==========

to

==========
Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

TBR=tina.legrand@webrtc.org
==========

[aluebs-webrtc, 2016-04-05 22:21:41]

The CQ bit was checked by aluebs@webrtc.org

[commit-bot: I haz the power, 2016-04-05 22:21:44]

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

[commit-bot: I haz the power, 2016-04-06 00:11:19]

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

[commit-bot: I haz the power, 2016-04-06 00:11:19]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[aluebs-webrtc, 2016-04-06 00:29:33]

Description was changed from

==========
Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

TBR=tina.legrand@webrtc.org
==========

to

==========
Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

R=henrik.lundin@webrtc.org, peah@webrtc.org
TBR=tina.legrand@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/40cbec54154da73b9d1019579...
==========

[aluebs-webrtc, 2016-04-06 00:29:35]

Committed patchset #3 (id:40001) manually as
40cbec54154da73b9d10195799894b861796f965 (presubmit successful).

[commit-bot: I haz the power, 2016-04-06 00:29:36]

Description was changed from

==========
Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

R=henrik.lundin@webrtc.org, peah@webrtc.org
TBR=tina.legrand@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/40cbec54154da73b9d1019579...
==========

to

==========
Fix the number of frames used when interleaving in AudioBuffer::InterleaveTo()

R=henrik.lundin@webrtc.org, peah@webrtc.org
TBR=tina.legrand@webrtc.org

Committed: https://crrev.com/40cbec54154da73b9d10195799894b861796f965
Cr-Commit-Position: refs/heads/master@{#12249}
==========

[commit-bot: I haz the power, 2016-04-06 00:29:38]

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