Multi frequency DTMF support - sender side

webrtc/media/engine/webrtcvoiceengine.cc

 /*
  *  Copyright (c) 2004 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.
  */
 
@@ skipping 1222 matching lines @@
     max_send_bitrate_bps_ = bps;
 
     if (config_.send_codec_spec.codec_inst.rate != *send_rate) {
       // Recreate AudioSendStream with new bit rate.
       config_.send_codec_spec.codec_inst.rate = *send_rate;
       RecreateAudioSendStream();
     }
     return true;
   }
 
-  bool SendTelephoneEvent(int payload_type, int event, int duration_ms) {
+  bool SendTelephoneEvent(int payload_type, int payload_freq, int event,
+                          int duration_ms) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     RTC_DCHECK(stream_);
-    return stream_->SendTelephoneEvent(payload_type, event, duration_ms);
+    return stream_->SendTelephoneEvent(payload_type, payload_freq, event,
+                                       duration_ms);
   }
 
   void SetSend(bool send) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     send_ = send;
     UpdateSendState();
   }
 
   void SetMuted(bool muted) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
@@ skipping 558 matching lines @@
 // codec settings from the given list of codecs (originally from SDP). Both send
 // and receive streams may be reconfigured based on the new settings.
 bool WebRtcVoiceMediaChannel::SetSendCodecs(
     const std::vector<AudioCodec>& codecs) {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   // TODO(solenberg): Validate input - that payload types don't overlap, are
   //                  within range, filter out codecs we don't support,
   //                  redundant codecs etc - the same way it is done for
   //                  RtpHeaderExtensions.
 
-  // Find the DTMF telephone event "codec" payload type.
+  // Find PT of telephone-event codec with lowest clockrate, as a fallback, in
+  // case we don't have a DTMF codec with a rate matching the send codec's, or
+  // if this function returns early.
   dtmf_payload_type_ = rtc::Optional<int>();
+  dtmf_payload_freq_ = -1;
+  std::vector<AudioCodec> dtmf_codecs;
   for (const AudioCodec& codec : codecs) {
+    if (codec.id < kMinPayloadType || codec.id > kMaxPayloadType) {
+      // Note that we return if *any* codec's PT is out of range.

[hlundin-webrtc, 2016/10/28 08:12:36]

This is more like an overall sanity check of the input. I think you should break
this out of the otherwise DTMF-specific for loop, and put it on top. Consider
using std::find_if_not.

[the sun, 2016/11/07 13:33:31]

Done.

+      return false;
+    }
     if (IsCodec(codec, kDtmfCodecName)) {
-      if (codec.id < kMinPayloadType || codec.id > kMaxPayloadType) {
-        return false;
-      }
-      dtmf_payload_type_ = rtc::Optional<int>(codec.id);
-      break;
+      dtmf_codecs.push_back(codec);
     }
   }
+  if (!dtmf_codecs.empty()) {
+    std::sort(dtmf_codecs.begin(), dtmf_codecs.end(),
+        [](const AudioCodec& a, const AudioCodec& b) {
+          return a.clockrate < b.clockrate;
+        });
+    dtmf_payload_type_ = rtc::Optional<int>(dtmf_codecs[0].id);
+    dtmf_payload_freq_ = dtmf_codecs[0].clockrate;
+  }
 
   // Scan through the list to figure out the codec to use for sending, along
   // with the proper configuration for VAD, CNG, NACK and Opus-specific
   // parameters.
   // TODO(solenberg): Refactor this logic once we create AudioEncoders here.
   webrtc::AudioSendStream::Config::SendCodecSpec send_codec_spec;
   {
     send_codec_spec.nack_enabled = send_codec_spec_.nack_enabled;
 
     // Find send codec (the first non-telephone-event/CN codec).
@@ skipping 54 matching lines @@
                             << " not supported.";
             continue;
         }
         send_codec_spec.cng_payload_type = codec.id;
         send_codec_spec.cng_plfreq = cng_plfreq;
         break;
       }
     }
   }
 
+  // Find the telephone-event PT exactly matching the preferred send codec.
+  for (const AudioCodec& codec : dtmf_codecs) {
+    if (codec.clockrate == send_codec_spec.codec_inst.plfreq) {
+      dtmf_payload_type_ = rtc::Optional<int>(codec.id);
+      dtmf_payload_freq_ = codec.clockrate;

[hlundin-webrtc, 2016/10/28 08:12:36]

Is there any reason to continue the for loop after finding a match?

You could use std::find_if, but I'm not sure the code gets any more readable:

const auto result = std::find_if(dtmf_codecs.begin(), dtmf_codecs.end(), 
  [&send_codec_spec](const AudioCodec& a) {return a.clockrate ==
send_codec_spec.codec_inst.plfreq;});
if (result != dtmf_codecs.end()) {
  dtmf_payload_type_ = rtc::Optional<int>(result->id);
  dtmf_payload_freq_ = result->clockrate;
}

Yeah, not very pretty...

[the sun, 2016/11/07 13:33:31]

Added a simple "break" instead.

+    }
+  }
+
   // Apply new settings to all streams.
   if (send_codec_spec_ != send_codec_spec) {
     send_codec_spec_ = std::move(send_codec_spec);
     for (const auto& kv : send_streams_) {
       kv.second->RecreateAudioSendStream(send_codec_spec_);
     }
   }
 
   // Check if the transport cc feedback or NACK status has changed on the
   // preferred send codec, and in that case reconfigure all receive streams.
@@ skipping 386 matching lines @@
   if (event < kMinTelephoneEventCode ||
       event > kMaxTelephoneEventCode) {
     LOG(LS_WARNING) << "DTMF event code " << event << " out of range.";
     return false;
   }
   if (duration < kMinTelephoneEventDuration ||
       duration > kMaxTelephoneEventDuration) {
     LOG(LS_WARNING) << "DTMF event duration " << duration << " out of range.";
     return false;
   }
-  return it->second->SendTelephoneEvent(*dtmf_payload_type_, event, duration);
+  RTC_DCHECK_NE(-1, dtmf_payload_freq_);
+  return it->second->SendTelephoneEvent(*dtmf_payload_type_, dtmf_payload_freq_,
+                                        event, duration);
 }
 
 void WebRtcVoiceMediaChannel::OnPacketReceived(
     rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time) {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
 
   const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp,
                                               packet_time.not_before);
   webrtc::PacketReceiver::DeliveryStatus delivery_result =
       call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO,
@@ skipping 211 matching lines @@
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   const auto it = send_streams_.find(ssrc);
   if (it != send_streams_.end()) {
     return it->second->channel();
   }
   return -1;
 }
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE