diff --git a/webrtc/modules/audio_coding/acm2/acm_receiver.cc b/webrtc/modules/audio_coding/acm2/acm_receiver.cc index a2a5eb7728..8f21e89448 100644 --- a/webrtc/modules/audio_coding/acm2/acm_receiver.cc +++ b/webrtc/modules/audio_coding/acm2/acm_receiver.cc @@ -219,8 +219,7 @@ int32_t AcmReceiver::AddCodec(int acm_codec_id, return 0; } - if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK && - neteq_->LastError() != NetEq::kDecoderNotFound) { + if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK) { LOG(LERROR) << "Cannot remove payload " << static_cast(payload_type); return -1; } @@ -249,8 +248,7 @@ bool AcmReceiver::AddCodec(int rtp_payload_type, return true; } - if (neteq_->RemovePayloadType(rtp_payload_type) != NetEq::kOK && - neteq_->LastError() != NetEq::kDecoderNotFound) { + if (neteq_->RemovePayloadType(rtp_payload_type) != NetEq::kOK) { LOG(LERROR) << "AcmReceiver::AddCodec: Could not remove existing decoder" " for payload type " << rtp_payload_type; @@ -280,9 +278,9 @@ void AcmReceiver::RemoveAllCodecs() { int AcmReceiver::RemoveCodec(uint8_t payload_type) { rtc::CritScope lock(&crit_sect_); - if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK && - neteq_->LastError() != NetEq::kDecoderNotFound) { - LOG(LERROR) << "AcmReceiver::RemoveCodec" << static_cast(payload_type); + if (neteq_->RemovePayloadType(payload_type) != NetEq::kOK) { + LOG(LERROR) << "AcmReceiver::RemoveCodec " + << static_cast(payload_type); return -1; } if (last_audio_decoder_ && payload_type == last_audio_decoder_->pltype) { diff --git a/webrtc/modules/audio_coding/neteq/include/neteq.h b/webrtc/modules/audio_coding/neteq/include/neteq.h index 45aae11048..648c04f8f5 100644 --- a/webrtc/modules/audio_coding/neteq/include/neteq.h +++ b/webrtc/modules/audio_coding/neteq/include/neteq.h @@ -102,32 +102,6 @@ class NetEq { kNotImplemented = -2 }; - enum ErrorCodes { - kNoError = 0, - kOtherError, - kInvalidRtpPayloadType, - kUnknownRtpPayloadType, - kCodecNotSupported, - kDecoderExists, - kDecoderNotFound, - kInvalidSampleRate, - kInvalidPointer, - kAccelerateError, - kPreemptiveExpandError, - kComfortNoiseErrorCode, - kDecoderErrorCode, - kOtherDecoderError, - kInvalidOperation, - kDtmfParameterError, - kDtmfParsingError, - kDtmfInsertError, - kStereoNotSupported, - kSampleUnderrun, - kDecodedTooMuch, - kRedundancySplitError, - kPacketBufferCorruption - }; - // Creates a new NetEq object, with parameters set in |config|. The |config| // object will only have to be valid for the duration of the call to this // method. @@ -191,7 +165,8 @@ class NetEq { const SdpAudioFormat& audio_format) = 0; // Removes |rtp_payload_type| from the codec database. Returns 0 on success, - // -1 on failure. + // -1 on failure. Removing a payload type that is not registered is ok and + // will not result in an error. virtual int RemovePayloadType(uint8_t rtp_payload_type) = 0; // Removes all payload types from the codec database. @@ -282,15 +257,6 @@ class NetEq { // Not implemented. virtual int SetTargetSampleRate() = 0; - // Returns the error code for the last occurred error. If no error has - // occurred, 0 is returned. - virtual int LastError() const = 0; - - // Returns the error code last returned by a decoder (audio or comfort noise). - // When LastError() returns kDecoderErrorCode or kComfortNoiseErrorCode, check - // this method to get the decoder's error code. - virtual int LastDecoderError() = 0; - // Flushes both the packet buffer and the sync buffer. virtual void FlushBuffers() = 0; diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index f9ec3bb44e..fb3cc11ec8 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -98,8 +98,6 @@ NetEqImpl::NetEqImpl(const NetEq::Config& config, reset_decoder_(false), ssrc_(0), first_packet_(true), - error_code_(0), - decoder_error_code_(0), background_noise_mode_(config.background_noise_mode), playout_mode_(config.playout_mode), enable_fast_accelerate_(config.enable_fast_accelerate), @@ -136,10 +134,7 @@ int NetEqImpl::InsertPacket(const RTPHeader& rtp_header, rtc::MsanCheckInitialized(payload); TRACE_EVENT0("webrtc", "NetEqImpl::InsertPacket"); rtc::CritScope lock(&crit_sect_); - int error = - InsertPacketInternal(rtp_header, payload, receive_timestamp); - if (error != 0) { - error_code_ = error; + if (InsertPacketInternal(rtp_header, payload, receive_timestamp) != 0) { return kFail; } return kOK; @@ -199,9 +194,7 @@ void SetAudioFrameActivityAndType(bool vad_enabled, int NetEqImpl::GetAudio(AudioFrame* audio_frame, bool* muted) { TRACE_EVENT0("webrtc", "NetEqImpl::GetAudio"); rtc::CritScope lock(&crit_sect_); - int error = GetAudioInternal(audio_frame, muted); - if (error != 0) { - error_code_ = error; + if (GetAudioInternal(audio_frame, muted) != 0) { return kFail; } RTC_DCHECK_EQ( @@ -235,21 +228,8 @@ int NetEqImpl::RegisterPayloadType(NetEqDecoder codec, LOG(LS_VERBOSE) << "RegisterPayloadType " << static_cast(rtp_payload_type) << " " << static_cast(codec); - int ret = decoder_database_->RegisterPayload(rtp_payload_type, codec, name); - if (ret != DecoderDatabase::kOK) { - switch (ret) { - case DecoderDatabase::kInvalidRtpPayloadType: - error_code_ = kInvalidRtpPayloadType; - break; - case DecoderDatabase::kCodecNotSupported: - error_code_ = kCodecNotSupported; - break; - case DecoderDatabase::kDecoderExists: - error_code_ = kDecoderExists; - break; - default: - error_code_ = kOtherError; - } + if (decoder_database_->RegisterPayload(rtp_payload_type, codec, name) != + DecoderDatabase::kOK) { return kFail; } return kOK; @@ -268,28 +248,8 @@ int NetEqImpl::RegisterExternalDecoder(AudioDecoder* decoder, assert(false); return kFail; } - int ret = decoder_database_->InsertExternal(rtp_payload_type, codec, - codec_name, decoder); - if (ret != DecoderDatabase::kOK) { - switch (ret) { - case DecoderDatabase::kInvalidRtpPayloadType: - error_code_ = kInvalidRtpPayloadType; - break; - case DecoderDatabase::kCodecNotSupported: - error_code_ = kCodecNotSupported; - break; - case DecoderDatabase::kDecoderExists: - error_code_ = kDecoderExists; - break; - case DecoderDatabase::kInvalidSampleRate: - error_code_ = kInvalidSampleRate; - break; - case DecoderDatabase::kInvalidPointer: - error_code_ = kInvalidPointer; - break; - default: - error_code_ = kOtherError; - } + if (decoder_database_->InsertExternal(rtp_payload_type, codec, codec_name, + decoder) != DecoderDatabase::kOK) { return kFail; } return kOK; @@ -300,40 +260,16 @@ bool NetEqImpl::RegisterPayloadType(int rtp_payload_type, LOG(LS_VERBOSE) << "NetEqImpl::RegisterPayloadType: payload type " << rtp_payload_type << ", codec " << audio_format; rtc::CritScope lock(&crit_sect_); - switch (decoder_database_->RegisterPayload(rtp_payload_type, audio_format)) { - case DecoderDatabase::kOK: - return true; - case DecoderDatabase::kInvalidRtpPayloadType: - error_code_ = kInvalidRtpPayloadType; - return false; - case DecoderDatabase::kCodecNotSupported: - error_code_ = kCodecNotSupported; - return false; - case DecoderDatabase::kDecoderExists: - error_code_ = kDecoderExists; - return false; - case DecoderDatabase::kInvalidSampleRate: - error_code_ = kInvalidSampleRate; - return false; - case DecoderDatabase::kInvalidPointer: - error_code_ = kInvalidPointer; - return false; - default: - error_code_ = kOtherError; - return false; - } + return decoder_database_->RegisterPayload(rtp_payload_type, audio_format) == + DecoderDatabase::kOK; } int NetEqImpl::RemovePayloadType(uint8_t rtp_payload_type) { rtc::CritScope lock(&crit_sect_); int ret = decoder_database_->Remove(rtp_payload_type); - if (ret == DecoderDatabase::kOK) { + if (ret == DecoderDatabase::kOK || ret == DecoderDatabase::kDecoderNotFound) { packet_buffer_->DiscardPacketsWithPayloadType(rtp_payload_type); return kOK; - } else if (ret == DecoderDatabase::kDecoderNotFound) { - error_code_ = kDecoderNotFound; - } else { - error_code_ = kOtherError; } return kFail; } @@ -527,16 +463,6 @@ int NetEqImpl::SetTargetSampleRate() { return kNotImplemented; } -int NetEqImpl::LastError() const { - rtc::CritScope lock(&crit_sect_); - return error_code_; -} - -int NetEqImpl::LastDecoderError() { - rtc::CritScope lock(&crit_sect_); - return decoder_error_code_; -} - void NetEqImpl::FlushBuffers() { rtc::CritScope lock(&crit_sect_); LOG(LS_VERBOSE) << "FlushBuffers"; @@ -1469,7 +1395,6 @@ int NetEqImpl::Decode(PacketList* packet_list, Operations* operation, error_code = decoder->ErrorCode(); if (error_code != 0) { // Got some error code from the decoder. - decoder_error_code_ = error_code; return_value = kDecoderErrorCode; LOG(LS_WARNING) << "Decoder returned error code: " << error_code; } else { @@ -1835,7 +1760,8 @@ int NetEqImpl::DoRfc3389Cng(PacketList* packet_list, bool play_dtmf) { dtmf_tone_generator_->Reset(); } if (cn_return == ComfortNoise::kInternalError) { - decoder_error_code_ = comfort_noise_->internal_error_code(); + LOG(LS_WARNING) << "Comfort noise generator returned error code: " + << comfort_noise_->internal_error_code(); return kComfortNoiseErrorCode; } else if (cn_return == ComfortNoise::kUnknownPayloadType) { return kUnknownRtpPayloadType; diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.h b/webrtc/modules/audio_coding/neteq/neteq_impl.h index 343676532d..d1d2039be7 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.h +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.h @@ -68,6 +68,26 @@ class NetEqImpl : public webrtc::NetEq { kVadPassive }; + enum ErrorCodes { + kNoError = 0, + kOtherError, + kUnknownRtpPayloadType, + kDecoderNotFound, + kInvalidPointer, + kAccelerateError, + kPreemptiveExpandError, + kComfortNoiseErrorCode, + kDecoderErrorCode, + kOtherDecoderError, + kInvalidOperation, + kDtmfParsingError, + kDtmfInsertError, + kSampleUnderrun, + kDecodedTooMuch, + kRedundancySplitError, + kPacketBufferCorruption + }; + struct Dependencies { // The constructor populates the Dependencies struct with the default // implementations of the objects. They can all be replaced by the user @@ -188,15 +208,6 @@ class NetEqImpl : public webrtc::NetEq { int SetTargetSampleRate() override; - // Returns the error code for the last occurred error. If no error has - // occurred, 0 is returned. - int LastError() const override; - - // Returns the error code last returned by a decoder (audio or comfort noise). - // When LastError() returns kDecoderErrorCode or kComfortNoiseErrorCode, check - // this method to get the decoder's error code. - int LastDecoderError() override; - // Flushes both the packet buffer and the sync buffer. void FlushBuffers() override; @@ -408,8 +419,6 @@ class NetEqImpl : public webrtc::NetEq { rtc::Optional current_cng_rtp_payload_type_ GUARDED_BY(crit_sect_); uint32_t ssrc_ GUARDED_BY(crit_sect_); bool first_packet_ GUARDED_BY(crit_sect_); - int error_code_ GUARDED_BY(crit_sect_); // Store last error code. - int decoder_error_code_ GUARDED_BY(crit_sect_); const BackgroundNoiseMode background_noise_mode_ GUARDED_BY(crit_sect_); NetEqPlayoutMode playout_mode_ GUARDED_BY(crit_sect_); bool enable_fast_accelerate_ GUARDED_BY(crit_sect_); diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index b6c4a77aaa..0676ad6803 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -281,8 +281,9 @@ TEST_F(NetEqImplTest, RemovePayloadType) { uint8_t rtp_payload_type = 0; EXPECT_CALL(*mock_decoder_database_, Remove(rtp_payload_type)) .WillOnce(Return(DecoderDatabase::kDecoderNotFound)); - // Check that kFail is returned when database returns kDecoderNotFound. - EXPECT_EQ(NetEq::kFail, neteq_->RemovePayloadType(rtp_payload_type)); + // Check that kOK is returned when database returns kDecoderNotFound, because + // removing a payload type that was never registered is not an error. + EXPECT_EQ(NetEq::kOK, neteq_->RemovePayloadType(rtp_payload_type)); } TEST_F(NetEqImplTest, RemoveAllPayloadTypes) { @@ -653,7 +654,6 @@ TEST_F(NetEqImplTest, FirstPacketUnknown) { // this packet will be rejected. EXPECT_EQ(NetEq::kFail, neteq_->InsertPacket(rtp_header, payload, kReceiveTime)); - EXPECT_EQ(NetEq::kUnknownRtpPayloadType, neteq_->LastError()); // Pull audio once. const size_t kMaxOutputSize = static_cast(10 * kSampleRateHz / 1000); @@ -911,10 +911,8 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { AudioFrame output; bool muted; // First call to GetAudio will try to decode the "faulty" packet. - // Expect kFail return value... + // Expect kFail return value. EXPECT_EQ(NetEq::kFail, neteq_->GetAudio(&output, &muted)); - // ... and kOtherDecoderError error code. - EXPECT_EQ(NetEq::kOtherDecoderError, neteq_->LastError()); // Output size and number of channels should be correct. const size_t kExpectedOutputSize = 10 * (kSampleRateHz / 1000) * kChannels; EXPECT_EQ(kExpectedOutputSize, output.samples_per_channel_ * kChannels); @@ -1123,8 +1121,6 @@ TEST_F(NetEqImplTest, DecodingError) { // Pull audio again. Decoder fails. EXPECT_EQ(NetEq::kFail, neteq_->GetAudio(&output, &muted)); - EXPECT_EQ(NetEq::kDecoderErrorCode, neteq_->LastError()); - EXPECT_EQ(kDecoderErrorCode, neteq_->LastDecoderError()); EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_); EXPECT_EQ(1u, output.num_channels_); // We are not expecting anything for output.speech_type_, since an error was @@ -1235,8 +1231,6 @@ TEST_F(NetEqImplTest, DecodingErrorDuringInternalCng) { // Pull audio again. Decoder fails. EXPECT_EQ(NetEq::kFail, neteq_->GetAudio(&output, &muted)); - EXPECT_EQ(NetEq::kDecoderErrorCode, neteq_->LastError()); - EXPECT_EQ(kDecoderErrorCode, neteq_->LastDecoderError()); EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_); EXPECT_EQ(1u, output.num_channels_); // We are not expecting anything for output.speech_type_, since an error was diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc index fae1e2324e..eed77405f8 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc @@ -830,7 +830,6 @@ TEST_F(NetEqDecodingTest, UnknownPayloadType) { PopulateRtpInfo(0, 0, &rtp_info); rtp_info.payloadType = 1; // Not registered as a decoder. EXPECT_EQ(NetEq::kFail, neteq_->InsertPacket(rtp_info, payload, 0)); - EXPECT_EQ(NetEq::kUnknownRtpPayloadType, neteq_->LastError()); } #if defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX) @@ -855,18 +854,7 @@ TEST_F(NetEqDecodingTest, MAYBE_DecoderError) { bool muted; EXPECT_EQ(NetEq::kFail, neteq_->GetAudio(&out_frame_, &muted)); ASSERT_FALSE(muted); - // Verify that there is a decoder error to check. - EXPECT_EQ(NetEq::kDecoderErrorCode, neteq_->LastError()); - enum NetEqDecoderError { - ISAC_LENGTH_MISMATCH = 6730, - ISAC_RANGE_ERROR_DECODE_FRAME_LENGTH = 6640 - }; -#if defined(WEBRTC_CODEC_ISAC) - EXPECT_EQ(ISAC_LENGTH_MISMATCH, neteq_->LastDecoderError()); -#elif defined(WEBRTC_CODEC_ISACFX) - EXPECT_EQ(ISAC_RANGE_ERROR_DECODE_FRAME_LENGTH, neteq_->LastDecoderError()); -#endif // Verify that the first 160 samples are set to 0. static const int kExpectedOutputLength = 160; // 10 ms at 16 kHz sample rate. const int16_t* const_out_frame_data = out_frame_.data(); diff --git a/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc b/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc index 34a1d50727..598ffd76f3 100644 --- a/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc +++ b/webrtc/modules/audio_coding/neteq/tools/neteq_test.cc @@ -18,21 +18,14 @@ namespace webrtc { namespace test { void DefaultNetEqTestErrorCallback::OnInsertPacketError( - int error_code, const NetEqInput::PacketData& packet) { - if (error_code == NetEq::kUnknownRtpPayloadType) { - std::cerr << "RTP Payload type " - << static_cast(packet.header.payloadType) << " is unknown." - << std::endl; - } else { - std::cerr << "InsertPacket returned error code " << error_code << std::endl; - } + std::cerr << "InsertPacket returned an error." << std::endl; std::cerr << "Packet data: " << packet.ToString() << std::endl; FATAL(); } -void DefaultNetEqTestErrorCallback::OnGetAudioError(int error_code) { - std::cerr << "GetAudio returned error code " << error_code << std::endl; +void DefaultNetEqTestErrorCallback::OnGetAudioError() { + std::cerr << "GetAudio returned an error." << std::endl; FATAL(); } @@ -70,8 +63,7 @@ int64_t NetEqTest::Run() { rtc::ArrayView(packet_data->payload), static_cast(packet_data->time_ms * sample_rate_hz_ / 1000)); if (error != NetEq::kOK && callbacks_.error_callback) { - callbacks_.error_callback->OnInsertPacketError(neteq_->LastError(), - *packet_data); + callbacks_.error_callback->OnInsertPacketError(*packet_data); } if (callbacks_.post_insert_packet) { callbacks_.post_insert_packet->AfterInsertPacket(*packet_data, @@ -91,7 +83,7 @@ int64_t NetEqTest::Run() { RTC_CHECK(!muted) << "The code does not handle enable_muted_state"; if (error != NetEq::kOK) { if (callbacks_.error_callback) { - callbacks_.error_callback->OnGetAudioError(neteq_->LastError()); + callbacks_.error_callback->OnGetAudioError(); } } else { sample_rate_hz_ = out_frame.sample_rate_hz_; diff --git a/webrtc/modules/audio_coding/neteq/tools/neteq_test.h b/webrtc/modules/audio_coding/neteq/tools/neteq_test.h index e4d3996e90..18fad9a971 100644 --- a/webrtc/modules/audio_coding/neteq/tools/neteq_test.h +++ b/webrtc/modules/audio_coding/neteq/tools/neteq_test.h @@ -26,15 +26,13 @@ namespace test { class NetEqTestErrorCallback { public: virtual ~NetEqTestErrorCallback() = default; - virtual void OnInsertPacketError(int error_code, - const NetEqInput::PacketData& packet) {} - virtual void OnGetAudioError(int error_code) {} + virtual void OnInsertPacketError(const NetEqInput::PacketData& packet) {} + virtual void OnGetAudioError() {} }; class DefaultNetEqTestErrorCallback : public NetEqTestErrorCallback { - void OnInsertPacketError(int error_code, - const NetEqInput::PacketData& packet) override; - void OnGetAudioError(int error_code) override; + void OnInsertPacketError(const NetEqInput::PacketData& packet) override; + void OnGetAudioError() override; }; class NetEqPostInsertPacket {