[Asterisk-code-review] formats: Restore previous fread() behavior (asterisk[13])

Sean Bright asteriskteam at digium.com
Tue Sep 5 10:09:19 CDT 2017


Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/6403


Change subject: formats: Restore previous fread() behavior
......................................................................

formats: Restore previous fread() behavior

Some formats are able to handle short reads while others are not, so
restore the previous behavior for the format modules so that we don't
have spurious errors when playing back files.

ASTERISK-27232 #close
Reported by: Jens T.

Change-Id: Iab7f52b25a394f277566c8a2a4b15a692280a300
---
M formats/format_g719.c
M formats/format_g723.c
M formats/format_g726.c
M formats/format_g729.c
M formats/format_gsm.c
M formats/format_h263.c
M formats/format_h264.c
M formats/format_ilbc.c
M formats/format_pcm.c
M formats/format_siren14.c
M formats/format_siren7.c
M formats/format_sln.c
M formats/format_vox.c
M formats/format_wav.c
M formats/format_wav_gsm.c
15 files changed, 87 insertions(+), 161 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/03/6403/1

diff --git a/formats/format_g719.c b/formats/format_g719.c
index 6678585..e27822d 100644
--- a/formats/format_g719.c
+++ b/formats/format_g719.c
@@ -42,20 +42,15 @@
 
 static struct ast_frame *g719read(struct ast_filestream *s, int *whennext)
 {
-	int res;
-	/* Send a frame from the file to the appropriate channel */
+	size_t res;
 
+	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_g723.c b/formats/format_g723.c
index 1182124..9b77033 100644
--- a/formats/format_g723.c
+++ b/formats/format_g723.c
@@ -42,7 +42,7 @@
 static struct ast_frame *g723_read(struct ast_filestream *s, int *whennext)
 {
 	unsigned short size;
-	int res;
+	size_t res;
 	int delay;
 	/* Read the delay for the next packet, and schedule again if necessary */
 	/* XXX is this ignored ? */
@@ -67,15 +67,10 @@
 	/* Read the data into the buffer */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, size);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_g726.c b/formats/format_g726.c
index 94623f4..50b558e 100644
--- a/formats/format_g726.c
+++ b/formats/format_g726.c
@@ -119,22 +119,17 @@
 
 static struct ast_frame *g726_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
 	struct g726_desc *fs = (struct g726_desc *)s->_private;
 
 	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, frame_size[fs->rate]);
 	s->fr.samples = 8 * FRAME_TIME;
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_g729.c b/formats/format_g729.c
index 8436446..35c68bd 100644
--- a/formats/format_g729.c
+++ b/formats/format_g729.c
@@ -48,20 +48,16 @@
 
 static struct ast_frame *g729_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
+
 	/* Send a frame from the file to the appropriate channel */
 	s->fr.samples = G729A_SAMPLES;
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res && res != 10) /* XXX what for ? */ {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_gsm.c b/formats/format_gsm.c
index cfc9452..783d955 100644
--- a/formats/format_gsm.c
+++ b/formats/format_gsm.c
@@ -55,19 +55,14 @@
 
 static struct ast_frame *gsm_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
 
 	AST_FRAME_SET_BUFFER(&(s->fr), s->buf, AST_FRIENDLY_OFFSET, GSM_FRAME_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, GSM_FRAME_SIZE, s->f)) != GSM_FRAME_SIZE) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), GSM_FRAME_SIZE, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), GSM_FRAME_SIZE, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_h263.c b/formats/format_h263.c
index 5d59972..be8e1df 100644
--- a/formats/format_h263.c
+++ b/formats/format_h263.c
@@ -69,7 +69,7 @@
 
 static struct ast_frame *h263_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
 	uint32_t mark;
 	unsigned short len;
 	unsigned int ts;
@@ -87,15 +87,10 @@
 	}
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, len);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_h264.c b/formats/format_h264.c
index f8906f4..3060400 100644
--- a/formats/format_h264.c
+++ b/formats/format_h264.c
@@ -61,7 +61,7 @@
 
 static struct ast_frame *h264_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
 	int mark = 0;
 	unsigned short len;
 	unsigned int ts;
@@ -79,15 +79,10 @@
 	}
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, len);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_ilbc.c b/formats/format_ilbc.c
index 6e06ef3..d4fbe96 100644
--- a/formats/format_ilbc.c
+++ b/formats/format_ilbc.c
@@ -47,19 +47,15 @@
 
 static struct ast_frame *ilbc_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
