AcmReceiver: Ask NetEq to delete all decoders at once instead of one by one It requires a new NetEq method, but it can no longer fail. And we no longer need to use AcmReceiver::decoders_, which we're trying to eliminate. (This is a re-land of https://codereview.webrtc.org/2342313002.) BUG=webrtc:5801 Review-Url: https://codereview.webrtc.org/2348233002 Cr-Commit-Position: refs/heads/master@{#14304}
diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc index 641818d..cbf3492 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc
@@ -246,27 +246,12 @@ neteq_->FlushBuffers(); } -// If failed in removing one of the codecs, this method continues to remove as -// many as it can. -int AcmReceiver::RemoveAllCodecs() { - int ret_val = 0; +void AcmReceiver::RemoveAllCodecs() { rtc::CritScope lock(&crit_sect_); - for (auto it = decoders_.begin(); it != decoders_.end(); ) { - auto cur = it; - ++it; // it will be valid even if we erase cur - if (neteq_->RemovePayloadType(cur->second.payload_type) == 0) { - decoders_.erase(cur); - } else { - LOG_F(LS_ERROR) << "Cannot remove payload " - << static_cast<int>(cur->second.payload_type); - ret_val = -1; - } - } - - // No codec is registered, invalidate last audio decoder. + neteq_->RemoveAllPayloadTypes(); + decoders_.clear(); last_audio_decoder_ = rtc::Optional<CodecInst>(); last_packet_sample_rate_hz_ = rtc::Optional<int>(); - return ret_val; } int AcmReceiver::RemoveCodec(uint8_t payload_type) {
diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.h b/webrtc/modules/audio_coding/acm2/acm_receiver.h index d39581e..53def2b 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.h +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.h
@@ -186,7 +186,7 @@ // // Remove all registered codecs. // - int RemoveAllCodecs(); + void RemoveAllCodecs(); // Returns the RTP timestamp for the last sample delivered by GetAudio(). // The return value will be empty if no valid timestamp is available.
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc index 4784a87..73f03a4 100644 --- a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc +++ b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
@@ -948,10 +948,8 @@ // If the receiver is already initialized then we want to destroy any // existing decoders. After a call to this function, we should have a clean // start-up. - if (receiver_initialized_) { - if (receiver_.RemoveAllCodecs() < 0) - return -1; - } + if (receiver_initialized_) + receiver_.RemoveAllCodecs(); receiver_.ResetInitialDelay(); receiver_.SetMinimumDelay(0); receiver_.SetMaximumDelay(0);
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc index b189e4b..f5fbad3 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc
@@ -169,6 +169,12 @@ return kOK; } +void DecoderDatabase::RemoveAll() { + decoders_.clear(); + active_decoder_type_ = -1; // No active decoder. + active_cng_decoder_type_ = -1; // No active CNG decoder. +} + const DecoderDatabase::DecoderInfo* DecoderDatabase::GetDecoderInfo( uint8_t rtp_payload_type) const { DecoderMap::const_iterator it = decoders_.find(rtp_payload_type);
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h index 6d277c9..3728d1d 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/decoder_database.h
@@ -136,6 +136,9 @@ // Returns kDecoderNotFound or kOK depending on the outcome of the operation. virtual int Remove(uint8_t rtp_payload_type); + // Remove all entries. + virtual void RemoveAll(); + // Returns a pointer to the DecoderInfo struct for |rtp_payload_type|. If // no decoder is registered with that |rtp_payload_type|, NULL is returned. virtual const DecoderInfo* GetDecoderInfo(uint8_t rtp_payload_type) const;
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc index 380e719..39c000c 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc
@@ -47,6 +47,21 @@ EXPECT_TRUE(db.Empty()); } +TEST(DecoderDatabase, InsertAndRemoveAll) { + DecoderDatabase db(new rtc::RefCountedObject<MockAudioDecoderFactory>); + const std::string kCodecName1 = "Robert\'); DROP TABLE Students;"; + const std::string kCodecName2 = "https://xkcd.com/327/"; + EXPECT_EQ(DecoderDatabase::kOK, + db.RegisterPayload(0, NetEqDecoder::kDecoderPCMu, kCodecName1)); + EXPECT_EQ(DecoderDatabase::kOK, + db.RegisterPayload(1, NetEqDecoder::kDecoderPCMa, kCodecName2)); + EXPECT_EQ(2, db.Size()); + EXPECT_FALSE(db.Empty()); + db.RemoveAll(); + EXPECT_EQ(0, db.Size()); + EXPECT_TRUE(db.Empty()); +} + TEST(DecoderDatabase, GetDecoderInfo) { rtc::scoped_refptr<MockAudioDecoderFactory> factory( new rtc::RefCountedObject<MockAudioDecoderFactory>);
diff --git a/webrtc/modules/audio_coding/neteq/include/neteq.h b/webrtc/modules/audio_coding/neteq/include/neteq.h index 22d994e3..952ab23 100644 --- a/webrtc/modules/audio_coding/neteq/include/neteq.h +++ b/webrtc/modules/audio_coding/neteq/include/neteq.h
@@ -183,6 +183,9 @@ // -1 on failure. virtual int RemovePayloadType(uint8_t rtp_payload_type) = 0; + // Removes all payload types from the codec database. + virtual void RemoveAllPayloadTypes() = 0; + // Sets a minimum delay in millisecond for packet buffer. The minimum is // maintained unless a higher latency is dictated by channel condition. // Returns true if the minimum is successfully applied, otherwise false is
diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h index 1f6d3bc..7347959 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h
@@ -42,6 +42,7 @@ AudioDecoder* decoder)); MOCK_METHOD1(Remove, int(uint8_t rtp_payload_type)); + MOCK_METHOD0(RemoveAll, void()); MOCK_CONST_METHOD1(GetDecoderInfo, const DecoderInfo*(uint8_t rtp_payload_type)); MOCK_CONST_METHOD1(GetRtpPayloadType,
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 27d47c1..ca20e5b 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ -293,6 +293,11 @@ return kFail; } +void NetEqImpl::RemoveAllPayloadTypes() { + rtc::CritScope lock(&crit_sect_); + decoder_database_->RemoveAll(); +} + bool NetEqImpl::SetMinimumDelay(int delay_ms) { rtc::CritScope lock(&crit_sect_); if (delay_ms >= 0 && delay_ms < 10000) {
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index 8795ef5..7903ba6 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h
@@ -123,6 +123,8 @@ // -1 on failure. int RemovePayloadType(uint8_t rtp_payload_type) override; + void RemoveAllPayloadTypes() override; + bool SetMinimumDelay(int delay_ms) override; bool SetMaximumDelay(int delay_ms) override;
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index 1b8ee82..4f98005 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -230,6 +230,12 @@ EXPECT_EQ(NetEq::kFail, neteq_->RemovePayloadType(rtp_payload_type)); } +TEST_F(NetEqImplTest, RemoveAllPayloadTypes) { + CreateInstance(); + EXPECT_CALL(*mock_decoder_database_, RemoveAll()).WillOnce(Return()); + neteq_->RemoveAllPayloadTypes(); +} + TEST_F(NetEqImplTest, InsertPacket) { CreateInstance(); const size_t kPayloadLength = 100;