From e46ea833904bf6bdfb4b0b618b0b67772a0c97dc Mon Sep 17 00:00:00 2001 From: Tom Yan Date: Fri, 15 Feb 2019 00:40:02 +0800 Subject: [PATCH 1/3] pulse: fix possible overflow for buffer_time option The int cast is wrong and unnecessary as internal->buffer_time is a pa_usec_t (which is uint64_t). It does not even make sense to cast it to a uint as it being multiplied by the sample rate (and then divided by 1000000 before it get fitted into battr.tlength, which is a uint32_t). Also ditch the "+7" trick for format->bits as the function would have returned already if it isn't 8/16(/24). --- src/plugins/pulse/ao_pulse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/pulse/ao_pulse.c b/src/plugins/pulse/ao_pulse.c index 2d10d57..f3e9376 100644 --- a/src/plugins/pulse/ao_pulse.c +++ b/src/plugins/pulse/ao_pulse.c @@ -258,8 +258,8 @@ int ao_plugin_open(ao_device *device, ao_sample_format *format) { /* buffering attributes */ battr.prebuf = battr.minreq = battr.fragsize = -1; - battr.tlength = (int)(internal->buffer_time * format->rate) / 1000000 * - ((format->bits+7)/8) * device->output_channels; + battr.tlength = internal->buffer_time * format->rate / 1000000 * + (format->bits / 8) * device->output_channels; battr.minreq = battr.tlength/4; battr.maxlength = battr.tlength+battr.minreq; From 684c302f09cac831318676bf03e138a6b7e379be Mon Sep 17 00:00:00 2001 From: Tom Yan Date: Fri, 15 Feb 2019 00:40:41 +0800 Subject: [PATCH 2/3] pulse: set battr.minreq and battr.maxlength to -1 We really just want to set battr.tlength (as its default is 2s). It's better to left the others for pulse to determine. --- src/plugins/pulse/ao_pulse.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/plugins/pulse/ao_pulse.c b/src/plugins/pulse/ao_pulse.c index f3e9376..9c5c008 100644 --- a/src/plugins/pulse/ao_pulse.c +++ b/src/plugins/pulse/ao_pulse.c @@ -256,12 +256,10 @@ int ao_plugin_open(ao_device *device, ao_sample_format *format) { } /* buffering attributes */ - battr.prebuf = battr.minreq = battr.fragsize = -1; + battr.prebuf = battr.minreq = battr.fragsize = battr.maxlength = -1; battr.tlength = internal->buffer_time * format->rate / 1000000 * (format->bits / 8) * device->output_channels; - battr.minreq = battr.tlength/4; - battr.maxlength = battr.tlength+battr.minreq; internal->simple = pa_simple_new(internal->server, t, PA_STREAM_PLAYBACK, internal->sink, t2, &ss, From 6872758ae0c31d93a7b256887f1738d4fb5887a7 Mon Sep 17 00:00:00 2001 From: Tom Yan Date: Fri, 15 Feb 2019 00:42:34 +0800 Subject: [PATCH 3/3] pulse: avoid unnecessary calculation --- src/plugins/pulse/ao_pulse.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plugins/pulse/ao_pulse.c b/src/plugins/pulse/ao_pulse.c index 9c5c008..6780830 100644 --- a/src/plugins/pulse/ao_pulse.c +++ b/src/plugins/pulse/ao_pulse.c @@ -41,7 +41,7 @@ #include #include -#define AO_PULSE_BUFFER_TIME 20000 +#define AO_PULSE_BUFFER_TIME 20 /* Unfortunately libao doesn't allow "const" for these structures... */ static char * ao_pulse_options[] = { @@ -73,7 +73,7 @@ typedef struct ao_pulse_internal { struct pa_simple *simple; char *server, *sink, *client_name; pa_usec_t static_delay; - pa_usec_t buffer_time; + int buffer_time; } ao_pulse_internal; /* Yes, this is very ugly, but required nonetheless... */ @@ -174,8 +174,8 @@ int ao_plugin_set_option(ao_device *device, const char *key, const char *value) } else if (!strcmp(key, "client_name")) { free(internal->client_name); internal->client_name = strdup(value); - }else if (!strcmp(key, "buffer_time")){ - internal->buffer_time = atoi(value) * 1000; + } else if (!strcmp(key, "buffer_time")) { + internal->buffer_time = atoi(value); } return 1; @@ -258,7 +258,7 @@ int ao_plugin_open(ao_device *device, ao_sample_format *format) { /* buffering attributes */ battr.prebuf = battr.minreq = battr.fragsize = battr.maxlength = -1; - battr.tlength = internal->buffer_time * format->rate / 1000000 * + battr.tlength = internal->buffer_time * format->rate / 1000 * (format->bits / 8) * device->output_channels; internal->simple = pa_simple_new(internal->server, t, PA_STREAM_PLAYBACK,