Andre Noll [Thu, 3 Mar 2011 16:39:11 +0000 (17:39 +0100)]
handle_connect(): Don't send anything to non-authorized clients.
Currently, if we don't receive a valid authentication request, we send
back an RC4-encrypted error message to the client, which is kind of
pointless since the RC4 keys are not set up at this point.
Of course we could send an unencryted error message in this case,
but in since the peer could be anything, it seems safer to just close
the connection.
Andre Noll [Thu, 3 Mar 2011 14:51:41 +0000 (15:51 +0100)]
score: Fix use of uninitialized memory on 64 bit machines.
The score of an audio file in the score table is defined as a quantity
which is sizeof(long) bytes large, i.e. 4 bytes on 32bit systems and
8 bytes on 64 bit systems. This is not a problem per se because the
score column lives only in memory, so we do not have to worry about
incompatibilities of the on-disk layout.
However, at several places in score.c we cast the pointer to the osl
object to (int *) rather than (long *). When writing to the object on
a 64 bit machine, this will only set 4 out of the 8 allocated bytes,
the other four bytes stay uninitialized. The "ls" command uses the
correct cast to (long *) and reads the full 8 bytes. This causes
valgrind to complain:
==5433== Conditional jump or move depends on uninitialised value(s)
==5433== at 0x4164F4: prepare_ls_row (aft.c:1334)
==5433== by 0x4E2F421: osl_rbtree_loop (osl.c:1457)
==5433== by 0x418935: admissible_file_loop (score.c:255)
==5433== by 0x41601A: com_ls_callback (aft.c:1363)
==5433== by 0x411FDE: command_post_select (afs.c:842)
==5433== by 0x41B67A: schedule (sched.c:76)
==5433== by 0x411ACF: afs_init (afs.c:986)
==5433== by 0x408863: main (server.c:451)
==5433==
==5433== Conditional jump or move depends on uninitialised value(s)
==5433== at 0x41650A: prepare_ls_row (aft.c:1334)
==5433== by 0x4E2F421: osl_rbtree_loop (osl.c:1457)
==5433== by 0x418935: admissible_file_loop (score.c:255)
==5433== by 0x41601A: com_ls_callback (aft.c:1363)
==5433== by 0x411FDE: command_post_select (afs.c:842)
==5433== by 0x41B67A: schedule (sched.c:76)
==5433== by 0x411ACF: afs_init (afs.c:986)
==5433== by 0x408863: main (server.c:451)
Andre Noll [Thu, 3 Mar 2011 14:09:53 +0000 (15:09 +0100)]
RC4: Fix invalid read.
Commit 7cb8fa26 (May 2010) created a target buffer for the RC4-encoded
data which is slightly larger than the input buffer because openssl
apparently wrote beyond the size it was told to write.
As it turns out, this was not enough as RC4() may also read-overflow
the input buffer. Valgrind says on Linux/x86_64:
==2423== Invalid read of size 8
==2423== at 0x5312020: RC4 (in /lib/libcrypto.so.0.9.8)
==2423== by 0x40F01D: rc4_send_bin_buffer (crypt.c:224)
==2423== by 0x40C724: com_stat (command.c:391)
==2423== by 0x40BABF: handle_connect (command.c:838)
==2423== by 0x408330: command_post_select (server.c:404)
==2423== by 0x41B5DA: schedule (sched.c:76)
==2423== by 0x4089C3: main (server.c:581)
==2423== Address 0x6cefeb8 is 232 bytes inside a block of size 235 alloc'd
==2423== at 0x4C275A2: realloc (vg_replace_malloc.c:525)
==2423== by 0x40DE74: para_realloc (string.c:40)
==2423== by 0x40E324: make_message (string.c:134)
==2423== by 0x40C5D0: com_stat (command.c:328)
==2423== by 0x40BABF: handle_connect (command.c:838)
==2423== by 0x408330: command_post_select (server.c:404)
==2423== by 0x41B5DA: schedule (sched.c:76)
==2423== by 0x4089C3: main (server.c:581)
Fix this by treating the last len % 8 bytes of the input
separately. It's ugly but it does silence valgrind and should not be
noticeably slower since we are only doing one extra copy of at most
7 bytes.
We need to round the input size up and down to a multiple of 8,
so this patch introduces generic macros in para.h for this purpose.
Andre Noll [Mon, 28 Feb 2011 13:05:31 +0000 (14:05 +0100)]
audiod: Update --no_default_filters help text.
This was quite outdated: We got the defaults right for udp and dccp
streams at least since commit 28f8405e (April 2010). Moreover, two new
audio formats (wma and spx) are supported now but are not mentioned
in the help text. Describe that the corresponding decoder is used as
the default filter rather than listing all supported audio formats.
Andre Noll [Mon, 28 Feb 2011 10:32:16 +0000 (11:32 +0100)]
Makefile: Stop building on ggo errors.
The dependency files *.d are implicitly made by -include command
during make's first pass. The "-" prefix instructs make to ignore any
non-existing files *and* all errors resulting from executing the rules
for the *.d targets . This prefix is necessary to avoid the warning
messages about non-existing dependency files, for example after a
"make clean2".
This implies that make proceeds even if a dependency file could not
be created due to a syntax error in a .ggo file. We'd like to stop
if this happens, but unfortunately gnu make does not have an option
for specifying this behaviour in an include command.
This patch works around this shortcoming by letting the "all"
target depend on the new phony target "dep" which depends on all .d
files. This way all *.d targets are considered twice: Once during the
first pass (where errors are ignored) and again due to the all->dep
dependency.
If no errors occur, "make dep" is a no-op, so this change should not
slow down the build noticeably. A slight drawback of this solution
is that, in case of an error in a ggo file, the error will be printed
twice. But this it is still better than silently ignoring the error.
Andre Noll [Fri, 25 Feb 2011 14:33:58 +0000 (15:33 +0100)]
Fix depend.sh on NetBSD.
NetBSD's sed apparently does not understand the \+ syntax to match the preceeding
pattern one or more times. OTOH, {1,} seems to work, so use this syntax instead.
This caused the build to abort silently because make gives up on the targets that
depend on files in the "cmdline" directory.
Andre Noll [Thu, 24 Feb 2011 13:11:18 +0000 (14:11 +0100)]
vorbis: Write out _all_ pending ogg pages.
ogg_stream_flush() does not necessarily write out all pending ogg
packets into a single ogg page. So we have to call this function
in a loop until it returns zero to make sure we have a complete
ogg/vorbis header.
This fixes a bug in the vorbis dummy header patch set which caused
corrupt replacement headers for ogg files whose header spanned more
than one ogg page.
Andre Noll [Sun, 20 Feb 2011 18:56:59 +0000 (19:56 +0100)]
Fix oss_init() error path.
A bug similar to the one fixed in the previous patch for the alsa
writer is also present in the oss writer: If oss_init() fails the
->private_data pointer is non-NULL, but its contents have already
been freed. glibc detects this, aborts the process and spits out
Andre Noll [Sun, 20 Feb 2011 16:01:15 +0000 (17:01 +0100)]
Fix alsa_init() error path.
If alsa_init() fails, alsa_write_post_select() removes the buffer tree
node and sets t->error to a negative value. para_audiod (or para_write)
then calls alsa->close() to deallocate any resources. At this point
wn->private_data is non-NULL while pad->handle _is_ NULL. The
subsequent call to snd_pcm_nonblock() therefore triggers the assertion
which aborts para_audiod. Fix this bug by freeing the ->private_data
pointer already in alsa_write_post_select() if alsa_init() fails. This
way, ->private_data and pad->handle are either both NULL or both
non-NULL, which meets the expectations of alsa_close().
Andre Noll [Mon, 5 Jul 2010 21:42:31 +0000 (23:42 +0200)]
Make autoconf-2.66 happy.
This removes more lines than it adds and avoids the following warning;
configure.ac:689: warning: AC_DEFINE_UNQUOTED: `
configure.ac:689: result=
configure.ac:689: for i in $status_items; do
configure.ac:689: result="$result SI_$(echo $i | tr 'a-z' 'A-Z')' is not a valid preprocessor define value
(cherry picked from commit 5842e3e7f2aa17fe16cb806b4ba955ce1f25ce53)
Andre Noll [Wed, 9 Feb 2011 17:18:17 +0000 (18:18 +0100)]
Speed up the oggdec filter and avoid wasting tons of memory.
Calls to ov_read appear to return at most 4K, no matter how much data
was requested. We allocated 64K per output chunk, so 60K of that space
is wasted. On the other hand we need large output buffers in order to
not sacrifice performance when para_filter decodes to stdout.
Fix this flaw by increasing the oggdec output buffer size from 64K to
640K, calling ov_read() until the output buffer is full or there is
nothing left to read, and then reallocating the buffer to the amount
of bytes actually read.
Assuming CD audio, the 640K output buffer size roughly matches
the decoded size of the 32K input buffer used by the stdin task,
so each input buffer corresponds roughly to one output buffer. The
patched version performs almost identically to the oggdec reference
implementation while previous versions were up to a factor of 2 slower.
Andre Noll [Mon, 17 Jan 2011 07:32:52 +0000 (08:32 +0100)]
oggdec: Replace stream start delay by minimum input queue size.
Waiting 300ms at stream start is both ugly and unnecessary if playback
does not start in the middle of the stream. This patch removes the
->stream_start member of struct private_oggdec_data and delays plaback
only if ov_read returns OV_HOLE, i.e. if streaming does not start at
the beginning of the file.
This delay is enforced by setting the minimal input queue size.
Andre Noll [Mon, 17 Jan 2011 07:24:37 +0000 (08:24 +0100)]
ogg_afh: Compute chunk time more accurately.
The chunk table is constructed so that each chunk corresponds to
frames_per_chunk samples as determined by the granule position of
the ogg page. Use this exact value for computing the duration of a
chunk rather than the avaerage num_frames / num_chunks.
Andre Noll [Tue, 18 Jan 2011 21:36:32 +0000 (22:36 +0100)]
gui: Don't exit without shutting down curses on config reload.
Currently, if a config file containing errors is being reloaded,
gui_cmdline_parser_ext() calls exit() which leaves the terminal
in an unusable state because para_gui had no chance to call
endwin() in order to reset the terminal.
Fix this flaw by instructing gengetopt to generate code that does
not exit on errors. We can still tell that the command line or the
config file contained errors by looking at the return value of the
various parsers.
Andre Noll [Thu, 10 Feb 2011 20:06:27 +0000 (21:06 +0100)]
Create three ogg pages when skipping vorbis comments.
This works around a shortcoming in older versions of libogg. It
makes get_header() create a separate ogg page for each of the
three header packets in an ogg/vorbis stream instead of combining
packets #2 and #3.
The patch also adds a couple of debug log messages which print the
size of the individual ogg packets and ogg pages.
Andre Noll [Mon, 1 Nov 2010 08:28:11 +0000 (09:28 +0100)]
vss: Use the stripped header also at stream start.
For ogg vorbis streams the stripped header is computed when a new audio file is
opened, but chunk zero, the unmodified header, is used for the first FEC group.
This not only wastes bandwidth but might cause vss to abort because the length
of the real header was not taken into account when computing the FEC
parameters.
Solve this by introducing vss_get_chunk() which returns either the stripped
header or calls afh_get_chunk() to obtain a reference to a "real" chunk.
Andre Noll [Sun, 3 Oct 2010 13:37:02 +0000 (15:37 +0200)]
Introduce get_header() for ogg/vorbis.
Define the get_header() method for the ogg vorbis audio format handler
which replaces ogg packet #2 by a dummy packet which contains no meta
data.
We call ogg_get_file_info() with afhi == NULL which only passes the
first three ogg packets to the given callback but does not compute
the chunk table. Ogg packet #1 and #3 are copied unmodified into the
header.
Andre Noll [Sun, 24 Oct 2010 17:27:55 +0000 (19:27 +0200)]
vss: Introduce extra_header_buf.
For the same reasons the extra data buffer is necessary, we also need to reserve
an extra buffer for the header in order to not read-overflow the header buffer
provided by the new get_header() function.
Andre Noll [Sun, 3 Oct 2010 11:07:42 +0000 (13:07 +0200)]
Add Infrastructure for dynamic header computation.
vorbis allows to add arbitrary many comments to an audio file. These
comments are stored in ogg packet #2, which is part of the audio file
header of ogg vorbis files. Hence ogg/vorbis headers may be arbitrary
large. Since the header is sent periodically, this wastes bandwidth
and may cause FEC parameters to become unnecessary large.
This introductory patch paves the way for replacing ogg packet #2 at
stream time by a dummy packet with does not contain any comments and
is therefore of bounded size.
In order to do so, we need audio file headers which are not necessarily
an unmodified portion of the audio file. Therefore we call back into
the audio format handler code at stream time to compute an appropriate
audio file header. Hence an optional new method ->get_header() is
added to struct audio_format_hander. If it is non-NULL, is is called
instead of using the first header_len bytes of the file.
Unfortunately, this implies that the virtual streaming system must
be aware of the audio format id in order to know which get_header()
method should be called. Thus we store the audio format id in struct
audio_file_data which is passed from the afs process to the server
process. Vss then calls afh_get_header() with the audio format id as
an additional argument.
ATM, the new functionality is not yet used as as no audio format
handler defines ->get_header() yet.
Andre Noll [Wed, 6 Oct 2010 20:21:07 +0000 (22:21 +0200)]
Kill afhi->header_offset.
It is always zero, and stored _twice_ in the osl table. Since we must
not change the layout of the on-disk afhi structure, rename these
fields to AFHI_UNUSED1_OFFSET and AFHI_UNUSED2_OFFSET, and set them
to zero in save_afhi().
Andre Noll [Mon, 6 Dec 2010 22:05:28 +0000 (23:05 +0100)]
client: Kill superfluous label "err".
The position of this label is identical to the "out" label, and "out"
is more to the point as we jump there not only in case of an error. So
jump always to "out" and remove the "err" label.
Andre Noll [Sun, 21 Nov 2010 20:48:42 +0000 (21:48 +0100)]
color: Simplify color error handling.
We exit on errors anyway, so get rid of the return value of
color_parse() and daemon_set_log_color() and abort immediately rather
than returning -1. Add the familiar "_or_die" suffix to these functions
to make it clear that no error handling is necessary in the caller.
Andre Noll [Sun, 28 Nov 2010 21:43:45 +0000 (22:43 +0100)]
fecdec: Use a fixed buffer pool size of 64K.
With the new variable sized FEC slices, it may happen that the
number of bytes per slice of the first FEC group is very small. It is
therefore no longer appropriate to base the size of the buffer pool
on this quantity. It happened to be much too small (< 1000 bytes)
for one aac file which caused the fecdec filter to abort early due
to a full buffer pool.
This patch uses a fixed buffer pool size of 64K for the fecdec filter,
which ought to be enough for everybody.
Andre Noll [Sun, 28 Nov 2010 21:32:13 +0000 (22:32 +0100)]
write_common: Don't abort if btr_exec_up() failed.
btr_exec_up() failure is unusual but possible if the upper btr node dies just in
the right moment. It happened for an aac file due to another bug in the fecdec
filter (fixed in a subsequent patch) which caused the fecdec and hence the aacdec
btr nodes to unregister themselves due to a full buffer tree pool.
So replace the assertion by a fat error message. This sets the sample rate,
channel count and the sample format to zero which makes the writer unregister
itsself. This is better than aborting.
Andre Noll [Mon, 1 Nov 2010 15:30:22 +0000 (16:30 +0100)]
vss: Avoid large FEC parameters for the DCCP transport.
Now that for DCCP streams the audio file header is sent only once at client
connect time, we can go one step further and send the header as its own FEC
group. Then all subsequent FEC groups contain data slices only, hence the
maximal required size for a FEC group reduces from
header_size + largest_chunk_size
to
max(header_size, largest_chunk_size)
This patch introduces a new helper function need_data_slices() which returns
false only at the beginning of a DCCP stream. In this case FEC group 0 consists
of the header only and an arbitrary time interval of 200ms is used for this
group.
Andre Noll [Sat, 6 Nov 2010 10:43:59 +0000 (11:43 +0100)]
osx_write: Make osx_write_open() a no-op.
Move the allocation of the private_osx_write_data struct to core_audio_init()
and adjust the the check whether core audio has been initialized accordingly.
Andre Noll [Fri, 5 Nov 2010 18:28:24 +0000 (19:28 +0100)]
file_writer: Make file_write_open() a no-op.
Move the allocation of the private_file_write_data struct to ->post_select()
and adjust the the check whether the output file has already been opened
accordingly.
If the output file has just been opened, pfwd->fd will never be set in
the write fd set of the scheduler, so we can skip this test.
Andre Noll [Fri, 5 Nov 2010 18:07:39 +0000 (19:07 +0100)]
alsa: Make alsa_open() a no-op.
Move the allocation of the private_alsa_write_data struct to
->post_select() and adjust the the check whether alsa has
been initialialized accordingly.
Andre Noll [Fri, 5 Nov 2010 07:45:20 +0000 (08:45 +0100)]
writers: Unify ->pre_select().
Always treat the easy cases "nothing to do", "error", and "not yet initialized"
first. For the alsa writer, this change fixes two minor bugs:
First, if data is available but alsa has not yet been initialized, we return
from ->pre_select() without setting a delay. This is wrong, we should init
the alsa handle ASAP in this case.
Second, on errors we wait 20ms which is both ugly and unnecessary.
Requesting a minimal delay is the right thing to do here as well.