Removed LEGACY_BITEXACT from neteq_impl.cc and updated the ACM unit tests.
I'll be rewriting AcmReceiver soon and am trying to reduce the amount of
old stuff that needs to be supported.
I've manually checked the outputs of the AcmReceiver bitexactness
tests with this change. A large part of the tests are still bitexact,
with one section only differing slightly in timings. Nothing audible
unless playing the old and new versions back simultaneously.
The output of NetEqDecoderTest were also changed due to this CL, although only on android. I built and ran the test locally and compared the audio output manually - the changes were the same as for the other tests; i.e. very slight timing changes for a part of the output.
I updated the network stats checksum for android without analyzing it further. I expect it goes hand-in-hand with the changes to the output; i.e. the changes in it are fine because the audio output is fine. Likely, the stats will show changes in the usage of CNG, since that is what the code changes.
BUG=webrtc:1361
Review-Url: https://codereview.webrtc.org/2117763002
Cr-Original-Commit-Position: refs/heads/master@{#13415}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 108ecec51ce5d55bcbe455f7a1cb778dd3cb2b22
diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc b/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
index ce8d86d..86f166b 100644
--- a/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
+++ b/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
@@ -958,34 +958,34 @@
#if (defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX)) && \
defined(WEBRTC_CODEC_ILBC) && defined(WEBRTC_CODEC_G722)
TEST_F(AcmReceiverBitExactnessOldApi, 8kHzOutput) {
- Run(8000, PlatformChecksum("90be25dd9505005aaadf91b77ee31624",
- "ac6dc4b5bf6d277f693889c4c916882e",
- "a607f7d0ba98683c9c236217f86aaa6b",
- "4a54f6ec712bda58484a388e1a332b42"),
+ Run(8000, PlatformChecksum("86f0e58421dafe3471e1bcd62316174e",
+ "e2c2176b822f109eab2c3317ff1f3a89",
+ "9c1058831bef8836d5880f86f03f02cd",
+ "7294944a869a39085ae69278dca875ad"),
std::vector<ExternalDecoder>());
}
TEST_F(AcmReceiverBitExactnessOldApi, 16kHzOutput) {
- Run(16000, PlatformChecksum("2c713197d41becd52c1ceecbd2b9f687",
- "130cc2a43063c74197122e3760690e7d",
- "cdc3d88f6d8e497d4e00c62c0e6dbb3c",
- "83edb67c157d0e3a0fb9f7d7b1ce5dc7"),
+ Run(16000, PlatformChecksum("0b895074ede8d93ff7d4b03e48ff4353",
+ "9b92ee1731bc07e7a802c46883551029",
+ "1a29721666a594cc56688b442790d0f9",
+ "c061a9486003d0c096504905521d0fe7"),
std::vector<ExternalDecoder>());
}
TEST_F(AcmReceiverBitExactnessOldApi, 32kHzOutput) {
- Run(32000, PlatformChecksum("fe5851d43c13df66a7ad30fdb124e62f",
- "309d24be4b287dc92c340f10a807a11e",
- "c4a0e0b2e031d62c693af2a9ff4337ac",
- "4cbfc6ab4d704f5d9b4f10406437fda9"),
+ Run(32000, PlatformChecksum("ed620557bab00d588db0af699dc05df9",
+ "9472cfaeba10282187a6376aeddba347",
+ "0071b9962de77b1d71ecd0a0e5accf45",
+ "c6cca0a4dd27bb77505343a15ea13c77"),
std::vector<ExternalDecoder>());
}
TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutput) {
- Run(48000, PlatformChecksum("a9241f426b4bf2ac650b6d287469a550",
- "30374fd4a932df942c1b1120e7b724ad",
- "22242dd832824046d48db9ea8a01f84c",
- "c7f46bf165400b266d9b57aee02d2747"),
+ Run(48000, PlatformChecksum("e68904bd63da1bad4b00f4bd221f2325",
+ "3418d41ee693f8fa6d5878de5a8c95d9",
+ "861e65072b1a10eec56ffde10c99bbce",
+ "244bafe12206b7e26276ff7c49fc1795"),
std::vector<ExternalDecoder>());
}
@@ -1043,10 +1043,10 @@
std::vector<ExternalDecoder> external_decoders;
external_decoders.push_back(ed);
- Run(48000, PlatformChecksum("a9241f426b4bf2ac650b6d287469a550",
- "30374fd4a932df942c1b1120e7b724ad",
- "22242dd832824046d48db9ea8a01f84c",
- "c7f46bf165400b266d9b57aee02d2747"),
+ Run(48000, PlatformChecksum("e68904bd63da1bad4b00f4bd221f2325",
+ "3418d41ee693f8fa6d5878de5a8c95d9",
+ "861e65072b1a10eec56ffde10c99bbce",
+ "244bafe12206b7e26276ff7c49fc1795"),
external_decoders);
EXPECT_CALL(mock_decoder, Die());
diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc
index 422252c..024c73d 100644
--- a/modules/audio_coding/neteq/neteq_impl.cc
+++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -47,11 +47,6 @@
#include "webrtc/modules/audio_coding/neteq/timestamp_scaler.h"
#include "webrtc/modules/include/module_common_types.h"
-// Modify the code to obtain backwards bit-exactness. Once bit-exactness is no
-// longer required, this #define should be removed (and the code that it
-// enables).
-#define LEGACY_BITEXACT
-
namespace webrtc {
NetEqImpl::Dependencies::Dependencies(
@@ -1145,15 +1140,8 @@
return -1;
}
timestamp_ = header->timestamp;
- if (*operation == kRfc3389CngNoPacket
-#ifndef LEGACY_BITEXACT
- // Without this check, it can happen that a non-CNG packet is sent to
- // the CNG decoder as if it was a SID frame. This is clearly a bug,
- // but is kept for now to maintain bit-exactness with the test
- // vectors.
- && decoder_database_->IsComfortNoise(header->payloadType)
-#endif
- ) {
+ if (*operation == kRfc3389CngNoPacket &&
+ decoder_database_->IsComfortNoise(header->payloadType)) {
// Change decision to CNG packet, since we do have a CNG packet, but it
// was considered too early to use. Now, use it anyway.
*operation = kRfc3389Cng;
@@ -1372,16 +1360,6 @@
reset_decoder_ = false;
}
-#ifdef LEGACY_BITEXACT
- // Due to a bug in old SignalMCU, it could happen that CNG operation was
- // decided, but a speech packet was provided. The speech packet will be used
- // to update the comfort noise decoder, as if it was a SID frame, which is
- // clearly wrong.
- if (*operation == kRfc3389Cng) {
- return 0;
- }
-#endif
-
*decoded_length = 0;
// Update codec-internal PLC state.
if ((*operation == kMerge) && decoder && decoder->HasDecodePlc()) {
@@ -1776,29 +1754,8 @@
Packet* packet = packet_list->front();
packet_list->pop_front();
if (!decoder_database_->IsComfortNoise(packet->header.payloadType)) {
-#ifdef LEGACY_BITEXACT
- // This can happen due to a bug in GetDecision. Change the payload type
- // to a CNG type, and move on. Note that this means that we are in fact
- // sending a non-CNG payload to the comfort noise decoder for decoding.
- // Clearly wrong, but will maintain bit-exactness with legacy.
- if (fs_hz_ == 8000) {
- packet->header.payloadType =
- decoder_database_->GetRtpPayloadType(NetEqDecoder::kDecoderCNGnb);
- } else if (fs_hz_ == 16000) {
- packet->header.payloadType =
- decoder_database_->GetRtpPayloadType(NetEqDecoder::kDecoderCNGwb);
- } else if (fs_hz_ == 32000) {
- packet->header.payloadType = decoder_database_->GetRtpPayloadType(
- NetEqDecoder::kDecoderCNGswb32kHz);
- } else if (fs_hz_ == 48000) {
- packet->header.payloadType = decoder_database_->GetRtpPayloadType(
- NetEqDecoder::kDecoderCNGswb48kHz);
- }
- assert(decoder_database_->IsComfortNoise(packet->header.payloadType));
-#else
LOG(LS_ERROR) << "Trying to decode non-CNG payload as CNG.";
return kOtherError;
-#endif
}
// UpdateParameters() deletes |packet|.
if (comfort_noise_->UpdateParameters(packet) ==
diff --git a/modules/audio_coding/neteq/neteq_unittest.cc b/modules/audio_coding/neteq/neteq_unittest.cc
index 783c7ff..99eb384 100644
--- a/modules/audio_coding/neteq/neteq_unittest.cc
+++ b/modules/audio_coding/neteq/neteq_unittest.cc
@@ -462,13 +462,13 @@
const std::string output_checksum = PlatformChecksum(
"472ebe1126f41fdb6b5c63c87f625a52e7604e49",
- "d2a6b6ff54b340cf9f961c7f07768d86b3761073",
+ "36f6fc87c05de077e998173b46b83524de5e8fc2",
"472ebe1126f41fdb6b5c63c87f625a52e7604e49",
"f9749813dbc3fb59dae761de518fec65b8407c5b");
const std::string network_stats_checksum = PlatformChecksum(
"2cf380a05ee07080bd72471e8ec7777a39644ec9",
- "01be67dc4c3b8e74743a45cbd8684c0535dec9ad",
+ "f50795e25ec2bab2d418c694ab088012776fd450",
"2cf380a05ee07080bd72471e8ec7777a39644ec9",
"2cf380a05ee07080bd72471e8ec7777a39644ec9");
