[alsa-devel] [PATCH] aplay: Fix header for raw format

[Takashi Iwai]

On Fri, 30 Jun 2017 15:11:38 +0200,
Daniel Baluta wrote:
> 
> Hi Takashi,
> 
> Thanks a lot for your feedback.
> 
> On Fri, Jun 30, 2017 at 9:54 AM, Takashi Iwai <tiwai at suse.de> wrote:
> > On Thu, 29 Jun 2017 13:17:16 +0200,
> > Ion-Horia Petrisor wrote:
> >>
> >> Raw format has no header, so use 0 when calling playback_go.
> >
> > It's the value passed for the length that has been already loaded.
> > The program has read dta bytes for parsing the header, and it's raw
> > data without header.  Thus you need to pass dta there.
> >
> With the current code we assume that the raw files have at least 26 bytes.
> (sizeof (VocHeader).
> 
> I think it doesn't matter here that dta bytes has already been loaded. These
> bytes useful to figure out the file header type.
> 
> To keep things simple we can assume that in the case of raw files there is no
> header, so second parameter of playback_go (named loaded) can be set to 0.

The loaded parameter of playback_go() doesn't mean that.  It's the
size that has been already read onto the audio buffer.  If you pass 0
there, it means that you discard the first 26 bytes samples you have
already read without playing.

> The worst things that can happen is that we re-read the first 26 bytes
> (previously
> read as VoC header).

Where is re-read...?

> We are trying to introduce a new parameter --samples which tells aplay to  only
> playback/record s number of samples.
> 
> We modified calc_count() accordingly so that when receives the
> --samples parameter
> computes the correct number of bytes to be read.
> 
> Anyhow, it doesn't work as expected because aplay expects files to have at least
> 26 bytes.
> 
> The correct solution would be that in the case of raw files not assume
> any header
> and read samples * sample_size bytes from the beginning of the file.

I see now what you want, but the patch is incorrect.
I guess a patch something like below is what you want.


thanks,

Takashi

---
diff --git a/aplay/aplay.c b/aplay/aplay.c
index 00af66246cad..a902be5cf9cb 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -2782,12 +2782,65 @@ static void playback_go(int fd, size_t loaded, off64_t count, int rtype, char *n
  *  let's play or capture it (capture_type says VOC/WAVE/raw)
  */
 
-static void playback(char *name)
+static void read_header(int *loaded, int header_size)
+{
+	if (*loaded < header_size) {
+		header_size -= *loaded;
+		if (safe_read(fd, audiobuf, header_size) != header_size) {
+			error(_("read error"));
+			prg_exit(EXIT_FAILURE);
+		}
+		*loaded += header_size;
+	}
+}
+
+static int playback_au(char *name, int *loaded)
+{
+	read_header(loaded, sizeof(AuHeader));
+	if (test_au(fd, audiobuf) < 0)
+		return -1;
+	rhwparams.format = hwparams.format;
+	pbrec_count = calc_count();
+	playback_go(fd, *loaded - sizeof(AuHeader), pbrec_count, FORMAT_AU, name);
+	return 0;
+}
+
+static int playback_voc(char *name, int *loaded)
 {
 	int ofs;
-	size_t dta;
+
+	read_header(loaded, sizeof(VocHeader));
+	if ((ofs = test_vocfile(audiobuf)) < 0)
+		return -1;
+	pbrec_count = calc_count();
+	voc_play(fd, ofs, name);
+	return 0;
+}
+
+static int playback_wave(char *name, int *loaded)
+{
 	ssize_t dtawave;
 
+	read_header(loaded, sizeof(WaveHeader));
+	if ((dtawave = test_wavefile(fd, audiobuf, *loaded)) < 0)
+		return -1;
+	pbrec_count = calc_count();
+	playback_go(fd, dtawave, pbrec_count, FORMAT_WAVE, name);
+	return 0;
+}
+
+static int playback_raw(char *name, int *loaded)
+{
+	init_raw_data();
+	pbrec_count = calc_count();
+	playback_go(fd, *loaded, pbrec_count, FORMAT_RAW, name);
+	return 0;
+}
+
+static void playback(char *name)
+{
+	int loaded = 0;
+
 	pbrec_count = LLONG_MAX;
 	fdcount = 0;
 	if (!name || !strcmp(name, "-")) {
@@ -2800,40 +2853,29 @@ static void playback(char *name)
 			prg_exit(EXIT_FAILURE);
 		}
 	}
-	/* read the file header */
-	dta = sizeof(AuHeader);
-	if ((size_t)safe_read(fd, audiobuf, dta) != dta) {
-		error(_("read error"));
-		prg_exit(EXIT_FAILURE);
-	}
-	if (test_au(fd, audiobuf) >= 0) {
-		rhwparams.format = hwparams.format;
-		pbrec_count = calc_count();
-		playback_go(fd, 0, pbrec_count, FORMAT_AU, name);
-		goto __end;
-	}
-	dta = sizeof(VocHeader);
-	if ((size_t)safe_read(fd, audiobuf + sizeof(AuHeader),
-		 dta - sizeof(AuHeader)) != dta - sizeof(AuHeader)) {
-		error(_("read error"));
-		prg_exit(EXIT_FAILURE);;
-	}
-	if ((ofs = test_vocfile(audiobuf)) >= 0) {
-		pbrec_count = calc_count();
-		voc_play(fd, ofs, name);
-		goto __end;
-	}
-	/* read bytes for WAVE-header */
-	if ((dtawave = test_wavefile(fd, audiobuf, dta)) >= 0) {
-		pbrec_count = calc_count();
-		playback_go(fd, dtawave, pbrec_count, FORMAT_WAVE, name);
-	} else {
-		/* should be raw data */
-		init_raw_data();
-		pbrec_count = calc_count();
-		playback_go(fd, dta, pbrec_count, FORMAT_RAW, name);
+
+	switch (file_type) {
+	case FORMAT_AU:
+		playback_au(name, &loaded);
+		break;
+	case FORMAT_VOC:
+		playback_voc(name, &loaded);
+		break;
+	case FORMAT_WAVE:
+		playback_wave(name, &loaded);
+		break;
+	case FORMAT_RAW:
+		playback_raw(name, &loaded);
+		break;
+	default:
+		/* parse the file header */
+		if (playback_au(name, &loaded) < 0 &&
+		    playback_voc(name, &loaded) < 0 &&
+		    playback_wave(name, &loaded) < 0)
+			playback_raw(name, &loaded); /* should be raw data */
+		break;
 	}
-      __end:
+
 	if (fd != 0)
 		close(fd);
 }