<p>Jaco Kroon has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15629">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">format/g729: Improved CNG and ptime!=20 handling.<br><br>In spite of asterisk passing annex=b we're still seeing a significant<br>amount of CNG being transmitted towards us. Additionally, we're seeing<br>at least one local provider that insists on ptime=60 and another using<br>ptime=30.<br><br>Changes:<br><br>1. Don't assume that a received frame % 10 isn't valid, CNG frames are<br> two bytes in size, so accomodate those by discarding them.<br>2. Only warn once on detecting CNG frames (per stream).<br>3. Return partial valid reads, so reading 10 bytes from an input file<br> is valid, since we could have an audio file of say 50ms which will<br> be 50 bytes.<br>4. Log invalid seeks.<br><br>This does not deal with the "all zero" frames ending up in the output<br>file ... resulting in beeps when transcoding these "gaps". This is<br>existing behaviour. The correct way of dealing with this is noise<br>generation based on CNG parameters. Alternatively a set of "silent"<br>frames which we inject randomly (from say sounds/en/silence/10.g729).<br><br>Change-Id: I3c64eeeef5005c55b677d7a1fa46797c831f3429<br>Signed-off-by: Jaco Kroon <jaco@uls.co.za><br>---<br>M formats/format_g729.c<br>1 file changed, 40 insertions(+), 11 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/29/15629/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/formats/format_g729.c b/formats/format_g729.c</span><br><span>index a6539a3..2d692ff 100644</span><br><span>--- a/formats/format_g729.c</span><br><span>+++ b/formats/format_g729.c</span><br><span>@@ -41,24 +41,38 @@</span><br><span> </span><br><span> /* Portions of the conversion code are by guido@sienanet.it */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-#define BUF_SIZE 20 /* two G729 frames */</span><br><span style="color: hsl(0, 100%, 40%);">-#define G729A_SAMPLES 160</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_FRAME_SIZE 10</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_FRAME_SAMPLES 80</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_CNG_SIZE 2</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* original code assumed ptime=20 (which is the norm), keep these for the</span><br><span style="color: hsl(120, 100%, 40%);">+ * moment, or two frames per packet */</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_BUF_SIZE (G729A_FRAME_SIZE * 2) /* two G729 frames */</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_BUF_SAMPLES (G729A_FRAME_SAMPLES * 2)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_SIZE2SAMPLES(b) ((b) * G729A_FRAME_SAMPLES / G729A_FRAME_SIZE)</span><br><span style="color: hsl(120, 100%, 40%);">+#define G729A_SAMPLES2SIZE(s) ((s) * G729A_FRAME_SIZE / G729A_FRAME_SAMPLES)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+struct ast_g729_pvt {</span><br><span style="color: hsl(120, 100%, 40%);">+ int detected_cng:1; /* simple flag indicating whether or not we detected</span><br><span style="color: hsl(120, 100%, 40%);">+ and reported on VAD/CNG for this write stream yet */</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span> </span><br><span> static struct ast_frame *g729_read(struct ast_filestream *s, int *whennext)</span><br><span> {</span><br><span> size_t res;</span><br><span> </span><br><span> /* Send a frame from the file to the appropriate channel */</span><br><span style="color: hsl(0, 100%, 40%);">- s->fr.samples = G729A_SAMPLES;</span><br><span style="color: hsl(0, 100%, 40%);">- AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, G729A_BUF_SIZE);</span><br><span> if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (res && res != 10) /* XXX what for ? */ {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (res % G729A_FRAME_SIZE) /* G729 must be multiple of G729A_FRAME_SIZE */ {</span><br><span> ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",</span><br><span> ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,</span><br><span> strerror(errno));</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+ s->fr.samples = G729A_SIZE2SAMPLES(res);</span><br><span> *whennext = s->fr.samples;</span><br><span> return &s->fr;</span><br><span> }</span><br><span>@@ -66,9 +80,18 @@</span><br><span> static int g729_write(struct ast_filestream *fs, struct ast_frame *f)</span><br><span> {</span><br><span> int res;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_g729_pvt* pvt = fs->_private;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (f->datalen % 10) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "Invalid data length, %d, should be multiple of 10\n", f->datalen);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (f->datalen % G729A_FRAME_SAMPLES) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (f->datalen % G729A_FRAME_SIZE == G729A_CNG_SIZE) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!pvt->detected_cng) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_WARNING, "Probable CNG parameter update detected, this is a bug in the sender who should not be sending CNG updates - try and switch off Voice Activity Detected (VAD) or Silence Suppression on the sender.\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ pvt->detected_cng = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_WARNING, "Invalid data length, %d, should be multiple of 10\n", f->datalen);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* can we avoid this for the CNG case? assuming it's not a bare CNG frame */</span><br><span> return -1;</span><br><span> }</span><br><span> if ((res = fwrite(f->data.ptr, 1, f->datalen, fs->f)) != f->datalen) {</span><br><span>@@ -87,7 +110,12 @@</span><br><span> fseeko(fs->f, 0, SEEK_END);</span><br><span> max = ftello(fs->f);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- bytes = BUF_SIZE * (sample_offset / G729A_SAMPLES);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sample_offset % G729A_FRAME_SAMPLES) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(AST_LOG_ERROR, "Invalid seek, sample_offset=%ld not a multiple of frame_samples=%d.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ sample_offset, G729A_FRAME_SAMPLES);</span><br><span style="color: hsl(120, 100%, 40%);">+ // return -1; /* we should flag error, but old code did not */</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ bytes = G729A_SAMPLES2SIZE(sample_offset);</span><br><span> if (whence == SEEK_SET)</span><br><span> offset = bytes;</span><br><span> else if (whence == SEEK_CUR || whence == SEEK_FORCECUR)</span><br><span>@@ -124,7 +152,7 @@</span><br><span> static off_t g729_tell(struct ast_filestream *fs)</span><br><span> {</span><br><span> off_t offset = ftello(fs->f);</span><br><span style="color: hsl(0, 100%, 40%);">- return (offset/BUF_SIZE)*G729A_SAMPLES;</span><br><span style="color: hsl(120, 100%, 40%);">+ return G729A_SIZE2SAMPLES(offset);</span><br><span> }</span><br><span> </span><br><span> static struct ast_format_def g729_f = {</span><br><span>@@ -135,7 +163,8 @@</span><br><span> .trunc = g729_trunc,</span><br><span> .tell = g729_tell,</span><br><span> .read = g729_read,</span><br><span style="color: hsl(0, 100%, 40%);">- .buf_size = BUF_SIZE + AST_FRIENDLY_OFFSET,</span><br><span style="color: hsl(120, 100%, 40%);">+ .buf_size = G729A_BUF_SIZE + AST_FRIENDLY_OFFSET,</span><br><span style="color: hsl(120, 100%, 40%);">+ .desc_size = sizeof(struct ast_g729_pvt),</span><br><span> };</span><br><span> </span><br><span> static int load_module(void)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15629">change 15629</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/15629"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I3c64eeeef5005c55b677d7a1fa46797c831f3429 </div>
<div style="display:none"> Gerrit-Change-Number: 15629 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>