Andre Noll [Sun, 31 Mar 2013 12:32:07 +0000 (12:32 +0000)]
sched: Provide alternative post_select variant.
Code which accesses the ->error field of another task has been
a source of bugs in the past. This patch is a first step to make
->error private to sched.c.
Currently the ->post_select() methods of all tasks are supposed
to set ->error to a negative value to indicate that the task
should be terminated, i.e. t->error being negative instructs the
scheduler to not call t->pre_select() or t->post_select() any more
in subsequent iterations. An equivalent way to achieve the same
is to let ->post_select() return an error code instead. Benefits
include
* It reduces the use of ->error outside of sched.c
* It's impossible to miss setting the error code
* It simplifies code slightly
Therefore we'd like to change the prototype of the ->post_select()
methods from
void (*post_select)(struct sched *s, struct task *t);
to
int (*post_select)(struct sched *s, struct task *t);
Changes to struct sched affects many parts of the code base,
hence converting all users at once would result in a very
large patch. Therefore we temporarily introduce an additional
->new_post_select() method according to the second declaration
above. The scheduler calls the new method if it is defined and falls
back to the old variant otherwise. This approach allows to switch
over one task after another without breaking things.
This patch introduces the infrastructure just described and switches
over the http receiver. Subsequent commits will change all other tasks
in the same way. After all tasks have been switched over we can get
rid of the old ->post_select variant and rename ->new_post_select()
back to ->post_select().
Andre Noll [Sun, 31 Mar 2013 02:03:04 +0000 (02:03 +0000)]
Replace gettimeofday() by clock_gettime().
POSIX.1-2008 marks gettimeofday() as obsolete, so let's switch to
clock_gettime().
clock_gettime() operates on timespecs rather than on timevals like
gettimeofday() does. Since timevals are extensively used in all
parts of paraslash, and select() takes a timeval pointer as the
timeout parameter, it seems to be easiest to add a new wrapper,
clock_get_realtime(). It calls clock_gettime(), performs error
checking (all errors are treated fatal and abort the program), and
converts the result to a timeval.
Another difference between gettimeofday() and clock_gettime()
is that sys/time.h needs to be included for gettimeofday(), while
clock_gettime() is declared in time.h which gets included from para.h.
Hence we can remove the include statement for sys/time.h everywhere.
Programs which call clock_gettime need to be linked against librt on
glibc versions before 2.17 while BSD and newer glibc-based systems
have no such requirement. To make matters more interesting,
MacOS lacks clock_gettime() completely although this function conforms
to SUSv2 and POSIX.1-2001.
We'd like to avoid the unnecessary dependence on librt on systems that
have clock_gettime() in -lc, and we must fall back to gettimeofday()
on MacOS. Hence this commit also introduces a check in configure.ac
which determines whether clock_gettime() is available and, if it is,
whether -lrt is needed. Executables are only linked with -lrt if
configure found that this is necessary.
Andre Noll [Tue, 2 Apr 2013 17:10:08 +0000 (17:10 +0000)]
compress: Further optimize inner loop.
This kills the local adjusted_sample variable of the inner loop of
compress_post_select(). Surprisingly enough, this saves another 5%
of execution time.
Andre Noll [Tue, 2 Apr 2013 15:02:09 +0000 (15:02 +0000)]
compress: Avoid PARA_ABS and PARA_MAX in inner loop.
These macros are type-safe and evaluate their arguments only once.
However, they are also slow. For the compress filter we are dealing
with ints and unsigned ints only, so the additional checks performed
by the macros only slow things down. Getting rid of the macros reduces
the running time of
Andre Noll [Tue, 2 Apr 2013 14:46:35 +0000 (14:46 +0000)]
compress: Compile with -O3.
By default all objects are compiled with -Os. For the compress filter,
which has to deal with large amounts of uncompressed audio data,
gcc emits rather inefficient code. Switching to -O3 resulted in a 10%
performance improvement on my Lenovo laptop.
Andre Noll [Sat, 30 Mar 2013 22:43:38 +0000 (22:43 +0000)]
sched: Get rid of (pre)select shortcuts.
These shortcuts let the scheduler skip any subsequent ->pre_select()
calls as well as the actual call to select(2) if the current task's
->pre_select() requested a zero timeout.
This idea turned out to be dubios at best. The main problem is that
->post_select no longer knows whether ->pre_select() and select()
have been called. For example, if ->pre_select() monitors some file
descriptor, ->post_select() might find it not ready for I/O even when
it actually is because ->pre_select() or select() was optimized away.
Another problem are tasks whose ->post_select() method expects that
->pre_select() sets some variable in a shared data structure.
This patch removes the scheduler optimizations so that all
->pre_select() methods of all tasks are called once per scheduler
interval.
Andre Noll [Sun, 31 Mar 2013 14:58:18 +0000 (14:58 +0000)]
audiod: Avoid delay when closing slot.
Currently we might wait up to a full scheduler interval until a slot
is closed.
This commit changes the pre_select method of the status task to check
whether some slot can be closed. In this case it requests a minimal
delay from the scheduler.
The current try_to_close_slot() is split into two functions,
must_close_slot() and close_unused_slots(). The former function is
called from ->pre_select(). It only checks whether a slot must be
closed but performs no action.
The latter function, close_unused_slots() probes all slots and closes
those for which must_close_slot() returns true.
Andre Noll [Wed, 27 Mar 2013 02:51:38 +0000 (02:51 +0000)]
configure.ac: Shorten config summary.
With many filters/writers the output line becomes rather long.
Moreover, filters and writers are also used by para_play but para_play
is not mentionend in the prefix. Just skip the prefix.
Andre Noll [Thu, 4 Apr 2013 17:00:01 +0000 (17:00 +0000)]
Add documentation of public functions in check_wav.c.
The init, shutdown, pre_select and post_select functions of this
file are exported via check_wav.h, so they should be documented. This
commit adds doxygen comments for all of them.
Andre Noll [Tue, 24 Jul 2012 06:14:12 +0000 (08:14 +0200)]
Allow addblob commands to create output.
Currently all addblob commands remain silent even on fatal errors
because para_client either reads the command output, or sends its
stdin, but never both.
This patch overcomes this shortcoming. It makes para_client create
two buffer trees instead of one. The client states CL_SENT_COMMAND
and CL_RECEIVING are replaced by a single CL_EXECUTING state.
Blob data is now sent as a sideband packet. The change breaks
compatibility with earlier 0.4.x versions again, but that's OK as
this is 0.5.0 material anyway.
Andre Noll [Tue, 24 Jul 2012 06:11:24 +0000 (08:11 +0200)]
client: Remove client_recv_buffer().
After the cleanup of the previous patch, only a single caller of
this function remains. It calls client_recv_buffer() when the client
state is CL_CONNECTED, in which case the function is just a wrapper
for read_nonblock(). So we may as well call read_nonblock() directly
and get rid of client_recv_buffer().
Andre Noll [Mon, 23 Jul 2012 17:07:07 +0000 (19:07 +0200)]
Reject non-sideband connections.
This makes the client abort the connection if the server does not
support sideband. Conversely, the server closes the socket if the
client did not request a sideband connection.
This breaks compatibility with earlier versions of para_server and
para_client but allows to clean up the code considerably.
Andre Noll [Sun, 10 Mar 2013 14:53:05 +0000 (15:53 +0100)]
blob: Avoid zero sized memcpy().
Calling memcpy() with a size argument of zero is a noop on Linux,
so this should be OK. However, memcpy(3) does not say anything about
this case, so let's be conservative here and call it only if there is
anything to copy.
Andre Noll [Sun, 27 Jan 2013 18:12:57 +0000 (19:12 +0100)]
Fix mood reload.
There was some confusion about how to determine whether the dummy mode
is currently active. At some places we tested if current_mood is NULL,
at others if current_mood->name is NULL. In fact neither check works:
* if a playlist is open, current_mood is NULL (but the dummy
mood is not loaded),
* loading the dummy mood currently sets current_mood->name to
"(dummy)".
This patch makes the latter variant the official one and fixes up
the code accordingly.
This also fixes the critical log message
Jan 27 18:57:29 (5) (13487) afs_event: table moods, event 9: key not found in rbtree
which occured if the dummy mood was active while an event caused the
mood to be reloaded.
Andre Noll [Tue, 1 Jan 2013 23:20:04 +0000 (23:20 +0000)]
afs: Add assertion to afs_cb_result_handler().
If cc is NULL we always have a bug, so it's OK to dereference this
pointer unconditionally. However, if the command handler crashes
due to cc being NULL, it is not obvious what has happened, so make
this explicit.
Andre Noll [Sat, 9 Feb 2013 18:29:14 +0000 (19:29 +0100)]
signal: Restore errno on exit from signal handler.
This probably is not necessary since generic_signal_handler() calls
exit(3) if the write to the signal pipe fails. However, nobody is
going to stop write(2) from setting errno also on success, so let's
play safe and always restore its value on exit.
Andre Noll [Mon, 31 Dec 2012 00:10:50 +0000 (00:10 +0000)]
Speed up dep target.
By default "make" removes intermediate files during the build. However,
some of the files generated by gengetopt are needed again later for
a different target.
This patch declares these generated files as "precious", so they
will no longer be removed. On one system the build time went down
from 30s to 25s due to this one-liner.
Andre Noll [Sun, 3 Jun 2012 21:22:13 +0000 (23:22 +0200)]
gui: Try to link against libncursesw.
This variant of the curses library supports wide characters so we
should link against it if it is available. If it is not, we may still
fall back to libcurses instead.
Andre Noll [Sun, 3 Jun 2012 12:08:55 +0000 (14:08 +0200)]
UTF-8 support for para_gui.
This adds two public helper functions to string.c which operate on
multibyte strings if the character encoding used in the selected
locale is UTF-8.
The first new helper, strwidth(), computes the width of the UTF-8
string while skip_cells() determines the number of bytes the given
multibyte string must be advanced in order to skip the given number
of cells.
para_gui is changed to use the new functions to properly display
UTF-8 encoded data in its top and bottom windows.
Andre Noll [Sun, 3 Jun 2012 20:00:31 +0000 (22:00 +0200)]
mp3_afh: Switch to UTF-8 encoding.
The interface of libid3tag allows to get the tag contents in various
formats. Currently, we always call the latin1 variants of these
library functions.
However, as UTF-8 encoding is more widespread these days and almost
all distributions, including the two year old Ubuntu lucid, have
switched to UTF-8, UTF-8 is probably a better choice today.
This patch changes mp3_afh.c to use the UTF-8 variants instead.
Andre Noll [Sun, 3 Jun 2012 20:42:42 +0000 (22:42 +0200)]
Fix --with-curses configure option.
We only honored the provided -L<dir> for the tests in configure but
not when linking para_gui. This patch creates an output variable,
gui_ldflags, which is used in the Makefile for linking para_gui.
Andre Noll [Thu, 20 Dec 2012 12:16:17 +0000 (13:16 +0100)]
Fix 'install' target.
This broke in commit 7024574d, where the AC_SUBST() was moved above
the AC_PROG_INSTALL on which it depends. This resulted in an empty
$install_sh variable, breaking 'make install'.
Fix this bug by moving the AC_SUBST() below AC_PROG_INSTALL.
Andre Noll [Sun, 16 Dec 2012 23:09:22 +0000 (00:09 +0100)]
Merge branch 't/resample'
Was cooking for quite some time. The merge conflicted slightly,
but this was easy to fix.
However, even with the conflict resolved, the merged tree would not
compile because after the merge para_play depends on libsamplerate,
but this dependency was not encoded in configure.ac.
There was no way to fix this issue in either of the two branches
involved without rewriting history: The recently merged t/afh_receiver
branch (which introduced para_play) had no idea of libsamplerate
while t/resample has no idea of para_play.
This merge commit fixes both issues.
0eb69b resample filter: Implementation.
cad284 resample filter: Infrastructure.
37e0df check_wav: Ask parent nodes before falling back to defaults.
216399 Replace check_wav_task by write_task.
1af65c Move wav detection code to a separate file.
d5dc1c write: Make wav-related config options modular.
c25d04 para_filter: Call proper ->free_config method on shutdown.
Andre Noll [Mon, 12 Nov 2012 14:15:43 +0000 (15:15 +0100)]
Do the RC4_ALIGN dance also for sideband connections.
In commit 1199398 (March 2011) sc_send_bin_buffer() and
sc_recv_bin_buffer() received a fix for invalid reads/writes in
RC4(). The same fix is needed also for sc_crypt(), which was introduced
later in 2830b9f8 as part of the sideband API.
Without the fix, valgrind complains like this:
==14435== Invalid read of size 8
==14435== at 0x510A03A: RC4 (in /ebio/maan/arch/Linux_x86_64/lib/libcrypto.so.0.9.8)
==14435== by 0x40937E: sc_crypt (crypt.c:333)
==14435== by 0x407233: sb_received (sideband.c:249)
==14435== by 0x40770F: recv_sb (client_common.c:215)
==14435== by 0x407FF3: client_post_select (client_common.c:427)
==14435== by 0x406BE8: schedule (sched.c:70)
==14435== by 0x404AE7: execute_client_command (client.c:115)
==14435== by 0x404B77: extract_matches_from_command (client.c:133)
==14435== by 0x404C16: generic_blob_complete (client.c:163)
==14435== by 0x404E13: complete_lsblob (client.c:181)
==14435== by 0x409AB0: create_matches (interactive.c:100)
==14435== by 0x409BD0: completion_generator (interactive.c:134)
==14435== Address 0x608bfd0 is 0 bytes inside a block of size 6 alloc'd
==14435== at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==14435== by 0x405E64: para_malloc (string.c:65)
==14435== by 0x40935E: sc_crypt (crypt.c:330)
==14435== by 0x407233: sb_received (sideband.c:249)
==14435== by 0x40770F: recv_sb (client_common.c:215)
==14435== by 0x407FF3: client_post_select (client_common.c:427)
==14435== by 0x406BE8: schedule (sched.c:70)
==14435== by 0x404AE7: execute_client_command (client.c:115)
==14435== by 0x404B77: extract_matches_from_command (client.c:133)
==14435== by 0x404C16: generic_blob_complete (client.c:163)
==14435== by 0x404E13: complete_lsblob (client.c:181)
==14435== by 0x409AB0: create_matches (interactive.c:100)
Andre Noll [Mon, 12 Nov 2012 12:52:50 +0000 (13:52 +0100)]
client: Fix double free bug in interactive mode.
This can easily be reproduced: Enter "select" and type "<tab>" twice
to complete on all moods and playlists, then hit enter to trigger
the bug. On newer glibc-based systems para_client aborts due to the
double free, and valgrind outputs the following:
==10084== Invalid read of size 4
==10084== at 0x804CA24: free_argv (string.c:813)
==10084== by 0x804E86A: client_disconnect (client_common.c:46)
==10084== by 0x804BF24: client_i9e_line_handler (client.c:448)
==10084== by 0x8050140: i9e_line_handler (interactive.c:245)
==10084== by 0x428DCF8: rl_callback_read_char (callback.c:217)
==10084== by 0x8050264: i9e_post_select (interactive.c:265)
==10084== by 0x804D183: schedule (sched.c:70)
==10084== by 0x804A0C2: main (client.c:502)
==10084== Address 0x452a928 is 0 bytes inside a block of size 8 free'd
==10084== at 0x4028BDC: free (vg_replace_malloc.c:446)
==10084== by 0x804E86A: client_disconnect (client_common.c:46)
==10084== by 0x804B6B8: execute_client_command (client.c:118)
==10084== by 0x804BA5E: select_completer (client.c:387)
==10084== by 0x80503BF: create_matches (interactive.c:100)
==10084== by 0x80504D0: completion_generator (interactive.c:134)
==10084== by 0x4280B1F: rl_completion_matches (complete.c:1992)
==10084== by 0x4280BBD: gen_completion_matches (complete.c:1062)
==10084== by 0x4281539: rl_complete_internal (complete.c:1830)
==10084== by 0x42819C1: rl_complete (complete.c:401)
==10084== by 0x4278E9C: _rl_dispatch_subseq (readline.c:774)
==10084== by 0x427917E: _rl_dispatch (readline.c:724)
The bug happens since we did not set the ->features pointer of
struct client task to NULL after the feature list was freed. The fix
is trivial.
Andre Noll [Sun, 28 Oct 2012 14:10:59 +0000 (15:10 +0100)]
resample filter: Implementation.
The resample filter allows to change the sample rate of a stream on the
fly. All the magic happens within libsamplerate, so the implementation
is quite simple.
However, one tricky thing to consider is how wav headers are treated:
Although the resample filter creates only a single task, it adds
two different nodes to the buffer tree, one for the wav detector and
another one for the resample filter itself.
At startup the generic code of para_filter or para_audiod adds only
the resample filter node, which in turn inserts the wav detector as
its own parent node. This requires to insert a new internal node to
the buffer tree which is currently not supported by the buffer tree
API. It is easy to implement this feature though, so this commit adds
the missing functionality to buffer_tree.c.
Andre Noll [Mon, 8 Oct 2012 21:29:08 +0000 (23:29 +0200)]
resample filter: Infrastructure.
This adds configure command line options and tests for libsamplerate,
and provides a dummy implementation of the resample filter. A simple
m4 file for the configuration options is included as well.
Andre Noll [Sun, 28 Oct 2012 14:21:56 +0000 (15:21 +0100)]
check_wav: Ask parent nodes before falling back to defaults.
This instructs the ->execute handler of the check_wav task to ask its
parent nodes for the channel count, sample rate and sample format in
case no wav header was found and no value for the query was given at
the command line. Currently this can never succeed, since the only
parent node of the check_wav task is the stdin task, which has no
clue about these things. However, once check_wav is being used by
the resample filter, one parent might be a decoder which can tell.
This requires to add the new public btr_parent() to buffer_tree.c to
let the check_wav node obtain the node to start the search from.
Andre Noll [Sun, 28 Oct 2012 14:03:55 +0000 (15:03 +0100)]
Replace check_wav_task by write_task.
Besides the task(s) associated with the given writer(s), para_write
currently sets up two other tasks: The stdin task and the check_wav
task.
The resample filter, which is to be introduced in subsequent patches,
also needs the functionality that is provided by the check wav task.
However, as filters are not supposed to create their own tasks, there
should not be a dedicated task instance for checking the wav header.
To overcome this problem we move the task member from
check_wav.c to write.c and rename the check_wav_task struct to
check_wav_context. Hence the ->pre_select and ->post_select methods
become part of para_write that call into check_wav.c to do the work.
The patch is quite large but large parts of the changes are just
trivial cwt -> cwc conversions.
Andre Noll [Sun, 28 Oct 2012 13:01:01 +0000 (14:01 +0100)]
Move wav detection code to a separate file.
The logic to detect a wav header and delete it from an input stream
is useful also for the upcoming resample filter, so let's make this
code reusable.
The first step for doing so is to remove the relevant parts from
write.c to check_wav.c and exports those functions which are called
from write.c to check_wav.h.
There is one problem though: The wav detector code needs to know
the arguments for the --channels, --sample_rate and --sample_format
options given to para_write, but the code would not be reusable if
check_wav.c depended on the gengetop args info struct of para_write.
Hence we introduce the public structure wav_params in check_wav.h
for this information, which is independent of gengetopt. A simple
convenience helper for copying the command line arguments from an
(arbitrary) args info structure to a struct wav_params is provided
as well.
Andre Noll [Sat, 27 Oct 2012 13:36:22 +0000 (15:36 +0200)]
write: Make wav-related config options modular.
The --channels, --sample-rate and --sample-format options will also
be used by the resample filter. This moves these options to separate
files which are included from write.m4. This allows to include them
also from the config file of the resample filter.
Andre Noll [Sun, 11 Nov 2012 09:14:39 +0000 (10:14 +0100)]
para_filter: Call proper ->free_config method on shutdown.
Currently we just free the conf struct. For some filters this not
enough to deallocate all resources and therefore results in a memory
leak. This leak is not serious though, since it does not affect
para_audiod, and para_filter is about to exit anyway at this point.
Andre Noll [Sun, 25 Nov 2012 13:00:08 +0000 (14:00 +0100)]
Merge branch 't/osx_writer_improvements'
Cooking since 2012-10-30.
9eaa22 osx_write: Check return value of AudioOutputUnitStart().
831db3 osx_write: Be careful when dereferencing the private_data pointer.
97902a osx: Treat writer node as an internal buffer tree node.
1c6fa9 osx_write: Add big fat comment on callback btr node.
Andre Noll [Mon, 19 Nov 2012 21:51:50 +0000 (22:51 +0100)]
play: Fix segfault if decoder is not supported.
When paraslash was build without libmad, para_play still recognizes mp3
files (because the mp3 audio format handler does not require libmad),
but decoding fails of course.
Specifically, the check_filter_arg() call in load_file() fails, but
the error handling code misses to clear the receiver node structure,
which results in a subsequent seqfault due to a double free.
This patch calls wipe_receiver_node() instead of open coding the
cleanup. This fixes the bug and actually simplifies the code a bit.
Andre Noll [Thu, 5 Apr 2012 23:18:23 +0000 (01:18 +0200)]
para_play, implementation.
This replaces the dummy implementation added in the previous commit
by a real implementation.
para_play makes use of all new concepts that have been previously
introduced. In particular, it employs the newly added single key
mode of the i9e subsystem and uses task notifications to shut down
the writer node when the user stops playback.
Andre Noll [Mon, 25 Jun 2012 23:48:04 +0000 (01:48 +0200)]
para_play, infrastructure.
Now that the preparatory changes to the interactive subsystem
(single key mode, status bar, signal handling) and the writer nodes
(notifications) and receiver nodes (btr exec mechanism) are in place,
we can move on to the para_play executable.
This patch adds only the necessary changes to the build system and
provides a dummy implementation of para_play. The real implementation
will be provided as as a separate patch in the next commit.
Andre Noll [Mon, 9 Jul 2012 17:44:03 +0000 (19:44 +0200)]
Make writer nodes honor notifications.
Currently, nobody is notifying any writer node but the para_play
executable, which will be introduced in subsequent patches, will use
this facility to terminate the audio stream.
Andre Noll [Thu, 12 Apr 2012 04:20:42 +0000 (06:20 +0200)]
interactive: Introduce i9e_print_status_bar().
When operating in single key mode, the users of the i9e API
may write a single line, called status bar, to stderr.
This patch adds a new public function to the i9e subsystem for
this purpose. It honors the width of the terminal and shortens
the given status bar string to fit in one line if necessary.
Andre Noll [Sun, 8 Apr 2012 23:27:11 +0000 (01:27 +0200)]
interactive: Implement single key mode.
This changes the i9e subsystem to operate in single key mode while
a command is running as determined by the existence of a buffer tree
node which is attached to the stdout_btrn of i9e.
In single key mode a different readline key map is activated and
single keystrokes are passed to the application via the new key
handler function pointer of struct i9e_client_info.
Andre Noll [Wed, 27 Jun 2012 20:32:16 +0000 (22:32 +0200)]
Interactive: Introduce i9e_get_error().
Currently there is no easy way for a paraslash task to find out
whether the i9e task is still up and running. This commit adds the
new public function i9e_get_error() which simply returns the error
state of the i9e task.
Andre Noll [Tue, 7 Aug 2012 11:32:44 +0000 (13:32 +0200)]
interactive: Fix wipe_bottom_line() on MacOS.
Once readline has been initialized, writing more than 68 characters in one go
to stderr using stdio causes interesting effects on Mac OS. Specifically, the
terminal is completely messed up. To increase the anticipated weirdness level,
writing the same string in smaller chunks just works fine. This patch adds such
code to interactive.c.
Andre Noll [Mon, 9 Apr 2012 01:02:59 +0000 (03:02 +0200)]
interactive: Honor SIGWINCH.
For the upcoming single key mode of the i9e subsystem, we need to
know the width of the terminal.
This makes this information available to all functions in interactive.c
via the i9e_private structure. The new num_columns variable of this
structure is updated whenever i9e_signal_dispatch() is called with
sig_num equal to SIGWINCH.
Andre Noll [Sun, 8 Apr 2012 03:03:52 +0000 (05:03 +0200)]
interactive: Honor SIGTERM.
Currently i9e_signal_dispatch() only looks at SIGINT and ignores all
other signals. We'd like to honor SIGTERM as well because this provides
a handy way for other paraslash tasks to shut down the i9e subsystem,
even if no signal was received.