RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up

webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h"
 
 #include <assert.h>  // assert
 #include <math.h>   // pow()
 #include <string.h>  // memcpy()
 
 #include "webrtc/base/logging.h"
 #include "webrtc/base/trace_event.h"
 
 namespace webrtc {
 RTPReceiverStrategy* RTPReceiverStrategy::CreateAudioStrategy(
     RtpData* data_callback) {
   return new RTPReceiverAudio(data_callback);
 }
 
 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;
   memset(current_remote_energy_, 0, sizeof(current_remote_energy_));
 }
 
 // Outband TelephoneEvent(DTMF) detection
 void RTPReceiverAudio::SetTelephoneEventForwardToDecoder(
     bool forward_to_decoder) {
   rtc::CritScope lock(&crit_sect_);
   telephone_event_forward_to_decoder_ = forward_to_decoder;
 }
 
 // Is forwarding of outband telephone events turned on/off?
 bool RTPReceiverAudio::TelephoneEventForwardToDecoder() const {
   rtc::CritScope lock(&crit_sect_);
   return telephone_event_forward_to_decoder_;
 }
 
 bool RTPReceiverAudio::TelephoneEventPayloadType(
     int8_t payload_type) const {
   rtc::CritScope lock(&crit_sect_);
   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.

[hlundin-webrtc, 2016/10/04 11:38:18]

Wording is a bit choppy in this comment. Bonus points if you improve that too.

[ossu, 2016/10/04 12:40:24]

I've reworded here and moved it down to where the other similar comment was. It
makes more sense to clarify there than here.

-  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 {
   // Don't do this for DTMF packets, otherwise it's fine.
   return !TelephoneEventPayloadType(payload_type);
 }
 
 // -   Sample based or frame based codecs based on RFC 3551
 // -
 // -   NOTE! There is one error in the RFC, stating G.722 uses 8 bits/samples.
@@ skipping 29 matching lines @@
 int32_t RTPReceiverAudio::OnNewPayloadTypeCreated(
     const char payload_name[RTP_PAYLOAD_NAME_SIZE],
     int8_t payload_type,
     uint32_t frequency) {
   rtc::CritScope lock(&crit_sect_);
 
   if (RtpUtility::StringCompare(payload_name, "telephone-event", 15)) {
     telephone_event_payload_type_ = payload_type;
   }
   if (RtpUtility::StringCompare(payload_name, "cn", 2)) {
-    //  we can have three CNG on 8000Hz, 16000Hz and 32000Hz

[hlundin-webrtc, 2016/10/04 11:38:18]

lol

     if (frequency == 8000) {
       cng_nb_payload_type_ = payload_type;
     } else if (frequency == 16000) {
       cng_wb_payload_type_ = payload_type;
     } else if (frequency == 32000) {
       cng_swb_payload_type_ = payload_type;
     } else if (frequency == 48000) {
       cng_fb_payload_type_ = payload_type;
     } else {
       assert(false);
@@ skipping 26 matching lines @@
     LOG(LS_INFO) << "Received first audio RTP packet";
   }
 
   return ParseAudioCodecSpecific(rtp_header,
                                  payload,
                                  payload_length,
                                  specific_payload.Audio,
                                  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 {
 
   // Our CNG is 9 bytes; if it's a likely CNG the receiver needs to check
   // kRtpNoRtp against NetEq speech_type kOutputPLCtoCNG.
   if (last_payload_length < 10) {  // our CNG is 9 bytes
     return kRtpNoRtp;
   } else {
     return kRtpDead;
   }
 }
 
 void RTPReceiverAudio::CheckPayloadChanged(int8_t payload_type,

[danilchap, 2016/10/03 14:29:38]

may be pull string further and change signature of this function into something
like
bool RTPReceiverStrategy::ShouldDiscardChanges(int payload_type);

[hlundin-webrtc, 2016/10/04 11:38:18]

Would be nice, but I'm afraid that will lead into RTPReceiverVideo as well.

                                            PayloadUnion* specific_payload,

[hlundin-webrtc, 2016/10/04 11:38:18]

/* specific_payload */

[ossu, 2016/10/04 12:40:24]

Done.

                                            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 {
   rtc::CritScope cs(&crit_sect_);
 
   assert(num_energy_ <= kRtpCsrcSize);
 
   if (num_energy_ > 0) {
     memcpy(array_of_energy, current_remote_energy_,
            sizeof(uint8_t) * num_energy_);
@@ skipping 72 matching lines @@
 
     // RFC 4733 2.5.1.3 & 2.5.2.3 Long-Duration Events
     // should not be a problem since we don't care about the duration
 
     // RFC 4733 See 2.5.1.5. & 2.5.2.4.  Multiple Events in a Packet
   }
 
   {
     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 {
       rtp_header->frameType = kAudioFrameSpeech;
       rtp_header->type.Audio.isCNG = false;
     }
 
     // check if it's a DTMF event, hence something we can playout
     if (telephone_event_packet) {
       if (!telephone_event_forward_to_decoder_) {
@@ skipping 16 matching lines @@
     // only one frame in the RED strip the one byte to help NetEq
     return data_callback_->OnReceivedPayloadData(
         payload_data + 1, payload_length - 1, rtp_header);
   }
 
   rtp_header->type.Audio.channel = audio_specific.channels;
   return data_callback_->OnReceivedPayloadData(
       payload_data, payload_length, rtp_header);
 }
 }  // namespace webrtc