Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting telephone-event/16000 #22

Open
evgeny-sidorov-dev opened this issue Jul 14, 2023 · 17 comments
Open

Supporting telephone-event/16000 #22

evgeny-sidorov-dev opened this issue Jul 14, 2023 · 17 comments

Comments

@evgeny-sidorov-dev
Copy link

AMR-WB codec works at a frequency 16000, which is not supported by Asterisk. When i receive INVITE with AMR-WB, DTMF fails.

Exists any solution to solve this?

@shmuel4
Copy link

shmuel4 commented Oct 22, 2023

Has anyone been able to solve or add this?
@traud Could you please try to add it?

Thanks!

@traud
Copy link
Owner

traud commented Oct 23, 2023

In July, via private E-mail, I sent this to Evgeny:

When it comes to telephone-event/16000, you have to report that upstream in Asterisk itself. However, I recommend to explain that in much more detail, especially how to reproduce your issue. For example, I never met a VoIP client which had an issue with that, all had working DTMF. Furthermore, with your VoIP client, this should affect G.722 and Opus-Codec as well, making it easier for the Asterisk community to reproduce.

I guess, you are about RTP in-band DMTF, right? When you created that issue report (or better start with a Community post or via the E-mail mailing list), please, update this issue here, so those are linked and someone can look at it.

@shmuel4
Copy link

shmuel4 commented Oct 23, 2023

What happens is that the call is on AMR-WB/16000 - no telephone-event is sent at all.
that the conversation is conducted in a different codec - the events are sent properly.

@traud
Copy link
Owner

traud commented Oct 23, 2023

  1. Who sends no telephone-event/8000, Asterisk?
  2. In which scenario, Asterisk as caller or Asterisk as callee?
  3. If it is not Asterisk, does the session include further SD audio codecs or just HD ones?
  4. Which effect does this have on the user level; no way to use other means for DTMF?

Perhaps you realize, this issue is not that easy. Although you face it, I have to understand it. Then, I have to reproduce it. Then, I have to understand why Asterisk behaves that way (bug and supported, or not supported at all, bug or feature request). And so on … with the current understanding of this issue, it is not limited to AMR-WB but affects all HD codecs. Therefore, it is an issue upstream. Therefore, it has to be discussed upstream. I might have a look at it, if I find time, and might even take it over, if you are lucky. However, right now, it is an upstream issue.

@oej
Copy link

oej commented Oct 24, 2023

It is asterisk. Also note that if the offer includes both an 8000 and 16000 hz codec, Asterisk needs to send TWO offers for DTMF.

@traud
Copy link
Owner

traud commented Oct 24, 2023

I am asking several questions and get one answer. I cannot work on this issue, if my questions are not answered step by step. I am not asking questions for fun.

if the offer includes both an 8000 and 16000 hz codec, Asterisk needs to send TWO offers for DTMF.

Such statements must be supported by an interoperability case (a VoIP implementation which is effected by this already) or a reference design (IETF RFC, 3GPP, …) that indicates a future interoperability issue. Furthermore, there is still the statement that this is not an AMR-WB specific thing and effects all HD codecs, even G.722. Therefore, this is still considered an issue upstream. Again, please, connect an upstream discussion so this can be followed-up there.

Furthermore, the wording offer-offer indicates the scenario Asterisk as VoIP/SIP proxy forwarding an SDP offer to another party. I thought we are talking about the scenario when Asterisk itself is the PBX.

Finally, I recommend reading one of the several how-to-write-issue/bug-report articles floating around the Internet. I do not know which call scenario is affected. I do not even know which DTMF scenario is affected – is it in-band? DTMF works here with my G.722 and AMR-WB implementations. And always consider whom you explain it. For example if you want me to work on this, consider me as a guy who added several HD audio codecs to Asterisk, for voice. Now, you have to explain your ‘DTMF’ scenario to someone like this. Otherwise, I have to ask questions which wastes everyones time.

