From ca101e6bb46ac6fa003e5724af4d7eafd910ee78 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Mon, 4 Apr 2022 21:42:55 +0200 Subject: [PATCH] Count consecutive expands by last mode in NetEq decision logic. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a slight change in behavior that fixes a bug where all expansions are not counted due to more than 10ms available in the sync buffer, which can happen after repeated expansions. The counter should also be updated when in muted mode. Bug: webrtc:13322 Change-Id: I067689ee251d3d1ae990a27cdd271f718b0d6f2f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257360 Reviewed-by: Ivo Creusen Commit-Queue: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#36483} --- .../acm2/audio_coding_module_unittest.cc | 10 +++---- modules/audio_coding/neteq/decision_logic.cc | 29 ++++++++++--------- modules/audio_coding/neteq/decision_logic.h | 8 ++--- .../audio_coding/neteq/neteq_impl_unittest.cc | 7 ----- modules/audio_coding/neteq/neteq_unittest.cc | 10 +++---- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/modules/audio_coding/acm2/audio_coding_module_unittest.cc index 1a63fcdcd2..25a554a453 100644 --- a/modules/audio_coding/acm2/audio_coding_module_unittest.cc +++ b/modules/audio_coding/acm2/audio_coding_module_unittest.cc @@ -895,28 +895,28 @@ class AcmReceiverBitExactnessOldApi : public ::testing::Test { defined(WEBRTC_ARCH_X86_64) TEST_F(AcmReceiverBitExactnessOldApi, 8kHzOutput) { std::string checksum_reference = GetCPUInfo(kAVX2) != 0 - ? "d8671dd38dab43fc9ca64a45c048c218" + ? "f531f3b7dabe96d9e928dece1d3a340b" : "4710c99559aec2f9f02a983ba2146f2d"; Run(/*output_freq_hz=*/8000, checksum_reference); } TEST_F(AcmReceiverBitExactnessOldApi, 16kHzOutput) { std::string checksum_reference = GetCPUInfo(kAVX2) != 0 - ? "abcb31509af46545edb4f6700728a4de" + ? "c68d7ee520bb35b6d053e017b37bc2b3" : "70b3217df49834b7093c631531068bd0"; Run(/*output_freq_hz=*/16000, checksum_reference); } TEST_F(AcmReceiverBitExactnessOldApi, 32kHzOutput) { std::string checksum_reference = GetCPUInfo(kAVX2) != 0 - ? "8489b7743d6cd1903807ac81e5ee493d" + ? "dc790e447442ff6105467f29ab7315ae" : "2679e4e596e33259228c62df545eb635"; Run(/*output_freq_hz=*/32000, checksum_reference); } TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutput) { std::string checksum_reference = GetCPUInfo(kAVX2) != 0 - ? "454996a7adb3f62b259a53a09ff624cf" + ? "d118436e154a976009171c4d451d5574" : "f0148c5ef84e74e019ac7057af839102"; Run(/*output_freq_hz=*/48000, checksum_reference); } @@ -996,7 +996,7 @@ TEST_F(AcmReceiverBitExactnessOldApi, 48kHzOutputExternalDecoder) { auto factory = rtc::make_ref_counted(); std::string checksum_reference = GetCPUInfo(kAVX2) != 0 - ? "454996a7adb3f62b259a53a09ff624cf" + ? "d118436e154a976009171c4d451d5574" : "f0148c5ef84e74e019ac7057af839102"; Run(48000, checksum_reference, factory, [](AudioCodingModule* acm) { diff --git a/modules/audio_coding/neteq/decision_logic.cc b/modules/audio_coding/neteq/decision_logic.cc index d5a216ea2a..f81535c84d 100644 --- a/modules/audio_coding/neteq/decision_logic.cc +++ b/modules/audio_coding/neteq/decision_logic.cc @@ -39,6 +39,10 @@ std::unique_ptr CreateDelayManager( return std::make_unique(config, neteq_config.tick_timer); } +bool IsExpand(NetEq::Mode mode) { + return mode == NetEq::Mode::kExpand || mode == NetEq::Mode::kCodecPlc; +} + } // namespace DecisionLogic::DecisionLogic(NetEqController::Config config) @@ -112,6 +116,12 @@ NetEq::Operation DecisionLogic::GetDecision(const NetEqStatus& status, cng_state_ = kCngInternalOn; } + if (IsExpand(status.last_mode)) { + ++num_consecutive_expands_; + } else { + num_consecutive_expands_ = 0; + } + prev_time_scale_ = prev_time_scale_ && (status.last_mode == NetEq::Mode::kAccelerateSuccess || @@ -163,9 +173,7 @@ NetEq::Operation DecisionLogic::GetDecision(const NetEqStatus& status, // Note that the MuteFactor is in Q14, so a value of 16384 corresponds to 1. const int target_level_samples = delay_manager_->TargetDelayMs() * sample_rate_ / 1000; - if ((status.last_mode == NetEq::Mode::kExpand || - status.last_mode == NetEq::Mode::kCodecPlc) && - status.expand_mutefactor < 16384 / 2 && + if (IsExpand(status.last_mode) && status.expand_mutefactor < 16384 / 2 && status.packet_buffer_info.span_samples < static_cast(target_level_samples * kPostponeDecodingLevel / 100) && @@ -192,12 +200,8 @@ NetEq::Operation DecisionLogic::GetDecision(const NetEqStatus& status, } } -void DecisionLogic::ExpandDecision(NetEq::Operation operation) { - if (operation == NetEq::Operation::kExpand) { - num_consecutive_expands_++; - } else { - num_consecutive_expands_ = 0; - } +void DecisionLogic::NotifyMutedState() { + ++num_consecutive_expands_; } absl::optional DecisionLogic::PacketArrived( @@ -334,10 +338,9 @@ NetEq::Operation DecisionLogic::FuturePacketAvailable( // Check if we should continue with an ongoing expand because the new packet // is too far into the future. uint32_t timestamp_leap = available_timestamp - target_timestamp; - if ((prev_mode == NetEq::Mode::kExpand || - prev_mode == NetEq::Mode::kCodecPlc) && - !ReinitAfterExpands(timestamp_leap) && !MaxWaitForPacket() && - PacketTooEarly(timestamp_leap) && UnderTargetLevel()) { + if (IsExpand(prev_mode) && !ReinitAfterExpands(timestamp_leap) && + !MaxWaitForPacket() && PacketTooEarly(timestamp_leap) && + UnderTargetLevel()) { if (play_dtmf) { // Still have DTMF to play, so do not do expand. return NetEq::Operation::kDtmf; diff --git a/modules/audio_coding/neteq/decision_logic.h b/modules/audio_coding/neteq/decision_logic.h index 2030c4e66e..22fb9f7748 100644 --- a/modules/audio_coding/neteq/decision_logic.h +++ b/modules/audio_coding/neteq/decision_logic.h @@ -68,11 +68,7 @@ class DecisionLogic : public NetEqController { // Resets the `cng_state_` to kCngOff. void SetCngOff() override { cng_state_ = kCngOff; } - // Reports back to DecisionLogic whether the decision to do expand remains or - // not. Note that this is necessary, since an expand decision can be changed - // to kNormal in NetEqImpl::GetDecision if there is still enough data in the - // sync buffer. - void ExpandDecision(NetEq::Operation operation) override; + void ExpandDecision(NetEq::Operation operation) override {} // Adds `value` to `sample_memory_`. void AddSampleMemory(int32_t value) override { sample_memory_ += value; } @@ -85,7 +81,7 @@ class DecisionLogic : public NetEqController { void RegisterEmptyPacket() override {} - void NotifyMutedState() override {} + void NotifyMutedState() override; bool SetMaximumDelay(int delay_ms) override { return delay_manager_->SetMaximumDelay(delay_ms); diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index b39a880292..af079a5de5 100644 --- a/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -1367,13 +1367,6 @@ TEST_F(NetEqImplTest, DecodingError) { // We are not expecting anything for output.speech_type_, since an error was // returned. - // Pull audio again, should continue an expansion. - EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output, &muted)); - EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_); - EXPECT_EQ(1u, output.num_channels_); - EXPECT_EQ(AudioFrame::kPLC, output.speech_type_); - EXPECT_THAT(output.packet_infos_, IsEmpty()); - // Pull audio again, should behave normal. EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output, &muted)); EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_); diff --git a/modules/audio_coding/neteq/neteq_unittest.cc b/modules/audio_coding/neteq/neteq_unittest.cc index 4ff1a431e1..f387b15be6 100644 --- a/modules/audio_coding/neteq/neteq_unittest.cc +++ b/modules/audio_coding/neteq/neteq_unittest.cc @@ -57,10 +57,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestBitExactness) { webrtc::test::ResourcePath("audio_coding/neteq_universal_new", "rtp"); const std::string output_checksum = - "ba4fae83a52f5e9d95b0910f05d540114285697b"; + "5e56fabfacd6fa202f3a00bcb4e034d6d817e6b3"; const std::string network_stats_checksum = - "fa878a8464ef1cb3d01503b7f927c3e2ce6f02c4"; + "dfbf60f913a25a1f2f1066f85b4b08c24eed0ef2"; DecodeAndCompare(input_rtp_file, output_checksum, network_stats_checksum, absl::GetFlag(FLAGS_gen_ref)); @@ -79,11 +79,11 @@ TEST_F(NetEqDecodingTest, MAYBE_TestOpusBitExactness) { // The checksum depends on SSE being enabled, the second part is the non-SSE // checksum. const std::string output_checksum = - "6e23d8827ae54ca352e1448ae363bdfd2878c78e|" - "47cddbf3494b0233f48cb350676e704807237545"; + "919e5eb3ba901192878f2354b981a15508c8373c|" + "c5eb0a8fcf7e8255a40f821cb815e1096619efeb"; const std::string network_stats_checksum = - "f89a9533dbb35a4c449b44c3ed120f7f1c7f90b6"; + "3d043e47e5f4bb81d37e7bce8c44bf802965c853"; DecodeAndCompare(input_rtp_file, output_checksum, network_stats_checksum, absl::GetFlag(FLAGS_gen_ref));