[asterisk-commits] russell: branch 1.8 r366296 - /branches/1.8/addons/format_mp3.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri May 11 18:53:42 CDT 2012


Author: russell
Date: Fri May 11 18:53:38 2012
New Revision: 366296

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=366296
Log:
format_mp3: Fix a possible crash mp3_read().

This patch fixes a potential crash in mp3_read() by not assuming that
dbuf has enough data to finish filling up the output buffer.  The patch
also makes sure that the dbuf state gets reset after we know we read
everything out of it already.

In passing, this patch includes some other cleanups of this module,
including stripping trailing whitespace, formatting fixes based on
coding guidelines, and removing a number of unused members from the
private state struct.

(closes issue ASTERISK-19761)
Reported by: Chris Maciejewsk
Tested by: Chris Maciejewsk

Modified:
    branches/1.8/addons/format_mp3.c

Modified: branches/1.8/addons/format_mp3.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/addons/format_mp3.c?view=diff&rev=366296&r1=366295&r2=366296
==============================================================================
--- branches/1.8/addons/format_mp3.c (original)
+++ branches/1.8/addons/format_mp3.c Fri May 11 18:53:38 2012
@@ -9,7 +9,7 @@
  * Thanks to mpglib from http://www.mpg123.org/
  * and Chris Stenton [jacs at gnome.co.uk]
  * for coding the ability to play stereo and non-8khz files
- 
+
  * See http://www.asterisk.org for more information about
  * the Asterisk project. Please do not directly contact
  * any of the maintainers of this project for assistance;
@@ -48,20 +48,20 @@
 #define MP3_DCACHE 8192
 
 struct mp3_private {
-	char waste[AST_FRIENDLY_OFFSET];	/* Buffer for sending frames, etc */
-	char empty;				/* Empty character */
-	int lasttimeout;
-	int maxlen;
-	struct timeval last;
+	/*! state for the mp3 decoder */
 	struct mpstr mp;
+	/*! buffer to hold mp3 data after read from disk */
 	char sbuf[MP3_SCACHE];
+	/*! buffer for slinear audio after being decoded out of sbuf */
 	char dbuf[MP3_DCACHE];
+	/*! how much data has been written to the output buffer in the ast_filestream */
 	int buflen;
+	/*! how much data has been written to sbuf */
 	int sbuflen;
+	/*! how much data is left to be read out of dbuf, starting at dbufoffset */
 	int dbuflen;
+	/*! current offset for reading data out of dbuf */
 	int dbufoffset;
-	int sbufoffset;
-	int lastseek;
 	int offset;
 	long seek;
 };
@@ -107,17 +107,17 @@
 static void mp3_close(struct ast_filestream *s)
 {
 	struct mp3_private *p = s->_private;
-	
+
 	ExitMP3(&p->mp);
 	return;
 }
 