@hardrock13
Copy link

@traud Hello. Got a similar thing here. Let me elaborate.
The main problem itself concerns the fact that Asterisk does not have processing for telephone-event/16000. Like at all. And all 16k codecs require to be followed by telephone-event/16000 according to RFC 4733.
As such, in a production environment on commercial SBCs, for EVS and AMR-WB codecs to work at all, is required the use of telephone-event/16000 for DTMF. Unless you use SIP INFO for DTMF, which is not an option in my case.
In a simulation case the RFC can be disregarded and so both EVS and AMR-WB will work with telephone-event/8000. But in my case, this is not possible.
Find the flow below:

  1. Asterisk is the callee. It receives the INVITE from SBC and replies in 200 OK with the available codecs from the SBC list and the telephone-event/8000.
  2. The SBC in question sees that only te-8k is available and selects from the list of codecs the codec with the most priority that is also 8000 Hz.
  3. Despite EVS and AMR-WB having higher priority, the SBC will not select them when performing a Re-INVITE, because they are 16000 Hz codecs.

So I have asked Joshua Colp regarding this. Initially I questioned the use of RTPengine (the Kamailio one) as a plug-in module for handling the RTP traffic, but Joshua said this will do nothing as the processing for DTMF is outside the pluggable architecture of RTP. He suggested looking into rtp_engine.c as a starting point.

Interesting fact in RFC 2833 in chapter 3.5 Payload Format. from what In understood, the te-16k is identical in processing to te-8k with the only exception that the duration is 16k timestamp units, while the te-8k has 8k timestamp units. This makes me believe that Asterisk should be able to handle it.

My thoughts was to play with the hypothesis and use the AST_RTP_DTMF element to add te-16 support. In case I fail and the duration needs to be modified as well, I think this warrants a duplication of all DTMF related functions and variables with the inclusion of duration for 16k timestamp units while processing.
What are your thoughts on this?

@walbus
Copy link

walbus commented Nov 21, 2023

I have gotten it to a state where it seems to work.
rtp_engine.h:

