Summary:	ASTERISK-25483: [patch] Built-in sounds are send at 40,20,40,20ms intervals for iLBC
Reporter:	Alexander Traud (traud)	Labels:
Date Opened:	2015-10-21 06:36:07	Date Closed:	2015-11-26 04:53:18.000-0600
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Formats/General
Versions:	11.20.0 13.6.0	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 10ms_Packetization_for_Asterisk_11.patch ( 1) 10ms_Packetization_for_Asterisk_13_part_2.patch ( 2) 10ms_Packetization_for_Asterisk_13.patch
Description:	For example, when you call the echo test and iLBC is negotiated as audio codec, Asterisk sends the RTP payload after 40ms, 20ms, 40ms, 20ms and so on. One would expect audio every 30ms. This creates an unnecessary delay of 10ms. Furthermore, packet-delay varies which might confuse/increase a remote jitter buffer. Because I am not aware of any real-world effects of this, I classified this issue not as major but as minor. As explained in ASTERISK-21492, Asterisk does not have an internal real-time clock for media but uses the source media as clock generator. In contrast to that other issue, when calling a built-in sound file (like the echo-test), Asterisk is the source of the media as well. Therefore, Asterisk is able to change behaviour. *Cause* Currently, Asterisk is reading its sound file every 20ms. Because iLBC has a default packetization time of 30ms, Asterisk has to read twice to send the first RTP packet on the wire. Then, it reads one additional chunk (20ms) and is able to create a new 30ms packet immediately. This creates this 40, 20, 40, 20. By the way, amount of samples (RTP payload) and the RTP timestamp are always 30ms. Just the real-time is 40, 20, 40 … *Affected* Codecs with a packetization time which is not a multiple of 20ms. Therefore, iLBC 30 is affected. Furthermore, this issue occurs when non-default packetization times are configured (like sip.conf:allow=g729:10) or negotiated (like sip.conf:autoframing=yes). *Patch* The default reading was changed from 20ms to 10ms. This helps all audio codecs with increment with 10ms or 30ms, like G.729, iLBC, Speex, G.722, and G.711. *Remaining Limitations* This patch changes the default of SLN, PCM, WAV, and G.729 only, because I do not have the resources to change/test all existing formats. However, this covers all [currently downloadable formats|http://downloads.asterisk.org/pub/telephony/sounds/], except Siren (which is [not supported|https://www.digium.com/products/asterisk/downloads] in Asterisk 13 anymore) and GSM. Theoretically, Opus allows a packetization time of 5ms, even 2.5ms increments. I do not need this and therefore went for 10ms. Even with those remaining limitations, I hope this patch is useful.
Comments:	By: Joshua C. Colp (jcolp) 2015-10-22 05:58:04.418-0500 Copied from gerrit: What kind of performance impact does this have on the system? Normally 20ms is what is used and if this causes a different performance profile then I'm not sure if I agree with it, since the vast majority of people are using 20ms. I've put this at -1 for now until that is explored but will re-evaluate. By: Richard Mudgett (rmudgett) 2015-10-22 13:35:31.152-0500 I don't think the performance loss associated with this patch is really worth it.  The very definition of VoIP jitter is a variance in packet arrival timing.  Jitter is something that all endpoints must be able to handle anyway.  The fact that Asterisk itself is creating the jitter on a media stream it is originating is immaterial as the network itself can add its own jitter. By: Frederic LE FOLL (flefoll) 2015-10-29 08:40:32.367-0500 Hello, If format_pcm.c and others provide 10 ms chunks, I think that channel.c:generator_force() should also be changed : this has to do with music on hold (at least), and the function includes a hard-coded 1/50s (20ms) timing : see line "res = generate(chan, tmp, 0, ast_format_get_sample_rate(ast_channel_writeformat(chan)) / 50);". With 10ms chunks, the rate should become 100. By: Frederic LE FOLL (flefoll) 2015-10-29 09:05:48.507-0500 To be checked also: app_meetme.c with its "#define CONF_SIZE  320", (320 for 160 16-bit samples). By: Rusty Newton (rnewton) 2015-11-04 14:38:20.561-0600 Opening this up and noting that activity is still pending on https://gerrit.asterisk.org/#/c/1460/ By: Alexander Traud (traud) 2015-11-26 04:52:33.911-0600 Thank you for the valuable comments on this issue and the within the review. Especially, the other parts of the source code which are affected by this. A small fraction of users is affected by this issue (iLBC 30, {{autoframing=yes}}, and those who specify a non-20ms ptime directly with a codec). However, I am not going to apply this change myself. Therefore, I am not going to adopt the change as mentioned in the review process = change to a define. If somebody is still interested in this change, please, reply here – then, I love to help to get that change passing the review process.