[Asterisk-code-review] chan_pjsip: digit_begin - constant DTMF tone if RTP is not setup yet (asterisk[17])

[Friendly Automation]

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

Change subject: chan_pjsip: digit_begin - constant DTMF tone if RTP is not setup yet
......................................................................

chan_pjsip: digit_begin - constant DTMF tone if RTP is not setup yet

If chan_pjsip is configured for DTMF_RFC_4733, and the core triggers a
digit begin before media, or rtp has been setup then it's possible the
outgoing channel will hear a constant DTMF tone upon answering.

This happens because when there is no media, or rtp chan_pjsip notifies
the core to initiate inband DTMF. However, upon digit end if media, and
rtp become available then chan_pjsip does not notify the core to stop
inband DTMF. Thus the tone continues playing.

This patch makes it so chan_pjsip only notifies the core to start
inband DTMF in only the required cases. Now if there is no media, or
rtp availabe upon digit begin chan_pjsip does nothing, but tells the
core it handled it.

ASTERISK-28817 #close

Change-Id: I0dbea9fff444a2595fb18c64b89653e90d2f6eb5
---
M channels/chan_pjsip.c
1 file changed, 18 insertions(+), 15 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
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 590843e..5df39c3 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -2163,20 +2163,23 @@
 {
 	struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(chan);
 	struct ast_sip_session_media *media;
-	int res = 0;
 
 	media = channel->session->active_media_state->default_session[AST_MEDIA_TYPE_AUDIO];
 
 	switch (channel->session->dtmf) {
 	case AST_SIP_DTMF_RFC_4733:
 		if (!media || !media->rtp) {
-			return -1;
+			return 0;
 		}
 
 		ast_rtp_instance_dtmf_begin(media->rtp, digit);
 		break;
 	case AST_SIP_DTMF_AUTO:
-		if (!media || !media->rtp || (ast_rtp_instance_dtmf_mode_get(media->rtp) == AST_RTP_DTMF_MODE_INBAND)) {
+		if (!media || !media->rtp) {
+			return 0;
+		}
+
+		if (ast_rtp_instance_dtmf_mode_get(media->rtp) == AST_RTP_DTMF_MODE_INBAND) {
 			return -1;
 		}
 
@@ -2191,13 +2194,12 @@
 	case AST_SIP_DTMF_NONE:
 		break;
 	case AST_SIP_DTMF_INBAND:
-		res = -1;
-		break;
+		return -1;
 	default:
 		break;
 	}
 
-	return res;
+	return 0;
 }
 
 struct info_dtmf_data {
@@ -2284,7 +2286,6 @@
 {
 	struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(ast);
 	struct ast_sip_session_media *media;
-	int res = 0;
 
 	if (!channel || !channel->session) {
 		/* This happens when the channel is hungup while a DTMF digit is playing. See ASTERISK-28086 */
@@ -2298,8 +2299,9 @@
 	case AST_SIP_DTMF_AUTO_INFO:
 	{
 		if (!media || !media->rtp) {
-			return -1;
+			return 0;
 		}
+
 		if (ast_rtp_instance_dtmf_mode_get(media->rtp) != AST_RTP_DTMF_MODE_NONE) {
 			ast_debug(3, "Told to send end of digit on Auto-Info channel %s RFC4733 negotiated so using it.\n", ast_channel_name(ast));
 			ast_rtp_instance_dtmf_end_with_duration(media->rtp, digit, duration);
@@ -2337,28 +2339,29 @@
 	}
 	case AST_SIP_DTMF_RFC_4733:
 		if (!media || !media->rtp) {
-			return -1;
+			return 0;
 		}
 
 		ast_rtp_instance_dtmf_end_with_duration(media->rtp, digit, duration);
 		break;
 	case AST_SIP_DTMF_AUTO:
-		if (!media || !media->rtp || (ast_rtp_instance_dtmf_mode_get(media->rtp) == AST_RTP_DTMF_MODE_INBAND)) {
+		if (!media || !media->rtp) {
+			return 0;
+		}
+
+		if (ast_rtp_instance_dtmf_mode_get(media->rtp) == AST_RTP_DTMF_MODE_INBAND) {
 			 return -1;
 		}
 
 		ast_rtp_instance_dtmf_end_with_duration(media->rtp, digit, duration);
 		break;
-
-
 	case AST_SIP_DTMF_NONE:
 		break;
 	case AST_SIP_DTMF_INBAND:
-		res = -1;
-		break;
+		return -1;
 	}
 
-	return res;
+	return 0;
 }
 
 static void update_initial_connected_line(struct ast_sip_session *session)

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

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