From 425a6ccac3c6c485048685566b259c540280cc2a Mon Sep 17 00:00:00 2001 From: ossu Date: Wed, 5 Oct 2016 08:44:22 -0700 Subject: [PATCH] RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up CheckPayloadChanged. Removed last_received_frequency_, cng_payload_type_, g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and cleaned up most of the related, now dead code. Since g722_payload_type_ was never set, neither was last_received_g722_, which means the frequency change in CNGPayloadType was never done. Setting the frequency to the standard values also proved unnecessary, since they were already set before the call. Even if frequency would have been changed by RTPReceiverAudio, I was not able to find a place where that would actually have mattered. The ACM and NetEq, for example, which eventually gets these packages, don't care about that value. Also, GetPayloadTypeFrequency was never called, so keeping track of last_received_frequency_ proved unnecessary. cng_payload_type_ was stored to be able to check in CNGPayloadType if cng_payload_type_has_changed. This flag was also never read, so these all disappear. The main reason for starting this change was to root out any G722 specific code we have sprinkled around the code base (specifically dealing with the fact that for G722 clock rate != sample rate). In this case, once I started pulling at one end of the string, the whole thing came unraveled. BUG=webrtc:5805 Review-Url: https://codereview.webrtc.org/2383103002 Cr-Commit-Position: refs/heads/master@{#14530} --- .../rtp_rtcp/source/rtp_receiver_audio.cc | 97 ++----------------- .../rtp_rtcp/source/rtp_receiver_audio.h | 17 +--- .../rtp_rtcp/source/rtp_receiver_strategy.h | 3 - .../rtp_rtcp/source/rtp_receiver_video.cc | 4 - .../rtp_rtcp/source/rtp_receiver_video.h | 2 - 5 files changed, 13 insertions(+), 110 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc index 38b2830b79..082d2a8b18 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc @@ -26,16 +26,12 @@ RTPReceiverStrategy* RTPReceiverStrategy::CreateAudioStrategy( RTPReceiverAudio::RTPReceiverAudio(RtpData* data_callback) : RTPReceiverStrategy(data_callback), TelephoneEventHandler(), - last_received_frequency_(8000), telephone_event_forward_to_decoder_(false), telephone_event_payload_type_(-1), cng_nb_payload_type_(-1), cng_wb_payload_type_(-1), cng_swb_payload_type_(-1), cng_fb_payload_type_(-1), - cng_payload_type_(-1), - g722_payload_type_(-1), - last_received_g722_(false), num_energy_(0), current_remote_energy_() { last_payload_.Audio.channels = 1; @@ -61,53 +57,12 @@ bool RTPReceiverAudio::TelephoneEventPayloadType( return telephone_event_payload_type_ == payload_type; } -bool RTPReceiverAudio::CNGPayloadType(int8_t payload_type, - uint32_t* frequency, - bool* cng_payload_type_has_changed) { +bool RTPReceiverAudio::CNGPayloadType(int8_t payload_type) { rtc::CritScope lock(&crit_sect_); - *cng_payload_type_has_changed = false; - - // We can have four CNG on 8000Hz, 16000Hz, 32000Hz and 48000Hz. - if (cng_nb_payload_type_ == payload_type) { - *frequency = 8000; - if (cng_payload_type_ != -1 && cng_payload_type_ != cng_nb_payload_type_) - *cng_payload_type_has_changed = true; - - cng_payload_type_ = cng_nb_payload_type_; - return true; - } else if (cng_wb_payload_type_ == payload_type) { - // if last received codec is G.722 we must use frequency 8000 - if (last_received_g722_) { - *frequency = 8000; - } else { - *frequency = 16000; - } - if (cng_payload_type_ != -1 && cng_payload_type_ != cng_wb_payload_type_) - *cng_payload_type_has_changed = true; - cng_payload_type_ = cng_wb_payload_type_; - return true; - } else if (cng_swb_payload_type_ == payload_type) { - *frequency = 32000; - if ((cng_payload_type_ != -1) && - (cng_payload_type_ != cng_swb_payload_type_)) - *cng_payload_type_has_changed = true; - cng_payload_type_ = cng_swb_payload_type_; - return true; - } else if (cng_fb_payload_type_ == payload_type) { - *frequency = 48000; - if (cng_payload_type_ != -1 && cng_payload_type_ != cng_fb_payload_type_) - *cng_payload_type_has_changed = true; - cng_payload_type_ = cng_fb_payload_type_; - return true; - } else { - // not CNG - if (g722_payload_type_ == payload_type) { - last_received_g722_ = true; - } else { - last_received_g722_ = false; - } - } - return false; + return payload_type == cng_nb_payload_type_ || + payload_type == cng_wb_payload_type_ || + payload_type == cng_swb_payload_type_ || + payload_type == cng_fb_payload_type_; } bool RTPReceiverAudio::ShouldReportCsrcChanges(uint8_t payload_type) const { @@ -157,7 +112,7 @@ int32_t RTPReceiverAudio::OnNewPayloadTypeCreated( telephone_event_payload_type_ = payload_type; } if (RtpUtility::StringCompare(payload_name, "cn", 2)) { - // we can have three CNG on 8000Hz, 16000Hz and 32000Hz + // We support comfort noise at four different frequencies. if (frequency == 8000) { cng_nb_payload_type_ = payload_type; } else if (frequency == 16000) { @@ -204,14 +159,6 @@ int32_t RTPReceiverAudio::ParseRtpPacket(WebRtcRTPHeader* rtp_header, is_red); } -int RTPReceiverAudio::GetPayloadTypeFrequency() const { - rtc::CritScope lock(&crit_sect_); - if (last_received_g722_) { - return 8000; - } - return last_received_frequency_; -} - RTPAliveType RTPReceiverAudio::ProcessDeadOrAlive( uint16_t last_payload_length) const { @@ -225,26 +172,10 @@ RTPAliveType RTPReceiverAudio::ProcessDeadOrAlive( } void RTPReceiverAudio::CheckPayloadChanged(int8_t payload_type, - PayloadUnion* specific_payload, + PayloadUnion* /* specific_payload */, bool* should_discard_changes) { - *should_discard_changes = false; - - if (TelephoneEventPayloadType(payload_type)) { - // Don't do callbacks for DTMF packets. - *should_discard_changes = true; - return; - } - // frequency is updated for CNG - bool cng_payload_type_has_changed = false; - bool is_cng_payload_type = CNGPayloadType(payload_type, - &specific_payload->Audio.frequency, - &cng_payload_type_has_changed); - - if (is_cng_payload_type) { - // Don't do callbacks for DTMF packets. - *should_discard_changes = true; - return; - } + *should_discard_changes = + TelephoneEventPayloadType(payload_type) || CNGPayloadType(payload_type); } int RTPReceiverAudio::Energy(uint8_t array_of_energy[kRtpCsrcSize]) const { @@ -337,16 +268,8 @@ int32_t RTPReceiverAudio::ParseAudioCodecSpecific( { rtc::CritScope lock(&crit_sect_); - if (!telephone_event_packet) { - last_received_frequency_ = audio_specific.frequency; - } - // Check if this is a CNG packet, receiver might want to know - uint32_t ignored; - bool also_ignored; - if (CNGPayloadType(rtp_header->header.payloadType, - &ignored, - &also_ignored)) { + if (CNGPayloadType(rtp_header->header.payloadType)) { rtp_header->type.Audio.isCNG = true; rtp_header->frameType = kAudioFrameCN; } else { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h index d5d89bae2d..29a4c7c391 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h @@ -36,16 +36,13 @@ class RTPReceiverAudio : public RTPReceiverStrategy, // Is forwarding of outband telephone events turned on/off? bool TelephoneEventForwardToDecoder() const override; - // Is TelephoneEvent configured with payload type payload_type + // Is TelephoneEvent configured with |payload_type|. bool TelephoneEventPayloadType(const int8_t payload_type) const override; TelephoneEventHandler* GetTelephoneEventHandler() override { return this; } - // Returns true if CNG is configured with payload type payload_type. If so, - // the frequency and cng_payload_type_has_changed are filled in. - bool CNGPayloadType(const int8_t payload_type, - uint32_t* frequency, - bool* cng_payload_type_has_changed); + // Returns true if CNG is configured with |payload_type|. + bool CNGPayloadType(const int8_t payload_type); int32_t ParseRtpPacket(WebRtcRTPHeader* rtp_header, const PayloadUnion& specific_payload, @@ -55,8 +52,6 @@ class RTPReceiverAudio : public RTPReceiverStrategy, int64_t timestamp_ms, bool is_first_packet) override; - int GetPayloadTypeFrequency() const override; - RTPAliveType ProcessDeadOrAlive(uint16_t last_payload_length) const override; bool ShouldReportCsrcChanges(uint8_t payload_type) const override; @@ -107,12 +102,6 @@ class RTPReceiverAudio : public RTPReceiverStrategy, int8_t cng_wb_payload_type_; int8_t cng_swb_payload_type_; int8_t cng_fb_payload_type_; - int8_t cng_payload_type_; - - // G722 is special since it use the wrong number of RTP samples in timestamp - // VS. number of samples in the frame - int8_t g722_payload_type_; - bool last_received_g722_; uint8_t num_energy_; uint8_t current_remote_energy_[kRtpCsrcSize]; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h index 663b883295..f435231764 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h @@ -46,9 +46,6 @@ class RTPReceiverStrategy { virtual TelephoneEventHandler* GetTelephoneEventHandler() = 0; - // Retrieves the last known applicable frequency. - virtual int GetPayloadTypeFrequency() const = 0; - // Computes the current dead-or-alive state. virtual RTPAliveType ProcessDeadOrAlive( uint16_t last_payload_length) const = 0; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc index 9f6d80ae0a..90e19c0645 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -108,10 +108,6 @@ int32_t RTPReceiverVideo::ParseRtpPacket(WebRtcRTPHeader* rtp_header, : -1; } -int RTPReceiverVideo::GetPayloadTypeFrequency() const { - return kVideoPayloadTypeFrequency; -} - RTPAliveType RTPReceiverVideo::ProcessDeadOrAlive( uint16_t last_payload_length) const { return kRtpDead; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h index a8aaf5da18..f1628b10ef 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h @@ -35,8 +35,6 @@ class RTPReceiverVideo : public RTPReceiverStrategy { TelephoneEventHandler* GetTelephoneEventHandler() override { return NULL; } - int GetPayloadTypeFrequency() const override; - RTPAliveType ProcessDeadOrAlive(uint16_t last_payload_length) const override; bool ShouldReportCsrcChanges(uint8_t payload_type) const override;