Allow DTMF delay configurability
This commit enables developers to configure the "," delay value from
the WebRTC spec value of 2 seconds. This flexibility allows developers
to comply with existing WebRTC clients.
Bug: webrtc:11273
Change-Id: Ia94b99e041df882e2396d0926a8f4188afe55885
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165700
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30354}
diff --git a/api/dtmf_sender_interface.h b/api/dtmf_sender_interface.h
index 9cdfba1..7c0e2ce 100644
--- a/api/dtmf_sender_interface.h
+++ b/api/dtmf_sender_interface.h
@@ -44,6 +44,9 @@
// See: https://www.w3.org/TR/webrtc/#peer-to-peer-dtmf
class DtmfSenderInterface : public rtc::RefCountInterface {
public:
+ // Provides the spec compliant default 2 second delay for the ',' character.
+ static const int kDtmfDefaultCommaDelayMs = 2000;
+
// Used to receive events from the DTMF sender. Only one observer can be
// registered at a time. UnregisterObserver should be called before the
// observer object is destroyed.
@@ -71,12 +74,29 @@
// |inter_tone_gap| must be at least 50 ms but should be as short as
// possible.
//
+ // The |comma_delay| parameter indicates the delay after the ','
+ // character. InsertDtmf specifies |comma_delay| as an argument
+ // with a default value of 2 seconds as per the WebRTC spec. This parameter
+ // allows users to comply with legacy WebRTC clients. The |comma_delay|
+ // must be at least 50 ms.
+ //
// If InsertDtmf is called on the same object while an existing task for this
// object to generate DTMF is still running, the previous task is canceled.
// Returns true on success and false on failure.
virtual bool InsertDtmf(const std::string& tones,
int duration,
- int inter_tone_gap) = 0;
+ int inter_tone_gap) {
+ return InsertDtmf(tones, duration, inter_tone_gap,
+ kDtmfDefaultCommaDelayMs);
+ }
+ virtual bool InsertDtmf(const std::string& tones,
+ int duration,
+ int inter_tone_gap,
+ int comma_delay) {
+ // TODO(bugs.webrtc.org/165700): Remove once downstream implementations
+ // override this signature rather than the 3-parameter one.
+ return InsertDtmf(tones, duration, inter_tone_gap);
+ }
// Returns the tones remaining to be played out.
virtual std::string tones() const = 0;
@@ -91,6 +111,11 @@
// default value of 50 ms if InsertDtmf() was never called.
virtual int inter_tone_gap() const = 0;
+ // Returns the current value of the "," character delay in ms.
+ // This value will be the value last set via the InsertDtmf() method, or the
+ // default value of 2000 ms if InsertDtmf() was never called.
+ virtual int comma_delay() const { return kDtmfDefaultCommaDelayMs; }
+
protected:
~DtmfSenderInterface() override = default;
};
diff --git a/pc/dtmf_sender.cc b/pc/dtmf_sender.cc
index af5b809..1037802 100644
--- a/pc/dtmf_sender.cc
+++ b/pc/dtmf_sender.cc
@@ -33,8 +33,7 @@
// +-------+--------+------+---------+
// The "," is a special event defined by the WebRTC spec. It means to delay for
// 2 seconds before processing the next tone. We use -1 as its code.
-static const int kDtmfCodeTwoSecondDelay = -1;
-static const int kDtmfTwoSecondInMs = 2000;
+static const int kDtmfCommaDelay = -1;
static const char kDtmfValidTones[] = ",0123456789*#ABCDabcd";
static const char kDtmfTonesTable[] = ",0123456789*#ABCD";
// The duration cannot be more than 6000ms or less than 40ms. The gap between
@@ -76,7 +75,8 @@
signaling_thread_(signaling_thread),
provider_(provider),
duration_(kDtmfDefaultDurationMs),
- inter_tone_gap_(kDtmfDefaultGapMs) {
+ inter_tone_gap_(kDtmfDefaultGapMs),
+ comma_delay_(kDtmfDefaultCommaDelayMs) {
RTC_DCHECK(signaling_thread_);
if (provider_) {
RTC_DCHECK(provider_->GetOnDestroyedSignal());
@@ -107,11 +107,12 @@
bool DtmfSender::InsertDtmf(const std::string& tones,
int duration,
- int inter_tone_gap) {
+ int inter_tone_gap,
+ int comma_delay) {
RTC_DCHECK(signaling_thread_->IsCurrent());
if (duration > kDtmfMaxDurationMs || duration < kDtmfMinDurationMs ||
- inter_tone_gap < kDtmfMinGapMs) {
+ inter_tone_gap < kDtmfMinGapMs || comma_delay < kDtmfMinGapMs) {
RTC_LOG(LS_ERROR)
<< "InsertDtmf is called with invalid duration or tones gap. "
"The duration cannot be more than "
@@ -130,6 +131,7 @@
tones_ = tones;
duration_ = duration;
inter_tone_gap_ = inter_tone_gap;
+ comma_delay_ = comma_delay;
// Clear the previous queue.
dtmf_driver_.Clear();
// Kick off a new DTMF task queue.
@@ -149,6 +151,10 @@
return inter_tone_gap_;
}
+int DtmfSender::comma_delay() const {
+ return comma_delay_;
+}
+
void DtmfSender::QueueInsertDtmf(const rtc::Location& posted_from,
uint32_t delay_ms) {
dtmf_driver_.AsyncInvokeDelayed<void>(
@@ -180,10 +186,12 @@
}
int tone_gap = inter_tone_gap_;
- if (code == kDtmfCodeTwoSecondDelay) {
- // Special case defined by WebRTC - The character',' indicates a delay of 2
- // seconds before processing the next character in the tones parameter.
- tone_gap = kDtmfTwoSecondInMs;
+ if (code == kDtmfCommaDelay) {
+ // Special case defined by WebRTC - By default, the character ',' indicates
+ // a delay of 2 seconds before processing the next character in the tones
+ // parameter. The comma delay can be set to a non default value via
+ // InsertDtmf to comply with legacy WebRTC clients.
+ tone_gap = comma_delay_;
} else {
if (!provider_) {
RTC_LOG(LS_ERROR) << "The DtmfProvider has been destroyed.";
diff --git a/pc/dtmf_sender.h b/pc/dtmf_sender.h
index 692c74b..e332a7e 100644
--- a/pc/dtmf_sender.h
+++ b/pc/dtmf_sender.h
@@ -56,10 +56,12 @@
bool CanInsertDtmf() override;
bool InsertDtmf(const std::string& tones,
int duration,
- int inter_tone_gap) override;
+ int inter_tone_gap,
+ int comma_delay = kDtmfDefaultCommaDelayMs) override;
std::string tones() const override;
int duration() const override;
int inter_tone_gap() const override;
+ int comma_delay() const override;
protected:
DtmfSender(rtc::Thread* signaling_thread, DtmfProviderInterface* provider);
@@ -83,6 +85,7 @@
std::string tones_;
int duration_;
int inter_tone_gap_;
+ int comma_delay_;
// Invoker for running delayed tasks which feed the DTMF provider one tone at
// a time.
rtc::AsyncInvoker dtmf_driver_;
@@ -96,10 +99,11 @@
PROXY_METHOD1(void, RegisterObserver, DtmfSenderObserverInterface*)
PROXY_METHOD0(void, UnregisterObserver)
PROXY_METHOD0(bool, CanInsertDtmf)
-PROXY_METHOD3(bool, InsertDtmf, const std::string&, int, int)
+PROXY_METHOD4(bool, InsertDtmf, const std::string&, int, int, int)
PROXY_CONSTMETHOD0(std::string, tones)
PROXY_CONSTMETHOD0(int, duration)
PROXY_CONSTMETHOD0(int, inter_tone_gap)
+PROXY_CONSTMETHOD0(int, comma_delay)
END_PROXY_MAP()
// Get DTMF code from the DTMF event character.
diff --git a/pc/dtmf_sender_unittest.cc b/pc/dtmf_sender_unittest.cc
index 3f59af0..f7f229a 100644
--- a/pc/dtmf_sender_unittest.cc
+++ b/pc/dtmf_sender_unittest.cc
@@ -133,10 +133,12 @@
// Constructs a list of DtmfInfo from |tones|, |duration| and
// |inter_tone_gap|.
- void GetDtmfInfoFromString(const std::string& tones,
- int duration,
- int inter_tone_gap,
- std::vector<FakeDtmfProvider::DtmfInfo>* dtmfs) {
+ void GetDtmfInfoFromString(
+ const std::string& tones,
+ int duration,
+ int inter_tone_gap,
+ std::vector<FakeDtmfProvider::DtmfInfo>* dtmfs,
+ int comma_delay = webrtc::DtmfSender::kDtmfDefaultCommaDelayMs) {
// Init extra_delay as -inter_tone_gap - duration to ensure the first
// DtmfInfo's gap field will be 0.
int extra_delay = -1 * (inter_tone_gap + duration);
@@ -147,7 +149,7 @@
int code = 0;
webrtc::GetDtmfCode(tone, &code);
if (tone == ',') {
- extra_delay = 2000; // 2 seconds
+ extra_delay = comma_delay;
} else {
dtmfs->push_back(FakeDtmfProvider::DtmfInfo(
code, duration, duration + inter_tone_gap + extra_delay));
@@ -165,11 +167,14 @@
}
// Verify the provider got all the expected calls.
- void VerifyOnProvider(const std::string& tones,
- int duration,
- int inter_tone_gap) {
+ void VerifyOnProvider(
+ const std::string& tones,
+ int duration,
+ int inter_tone_gap,
+ int comma_delay = webrtc::DtmfSender::kDtmfDefaultCommaDelayMs) {
std::vector<FakeDtmfProvider::DtmfInfo> dtmf_queue_ref;
- GetDtmfInfoFromString(tones, duration, inter_tone_gap, &dtmf_queue_ref);
+ GetDtmfInfoFromString(tones, duration, inter_tone_gap, &dtmf_queue_ref,
+ comma_delay);
VerifyOnProvider(dtmf_queue_ref);
}
@@ -310,15 +315,33 @@
VerifyOnObserver("1");
}
-TEST_F(DtmfSenderTest, InsertDtmfWithCommaAsDelay) {
+TEST_F(DtmfSenderTest, InsertDtmfWithDefaultCommaDelay) {
std::string tones = "3,4";
int duration = 100;
int inter_tone_gap = 50;
+ int default_comma_delay = webrtc::DtmfSender::kDtmfDefaultCommaDelayMs;
+ EXPECT_EQ(dtmf_->comma_delay(), default_comma_delay);
EXPECT_TRUE(dtmf_->InsertDtmf(tones, duration, inter_tone_gap));
EXPECT_TRUE_SIMULATED_WAIT(observer_->completed(), kMaxWaitMs, fake_clock_);
VerifyOnProvider(tones, duration, inter_tone_gap);
VerifyOnObserver(tones);
+ EXPECT_EQ(dtmf_->comma_delay(), default_comma_delay);
+}
+
+TEST_F(DtmfSenderTest, InsertDtmfWithNonDefaultCommaDelay) {
+ std::string tones = "3,4";
+ int duration = 100;
+ int inter_tone_gap = 50;
+ int default_comma_delay = webrtc::DtmfSender::kDtmfDefaultCommaDelayMs;
+ int comma_delay = 500;
+ EXPECT_EQ(dtmf_->comma_delay(), default_comma_delay);
+ EXPECT_TRUE(dtmf_->InsertDtmf(tones, duration, inter_tone_gap, comma_delay));
+ EXPECT_TRUE_SIMULATED_WAIT(observer_->completed(), kMaxWaitMs, fake_clock_);
+
+ VerifyOnProvider(tones, duration, inter_tone_gap, comma_delay);
+ VerifyOnObserver(tones);
+ EXPECT_EQ(dtmf_->comma_delay(), comma_delay);
}
TEST_F(DtmfSenderTest, TryInsertDtmfWhenItDoesNotWork) {
@@ -337,6 +360,7 @@
EXPECT_FALSE(dtmf_->InsertDtmf(tones, 6001, inter_tone_gap));
EXPECT_FALSE(dtmf_->InsertDtmf(tones, 39, inter_tone_gap));
EXPECT_FALSE(dtmf_->InsertDtmf(tones, duration, 29));
+ EXPECT_FALSE(dtmf_->InsertDtmf(tones, duration, inter_tone_gap, 29));
EXPECT_TRUE(dtmf_->InsertDtmf(tones, duration, inter_tone_gap));
}