Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

This change results in different checksums for the following tests:

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

Because they make an ACM and then ask it to decode both 16 kHz and 32
kHz iSAC. (The arm32 and arm64 checksums didn't change, because the
tests skip 32 kHz iSAC on arm.)

BUG=webrtc:5801
Committed: https://crrev.com/bfde543f73681b7e4cb41edc0be0088b9068f29f
Cr-Commit-Position: refs/heads/master@{#12463}

[kwiberg-webrtc, 2016-04-21 11:03:16]

kwiberg@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[kwiberg-webrtc, 2016-04-21 11:03:16]

I'm doing this small piece as a separate CL because it requires updates to the
md5 sums for 

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

(Only tests that run both 16 kHz and 32 kHz iSAC are affected, as expected.)

Obviously, I also have to actually update the expected md5 sums in the tests,
but I thought I'd get your input before I did all that work.

[kwiberg-webrtc, 2016-04-21 11:35:27]

Description was changed from

==========
Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

BUG=webrtc:5801
==========

to

==========
Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

This change results in different checksums for the following tests:

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

Because they make an ACM and then ask it to decode both 16 kHz and 32
kHz iSAC. (The arm32 and arm64 checksums didn't change, because the
tests skip 32 kHz iSAC on arm.)

BUG=webrtc:5801
==========

[kwiberg-webrtc, 2016-04-21 11:37:10]

On 2016/04/21 11:03:16, kwiberg-webrtc wrote:

> Obviously, I also have to actually update the expected md5 sums in the tests,
> but I thought I'd get your input before I did all that work.

Checksums updated.

[ossu, 2016-04-21 11:42:58]

On 2016/04/21 11:03:16, kwiberg-webrtc wrote:
> I'm doing this small piece as a separate CL because it requires updates to the
> md5 sums for 
> 
> AcmReceiverBitExactnessOldApi.8kHzOutput
> AcmReceiverBitExactnessOldApi.16kHzOutput
> AcmReceiverBitExactnessOldApi.32kHzOutput
> AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
> AcmReceiverBitExactnessOldApi.48kHzOutput
> 
> (Only tests that run both 16 kHz and 32 kHz iSAC are affected, as expected.)
> 
> Obviously, I also have to actually update the expected md5 sums in the tests,
> but I thought I'd get your input before I did all that work.

Seems reasonable. I guess the iSAC decoder is kept separate like this for now,
but that that will eventually go away?

[kwiberg-webrtc, 2016-04-21 11:46:49]

On 2016/04/21 11:42:58, ossu wrote:
> On 2016/04/21 11:03:16, kwiberg-webrtc wrote:
> > I'm doing this small piece as a separate CL because it requires updates to
the
> > md5 sums for 
> > 
> > AcmReceiverBitExactnessOldApi.8kHzOutput
> > AcmReceiverBitExactnessOldApi.16kHzOutput
> > AcmReceiverBitExactnessOldApi.32kHzOutput
> > AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
> > AcmReceiverBitExactnessOldApi.48kHzOutput
> > 
> > (Only tests that run both 16 kHz and 32 kHz iSAC are affected, as expected.)
> > 
> > Obviously, I also have to actually update the expected md5 sums in the
tests,
> > but I thought I'd get your input before I did all that work.
> 
> Seems reasonable. I guess the iSAC decoder is kept separate like this for now,
> but that that will eventually go away?

The special case for iSAC where it's injected into the ACM rather than created
inside it like the other decoders? Yes, that'll go away once we start injecting
the entire decoder factory. (But this checksum change would have happened at
some point along the way anyway, so it's better to get it over with in a tiny CL
like this one.)

[hlundin-webrtc, 2016-04-22 07:03:29]

On 2016/04/21 11:46:49, kwiberg-webrtc wrote:
> On 2016/04/21 11:42:58, ossu wrote:
> > On 2016/04/21 11:03:16, kwiberg-webrtc wrote:
> > > I'm doing this small piece as a separate CL because it requires updates to
> the
> > > md5 sums for 
> > > 
> > > AcmReceiverBitExactnessOldApi.8kHzOutput
> > > AcmReceiverBitExactnessOldApi.16kHzOutput
> > > AcmReceiverBitExactnessOldApi.32kHzOutput
> > > AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
> > > AcmReceiverBitExactnessOldApi.48kHzOutput
> > > 
> > > (Only tests that run both 16 kHz and 32 kHz iSAC are affected, as
expected.)
> > > 
> > > Obviously, I also have to actually update the expected md5 sums in the
> tests,
> > > but I thought I'd get your input before I did all that work.
> > 
> > Seems reasonable. I guess the iSAC decoder is kept separate like this for
now,
> > but that that will eventually go away?
> 
> The special case for iSAC where it's injected into the ACM rather than created
> inside it like the other decoders? Yes, that'll go away once we start
injecting
> the entire decoder factory. (But this checksum change would have happened at
> some point along the way anyway, so it's better to get it over with in a tiny
CL
> like this one.)

LG, but I have to verify that I understand correctly. One of the reasons that
iSAC 16 and 32 share the same instance is that we want to support using iSAC's
internal BWE even though we're sending 16 kHz one way and 32 kHz the other way.
Therefore, it was previously necessary that the decoder instances were in fact
the same for both rates, since they had to be the same as the sending encoder.
Will both of the instances you create in this CL share one and the same
LockedIsacBandwidthInfo object?

[kwiberg-webrtc, 2016-04-22 07:11:36]

On 2016/04/22 07:03:29, hlundin-webrtc wrote:
> On 2016/04/21 11:46:49, kwiberg-webrtc wrote:
> > On 2016/04/21 11:42:58, ossu wrote:
> > > On 2016/04/21 11:03:16, kwiberg-webrtc wrote:
> > > > I'm doing this small piece as a separate CL because it requires updates
to
> > the
> > > > md5 sums for 
> > > > 
> > > > AcmReceiverBitExactnessOldApi.8kHzOutput
> > > > AcmReceiverBitExactnessOldApi.16kHzOutput
> > > > AcmReceiverBitExactnessOldApi.32kHzOutput
> > > > AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
> > > > AcmReceiverBitExactnessOldApi.48kHzOutput
> > > > 
> > > > (Only tests that run both 16 kHz and 32 kHz iSAC are affected, as
> expected.)
> > > > 
> > > > Obviously, I also have to actually update the expected md5 sums in the
> > tests,
> > > > but I thought I'd get your input before I did all that work.
> > > 
> > > Seems reasonable. I guess the iSAC decoder is kept separate like this for
> now,
> > > but that that will eventually go away?
> > 
> > The special case for iSAC where it's injected into the ACM rather than
created
> > inside it like the other decoders? Yes, that'll go away once we start
> injecting
> > the entire decoder factory. (But this checksum change would have happened at
> > some point along the way anyway, so it's better to get it over with in a
tiny
> CL
> > like this one.)
> 
> LG, but I have to verify that I understand correctly. One of the reasons that
> iSAC 16 and 32 share the same instance is that we want to support using iSAC's
> internal BWE even though we're sending 16 kHz one way and 32 kHz the other
way.
> Therefore, it was previously necessary that the decoder instances were in fact
> the same for both rates, since they had to be the same as the sending encoder.
> Will both of the instances you create in this CL share one and the same
> LockedIsacBandwidthInfo object?

Yes, if the caller uses the same Rent-A-Codec to create both iSAC decoders,
they'll share a LockedIsacBandwidthInfo.

[hlundin-webrtc, 2016-04-22 07:14:37]

lgtm

[kwiberg-webrtc, 2016-04-22 07:20:22]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2016-04-22 07:20:32]

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

