Multi frequency DTMF support - sender side

webrtc/modules/rtp_rtcp/source/dtmf_queue.cc

 /*
  *  Copyright (c) 2011 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/dtmf_queue.h"
-
-#include <string.h>
+#include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h"

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

I'd rather you moved DTMF_OUTBAND_MAX to this file instead, since it is only
used here.

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

Agree.

 
 namespace webrtc {
-DTMFqueue::DTMFqueue() : next_empty_index_(0) {
-  memset(dtmf_key_, 0, sizeof(dtmf_key_));
-  memset(dtmf_length, 0, sizeof(dtmf_length));
-  memset(dtmf_level_, 0, sizeof(dtmf_level_));
-}
+DTMFQueue::DTMFQueue() {}
 
-DTMFqueue::~DTMFqueue() {}
+DTMFQueue::~DTMFQueue() {}
 
-int32_t DTMFqueue::AddDTMF(uint8_t key, uint16_t len, uint8_t level) {
+int DTMFQueue::AddDTMF(const Event& event) {

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

This looks like a bool-returning method to me.

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

Done.

   rtc::CritScope lock(&dtmf_critsect_);
-
-  if (next_empty_index_ >= DTMF_OUTBAND_MAX) {
+  if (queue_.size() >= DTMF_OUTBAND_MAX) {

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

Probably no longer needed as such, since queue_ is not limited in size as
before. Could of course be useful to keep it from growing indefinitely in case
of some weird error in the calling code.

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

Yes.

     return -1;
   }
-  int32_t index = next_empty_index_;
-  dtmf_key_[index] = key;
-  dtmf_length[index] = len;
-  dtmf_level_[index] = level;
-  next_empty_index_++;
+  queue_.push_back(event);
   return 0;
 }
 
-int8_t DTMFqueue::NextDTMF(uint8_t* dtmf_key, uint16_t* len, uint8_t* level) {
+int DTMFQueue::NextDTMF(Event* event) {

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

Wouldn't hurt to change this to return an rtc::Optional<event>.

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

Changed it at first, but decided against. since it would require a larger change
to the logic in the one place the method is being called.

[hlundin-webrtc, 2016/11/08 09:01:59]

Acknowledged.

+  RTC_DCHECK(event);
   rtc::CritScope lock(&dtmf_critsect_);
-  if (next_empty_index_ == 0)
+  if (queue_.empty()) {
     return -1;
+  }
 
-  *dtmf_key = dtmf_key_[0];
-  *len = dtmf_length[0];
-  *level = dtmf_level_[0];
-
-  memmove(&(dtmf_key_[0]), &(dtmf_key_[1]),
-          next_empty_index_ * sizeof(uint8_t));
-  memmove(&(dtmf_length[0]), &(dtmf_length[1]),
-          next_empty_index_ * sizeof(uint16_t));
-  memmove(&(dtmf_level_[0]), &(dtmf_level_[1]),
-          next_empty_index_ * sizeof(uint8_t));
-
-  next_empty_index_--;
+  *event = queue_.front();
+  queue_.pop_front();
   return 0;
 }
 
-bool DTMFqueue::PendingDTMF() {
+bool DTMFQueue::PendingDTMF() {
   rtc::CritScope lock(&dtmf_critsect_);
-  return next_empty_index_ > 0;
+  return !queue_.empty();
 }
 }  // namespace webrtc