Bug in aplay?

Bug in aplay?

[Damien Zammit]

Hi,
I think i found a small bug in alsa utility "aplay".

If you have a sound device that supports the S24_3BE format natively,
and you have a wav file of the same format, aplay will not play it
because it detects the wrong format and assumes it is default format.
(ie 8000Hz Mono 1 channel).
I compiled my own version of aplay and it works because I hacked the
default format to match this format, but it is not the ideal solution.

pi@raspberrypi ~ $ aplay --version
aplay: version 1.0.25 by Jaroslav Kysela <perex@perex.cz>

pi@raspberrypi ~ $ aplay -D hw:1 48000-S24_3BE.wav
Playing raw data '48000-S24_3BE.wav' : Unsigned 8 bit, Rate 8000 Hz, Mono
aplay: set_params:1081: Sample format non available
Available formats:
- S24_3BE

My version:
pi@raspberrypi ~ $ ./aplay -D hw:1 48000-S24_3BE.wav
Playing raw data '48000-S24_3BE.wav' : Signed 24 bit Big Endian in
3bytes, Rate 48000 Hz, Stereo
aplay: set_params:1145: Unable to install hw params:
ACCESS:  RW_INTERLEAVED
FORMAT:  S24_3BE
SUBFORMAT:  STD
SAMPLE_BITS: 24
FRAME_BITS: 48
CHANNELS: 2
RATE: 48000
PERIOD_TIME: 125000
PERIOD_SIZE: 6000
PERIOD_BYTES: 36000
PERIODS: 4
BUFFER_TIME: 500000
BUFFER_SIZE: 24000
BUFFER_BYTES: 144000
TICK_TIME: 0
pi@raspberrypi ~ $

This is making it tricky for me to use my sound card.  For some reason
it works better with jackd.  The error above is also a mystery to me,
but it seems to be detecting the format better.

Regards,
Damien

Re: Bug in aplay?

[Takashi Iwai]

At Wed, 12 Dec 2012 22:01:38 +1100,
Damien Zammit wrote:
> 
> Hi,
> I think i found a small bug in alsa utility "aplay".
> 
> If you have a sound device that supports the S24_3BE format natively,
> and you have a wav file of the same format, aplay will not play it
> because it detects the wrong format and assumes it is default format.
> (ie 8000Hz Mono 1 channel).
> I compiled my own version of aplay and it works because I hacked the
> default format to match this format, but it is not the ideal solution.
> 
> pi@raspberrypi ~ $ aplay --version
> aplay: version 1.0.25 by Jaroslav Kysela <perex@perex.cz>
> 
> pi@raspberrypi ~ $ aplay -D hw:1 48000-S24_3BE.wav
> Playing raw data '48000-S24_3BE.wav' : Unsigned 8 bit, Rate 8000 Hz, Mono
> aplay: set_params:1081: Sample format non available
> Available formats:
> - S24_3BE

Does your WAV file have a valid WAV_FMT_* chunk indicating the 24bit
format?  aplay checks bit_p_spl and byte_p_spl fields of WaveFmtBody
defined in formats.h.


Takashi

Re: Bug in aplay?

[Damien Zammit]

