[alsa-devel] [PATCH] [alsa-utils.git] Fix a strange regression in aplay's '-d' option
Takashi Iwai
tiwai at suse.de
Mon Sep 7 17:27:04 CEST 2015
On Sat, 05 Sep 2015 11:39:10 +0200,
Grond wrote:
>
>
>
> Hello.
> (J. Random Hacker here)
>
> While using aplay/arecord (version 1.0.28) to generate some noise, I ran
> across a regression in the use of the '-d' option.
Do you mean a regression from any previous versions? I couldn't find
any relevant change in the code you cited. It's been so from the very
original version, as far as I look at git commits.
> Specifically:
> According to the man page:
> -d, --duration=#
> Interrupt after # seconds. A value of zero means infinity.
> The default is zero, so if this option is omitted then the
> arecord process will run until it is killed.
> [snip]
> arecord -d 10 -f cd -t wav -D copy foobar.wav
> will record foobar.wav as a 10-second, CD-quality wave file,
> using the PCM "copy" (which might be defined in the user's
> .asoundrc file as:
> pcm.copy {
> type plug
> slave {
> pcm hw
> }
> route_policy copy
> }
> [snip]
>
> So the behavior I expect to see (and have experienced in the past) is:
>
> $ arecord -d 3 -f cd dump.wav
> Recording WAVE 'dump.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo
> $ ls -l
> -rw------- 1 grond66 grond66 529244 Sep 5 00:34 dump.wav
>
> What actually happens is:
>
> $ arecord -d 3 -f cd dump.wav
> Recording WAVE 'dump.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo
> [ program hangs here until SIGINT ]
> ^CAborted by signal Interrupt...
> arecord: pcm_read:2031: read error: Interrupted system call
> $ ls -l
> -rw------- 1 grond66 grond66 526444 Sep 5 00:39 dump-01.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-02.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-03.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-04.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-05.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-06.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-07.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-08.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-09.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-100.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-101.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-102.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-103.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-104.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-105.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-106.wav
> -rw------- 1 grond66 grond66 44 Sep 5 00:39 dump-107.wav
> [ etcetera... ]
> $ aplay dump-01.wav
> [ three seconds of hiss, as expected ]
> $ less dump-02.wav
> RIFF$^@^@^@WAVEfmt ^P^@^@^@^A^@^B^@D<AC>^@^@^P<B1>^B^@^D^@^P^@data^@^@^@^@
> $
>
> So arecord is recording the requested audio and putting it into the first file,
> then spamming WAV headers into all the subsequent files.
> This is not terribly useful.
Hmm, this doesn't happen on my machine. Which system and which
alsa-lib / alsa-utils versions are you using?
Takashi
> The problem appears to be six lines of code in aplay/aplay.c, in the functions
> pcm_read() and pcm_readv():
>
> 2006 static ssize_t pcm_read(u_char *data, size_t rcount)
> 2007 {
> 2008 ssize_t r;
> 2009 size_t result = 0;
> 2010 size_t count = rcount;
> 2011
> )> 2012 if (count != chunk_size) {
> )> 2013 count = chunk_size;
> )> 2014 }
> 2015
> 2016 while (count > 0 && !in_aborting) {
>
> 2045 static ssize_t pcm_readv(u_char **data, unsigned int channels, size_t rcount)
> 2046 {
> 2047 ssize_t r;
> 2048 size_t result = 0;
> 2049 size_t count = rcount;
> 2050
> )> 2051 if (count != chunk_size) {
> )> 2052 count = chunk_size;
> )> 2053 }
> 2054
> 2055 while (count > 0 && !in_aborting) {
>
> In this case, the 'rcount' arguments are the number of PCM
> integers to be read from the capture device.
> In theory. Because what lines 2012-2014 and 2051-2053 do is effectively:
>
> count = chunk_size;
>
> In other words, the argument is completely disregarded,
> and 'chunk_size' is used no matter what the functions are passed.
>
> But capture() and family use the following logic:
> "If pcm_read{,v}() returns with a value other than the
> number integers we told it to read off the wire,
> assume something went wrong":
>
> 2925 static void capture(char *orig_name)
> 2926 {
> 2927 int tostdout=0; /* boolean which describes output stream */
> 2928 int filecount=0; /* number of files written */
> 2929 char *name = orig_name; /* current filename */
> 2930 char namebuf[PATH_MAX+1];
> 2931 off64_t count, rest; /* number of bytes to capture */
> [...]
> 2964 do {
> 2965 /* open a file to write */
> 2966 if(!tostdout) {
> 2967 /* upon the second file we start the numbering scheme */
> 2968 if (filecount || use_strftime) {
> 2969 filecount = new_capture_file(orig_name, namebuf,
> 2970 sizeof(namebuf),
> 2971 filecount);
> 2972 name = namebuf;
> 2973 }
> 2974
> 2975 /* open a new file */
> 2976 remove(name);
> 2977 fd = safe_open(name);
> 2978 f (fd < 0) {
> 2979 perror(name);
> 2980 prg_exit(EXIT_FAILURE);
> 2981 }
> 2982 filecount++;
> 2983 }
> 2984
> 2985 rest = count;
> 2986 if (rest > fmt_rec_table[file_type].max_filesize)
> 2987 rest = fmt_rec_table[file_type].max_filesize;
> 2988 if (max_file_size && (rest > max_file_size))
> 2989 rest = max_file_size;
> 2990
> 2991 /* setup sample header */
> 2992 if (fmt_rec_table[file_type].start)
> 2993 fmt_rec_table[file_type].start(fd, rest);
> 2994
> 2995 /* capture */
> 2996 fdcount = 0;
> 2997 while (rest > 0 && recycle_capture_file == 0 && !in_aborting) {
> )> 2998 size_t c = (rest <= (off64_t)chunk_bytes) ?
> )> 2999 (size_t)rest : chunk_bytes;
> )> 3000 size_t f = c * 8 / bits_per_frame;
> )> 3001 if (pcm_read(audiobuf, f) != f)
> )> 3002 break;
> 3003 if (write(fd, audiobuf, c) != c) {
> 3004 perror(name);
> 3005 prg_exit(EXIT_FAILURE);
> 3006 }
> 3007 count -= c;
> 3008 rest -= c;
> 3009 fdcount += c;
> 3010 }
> 3011
> [...]
> 3027 /* repeat the loop when format is raw without timelimit or
> 3028 * requested counts of data are recorded
> 3029 */
> 3030 } while ((file_type == FORMAT_RAW && !timelimit) || count > 0);
> 3031 }
>
> These two modes of operation are incompatible.
> However, if we nix the strange requirement that all calls to pcm_read{,v}()
> must be for exactly 'chunk_size' samples, the problem goes away.
> This is the purpose of the patch I've included below.
> From eaa3b4acc17bbd7180a46d5c9f3e63442b159cd7 Mon Sep 17 00:00:00 2001
> From: Grond66 <grond66 at riseup.net>
> Date: Fri, 4 Sep 2015 20:05:19 -0700
> Subject: [PATCH 1/1] aplay: Remove weird code in pcm_read{,v}
>
> This fixes a regression in aplay/arecord which causes
> arecord to loop creating numerous empty numbered output files
> when the '-d' is used.
>
> Signed-off-by: Grond66 <grond66 at riseup.net>
>
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index 459f7dd..a26dbdb 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -2009,10 +2009,6 @@ static ssize_t pcm_read(u_char *data, size_t rcount)
> size_t result = 0;
> size_t count = rcount;
>
> - if (count != chunk_size) {
> - count = chunk_size;
> - }
> -
> while (count > 0 && !in_aborting) {
> if (test_position)
> do_test_position();
> @@ -2048,10 +2044,6 @@ static ssize_t pcm_readv(u_char **data, unsigned int channels, size_t rcount)
> size_t result = 0;
> size_t count = rcount;
>
> - if (count != chunk_size) {
> - count = chunk_size;
> - }
> -
> while (count > 0 && !in_aborting) {
> unsigned int channel;
> void *bufs[channels];
> --
> 2.1.4
>
> [1.3 PGP Key 0x55D89FD9. <application/pgp-keys (7bit)>]
>
> [2 Digital signature <application/pgp-signature (7bit)>]
>
More information about the Alsa-devel
mailing list