From 73deaadd0e7e7a1c07595503f571109d42721c14 Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Thu, 31 Jan 2013 13:32:51 +0000 Subject: [PATCH] Removing a hack for CNG However, two other "hacks" had to be added to maintain bit-exactness with legacy. Note that this change requires a new version of the universal.rtp test input, although the output reference stays the same. Moving reference files, and using a new input vector for NetEq4. The new input vector neteq_universal_new.rtp is identical to the old neteq_universal.rtp, except that the payload type for CNG packets that follows a wideband codec is changed to 98. Update to resources revision 15 where the new reference files are. Also changing a faulty log error. Review URL: https://webrtc-codereview.appspot.com/1078009 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3442 4adac7df-926f-26a2-2b94-8c16560cd09d --- DEPS | 2 +- .../neteq/webrtc_neteq_unittest.cc | 14 +++--- .../modules/audio_coding/neteq4/neteq_impl.cc | 44 +++++++++++++++---- .../audio_coding/neteq4/neteq_unittest.cc | 10 ++--- 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/DEPS b/DEPS index c49e85ee6..dc3e72a13 100644 --- a/DEPS +++ b/DEPS @@ -14,7 +14,7 @@ vars = { # External resources like video and audio files used for testing purposes. # Downloaded on demand when needed. - "webrtc_resources_revision": "14", + "webrtc_resources_revision": "15", } # NOTE: Prefer revision numbers to tags for svn deps. Use http rather than diff --git a/webrtc/modules/audio_coding/neteq/webrtc_neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/webrtc_neteq_unittest.cc index ebb96aab8..8b9478874 100644 --- a/webrtc/modules/audio_coding/neteq/webrtc_neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/webrtc_neteq_unittest.cc @@ -376,33 +376,33 @@ void NetEqDecodingTest::PopulateCng(int frame_index, TEST_F(NetEqDecodingTest, TestBitExactness) { const std::string kInputRtpFile = webrtc::test::ProjectRootPath() + - "resources/neteq_universal.rtp"; + "resources/audio_coding/neteq_universal.rtp"; #if defined(_MSC_VER) && (_MSC_VER >= 1700) // For Visual Studio 2012 and later, we will have to use the generic reference // file, rather than the windows-specific one. const std::string kInputRefFile = webrtc::test::ProjectRootPath() + - "resources/neteq_universal_ref.pcm"; + "resources/audio_coding/neteq_universal_ref.pcm"; #else const std::string kInputRefFile = - webrtc::test::ResourcePath("neteq_universal_ref", "pcm"); + webrtc::test::ResourcePath("audio_coding/neteq_universal_ref", "pcm"); #endif DecodeAndCompare(kInputRtpFile, kInputRefFile); } TEST_F(NetEqDecodingTest, TestNetworkStatistics) { const std::string kInputRtpFile = webrtc::test::ProjectRootPath() + - "resources/neteq_universal.rtp"; + "resources/audio_coding/neteq_universal.rtp"; #if defined(_MSC_VER) && (_MSC_VER >= 1700) // For Visual Studio 2012 and later, we will have to use the generic reference // file, rather than the windows-specific one. const std::string kNetworkStatRefFile = webrtc::test::ProjectRootPath() + - "resources/neteq_network_stats.dat"; + "resources/audio_coding/neteq_network_stats.dat"; #else const std::string kNetworkStatRefFile = - webrtc::test::ResourcePath("neteq_network_stats", "dat"); + webrtc::test::ResourcePath("audio_coding/neteq_network_stats", "dat"); #endif const std::string kRtcpStatRefFile = - webrtc::test::ResourcePath("neteq_rtcp_stats", "dat"); + webrtc::test::ResourcePath("audio_coding/neteq_rtcp_stats", "dat"); DecodeAndCheckStats(kInputRtpFile, kNetworkStatRefFile, kRtcpStatRefFile); } diff --git a/webrtc/modules/audio_coding/neteq4/neteq_impl.cc b/webrtc/modules/audio_coding/neteq4/neteq_impl.cc index e16f0830f..5508d0870 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_impl.cc @@ -374,7 +374,9 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, packet->primary = true; packet->waiting_time = 0; packet->payload = new uint8_t[packet->payload_length]; - LOG_F(LS_ERROR) << "Payload pointer is NULL."; + if (!packet->payload) { + LOG_F(LS_ERROR) << "Payload pointer is NULL."; + } assert(payload); // Already checked above. memcpy(packet->payload, payload, packet->payload_length); // Insert packet in a packet list. @@ -844,7 +846,14 @@ int NetEqImpl::GetDecision(Operations* operation, delay_manager_->Reset(); stats_.ResetMcu(); - if (*operation == kRfc3389CngNoPacket) { + 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 + ) { // 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; @@ -1400,14 +1409,31 @@ int NetEqImpl::DoRfc3389Cng(PacketList* packet_list, bool play_dtmf, assert(packet_list->size() == 1); Packet* packet = packet_list->front(); packet_list->pop_front(); - // Temp hack to get correct PT for CNG. - // TODO(hlundin): Update universal.rtp and remove this hack. - if (fs_hz_ == 16000) { - packet->header.payloadType = 98; - } else if (fs_hz_ == 32000) { - packet->header.payloadType = 99; + 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(kDecoderCNGnb); + } else if (fs_hz_ == 16000) { + packet->header.payloadType = + decoder_database_->GetRtpPayloadType(kDecoderCNGwb); + } else if (fs_hz_ == 32000) { + packet->header.payloadType = + decoder_database_->GetRtpPayloadType(kDecoderCNGswb32kHz); + } else if (fs_hz_ == 48000) { + packet->header.payloadType = + decoder_database_->GetRtpPayloadType(kDecoderCNGswb48kHz); + } + assert(decoder_database_->IsComfortNoise(packet->header.payloadType)); +#else + LOG(LS_ERROR) << "Trying to decode non-CNG payload as CNG."; + return kOtherError; +#endif } - // End of hack. // UpdateParameters() deletes |packet|. if (comfort_noise_->UpdateParameters(packet) == ComfortNoise::kInternalError) { diff --git a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc index 250ca0e79..18f43aa73 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc @@ -374,19 +374,19 @@ void NetEqDecodingTest::PopulateCng(int frame_index, TEST_F(NetEqDecodingTest, TestBitExactness) { const std::string kInputRtpFile = webrtc::test::ProjectRootPath() + - "resources/neteq_universal.rtp"; + "resources/audio_coding/neteq_universal_new.rtp"; const std::string kInputRefFile = - webrtc::test::ResourcePath("neteq_universal_ref", "pcm"); + webrtc::test::ResourcePath("audio_coding/neteq_universal_ref", "pcm"); DecodeAndCompare(kInputRtpFile, kInputRefFile); } TEST_F(NetEqDecodingTest, TestNetworkStatistics) { const std::string kInputRtpFile = webrtc::test::ProjectRootPath() + - "resources/neteq_universal.rtp"; + "resources/audio_coding/neteq_universal_new.rtp"; const std::string kNetworkStatRefFile = - webrtc::test::ResourcePath("neteq_network_stats", "dat"); + webrtc::test::ResourcePath("audio_coding/neteq_network_stats", "dat"); const std::string kRtcpStatRefFile = - webrtc::test::ResourcePath("neteq_rtcp_stats", "dat"); + webrtc::test::ResourcePath("audio_coding/neteq_rtcp_stats", "dat"); DecodeAndCheckStats(kInputRtpFile, kNetworkStatRefFile, kRtcpStatRefFile); }