alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: David Fries <David@Fries.net>
To: Jaroslav Kysela <perex@perex.cz>
Cc: alsa-devel@alsa-project.org
Subject: [PATCH] aplay: fix lurking capture file overwrite bug
Date: Wed, 13 Apr 2016 23:32:46 -0500	[thread overview]
Message-ID: <20160414043246.GB16174@spacedout.fries.net> (raw)

If -d was given to arecord while commit
8aa13eec80eac312e4b99423909387660fb99b8f (now reverted) was in effect,
the last read would be shorter than the chunk size, but pcm_read would
read and return the chunk size, the samples were discarded, and
capture() continued in a loop because count never reached 0.  arecord
opens a new file each loop iteration, if arecord is dynamically naming
files, --use-strftime option or beyond the wave 2GB limit, this will
generate a series of header only wave files.  If the file is unique
the originally recorded data is lost and it will continue overwriting
the same file with a header only wave file.

While the current pcm_read can't fail (it can exit), it is better to
just fix this lurking bug in case it is "fixed" again.
---
In my case I wanted to record for 16 hours, I did get the audio, but I
also found I had 3.1 million 44 byte files, which took the system
3 hours to unlink, I'm glad I at least discovered this problem the
next day..  Debian has the effectively broken pcm_read, after figuring
out what was going wrong, and that the current arecord doesn't exhibit
the problem I wanted to fix it anyway.  There are remaining problems,
pcm_read will read chunk_size, but if the requested size is smaller
samples are lost, I'm not trying to fix that here.  This will at least
gracefully close a wave file (update the counts) if read or write
returns a size different from requested.

 aplay/aplay.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/aplay/aplay.c b/aplay/aplay.c
index 7acaa83..2da7dda 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -3067,11 +3067,14 @@ static void capture(char *orig_name)
 			size_t c = (rest <= (off64_t)chunk_bytes) ?
 				(size_t)rest : chunk_bytes;
 			size_t f = c * 8 / bits_per_frame;
-			if (pcm_read(audiobuf, f) != f)
+			if (pcm_read(audiobuf, f) != f) {
+				in_aborting = 1;
 				break;
+			}
 			if (write(fd, audiobuf, c) != c) {
 				perror(name);
-				prg_exit(EXIT_FAILURE);
+				in_aborting = 1;
+				break;
 			}
 			count -= c;
 			rest -= c;
@@ -3091,7 +3094,7 @@ static void capture(char *orig_name)
 		}
 
 		if (in_aborting)
-			break;
+			prg_exit(EXIT_FAILURE);
 
 		/* repeat the loop when format is raw without timelimit or
 		 * requested counts of data are recorded
-- 
2.1.4

             reply	other threads:[~2016-04-14  4:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-14  4:32 David Fries [this message]
2016-04-14 12:36 ` [PATCH] aplay: fix lurking capture file overwrite bug Takashi Iwai
2016-04-15 20:25 ` Alan Horstmann
2016-04-18  3:34   ` David Fries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160414043246.GB16174@spacedout.fries.net \
    --to=david@fries.net \
    --cc=alsa-devel@alsa-project.org \
    --cc=perex@perex.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).