+
 	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, ILBC_BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_pcm.c b/formats/format_pcm.c
index bd7cf77..7b06482 100644
--- a/formats/format_pcm.c
+++ b/formats/format_pcm.c
@@ -80,21 +80,15 @@
 
 static struct ast_frame *pcm_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
-	
-	/* Send a frame from the file to the appropriate channel */
+	size_t res;
 
+	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) < 1) {
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_siren14.c b/formats/format_siren14.c
index 5aaa1f1..3e42bef 100644
--- a/formats/format_siren14.c
+++ b/formats/format_siren14.c
@@ -42,20 +42,15 @@
 
 static struct ast_frame *siren14read(struct ast_filestream *s, int *whennext)
 {
-	int res;
-	/* Send a frame from the file to the appropriate channel */
+	size_t res;
 
+	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_siren7.c b/formats/format_siren7.c
index 87e1372..f1bde00 100644
--- a/formats/format_siren7.c
+++ b/formats/format_siren7.c
@@ -42,20 +42,15 @@
 
 static struct ast_frame *siren7read(struct ast_filestream *s, int *whennext)
 {
-	int res;
-	/* Send a frame from the file to the appropriate channel */
+	size_t res;
 
+	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_sln.c b/formats/format_sln.c
index 2f4cc57..48bad8a 100644
--- a/formats/format_sln.c
+++ b/formats/format_sln.c
@@ -36,20 +36,15 @@
 
 static struct ast_frame *generic_read(struct ast_filestream *s, int *whennext, unsigned int buf_size)
 {
-	int res;
-	/* Send a frame from the file to the appropriate channel */
+	size_t res;
 
+	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, buf_size);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) < 1) {
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_vox.c b/formats/format_vox.c
index 26d4169..813dabf 100644
--- a/formats/format_vox.c
+++ b/formats/format_vox.c
@@ -42,20 +42,15 @@
 
 static struct ast_frame *vox_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
 
 	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) < 1) {
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_wav.c b/formats/format_wav.c
index 2903992..cead61c 100644
--- a/formats/format_wav.c
+++ b/formats/format_wav.c
@@ -371,7 +371,7 @@
 
 static struct ast_frame *wav_read(struct ast_filestream *s, int *whennext)
 {
-	int res;
+	size_t res;
 	int samples;	/* actual samples read */
 #if __BYTE_ORDER == __BIG_ENDIAN
 	int x;
@@ -393,16 +393,11 @@
 /* 	ast_debug(1, "here: %d, maxlen: %d, bytes: %d\n", here, s->maxlen, bytes); */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, bytes);
 
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (feof(s->f)) {
-			if (res) {
-				ast_debug(3, "Incomplete frame data at end of %s file "
-						  "(expected %d bytes, read %d)\n",
-						  ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
-			}
-		} else {
-			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) == 0) {
+		if (res) {
+			ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+					ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res,
+					strerror(errno));
 		}
 		return NULL;
 	}
diff --git a/formats/format_wav_gsm.c b/formats/format_wav_gsm.c
index 2f80a9a..423dfe4 100644
--- a/formats/format_wav_gsm.c
+++ b/formats/format_wav_gsm.c
@@ -421,18 +421,13 @@
 	} else {
 		/* read and convert */
 		unsigned char msdata[MSGSM_FRAME_SIZE];
-		int res;
-		
+		size_t res;
+
 		if ((res = fread(msdata, 1, MSGSM_FRAME_SIZE, s->f)) != MSGSM_FRAME_SIZE) {
-			if (feof(s->f)) {
-				if (res) {
-					ast_debug(3, "Incomplete frame data at end of %s file "
-							  "(expected %d bytes, read %d)\n",
-							  ast_format_get_name(s->fr.subclass.format), MSGSM_FRAME_SIZE, res);
-				}
-			} else {
-				ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
-						ast_format_get_name(s->fr.subclass.format), strerror(errno));
+			if (res && res != 1) {
+				ast_log(LOG_WARNING, "Short read of %s data (expected %d bytes, read %zu): %s\n",
+						ast_format_get_name(s->fr.subclass.format), MSGSM_FRAME_SIZE, res,
+						strerror(errno));
 			}
 			return NULL;
 		}

-- 
To view, visit https://gerrit.asterisk.org/6403
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iab7f52b25a394f277566c8a2a4b15a692280a300
Gerrit-Change-Number: 6403
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170905/7b4c7721/attachment-0001.html>


More information about the asterisk-code-review mailing list