[commit-bot: I haz the power, 2016-04-22 07:32:04]

Description was changed from

==========
Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

This change results in different checksums for the following tests:

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

Because they make an ACM and then ask it to decode both 16 kHz and 32
kHz iSAC. (The arm32 and arm64 checksums didn't change, because the
tests skip 32 kHz iSAC on arm.)

BUG=webrtc:5801
==========

to

==========
Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

This change results in different checksums for the following tests:

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

Because they make an ACM and then ask it to decode both 16 kHz and 32
kHz iSAC. (The arm32 and arm64 checksums didn't change, because the
tests skip 32 kHz iSAC on arm.)

BUG=webrtc:5801
==========

[commit-bot: I haz the power, 2016-04-22 07:32:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-22 07:32:13]

Description was changed from

==========
Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

This change results in different checksums for the following tests:

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

Because they make an ACM and then ask it to decode both 16 kHz and 32
kHz iSAC. (The arm32 and arm64 checksums didn't change, because the
tests skip 32 kHz iSAC on arm.)

BUG=webrtc:5801
==========

to

==========
Audio Coding Module: Use separate instances for 16 kHz and 32 kHz iSAC decoder

This will allow us to fix the sample rate of each AudioDecoder at
instantiation time.

This change results in different checksums for the following tests:

AcmReceiverBitExactnessOldApi.8kHzOutput
AcmReceiverBitExactnessOldApi.16kHzOutput
AcmReceiverBitExactnessOldApi.32kHzOutput
AcmReceiverBitExactnessOldApi.48kHzOutputExternalDecoder
AcmReceiverBitExactnessOldApi.48kHzOutput

Because they make an ACM and then ask it to decode both 16 kHz and 32
kHz iSAC. (The arm32 and arm64 checksums didn't change, because the
tests skip 32 kHz iSAC on arm.)

BUG=webrtc:5801

Committed: https://crrev.com/bfde543f73681b7e4cb41edc0be0088b9068f29f
Cr-Commit-Position: refs/heads/master@{#12463}
==========

[commit-bot: I haz the power, 2016-04-22 07:32:14]

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