From bfb78d12932a339d09fb3561ff11960d277fbaeb Mon Sep 17 00:00:00 2001 From: kwiberg Date: Sun, 18 Sep 2016 05:33:41 -0700 Subject: [PATCH] Revert of AcmReceiver: Ask NetEq to delete all decoders at once instead of one by one (patchset #2 id:20001 of https://codereview.webrtc.org/2342313002/ ) Reason for revert: Seems to have broken Chromium tests. Original issue's description: > 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. > > BUG=webrtc:5801 > > Committed: https://crrev.com/f6232b43a176e1717354b671a0a52b887d70de59 > Cr-Commit-Position: refs/heads/master@{#14275} TBR=ossu@webrtc.org,henrik.lundin@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:5801 Review-Url: https://codereview.webrtc.org/2349973002 Cr-Commit-Position: refs/heads/master@{#14278} --- .../modules/audio_coding/acm2/acm_receiver.cc | 21 ++++++++++++++++--- .../modules/audio_coding/acm2/acm_receiver.h | 2 +- .../audio_coding/acm2/audio_coding_module.cc | 6 ++++-- .../audio_coding/neteq/decoder_database.cc | 6 ------ .../audio_coding/neteq/decoder_database.h | 3 --- .../audio_coding/neteq/include/neteq.h | 3 --- .../modules/audio_coding/neteq/neteq_impl.cc | 5 ----- .../modules/audio_coding/neteq/neteq_impl.h | 2 -- 8 files changed, 23 insertions(+), 25 deletions(-) diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc index b89091f362..2ae75485e0 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc @@ -245,12 +245,27 @@ void AcmReceiver::FlushBuffers() { neteq_->FlushBuffers(); } -void AcmReceiver::RemoveAllCodecs() { +// 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; rtc::CritScope lock(&crit_sect_); - neteq_->RemoveAllPayloadTypes(); - decoders_.clear(); + 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(cur->second.payload_type); + ret_val = -1; + } + } + + // No codec is registered, invalidate last audio decoder. last_audio_decoder_ = rtc::Optional(); last_packet_sample_rate_hz_ = rtc::Optional(); + 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 ee3f81da59..5b864c2b70 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.h +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.h @@ -186,7 +186,7 @@ class AcmReceiver { // // Remove all registered codecs. // - void RemoveAllCodecs(); + int 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 73f03a47ad..4784a8721f 100644 --- a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc +++ b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc @@ -948,8 +948,10 @@ int AudioCodingModuleImpl::InitializeReceiverSafe() { // 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_) - receiver_.RemoveAllCodecs(); + if (receiver_initialized_) { + if (receiver_.RemoveAllCodecs() < 0) + return -1; + } 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 f5fbad3446..b189e4bb56 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc @@ -169,12 +169,6 @@ int DecoderDatabase::Remove(uint8_t rtp_payload_type) { 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 38498f5143..6d277c95c9 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/decoder_database.h @@ -136,9 +136,6 @@ class DecoderDatabase { // Returns kDecoderNotFound or kOK depending on the outcome of the operation. virtual int Remove(uint8_t rtp_payload_type); - // Remove all entries. - 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/include/neteq.h b/webrtc/modules/audio_coding/neteq/include/neteq.h index eb08151a5d..1f6205f1e4 100644 --- a/webrtc/modules/audio_coding/neteq/include/neteq.h +++ b/webrtc/modules/audio_coding/neteq/include/neteq.h @@ -183,9 +183,6 @@ class NetEq { // -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/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 5e2116409d..71eea2114c 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -291,11 +291,6 @@ int NetEqImpl::RemovePayloadType(uint8_t rtp_payload_type) { 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 7903ba665c..8795ef5210 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -123,8 +123,6 @@ class NetEqImpl : public webrtc::NetEq { // -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;