[Asterisk-code-review] channel.c: Resolve issue with receiving SIP INFO packets for DTMF (asterisk[master])

Friendly Automation asteriskteam at digium.com
Fri Dec 6 08:46:30 CST 2019


Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13343 )

Change subject: channel.c: Resolve issue with receiving SIP INFO packets for DTMF
......................................................................

channel.c: Resolve issue with receiving SIP INFO packets for DTMF

The problem is essentially the same as in ASTERISK~28245. Besides
the direct media scenario we have an additional scenario where a
special client is involved. This device mutes audio by default in
transmit direction (no rtp frames) and activates audio only by a
foot switch. In this situation dtmf input (pin for conferences,
transfer features codes , etc) using SIP INFO mode is not
understood properly especially when SIP INFO messages are sent
quickly.

This patch ensures that SIP INFO frames are properly queued and
processed in the above scenario. The patch also corrects situations
where successive dtmf events are received quicker than the
signalled event duration (plus minimum gap/pause) allows, i.e. DTMF
events have to be buffered in the ast channel read queue and
emulation has to be processed asynchronously at slower speed.

Reported by: Thomas Arimont
patches:
  trigger_dtmf_emulation.patch submitted by Thomas Arimont (license 5525)

Change-Id: I309bf61dd065c9978c8e48f5b9a936ab47de64c2
---
M main/channel.c
1 file changed, 102 insertions(+), 3 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/main/channel.c b/main/channel.c
index f096087..c350177 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -2819,6 +2819,33 @@
 	return (ast_channel_get_up_time_ms(chan) / 1000);
 }
 
