/* Copyright (C) 2000 Britton Leo Kerin, see LICENSE.txt. */ basename now needs libgen.h? Maybe time to eliminate this function. ***** RESOLVED: Its gone. Man page titles are slightly screwed up in apropos because the .NAME sections are wrong because of man page link usage for rawrec and rawplay. Fix it. What is the empty LIBS make variable doing there? It's not a built in like CFLAGS or LDFLAGS, so is it a convention or what? ***** RESOLVED: Just a common convenience variable, its gone now. What is emty LDFLAGS doing there? It is a built in, so does including it tell gcc to do linking as well (probably not) or is it just there by convention to let people know that it's empty? ***** RESOLVED: The linker arguments for libraries. Used. Is the function name "usage( )" traditional for showing the argument and option listing that gets printed when users enter invalid arguments or options? ***** RESOLVED: pretty much. need mutex for progname even? threads currently use it to report write errors to ordinary data file. Probably not cause it is only read, but it does get passed to library functions. ***** RESOLVED: nah, its read only. according to getopt man page, -W is reserved for 'implementation extensions'? ***** RESOLVED: yet, and in fact getopt() implements an extension which allows you so specify long options without having to use two hyphens. So we don't use -W. remember to check rlimit RLIMIT_MEMLOCK before using mlock() to lock down ring buffer. make sure mlock has the correct length argument -- recall info in Beginning Linux Programming stating that area returned by memory management calls may be slightly different than expected. ***** RESOLVED: by using mlockall(), which is more appropriate for RT apps, instead of mlock(), from mlock man page: child processes do not inherit page locks accross a fork means what for thread implementation (hopefully, nothing, since they are only processes under the hood)? ***** RESOLVED: definately nothing, since unlike processes, threads don't make copies of process memory space. do we correctly handle the opening (and playing/recording) of really big files? ***** RESOLVED: yes, now full LFS usage. Shouldn't we be able to take a long list of files instead of just one, and play/record them all with the same parameters (would be more like other unix tools)? ***** RESOLVED: When working with raw data, and considering the asymmetry it would induce between record and play, I think this is more of a headache than its worth basename() has apparently gone away with latest library version. I have heard hoopla about this being inconsistently defined for a while now. Supposedly it is in the library but not in the header or something equally convenient. This needs sorting out, for now I'm doing what was done in bplay (including in rawrec.h a line: extern char *basename(char *name); ). ***** RESOLVED: string.h include features.h, which if _GNU_SOURCE is defined defines __USE_GNU, which causes string.h to use GNU basename instead of weird XPG version sphinx double-checks all it's ioctls, like this: if (ioctl (dspFD, SNDCTL_DSP_SETFMT, &audioFormat) < 0){ E_ERROR("Audio ioctl(SETFMT 0x%x) failed: %s\n", audioFormat, strerror(errno)); close (dspFD); return NULL; } if (audioFormat != AUDIO_FORMAT) { E_ERROR("Audio ioctl(SETFMT): 0x%x, expected: 0x%x\n", audioFormat, AUDIO_FORMAT); close (dspFD); return NULL; } if (ioctl (dspFD, SNDCTL_DSP_SPEED, &sampleRate) < 0) { E_ERROR("Audio ioctl(SPEED %d) failed %s\n", sampleRate, strerror(errno)); close (dspFD); return NULL; } if (sampleRate != sps) { E_ERROR("Audio ioctl(SPEED): %d, expected: %d\n", sampleRate, sps); close (dspFD); return NULL; } etc. is this a good idea? ***** RESOLVED: sort of, just giving up is probably a bad idea, this issue is now handled from process_command_line by calling test_pcm_params Is this still the format for setting the fragment size: int arg = 0xMMMMSSSS; if (ioctl(audio_fd, SNDCTL_DSP_SETFRAGMENT, &arg)) error(); ***** RESOLVED: yes, apparently. How is LIBS variable from Makefile typically used? ***** RESOLVED: to hold names of .a files, we don't use any so it's gone exactly where do I need to synchronize the audio device? According to 4front web page, only when changing device params, going from record to playback, or when I want the device to play all data in buffers (close()ing the audio device does this automaticly). apparently there is a new option '-pthread' which automaticly takes care of both adds -D_REENTRANT during the cpp pass, and adds -lpthread during the link-edit. is it really new? should it be used? will stdin and stdout still work at all in a real-timish pipeline if the commands driving the other ends of their pipes have to compete with the high priority move_au and move_fd threads? ***** RESOLVED: yes, they work perfectly provided they make sane processing (and io) demands. There is plenty of free time when both threads are blocked. how to determine the largest allowable value of the fragment size (around line 92 of process_command_line.c)? ***** RESOLVED: don't bother, driver will automagicly select largest possible if something larger than possible is requested (6-13-2000) use at least gcc -Wall -Wpointer-arith -Wstrict-prototypes for compiling securly (from Secure-Programs HOWTO)? ***** RESOLVED: ok, they are now used. under gdb, using -f, the file size shows zero until two full -f sized fragments have been recorded, whereupon the file size jumps to 2 time the fragment size, but for all subsequent fragments, the file size is incremented once per fragment as expected. with debugging code in, but not under gdb, I don't see this behavior. could this be some subtle, insidious concurency bug? Update: in fact, I get a veriety of exceedingly screwy behavior under gdb that I can't duplicate outside it, including "read less than expected" errors, gdb reporting getting sigsegv's and causing other processes trying to access the same data file to block forever trying to open() that data file, while gdb hangs and refuses to quit untill kill -9 from without, etc. I suspect thread-related gdb buginess. there doesn't seem to be a problem with -v needing to report warning when rawrec has been invoked as (for example) ./rawrec -t 20 -s 22777 -v &, though Steven's book's discussion of SIGTTOU seems to indicate that there might be check accuracy of long sleeps ***** DONE: when using nanosleep under a non-realtime scheduling policy or for a sleep longer than 2 ms, at least the requested time + up to 10 ms do I really need to do this (from mlockall man page): Real-time processes should reserve enough locked stack pages before entering the time-critical section, so that no page fault can be caused by function calls. This can be achieved by calling a function which has a sufficiently large automatic variable and which writes to the memory occupied by this large array in order to touch these stack pages. This way, enough pages will be mapped for the stack and can be locked into RAM. The dummy writes ensure that not even copy-on-write page faults can occur in the critical section. ? ***** RESOLVED: mlockall man page says all pages are gauranteed to be in RAM when mlockall() returns. currently the thread function have only a couple of single automatic variables. I'm not inclined to use MCL_FUTURE flag to mlockall to lock down this one thing (and mlockall(MCL_FUTURE) introduces much potential wierdness for future sys calls, see man page), is it worth moving the mlock inside the thread code? ***** RESOLVEDISH: I really doubt it. it seems I only need to have root permissions at the time the pthreads are actually started, not when high priority attributes are set or sched_get_priority_max() is called (max root priority is returned by sched_get_priority_max() even when it is called from normal user). Is this POSIX or Linux-specific behavior? is it safe to call fprintf(stderr, "%s...", progname) as root where %s is program name, since malicious user could concievably create long progname to overflow buffer for %s? Note that use should be limited by os to maximum file length, which is probably less than what %s parameter buffer can handle. Currently, I do it this way. Currently, running threads have geteuid() == 0, which they inherit from their parent thread, main. Can/should this be changed without affecting their scheduling, or do I need to try to secure the thread code (masking signals, etc headache). ***** RESOLVED: yes it can be changed, should be changed, and is changed Under pathological loading with a small ring buffer, is it possible for the thread that starts first to wrap all the way around and grab the first buffer segment again before the second thread makes it from it's pthread_cond_wait() to it's pthread_mutex_lock(&seg_mutex[0])? Perhaps for strict correctness, the first thread should stop and pthread_cond_wait for the second thread to pthread_cond_signal that it's done grabbing the first segment before wrapping around for the first time? ***** RESOLVED: yes, in fact the load doesn't have to be high at all in the case where we are playing, the disk is fast and/or the buffer is small. the movd_fd thread raced around and grabbed the first buffer again before the move_au thread could even get going for buffers as big as a megabyte or so. This problem has been fixed by the addition of a new global wrap_ready flag condition variable and mutex which signal when the second thread to start has successfully grabbed the first buffer segment. currently ringbufp is an unsigned char *. According to opensound pages, should it stay like that or should it become a regular char * (it shouldn't matter really, the question is really which would be more typical?) ***** RESOLVED: it should stay like that is it correct usage to say that 'an argument must be integral' (when you mean it must be an integer)? when we return or exit (I havn't decided for certian which yet) from record or play prematurely following receipt of a processed signal, we skip over the end part of the record or play functions, which misses end pausing and recording at whatnot, which is what we intend, but we also skip ringbuf_close, which is perhaps not so good. What is the performance penalty if any of calling ringbuf_close explicitly before leaving the record or play? better checking of options, both for type and for reasonableness (atof doesn't really check enough for example) the whole limit_set mess desperately needs cleanup, probably by adding another type of structure for internal use which does away with time entirely in favor of samples, and then adding a bit more intelligence to process_command_line. LFS support badly needed ***** DONE appropriate permissions on created files. ***** DONE: data files are created with read/write permissions for ugo, subject to umask a signal handling thread (with priorities?) and worker thread signal polling to handle stopped jobs. This will have to wait until Linux threads conform fully to POSIX with respect to signal handling. possibly: facilities for handling 'at time' startup delays, and possibly 'unitl time' record play limits? These probably arn't worth the hassles for coder and user (creeping featurism). ***** MOOT: not worth it testbed for what atol does when target is bigger than maximum long but smaller than maximum unsigned long ***** MOOT: atol is bogus and does no error checking, it must be replaced, probably with strtol, which is more careful about this sort of issue error checking on the pthread calls protected by #ifdef _POSIX_THREAD_PRIO_INHERIT and _POSIX_THREAD_PRIO_CEILING. This will likely not happen until rawrec runs on a system that supports these mechanisms. make play() take advantage of all the things that record now does ***** DONE use of NAME_MAX for length of character arrays designed to hold filenames is apparently 'file system specific' and pathconf should be used instead (according to loopy Beginning Linux Programming book) when a dummy device is in use and the audio block size is small (either because of small kernel size (unlikely) or USER_FRGSZ), nanosleep may be called many times, and may ultimately result in drift. Perhaps a clock check should be made occasionally for long runs to try to keep things on track? ***** MOOT: for the moment at least, I'm not trying to do write at synchronous speed. requires too much unholy black magic to get it to work sort of right. update: dummy files were an ill-conceived feature and have been removed entirely warn user about frequency settings that arn't exactly what they asked for. i.e. user requests sampling rate of 16000, SB 128 returns 16036, if verbose warnings are enabled. ***** DONE: this is now handled by test_pcm_params() from process_command_line() Makefile could perhaps use PROG1 and PROG2, right now there is not a lot of point in using $(PROG), since build rules use ln -sf rawrec rawplay and clean uses 'rm -f rawrec rawplay *.o' anyway. ***** DONE: There are now vars PROG and ALT_INVOC for rawrec and rawplay and they are used consistently debug target in makefile is ugly, unconventional, and perhaps incorrect. Look for better way. you can't lseek() in pipes, so some options which use that facility will have to be rewritten if standard io is being used in place of a file name argument? ***** DONE: (except for -j and -J options, for which I'm not sure it makes sense) if you use lseek() to record silence, you better make sure 0 is silence. ***** CONFIRMED: zero is silence, under both signed and unsigned conventions no need to use SCHED_RR for disk io thread, use FIFO there also. ***** DONE all the root effective user id crap needs to be sorted out. ***** DONE: currently, seteuid and getuid are used to twiddle root permissions make sure that program always reads and writes full samples whenever possible, i.e. 16 bit stereo has each sample four bytes long, you should always read or write 4 bytes at a time or you will lose sync with the audio device. ***** DONE make sure that the user doesn't get the chance to set the fragment size to something smaller than the sample size (as per above) not very likely perhaps but mayby good to keep in mind. ***** RESOLVED: in fact, it's not possible given current maximum sample size (which is 16 bits * 2 channels = 4 bytes) < min frag size = 16 bytes use POSIX (now defunct 1003.1e) capabilities CAP_IPC_LOCK for memory locking and CAP_SYS_NICE for thread prioritization instead of or in addition to standard suid twiddling ***** MOOT: I no longer believe this is worthwhile look into security issues of using strcpy, strtol, strtod, anywhere in program ***** DONE: strcpy is the bad one, and all uses of it have been replaced with carefully checked uses of strncpy. strtol and strtod look ok, but they arn't used at the moment. clean up test_pcm_params and possibly add support for more return codes ***** SEMI-DONE: return codes are improved possibly: support for more formats? whole new interface that does what test_pcm_params currently does more more cleanly ***** MOOT: test_psm_params isn't that bad add an err_die function with variable number of args to take care of all the fprintf, perror, exit() code blocks practice of using constant slop for acceptable speed settings (currently, within 300 samples per second, 6-13-2000) means relatively large percent error at low speeds (my card appears capable of 4 kHz minimum rate). Elegant solution exist? did the order in which the prototypes are given in rawrec.h once mean something? should the order mean something? ***** RESOLVED: currently, listing order corresponds to call order in execution. This is funky and I will probably go to alphabetic order review uses of int vs. long set a gdb watch on ERRNO to see when it's getting set to EINTR ***** DONE: it's actually happening a couple levels of call inside a pthread function. Linux pthreads maintainer has been notified. should we be nice and not ignore SIGPWR, so that in the event of a system needing to shut down quickly due to low emergency power, we don't hog resources with all our high priority threads? It's Steven's book that talks about SIGPWR, which is kind of old, is it even an issue still? actualy use the error codes returned by the pthread functions to generate better error reporting ***** SEMI-DONE: done everywhere except in thread fctns move_au and move_fd consider being greedy and holding the audio device longer ***** DONE: if -h is specified, we now hold the device for basicly the whole execution, include pause time make should rebuild all files if an ordinary build is requested and the last build was a debug build, not just the ones that have changed. debug build currently works correctly, but it's because the debug build dependency is brute-forcish improve the maintain timing option so that it at least makes some kind of intelligent guess to account for the time required to actually copy the data to or from the dummy file ***** MOOT: dummy files no longer exist fix this (bad use of -d should not end up generate a BUG-tagged error): ./rawrec -b 8 -c 1 -t 0 -T 10 -d testfile.raw cloned_file.raw rawrec: BUG: SOUND_PCM_SYNC ioctl failed: Inappropriate ioctl for device ***** FIXED: added what sanity checks I could to process_command_line (above case is now caught there) and changed status of some diagnostics from BUG to normal pthread syscalls in move_au and move_fd could probably stand to check their return values, though I really hate to do it to them, both because of the clutter it adds to concurrent code which is already pretty confusing to read, and also because all the branches may induce some performance penalty, however small. locking down the copy buffer for use with target dummy files might give a slightly better performance gaurantee, but is it worth it? not while copy involved such inefficiency for short copies as it currently does. ***** MOOT: dummy files are out file record locking will have to be used to gaurantee that there are no bugs possible if users do insane things to datafiles while rawrec is trying to run. If done stupidly, this might actually hurt some legitimate uses, so care is needed. version 1.2 feature. the thread arguments are still using a 0/1 flag called recorder to distinguish between record and play runs. replace with something better. use strtol, strtod with error checking instead of relatively quick and dirty atoi, atol by default, should rawrec with no options (in particular, no -t or -T) record until a terminating signal is recieved? ***** DONE: Yes, and it now does. at the moment, jumping (with -j or -J) into an argument file is only allowed if the file is an ordinary file, jumping into standard input is not allowed. Should this be changed? ***** DONE: yes, you can now jump into pipes using dummy reads. currently the argument file is only checked in process_command_line if we are playing, it would be nice to check when recording to be sure that if the argument file exists it is not a directory, but unless we want to rely on error return from stat() we first need a graceful way to check if the argument file even exists yet currently the dummy file is only checked in process_command_line if we are recording from it, it would be nice to check when playing to be sure that if the dummy file exists it is not a directory, but unless we want to rely on error return from stat() we first need a graceful way to check if the argument file even exists yet ***** MOOT: dummy files are gone SSIZE_MAX should be used to determine the maximum allowable number of bytes for read() the bytes_read currently used to check for standard input starvation in move_fd could be made into an array of shared data between the threads, one element per buffer segment, so that move_au could write only as many bytes as were read, and some starvation could be tolerated. The big question is whether it's worth the hassle. ***** DONE: it was decided that this was worth the hassle. we don't starve now until a certain magic number of empty segments in a row are seen. okaaaay, so SOUND_PCM ioctls are obsolete and SNDCTL is now preferred (not the other way round). So they all should be changed. ***** DONE currently, when thread_functions.h needs to be included, rawrec.h needs to be included before it in order for things to work. This is horribly ugly and must be fixed (the header files need to be reworked). from 'if ( (clp->using_stdio == FALSE) && (clp->recorder == FALSE) ) {' onward in process_command_line is a relatively hideous mess in bad need of a cleanup. To do this right will probably require fixing lower layers which don't check app->limit_set as they should (because it didn't exist when they were first written). even though we are using extremely generic formats, we need to check that the device sets them correctly ***** DONE: with some changes in the options to allow specification of sample formats, if the requested format isn't supported, this is currently fatal, hopefully things are set up so that it won't be that difficult to implement on-the-fly translation between the basic format supported by the hardware and the format requested by the user use pthread_attr_setstacksize to gaurantee memory space for the threads before locking down the process memory space? does this protect automatic and dynamic allocation in thread functions from swapping, provided a large enough stacksize is allocated? Note that this is likely not worthwhile, but could be more strictly correct. write_silence_on_option uses an awkward method to do it's job. In particular, the memcpys should be fewer and move larger chunks, but mayby there is a more elegant solution to the whole thing anyway? check return values of all malloc and free to be sure they havn't returned zero (or some other error value) ***** DONE add --version flag as per FSF programming guidelines ***** DONE in the short time that I rationalized process_command_line being ugly, it has gotten really really ugly and is already in bad need of a cleanup. In particular it is horribly nasty to set all the limits to appropriate values and then not set the samp_limit_set boolean flags in the actual parameters structure (ap structure) and whatnot appropriately. Of course, setting them is not beautiful either, so mayby someday the whole shebang should be redone.