From: Takashi Iwai <tiwai@suse.de>
To: David Fries <David@Fries.net>
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH] aplay: fix lurking capture file overwrite bug
Date: Thu, 14 Apr 2016 14:36:36 +0200 [thread overview]
Message-ID: <s5hmvow4baj.wl-tiwai@suse.de> (raw)
In-Reply-To: <20160414043246.GB16174@spacedout.fries.net>
On Thu, 14 Apr 2016 06:32:46 +0200,
David Fries wrote:
>
> 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.
Applied, thanks.
Takashi
>
> 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
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
next prev parent reply other threads:[~2016-04-14 12:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-14 4:32 [PATCH] aplay: fix lurking capture file overwrite bug David Fries
2016-04-14 12:36 ` Takashi Iwai [this message]
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=s5hmvow4baj.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=David@Fries.net \
--cc=alsa-devel@alsa-project.org \
/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).