Multi frequency DTMF support - sender side

webrtc/media/engine/webrtcvoiceengine.cc

Index: webrtc/media/engine/webrtcvoiceengine.cc
diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc
index 222b246992af1a828d1fec3872f013013ad404fb..67a27d129e99922324d635891061a26c37df2268 100644
--- a/webrtc/media/engine/webrtcvoiceengine.cc
+++ b/webrtc/media/engine/webrtcvoiceengine.cc
@@ -1240,10 +1240,12 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     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) {
@@ -1822,17 +1824,29 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
   //                  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
@@ -1907,6 +1921,14 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
     }
   }
 
+  // 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);
@@ -2313,7 +2335,9 @@ bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc, int event,
     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(