enum ast_rtp_instance_rtcp {
...
 #define AST_RTP_CISCO_DTMF              (1 << 2)
+#define AST_RTP_DTMF_16000              (1 << 3)
/*! Maximum RTP-specific code */
-#define AST_RTP_MAX                     AST_RTP_CISCO_DTMF
+#define AST_RTP_MAX                     AST_RTP_DTMF_16000

rtp_engine.c:

int ast_rtp_engine_init(void)
...
+	set_next_mime_type(NULL, AST_RTP_DTMF_16000, "audio", "telephone-event", 16000);
...
+	add_static_payload(113, NULL, AST_RTP_DTMF_16000); // I had cases where 113 was needed, but maybe it could be -1?

res_rtp_asterisk.c:

static struct ast_frame *ast_rtp_interpret(struct ast_rtp_instance *instance, struct ast_srtp *srtp,
...
 			process_dtmf_rfc2833(instance, read_area + hdrlen, res - hdrlen, seqno, timestamp, payloadtype, mark, &frames);
+		} else if (payload->rtp_code == AST_RTP_DTMF_16000) {
+			/* process_dtmf_rfc2833 may need to return multiple frames. We do this
+			 * by passing the pointer to the frame list to it so that the method
+			 * can append frames to the list as needed.
+			 */
+			process_dtmf_rfc2833(instance, read_area + hdrlen, res - hdrlen, seqno, timestamp, payloadtype, mark, &frames);
 		} else if (payload->rtp_code == AST_RTP_CISCO_DTMF) {
 			f = process_dtmf_cisco(instance, read_area + hdrlen, res - hdrlen, seqno, timestamp, payloadtype, mark);
 		} else if (payload->rtp_code == AST_RTP_CN) {
...
 	/* Grab the payload that they expect the RFC2833 packet to be received in */
-	payload = ast_rtp_codecs_payload_code_tx(ast_rtp_instance_get_codecs(instance), 0, NULL, AST_RTP_DTMF);
+	if(ast_rtp_get_rate(rtp->f.subclass.format)==16000)
+		payload = ast_rtp_codecs_payload_code_tx(ast_rtp_instance_get_codecs(instance), 0, NULL, AST_RTP_DTMF_16000);
+	else
+		payload = ast_rtp_codecs_payload_code_tx(ast_rtp_instance_get_codecs(instance), 0, NULL, AST_RTP_DTMF);

These changes plus some changes to chan_sip.c to use the format made it work. (I am planing on changing to pjsip, but it is a lot of work)

@shmuel4
Copy link

shmuel4 commented Nov 21, 2023

@walbus
What changes did you make in chan_sip?
Is it possible that your patch will work with pjsip without changes to pjsip?

I really want it to work for me.

@shmuel4
Copy link

shmuel4 commented Nov 21, 2023

I made the changes you wrote and compiled an asterisk, but unfortunately it doesn't help, an asterisk still doesn't correspond to telephone-event/16000 😕.

I am using with Asterisk 20.5.0 and PJSIP.

@walbus
Copy link

walbus commented Nov 21, 2023

@shmuel4 I have not looked at pjsip. But in chan_sip i had to add AST_RTP_DTMF_16000 a lot of places.

 	dialog->noncodeccapability |= AST_RTP_DTMF;
+	dialog->noncodeccapability |= AST_RTP_DTMF_16000;
 	p->noncodeccapability |= AST_RTP_DTMF;
+	p->noncodeccapability |= AST_RTP_DTMF_16000;
-	if (format == AST_RTP_DTMF)	/* Indicate we support DTMF and FLASH... */
+	if (format == AST_RTP_DTMF || format == AST_RTP_DTMF_16000) 	/* Indicate we support DTMF and FLASH... */
-	if (newnoncodeccapability & AST_RTP_DTMF) {
+	if ( newnoncodeccapability & (AST_RTP_DTMF | AST_RTP_DTMF_16000)  ) {
 	if ((ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_RFC2833) ||
-	    (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO))
+	    (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO)) {
 		p->noncodeccapability |= AST_RTP_DTMF;
-	else
+		p->noncodeccapability |= AST_RTP_DTMF_16000;
+	} else {
 		p->noncodeccapability &= ~AST_RTP_DTMF;
+		p->noncodeccapability &= ~AST_RTP_DTMF_16000;
+	}
 		p->jointnoncodeccapability &= ~AST_RTP_DTMF;
+		p->jointnoncodeccapability &= ~AST_RTP_DTMF_16000;
 	} else if (!strcasecmp(mode, "shortinfo")) {
 		ast_clear_flag(&p->flags[0], SIP_DTMF);
 		ast_set_flag(&p->flags[0], SIP_DTMF_SHORTINFO);
 		p->jointnoncodeccapability &= ~AST_RTP_DTMF;
+		p->jointnoncodeccapability &= ~AST_RTP_DTMF_16000;
 	} else if (!strcasecmp(mode, "rfc2833")) {
 		ast_clear_flag(&p->flags[0], SIP_DTMF);
 		ast_set_flag(&p->flags[0], SIP_DTMF_RFC2833);
 		p->jointnoncodeccapability |= AST_RTP_DTMF;
+		p->jointnoncodeccapability |= AST_RTP_DTMF_16000;
 	} else if (!strcasecmp(mode, "inband")) {
 		ast_clear_flag(&p->flags[0], SIP_DTMF);
 		ast_set_flag(&p->flags[0], SIP_DTMF_INBAND);
 		p->jointnoncodeccapability &= ~AST_RTP_DTMF;
+		p->jointnoncodeccapability &= ~AST_RTP_DTMF_16000;

@walbus
Copy link

walbus commented Nov 21, 2023

@shmuel4
If you do it the way I did, it looks like you need to make changes in res_pjsip_sdp_rtp.c so it also can use AST_RTP_DTMF_16000.

@shmuel4
Copy link

shmuel4 commented Nov 21, 2023

@walbus

Yes, I already found it:

        int noncodec = (session->dtmf == AST_SIP_DTMF_RFC_4733 || session->dtmf == AST_SIP_DTMF_AUTO || session->dtmf == AST_SIP_DTMF_AUTO_INFO) ? AST_RTP_DTMF : 0;

If I change to:

        int noncodec = (session->dtmf == AST_SIP_DTMF_RFC_4733 || session->dtmf == AST_SIP_DTMF_AUTO || session->dtmf == AST_SIP_DTMF_AUTO_INFO) ? AST_RTP_DTMF_16000 : 0;

It works,

But then Kochavit does not process events at the rate of 8000.

I need to find a solution...

trying

@shmuel4
Copy link

shmuel4 commented Nov 21, 2023

The way I was able to solve it with PJSIP is this,

I don't know if this is a correct way to do it, but maybe after our research @traud can help give help and give a perfect solution.

In the file res/res_pjsip_sdp_rtp.c
I changed noncodec, instead of:

int noncodec = (session->dtmf == AST_SIP_DTMF_RFC_4733 || session->dtmf == AST_SIP_DTMF_AUTO || session->dtmf == AST_SIP_DTMF_AUTO_INFO) ? AST_RTP_DTMF : 0;

to the following form:

#define NONCODEC_MAX_SIZE 2
// Initialize noncodec as an array
int noncodec[NONCODEC_MAX_SIZE] = {0};
// Populate noncodec based on conditions
int noncodecIndex = 0;
if (session->dtmf == AST_SIP_DTMF_RFC_4733 || session->dtmf == AST_SIP_DTMF_AUTO || session->dtmf == AST_SIP_DTMF_AUTO_INFO) {
     noncodec[noncodecIndex++] = AST_RTP_DTMF;
     noncodec[noncodecIndex++] = AST_RTP_DTMF_16000;
}

then in:

/* Add non-codec formats */
if (ast_sip_session_is_pending_stream_default(session, stream) && media_type != AST_MEDIA_TYPE_VIDEO

instead of

if (!(noncodec & index)) {
continue;
}

And the part that follows, comes:

if (!index) {
continue;
}
for (int i = 0; i < noncodecIndex; i++) {
int currentNoncodec = noncodec[i];

// Replace your original bitwise check with a check against the current array element
rtp_code = ast_rtp_codecs_payload_code(
ast_rtp_instance_get_codecs(session_media->rtp), 0, NULL, currentNoncodec);
if (rtp_code == -1) {
continue;
}

if ((attr = generate_rtpmap_attr(session, media, pool, rtp_code, 0, NULL, currentNoncodec))) {
media->attr[media->attr_count++] = attr;
}

if (currentNoncodec == AST_RTP_DTMF || currentNoncodec == AST_RTP_DTMF_16000) {
snprintf(tmp, sizeof(tmp), "%d 0-16", rtp_code);
attr = pjmedia_sdp_attr_create(pool, "fmtp", pj_cstr(&stmp, tmp));
media->attr[media->attr_count++] = attr;
}

// Check if fmt_count reached its max
if (media->desc.fmt_count == PJMEDIA_MAX_SDP_FMT) {
break
}
}

In this situation, DTMF works for me in both ALAW and AMR-WB.

@shmuel4
Copy link

shmuel4 commented Nov 21, 2023

I do experience a problem of hanging up after about 30 seconds when using AMR-WB, I couldn't figure out where it comes from, or what causes it, it doesn't happen with other codecs..

Actually, more work is needed here to make it work well...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants
@oej @hardrock13 @traud @walbus @evgeny-sidorov-dev @shmuel4 and others