[svn-commits] mmichelson: branch 1.6.2 r269823 - in /branches/1.6.2: ./ main/channel.c

[SVN commits to the Digium repositories]

Author: mmichelson
Date: Thu Jun 10 14:50:35 2010
New Revision: 269823

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=269823
Log:
Merged revisions 269822 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

................
  r269822 | mmichelson | 2010-06-10 14:34:03 -0500 (Thu, 10 Jun 2010) | 25 lines
  
  Merged revisions 269821 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.4
  
  ........
    r269821 | mmichelson | 2010-06-10 14:30:12 -0500 (Thu, 10 Jun 2010) | 19 lines
    
    Fix potential crash when writing raw SLIN audio on a PLC-enabled channel.
    
    The issue here was that the frame created when adjusting for PLC had no offset
    to its audio data. If this frame were translated to another format prior to
    being sent out an RTP socket, all went well because the translation code would
    put an appropriate offset into the frame. However, if the SLIN audio were not
    translated before being sent out the RTP socket, bad things would happen.
    Specifically, the ast_rtp_raw_write makes the assumption that the frame has
    at least enough of an offset that it can accommodate an RTP header. This was
    not the case. As such, data was being written prior to the allocation, likely
    corrupting the data the memory allocator had written. Thus when the time came
    to free the data, all hell broke loose. ....Well, Asterisk crashed at least.
    
    The fix was just what one would expect. Offset the data in the frame by a reasonable
    amount. The method I used is a bit odd since the data in the frame is 16 bit integers
    and not bytes. I left a big ol' comment about it. This can be improved on if someone
    is interested. I was more interested in getting the crash resolved.
  ........
................

Modified:
    branches/1.6.2/   (props changed)
    branches/1.6.2/main/channel.c

Propchange: branches/1.6.2/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.2/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.2/main/channel.c?view=diff&rev=269823&r1=269822&r2=269823
==============================================================================
--- branches/1.6.2/main/channel.c (original)
+++ branches/1.6.2/main/channel.c Thu Jun 10 14:50:35 2010
@@ -3502,8 +3502,25 @@
 	int num_new_samples = frame->samples;
 	struct plc_ds *plc = datastore->data;
 
-
-	/* If this audio frame has no samples to fill in ignore it */
+	/* As a general note, let me explain the somewhat odd calculations used when taking
+	 * the frame offset into account here. According to documentation in frame.h, the frame's
+	 * offset field indicates the number of bytes that the audio is offset. The plc->samples_buf
+	 * is not an array of bytes, but rather an array of 16-bit integers since it holds SLIN
+	 * samples. So I had two choices to make here with the offset.
+	 * 
+	 * 1. Make the offset AST_FRIENDLY_OFFSET bytes. The main downside for this is that
+	 *    I can't just add AST_FRIENDLY_OFFSET to the plc->samples_buf and have the pointer
+	 *    arithmetic come out right. I would have to do some odd casting or division for this to
+	 *    work as I wanted.
+	 * 2. Make the offset AST_FRIENDLY_OFFSET * 2 bytes. This allows the pointer arithmetic
+	 *    to work out better with the plc->samples_buf. The downside here is that the buffer's
+	 *    allocation contains an extra 64 bytes of unused space.
+	 * 
+	 * I decided to go with option 2. This is why in the calloc statement and the statement that
+	 * sets the frame's offset, AST_FRIENDLY_OFFSET is multiplied by 2.
+	 */
+
+	/* If this audio frame has no samples to fill in, ignore it */
 	if (!num_new_samples) {
 		return;
 	}
@@ -3514,7 +3531,7 @@
 	 */
 	if (plc->num_samples < num_new_samples) {
 		ast_free(plc->samples_buf);
-		plc->samples_buf = ast_calloc(num_new_samples, sizeof(*plc->samples_buf));
+		plc->samples_buf = ast_calloc(1, (num_new_samples * sizeof(*plc->samples_buf)) + (AST_FRIENDLY_OFFSET * 2));
 		if (!plc->samples_buf) {
 			ast_channel_datastore_remove(chan, datastore);
 			ast_datastore_free(datastore);
@@ -3524,9 +3541,10 @@
 	}
 
 	if (frame->datalen == 0) {
-		plc_fillin(&plc->plc_state, plc->samples_buf, frame->samples);
-		frame->data.ptr = plc->samples_buf;
+		plc_fillin(&plc->plc_state, plc->samples_buf + AST_FRIENDLY_OFFSET, frame->samples);
+		frame->data.ptr = plc->samples_buf + AST_FRIENDLY_OFFSET;
 		frame->datalen = num_new_samples * 2;
+		frame->offset = AST_FRIENDLY_OFFSET * 2;
 	} else {
 		plc_rx(&plc->plc_state, frame->data.ptr, frame->samples);
 	}