-static int mp3_squeue(struct ast_filestream *s) 
+static int mp3_squeue(struct ast_filestream *s)
 {
 	struct mp3_private *p = s->_private;
 	int res=0;
-	
-	p->lastseek = ftell(s->f);
+
+	res = ftell(s->f);
 	p->sbuflen = fread(p->sbuf, 1, MP3_SCACHE, s->f);
 	if(p->sbuflen < 0) {
 		ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", p->sbuflen, strerror(errno));
@@ -131,11 +131,11 @@
 	return 0;
 }
 
-static int mp3_dqueue(struct ast_filestream *s) 
+static int mp3_dqueue(struct ast_filestream *s)
 {
 	struct mp3_private *p = s->_private;
 	int res=0;
-	
+
 	if((res = decodeMP3(&p->mp,NULL,0,p->dbuf,MP3_DCACHE,&p->dbuflen)) == MP3_OK) {
 		p->sbuflen -= p->dbuflen;
 		p->dbufoffset = 0;
@@ -147,7 +147,7 @@
 {
 	struct mp3_private *p = s->_private;
 	int res = 0, bytes = 0;
-	
+
 	if(p->seek) {
 		ExitMP3(&p->mp);
 		InitMP3(&p->mp, OUTSCALE);
@@ -167,7 +167,7 @@
 			if(res == MP3_ERR)
 				return -1;
 		}
-		
+
 		p->seek = 0;
 		return 0;
 	}
@@ -181,7 +181,7 @@
 			if(mp3_squeue(s))
 				return -1;
 		}
-		
+
 	}
 
 	return 0;
@@ -194,36 +194,41 @@
 	int delay =0;
 	int save=0;
 
-	/* Send a frame from the file to the appropriate channel */
-
-	if(mp3_queue(s))
+	/* Pre-populate the buffer that holds audio to be returned (dbuf) */
+	if (mp3_queue(s)) {
 		return NULL;
-
-	if(p->dbuflen) {
-		for(p->buflen=0; p->buflen < MP3_BUFLEN && p->buflen < p->dbuflen; p->buflen++) {
-			s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[p->buflen+p->dbufoffset];
-			p->sbufoffset++;
+	}
+
+	if (p->dbuflen) {
+		/* Read out what's waiting in dbuf */
+		for (p->buflen = 0; p->buflen < MP3_BUFLEN && p->buflen < p->dbuflen; p->buflen++) {
+			s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[p->buflen + p->dbufoffset];
 		}
 		p->dbufoffset += p->buflen;
 		p->dbuflen -= p->buflen;
-
-		if(p->buflen < MP3_BUFLEN) {
-			if(mp3_queue(s))
-				return NULL;
-
-			for(save = p->buflen; p->buflen < MP3_BUFLEN; p->buflen++) {
-				s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[(p->buflen-save)+p->dbufoffset];
-				p->sbufoffset++;
+	}
+
+	if (p->buflen < MP3_BUFLEN) {
+		/* dbuf didn't have enough, so reset dbuf, fill it back up and continue */
+		p->dbuflen = p->dbufoffset = 0;
+
+		if (mp3_queue(s)) {
+			return NULL;
+		}
+
+		/* Make sure dbuf has enough to complete this read attempt */
+		if (p->dbuflen >= (MP3_BUFLEN - p->buflen)) {
+			for (save = p->buflen; p->buflen < MP3_BUFLEN; p->buflen++) {
+				s->buf[p->buflen + AST_FRIENDLY_OFFSET] = p->dbuf[(p->buflen - save) + p->dbufoffset];
 			}
 			p->dbufoffset += (MP3_BUFLEN - save);
 			p->dbuflen -= (MP3_BUFLEN - save);
-
-		} 
-
-	}
-	
+		}
+
+	}
+
 	p->offset += p->buflen;
-	delay = p->buflen/2;
+	delay = p->buflen / 2;
 	s->fr.frametype = AST_FRAME_VOICE;
 	s->fr.subclass.codec = AST_FORMAT_SLINEAR;
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, p->buflen);
@@ -266,16 +271,16 @@
 
 	p->seek = offset;
 	return fseek(s->f, offset, SEEK_SET);
-	
-}
-
-static int mp3_rewrite(struct ast_filestream *s, const char *comment) 
+
+}
+
+static int mp3_rewrite(struct ast_filestream *s, const char *comment)
 {
 	ast_log(LOG_ERROR,"I Can't write MP3 only read them.\n");
 	return -1;
 }
 
-static int mp3_trunc(struct ast_filestream *s) 
+static int mp3_trunc(struct ast_filestream *s)
 {
 
 	ast_log(LOG_ERROR,"I Can't write MP3 only read them.\n");
@@ -285,7 +290,7 @@
 static off_t mp3_tell(struct ast_filestream *s)
 {
 	struct mp3_private *p = s->_private;
-	
+
 	return p->offset/2;
 }
 
@@ -321,6 +326,6 @@
 static int unload_module(void)
 {
 	return ast_format_unregister(name);
-}	
+}
 
 AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "MP3 format [Any rate but 8000hz mono is optimal]");




More information about the asterisk-commits mailing list