[Asterisk-code-review] format wav: Read 16khz wav samples properly (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5478 )

Change subject: format_wav: Read 16khz wav samples properly
......................................................................


format_wav: Read 16khz wav samples properly

When opening a PCM wave file for reading, we aren't tracking the
frequency of the opened file, so we treat 16khz files as 8khz and do
half reads.

This patch also cleans up some of the data types and an unnecessarily
complex `if` expression.

ASTERISK-26613 #close
Reported by: Vitaly K

Change-Id: I05f8b263058dc573ea8ffe0c62e7964506e11815
---
M formats/format_wav.c
1 file changed, 14 insertions(+), 10 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Mark Michelson: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/formats/format_wav.c b/formats/format_wav.c
index 42a27e4..90095eb 100644
--- a/formats/format_wav.c
+++ b/formats/format_wav.c
@@ -82,9 +82,8 @@
 
 static int check_header_fmt(FILE *f, int hsize, int hz)
 {
-	short format, chans, bysam, bisam;
-	int bysec;
-	int freq;
+	unsigned short format, chans, bysam, bisam;
+	unsigned int freq, bysec;
 	if (hsize < 16) {
 		ast_log(LOG_WARNING, "Unexpected header size %d\n", hsize);
 		return -1;
@@ -94,7 +93,7 @@
 		return -1;
 	}
 	if (ltohs(format) != 1) {
-		ast_log(LOG_WARNING, "Not a supported wav file format (%d). Only PCM encoded, 16 bit, mono, 8kHz files are supported with a lowercase '.wav' extension.\n", ltohs(format));
+		ast_log(LOG_WARNING, "Not a supported wav file format (%d). Only PCM encoded, 16 bit, mono, 8kHz/16kHz files are supported with a lowercase '.wav' extension.\n", ltohs(format));
 		return -1;
 	}
 	if (fread(&chans, 1, 2, f) != 2) {
@@ -109,10 +108,9 @@
 		ast_log(LOG_WARNING, "Read failed (freq)\n");
 		return -1;
 	}
-	if (((ltohl(freq) != 8000) && (ltohl(freq) != 16000)) ||
-		((ltohl(freq) == 8000) && (hz != 8000)) ||
-		((ltohl(freq) == 16000) && (hz != 16000))) {
-		ast_log(LOG_WARNING, "Unexpected frequency mismatch %d (expecting %d)\n", ltohl(freq),hz);
+	freq = ltohl(freq);
+	if ((freq != 8000 && freq != 16000) || freq != hz) {
+		ast_log(LOG_WARNING, "Unexpected frequency mismatch %d (expecting %d)\n", freq, hz);
 		return -1;
 	}
 	/* Ignore the byte frequency */
@@ -325,9 +323,15 @@
 	/* We don't have any header to read or anything really, but
 	   if we did, it would go here.  We also might want to check
 	   and be sure it's a valid file.  */
-	struct wav_desc *tmp = (struct wav_desc *)s->_private;
-	if ((tmp->maxlen = check_header(s->f, ast_format_get_sample_rate(s->fmt->format))) < 0)
+	struct wav_desc *tmp = s->_private;
+	unsigned int sample_rate = ast_format_get_sample_rate(s->fmt->format);
+
+	tmp->maxlen = check_header(s->f, sample_rate);
+	if (tmp->maxlen < 0) {
 		return -1;
+	}
+
+	tmp->hz = sample_rate;
 	return 0;
 }
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I05f8b263058dc573ea8ffe0c62e7964506e11815
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>