+/*!
+ * \brief Determine whether or not we have to trigger dtmf emulating using 50 fps timer events
+ * especially when no voice frames are received during dtmf processing (direct media or muted
+ * sender case using SIP INFO)
+ */
+static inline int should_trigger_dtmf_emulating(struct ast_channel *chan)
+{
+	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF | AST_FLAG_EMULATE_DTMF)) {
+		/* We're in the middle of emulating a digit, or DTMF has been
+		 * explicitly deferred. Trigger dtmf with periodic 50 pfs timer events, then. */
+		return 1;
+	}
+
+	if (!ast_tvzero(*ast_channel_dtmf_tv(chan)) &&
+			ast_tvdiff_ms(ast_tvnow(), *ast_channel_dtmf_tv(chan)) < 2*AST_MIN_DTMF_GAP) {
+		/*
+		 * We're not in the middle of a digit, but it hasn't been long enough
+		 * since the last digit, so we'll have to trigger DTMF furtheron.
+		 * Using 2 times AST_MIN_DTMF_GAP to trigger readq reading for possible
+		 * buffered next dtmf event
+		 */
+		return 1;
+	}
+
+	return 0;
+}
+
 static void deactivate_generator_nolock(struct ast_channel *chan)
 {
 	if (ast_channel_generatordata(chan)) {
@@ -2839,6 +2866,10 @@
 {
 	ast_channel_lock(chan);
 	deactivate_generator_nolock(chan);
+	if (should_trigger_dtmf_emulating(chan)) {
+		/* if in the middle of dtmf emulation keep 50 tick per sec timer on rolling */
+		ast_timer_set_rate(ast_channel_timer(chan), 50);
+	}
 	ast_channel_unlock(chan);
 }
 
@@ -3508,6 +3539,7 @@
 
 	if (ast_channel_timingfd(chan) > -1 && ast_channel_fdno(chan) == AST_TIMING_FD) {
 		enum ast_timer_event res;
+		int trigger_dtmf_emulating = should_trigger_dtmf_emulating(chan);
 
 		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_EXCEPTION);
 
@@ -3535,10 +3567,26 @@
 				if (got_ref) {
 					ao2_ref(data, -1);
 				}
+
+				if (trigger_dtmf_emulating) {
+					/*
+					 * Since we're breaking out of this switch block and not
+					 * returning, we need to re-lock the channel.
+					 */
+					ast_channel_lock(chan);
+					/* generate null frame to trigger dtmf emulating */
+					f = &ast_null_frame;
+					break;
+				}
+			} else if (trigger_dtmf_emulating) {
+				/* generate null frame to trigger dtmf emualating */
+				f = &ast_null_frame;
+				break;
 			} else {
 				ast_timer_set_rate(ast_channel_timer(chan), 0);
-				ast_channel_fdno_set(chan, -1);
-				ast_channel_unlock(chan);
+				/* generate very last null frame to trigger dtmf emulating */
+				f = &ast_null_frame;
+				break;
 			}
 
 			/* cannot 'goto done' because the channel is already unlocked */
@@ -3576,6 +3624,7 @@
 
 	/* Check for pending read queue */
 	if (!AST_LIST_EMPTY(ast_channel_readq(chan))) {
+		int skipped_dtmf_frame = 0;
 		int skip_dtmf = should_skip_dtmf(chan);
 
 		AST_LIST_TRAVERSE_SAFE_BEGIN(ast_channel_readq(chan), f, frame_list) {
@@ -3584,6 +3633,7 @@
 			 * some later time. */
 
 			if ( (f->frametype == AST_FRAME_DTMF_BEGIN || f->frametype == AST_FRAME_DTMF_END) && skip_dtmf) {
+				skipped_dtmf_frame = 1;
 				continue;
 			}
 
@@ -3595,7 +3645,19 @@
 		if (!f) {
 			/* There were no acceptable frames on the readq. */
 			f = &ast_null_frame;
-			ast_channel_alert_write(chan);
+			if (!skipped_dtmf_frame) {
+				/*
+				 * Do not trigger alert pipe if only buffered dtmf begin or end frames
+				 * are left in the readq.
+				 */
+				ast_channel_alert_write(chan);
+			} else {
+				/*
+				 * Safely disable continous timer events if only buffered dtmf begin or end
+				 * frames are left in the readq.
+				 */
+				ast_timer_disable_continuous(ast_channel_timer(chan));
+			}
 		}
 
 		/* Interpret hangup and end-of-Q frames to return NULL */
@@ -3801,6 +3863,16 @@
 					} else
 						ast_channel_emulate_dtmf_duration_set(chan, AST_DEFAULT_EMULATE_DTMF_DURATION);
 					ast_log(LOG_DTMF, "DTMF begin emulation of '%c' with duration %u queued on %s\n", f->subclass.integer, ast_channel_emulate_dtmf_duration(chan), ast_channel_name(chan));
+
+					/*
+					 * Start generating 50 fps timer events (null frames) for dtmf emulating
+					 * independently from any existing incoming voice frames.
+					 * If channel generator is already activated in regular mode use these
+					 * timer events to generate null frames.
+					 */
+					if (!ast_channel_generator(chan)) {
+						ast_timer_set_rate(ast_channel_timer(chan), 50);
+					}
 				}
 				if (ast_channel_audiohooks(chan)) {
 					struct ast_frame *old_frame = f;
@@ -3842,12 +3914,30 @@
 					ast_channel_emulate_dtmf_duration_set(chan, option_dtmfminduration - f->len);
 					ast_frfree(f);
 					f = &ast_null_frame;
+
+					/* Start generating 50 fps timer events (null frames) for dtmf emulating
+					 * independently from any existing incoming voice frames.
+					 * If channel generator is already activated in regular mode use these
+					 * timer events to generate null frames.
+					 */
+					if (!ast_channel_generator(chan)) {
+						ast_timer_set_rate(ast_channel_timer(chan), 50);
+					}
 				} else {
 					ast_log(LOG_DTMF, "DTMF end passthrough '%c' on %s\n", f->subclass.integer, ast_channel_name(chan));
 					if (f->len < option_dtmfminduration) {
 						f->len = option_dtmfminduration;
 					}
 					ast_channel_dtmf_tv_set(chan, &now);
+
+					/* Start generating 50 fps timer events (null frames) for dtmf emulating
+					 * independently from any existing incoming voice frames.
+					 * If channel generator is already activated in regular mode use these
+					 * timer events to generate null frames.
+					 */
+					if (!ast_channel_generator(chan)) {
+						ast_timer_set_rate(ast_channel_timer(chan), 50);
+					}
 				}
 				if (ast_channel_audiohooks(chan)) {
 					struct ast_frame *old_frame = f;
@@ -3901,6 +3991,15 @@
 							ast_frfree(old_frame);
 						}
 					}
+
+					/* Start generating 50 fps timer events (null frames) for dtmf emulating
+					 * independently from any existing incoming voice frames.
+					 * If channel generator is already activated in regular mode use these
+					 * timer events to generate null frames.
+					 */
+					if (!ast_channel_generator(chan)) {
+						ast_timer_set_rate(ast_channel_timer(chan), 50);
+					}
 				}
 			}
 			break;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13343
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I309bf61dd065c9978c8e48f5b9a936ab47de64c2
Gerrit-Change-Number: 13343
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191206/ebf86a79/attachment-0001.html>


More information about the asterisk-code-review mailing list