| Message ID | 20150905093910.GA5618@localhost.localdomain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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@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)>] >
From eaa3b4acc17bbd7180a46d5c9f3e63442b159cd7 Mon Sep 17 00:00:00 2001
From: Grond66 <grond66@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@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
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. 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. 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.