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;
