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

DTMF duration is wrong for G722 on receiver with RFC2833 #4117

Closed
R-Jeske opened this issue Oct 28, 2024 · 7 comments · Fixed by #4159
Closed

DTMF duration is wrong for G722 on receiver with RFC2833 #4117

R-Jeske opened this issue Oct 28, 2024 · 7 comments · Fixed by #4159

Comments

@R-Jeske
Copy link
Contributor

R-Jeske commented Oct 28, 2024

Describe the bug

When a DTMF Signal is transmitted with RFC2833 and DTMF events are used on the receiver, the duration from the callback cb_on_dtmf_event differs between G722 and PCMA codecs. When using the PCMA codec, the event is 200ms long and the duration from callback says 200ms (640 inside the packet), but when using the G722 Codec, the event is 200ms long and the duration from the callback says 100ms (640 inside the packet). I tested it with other lengths, and it is always the half in case of G722. Since the clock_rate of both PCMA and G722 is 8000, I would assume, that a value of 640 inside the packet is correct. Thus there might be a correction missing (like the pjmedia_stream_create PJMEDIA_HANDLE_G722_MPEG_BUG)

Steps to reproduce

  1. Call callee directly using G722 Codec
  2. send DTMF with RFC2833
pjsua_call_send_dtmf_param dtmf_param;
pjsua_call_send_dtmf_param_default(&dtmf_param);
dtmf_param.method = PJSUA_DTMF_METHOD_RFC2833;
char buf[2];
buf[0]='1' ;
buf[1]='\0';
const pj_str_t str=pj_str(buf);
dtmf_param.digits = str;
pjsua_call_send_dtmf(id, &dtmf_param);
  1. use callback to read duration:
void cb_on_dtmf_event(pjsua_call_id call_id,
                                        const pjsua_dtmf_event *event)
{
  bool dtmfEnd = (event->flags & PJMEDIA_STREAM_DTMF_IS_END) != 0;
  if (p->pCb_!=NULL && dtmfEnd)
  {
    a = event->duration;
  }
}

PJSIP version

2.14.1

Context

Should occur on all platforms, that support G722

Log, call stack, etc

@trengginas
Copy link
Member

The event duration calculation is done here.
G722 is using 16000 clockrate, and PCMA is using 8000, it is expected.
ref:

@R-Jeske
Copy link
Contributor Author

R-Jeske commented Oct 30, 2024

Clock Rate is 8000

RTP clock rate for the G722 payload format is 8,000 Hz

Sample Rate is 16000

sampling rate for G.722 audio is 16,000 Hz

as stated in the standard. This is the case when the payload is created:

    digit->duration += stream->rtp_tx_ts_len_per_pkt;
    if (digit->duration >= stream->dtmf_duration)
        digit->duration = stream->dtmf_duration; 

https://github.com/pjsip/pjproject/blob/master/pjmedia/src/pjmedia/stream.c#L998-L1000

because the time dependent values are adjusted to respect the different clock_rate of 8000 (half of 16000 sample rate):

    if (info->fmt.pt == PJMEDIA_RTP_PT_G722) {
        stream->has_g722_mpeg_bug = PJ_TRUE;
        /* RTP clock rate = 1/2 real clock rate */
        stream->rtp_tx_ts_len_per_pkt >>= 1;
#if defined(PJMEDIA_DTMF_DURATION_MSEC) && (PJMEDIA_DTMF_DURATION_MSEC > 0)
        stream->dtmf_duration >>= 1;
#endif

https://github.com/pjsip/pjproject/blob/master/pjmedia/src/pjmedia/stream.c#L2742-L2748

On receiver side, there is no correction for the clock rate of 8000, so the duration is 1/2 of the send value;

        dtmf_event.duration = (pj_uint16_t)(event_duration /
            (stream->codec_param.info.clock_rate / 1000));

https://github.com/pjsip/pjproject/blob/master/pjmedia/src/pjmedia/stream.c#L1801-L1802

This is exactly what I experience when I test the same duration on sender side (200ms), since I get a duration of 200 ms with PCMA, and 100 ms with G722, even both length in actual time (Wireshark) are 200ms.

If clock_rate would be 8000 on receiver, the duration would be correct. But since the sample rate is 16000, the duration must be corrected in reverse of stream->dtmf_duration >>= 1; for the RTP Payload G722 like:

  if (fmt.pt=PJMEDIA_RTP_PT_G722)
        dtmf_event.duration<<=1;

@sauwming
Copy link
Member

Thank you for fixing this in #4124 .

@R-Jeske
Copy link
Contributor Author

R-Jeske commented Nov 13, 2024

No, unfortunately this is not fixed with #4124, since this only handles those respects this behavior only on sender side. On receiver Side, it must me handled separately.

@sauwming
Copy link
Member

Ah sorry, thanks for clarifying.

Since in the above post, you already know the fix, would you also create a PR for it?

@sauwming sauwming reopened this Nov 13, 2024
@R-Jeske
Copy link
Contributor Author

R-Jeske commented Nov 13, 2024

Yes, i will create a PR tomorrow.

@R-Jeske
Copy link
Contributor Author

R-Jeske commented Nov 14, 2024

see #4159

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

Successfully merging a pull request may close this issue.

3 participants