pi@raspberrypi ~ $ hexdump -C 48000-S24_3BE.wav |head
00000000  52 49 46 58 01 c3 20 48  57 41 56 45 66 6d 74 20  |RIFX.. HWAVEfmt |
00000010  00 00 00 28 ff fe 00 02  00 00 bb 80 00 04 65 00  |...(..........e.|
00000020  00 06 00 18 00 16 00 18  00 00 00 03 00 01 00 00  |................|
00000030  00 00 10 00 80 00 00 aa  00 38 9b 71 66 61 63 74  |.........8.qfact|
00000040  00 00 00 04 00 4b 30 00  64 61 74 61 01 c3 20 00  |.....K0.data.. .|
00000050  ff f6 00 00 14 00 00 3f  00 00 26 00 00 25 00 00  |.......?..&..%..|
00000060  2c 00 ff cf 00 00 2b 00  00 0b 00 00 25 00 00 44  |,.....+.....%..D|
00000070  00 00 1d 00 00 1b 00 00  13 00 00 19 00 00 06 00  |................|
00000080  ff f8 00 00 00 00 ff ab  00 00 04 00 ff f1 00 00  |................|
00000090  0a 00 00 5a 00 00 0d 00  00 38 00 00 0a 00 00 08  |...Z.....8......|

On 12 December 2012 22:38, Takashi Iwai <tiwai@suse.de> wrote:
> At Wed, 12 Dec 2012 22:01:38 +1100,
> Damien Zammit wrote:
>>
>> Hi,
>> I think i found a small bug in alsa utility "aplay".
>>
>> If you have a sound device that supports the S24_3BE format natively,
>> and you have a wav file of the same format, aplay will not play it
>> because it detects the wrong format and assumes it is default format.
>> (ie 8000Hz Mono 1 channel).
>> I compiled my own version of aplay and it works because I hacked the
>> default format to match this format, but it is not the ideal solution.
>>
>> pi@raspberrypi ~ $ aplay --version
>> aplay: version 1.0.25 by Jaroslav Kysela <perex@perex.cz>
>>
>> pi@raspberrypi ~ $ aplay -D hw:1 48000-S24_3BE.wav
>> Playing raw data '48000-S24_3BE.wav' : Unsigned 8 bit, Rate 8000 Hz, Mono
>> aplay: set_params:1081: Sample format non available
>> Available formats:
>> - S24_3BE
>
> Does your WAV file have a valid WAV_FMT_* chunk indicating the 24bit
> format?  aplay checks bit_p_spl and byte_p_spl fields of WaveFmtBody
> defined in formats.h.
>
>
> Takashi

Re: Bug in aplay?

[Clemens Ladisch]

Damien Zammit wrote:
> pi@raspberrypi ~ $ hexdump -C 48000-S24_3BE.wav |head
> 00000000  52 49 46 58 01 c3 20 48  57 41 56 45 66 6d 74 20  |RIFX.. HWAVEfmt |

aplay does not support big-endian .wav files.


Regards,
Clemens

Re: Bug in aplay?

[Damien Zammit]

I wish to record and playback streams from my sound card which uses 24
bit big endian format, but I do not wish to resample it.  Is it
possible to reorder the endianess on the fly so that I can play and
record using alsa tools without losing any quality?

On 12 December 2012 22:59, Clemens Ladisch <clemens@ladisch.de> wrote:
> Damien Zammit wrote:
>> pi@raspberrypi ~ $ hexdump -C 48000-S24_3BE.wav |head
>> 00000000  52 49 46 58 01 c3 20 48  57 41 56 45 66 6d 74 20  |RIFX.. HWAVEfmt |
>
> aplay does not support big-endian .wav files.
>
>
> Regards,
> Clemens

Re: Bug in aplay?

[Clemens Ladisch]

Damien Zammit wrote:
> I wish to record and playback streams from my sound card which uses 24
> bit big endian format, but I do not wish to resample it.

You get resampling only if the sample rates do not match.


Regards,
Clemens

Re: Bug in aplay?

[Takashi Iwai]

At Wed, 12 Dec 2012 12:59:01 +0100,
Clemens Ladisch wrote:
> 
> Damien Zammit wrote:
> > pi@raspberrypi ~ $ hexdump -C 48000-S24_3BE.wav |head
> > 00000000  52 49 46 58 01 c3 20 48  57 41 56 45 66 6d 74 20  |RIFX.. HWAVEfmt |
> 
> aplay does not support big-endian .wav files.

Indeed.

The (untested) patch below should add the support for BE format.
Give it a try.


Takashi

---
diff --git a/aplay/aplay.c b/aplay/aplay.c
index f35f603..4852b20 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -892,11 +892,20 @@ static ssize_t test_wavefile(int fd, u_char *_buffer, size_t size)
 	WaveFmtBody *f;
 	WaveChunkHeader *c;
 	u_int type, len;
+	unsigned short format, channels;
+	int big_endian, native_format;
 
 	if (size < sizeof(WaveHeader))
 		return -1;
-	if (h->magic != WAV_RIFF || h->type != WAV_WAVE)
+	if (h->magic == WAV_RIFF)
+		big_endian = 0;
+	else if (h->magic == WAV_RIFX)
+		big_endian = 1;
+	else
+		return -1;
+	if (h->type != WAV_WAVE)
 		return -1;
+
 	if (size > sizeof(WaveHeader)) {
 		check_wavefile_space(buffer, size - sizeof(WaveHeader), blimit);
 		memcpy(buffer, _buffer + sizeof(WaveHeader), size - sizeof(WaveHeader));
@@ -907,7 +916,7 @@ static ssize_t test_wavefile(int fd, u_char *_buffer, size_t size)
 		test_wavefile_read(fd, buffer, &size, sizeof(WaveChunkHeader), __LINE__);
 		c = (WaveChunkHeader*)buffer;
 		type = c->type;
-		len = LE_INT(c->length);
+		len = TO_CPU_INT(c->length, big_endian);
 		len += len % 2;
 		if (size > sizeof(WaveChunkHeader))
 			memmove(buffer, buffer + sizeof(WaveChunkHeader), size - sizeof(WaveChunkHeader));
@@ -929,7 +938,8 @@ static ssize_t test_wavefile(int fd, u_char *_buffer, size_t size)
 	check_wavefile_space(buffer, len, blimit);
 	test_wavefile_read(fd, buffer, &size, len, __LINE__);
 	f = (WaveFmtBody*) buffer;
-	if (LE_SHORT(f->format) == WAV_FMT_EXTENSIBLE) {
+	format = TO_CPU_SHORT(f->format, big_endian);
+	if (format == WAV_FMT_EXTENSIBLE) {
 		WaveFmtExtensibleBody *fe = (WaveFmtExtensibleBody*)buffer;
 		if (len < sizeof(WaveFmtExtensibleBody)) {
 			error(_("unknown length of extensible 'fmt ' chunk (read %u, should be %u at least)"),
@@ -940,19 +950,20 @@ static ssize_t test_wavefile(int fd, u_char *_buffer, size_t size)
 			error(_("wrong format tag in extensible 'fmt ' chunk"));
 			prg_exit(EXIT_FAILURE);
 		}
-		f->format = fe->guid_format;
+		format = TO_CPU_SHORT(fe->guid_format, big_endian);
 	}
-        if (LE_SHORT(f->format) != WAV_FMT_PCM &&
-            LE_SHORT(f->format) != WAV_FMT_IEEE_FLOAT) {
-                error(_("can't play WAVE-file format 0x%04x which is not PCM or FLOAT encoded"), LE_SHORT(f->format));
+	if (format != WAV_FMT_PCM &&
+	    format != WAV_FMT_IEEE_FLOAT) {
+                error(_("can't play WAVE-file format 0x%04x which is not PCM or FLOAT encoded"), format);
 		prg_exit(EXIT_FAILURE);
 	}
-	if (LE_SHORT(f->channels) < 1) {
-		error(_("can't play WAVE-files with %d tracks"), LE_SHORT(f->channels));
+	channels = TO_CPU_SHORT(f->channels, big_endian);
+	if (channels < 1) {
+		error(_("can't play WAVE-files with %d tracks"), channels);
 		prg_exit(EXIT_FAILURE);
 	}
-	hwparams.channels = LE_SHORT(f->channels);
-	switch (LE_SHORT(f->bit_p_spl)) {
+	hwparams.channels = channels;
+	switch (TO_CPU_SHORT(f->bit_p_spl, big_endian)) {
 	case 8:
 		if (hwparams.format != DEFAULT_FORMAT &&
 		    hwparams.format != SND_PCM_FORMAT_U8)
@@ -960,43 +971,69 @@ static ssize_t test_wavefile(int fd, u_char *_buffer, size_t size)
 		hwparams.format = SND_PCM_FORMAT_U8;
 		break;
 	case 16:
+		if (big_endian)
+			native_format = SND_PCM_FORMAT_S16_BE;
+		else
+			native_format = SND_PCM_FORMAT_S16_LE;
 		if (hwparams.format != DEFAULT_FORMAT &&
-		    hwparams.format != SND_PCM_FORMAT_S16_LE)
-			fprintf(stderr, _("Warning: format is changed to S16_LE\n"));
-		hwparams.format = SND_PCM_FORMAT_S16_LE;
+		    hwparams.format != native_format)
+			fprintf(stderr, _("Warning: format is changed to %s\n"),
+				snd_pcm_format_name(native_format));
+		hwparams.format = native_format;
 		break;
 	case 24:
-		switch (LE_SHORT(f->byte_p_spl) / hwparams.channels) {
+		switch (TO_CPU_SHORT(f->byte_p_spl, big_endian) / hwparams.channels) {
 		case 3:
+			if (big_endian)
+				native_format = SND_PCM_FORMAT_S24_3BE;
+			else
+				native_format = SND_PCM_FORMAT_S24_3LE;
 			if (hwparams.format != DEFAULT_FORMAT &&
-			    hwparams.format != SND_PCM_FORMAT_S24_3LE)
-				fprintf(stderr, _("Warning: format is changed to S24_3LE\n"));
-			hwparams.format = SND_PCM_FORMAT_S24_3LE;
+			    hwparams.format != native_format)
+				fprintf(stderr, _("Warning: format is changed to %s\n"),
+					snd_pcm_format_name(native_format));
+			hwparams.format = native_format;
 			break;
 		case 4:
+			if (big_endian)
+				native_format = SND_PCM_FORMAT_S24_BE;
+			else
+				native_format = SND_PCM_FORMAT_S24_LE;
 			if (hwparams.format != DEFAULT_FORMAT &&
-			    hwparams.format != SND_PCM_FORMAT_S24_LE)
-				fprintf(stderr, _("Warning: format is changed to S24_LE\n"));
-			hwparams.format = SND_PCM_FORMAT_S24_LE;
+			    hwparams.format != native_format)
+				fprintf(stderr, _("Warning: format is changed to %s\n"),
+					snd_pcm_format_name(native_format));
+			hwparams.format = native_format;
 			break;
 		default:
 			error(_(" can't play WAVE-files with sample %d bits in %d bytes wide (%d channels)"),
-			      LE_SHORT(f->bit_p_spl), LE_SHORT(f->byte_p_spl), hwparams.channels);
+			      TO_CPU_SHORT(f->bit_p_spl, big_endian),
+			      TO_CPU_SHORT(f->byte_p_spl, big_endian),
+			      hwparams.channels);
 			prg_exit(EXIT_FAILURE);
 		}
 		break;
 	case 32:
-                if (LE_SHORT(f->format) == WAV_FMT_PCM)
-                        hwparams.format = SND_PCM_FORMAT_S32_LE;
-                else if (LE_SHORT(f->format) == WAV_FMT_IEEE_FLOAT)
-                        hwparams.format = SND_PCM_FORMAT_FLOAT_LE;
+		if (format == WAV_FMT_PCM) {
+			if (big_endian)
+				native_format = SND_PCM_FORMAT_S32_BE;
+			else
+				native_format = SND_PCM_FORMAT_S32_LE;
+                        hwparams.format = native_format;
+		} else if (format == WAV_FMT_IEEE_FLOAT) {
+			if (big_endian)
+				native_format = SND_PCM_FORMAT_FLOAT_BE;
+			else
+				native_format = SND_PCM_FORMAT_FLOAT_LE;
+			hwparams.format = native_format;
+		}
 		break;
 	default:
 		error(_(" can't play WAVE-files with sample %d bits wide"),
-		      LE_SHORT(f->bit_p_spl));
+		      TO_CPU_SHORT(f->bit_p_spl, big_endian));
 		prg_exit(EXIT_FAILURE);
 	}
-	hwparams.rate = LE_INT(f->sample_fq);
+	hwparams.rate = TO_CPU_INT(f->sample_fq, big_endian);
 	
 	if (size > len)
 		memmove(buffer, buffer + len, size - len);
@@ -1009,7 +1046,7 @@ static ssize_t test_wavefile(int fd, u_char *_buffer, size_t size)
 		test_wavefile_read(fd, buffer, &size, sizeof(WaveChunkHeader), __LINE__);
 		c = (WaveChunkHeader*)buffer;
 		type = c->type;
-		len = LE_INT(c->length);
+		len = TO_CPU_INT(c->length, big_endian);
 		if (size > sizeof(WaveChunkHeader))
 			memmove(buffer, buffer + sizeof(WaveChunkHeader), size - sizeof(WaveChunkHeader));
 		size -= sizeof(WaveChunkHeader);
diff --git a/aplay/formats.h b/aplay/formats.h
index b5314f9..ac0a2b0 100644
--- a/aplay/formats.h
+++ b/aplay/formats.h
@@ -60,7 +60,14 @@ typedef struct voc_ext_block {
 #error "Wrong endian"
 #endif
 
+/* Note: the following macros evaluate the parameter v twice */
+#define TO_CPU_SHORT(v, be) \
+	((be) ? BE_SHORT(v) : LE_SHORT(v))
+#define TO_CPU_INT(v, be) \
+	((be) ? BE_INT(v) : LE_INT(v))
+
 #define WAV_RIFF		COMPOSE_ID('R','I','F','F')
+#define WAV_RIFX		COMPOSE_ID('R','I','F','X')
 #define WAV_WAVE		COMPOSE_ID('W','A','V','E')
 #define WAV_FMT			COMPOSE_ID('f','m','t',' ')
 #define WAV_DATA		COMPOSE_ID('d','a','t','a')