Re: [pulseaudio-discuss] [PATCH 0/8] more Coverity fixes and cleanups
On 08.03.2017 16:09, Peter Meerwald-Stadler wrote: some more Coverity fixes and cleanups also addresses Hajime Fujita's earlier review comments Peter Meerwald-Stadler (8): raop: Fix potential dereference after NULL check raop: Fix potential NULL dereference raop: Log if pa_atoi() fails, latency is not used anyway raop: Error out on parsing server port component core: Assert return value of pa_shared_set/_remove() in dbus-shared core: Document behaviour of pa_shared_remove() in case name does not exist oss: Fix dead code pulse: Explicitly ignore pa_mainloop_run() return value in thread function src/modules/oss/module-oss.c | 2 +- src/modules/raop/raop-client.c | 50 +- src/modules/raop/raop-sink.c | 10 + src/pulse/thread-mainloop.c| 2 +- src/pulsecore/dbus-shared.c| 4 ++-- src/pulsecore/shared.c | 2 +- src/pulsecore/shared.h | 4 +++- 7 files changed, 44 insertions(+), 30 deletions(-) Look good to me, except where noted. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 4/8] raop: Error out on parsing server port component
On 08.03.2017 16:09, Peter Meerwald-Stadler wrote:
don't ignore server port parsing errors as suggested by Hajime Fujita
Signed-off-by: Peter Meerwald-Stadler
Cc: Hajime Fujita
---
src/modules/raop/raop-client.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
index 78f86a4..ae950f7 100644
--- a/src/modules/raop/raop-client.c
+++ b/src/modules/raop/raop-client.c
@@ -988,7 +988,7 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_
pa_socket_client *sc = NULL;
uint32_t sport = DEFAULT_UDP_AUDIO_PORT;
uint32_t cport =0, tport = 0;
-char *ajs, *token, *pc;
+char *ajs, *token, *pc, *trs;
const char *token_state = NULL;
char delimiters[] = ";";
@@ -1031,17 +1031,21 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp, pa_rtsp_state_t state, pa_rtsp_
pa_socket_client_unref(sc);
sc = NULL;
} else if (c->protocol == PA_RAOP_PROTOCOL_UDP) {
-char *trs = pa_xstrdup(pa_headerlist_gets(headers,
"Transport"));
+trs = pa_xstrdup(pa_headerlist_gets(headers, "Transport"));
if (trs) {
/* Now parse out the server port component of the
response. */
while ((token = pa_split(trs, delimiters, &token_state)))
{
if ((pc = strstr(token, "="))) {
*pc = 0;
-if (pa_streq(token, "control_port"))
-(void) pa_atou(pc + 1, &cport);
-if (pa_streq(token, "timing_port"))
-(void) pa_atou(pc + 1, &tport);
+if (pa_streq(token, "control_port")) {
+if (pa_atou(pc + 1, &cport) < 0)
+goto setup_error_parse;
+}
+if (pa_streq(token, "timing_port")) {
+if (pa_atou(pc + 1, &tport) < 0)
+goto setup_error_parse;
+}
*pc = '=';
}
pa_xfree(token);
@@ -1072,6 +1076,11 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_
pa_xfree(ajs);
You need to free trs here as well.
break;
+setup_error_parse:
+pa_log("Failed parsing server port components");
+pa_xfree(token);
+pa_xfree(trs);
+/* fall-thru */
setup_error:
if (c->tcp_sfd >= 0)
pa_close(c->tcp_sfd);
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] combine-sink: Use the correct percent symbol in comment/log
On Thu, 9 Mar 2017, at 11:41 AM, Georg Chini wrote:
> On 09.03.2017 05:36, Arun Raghavan wrote:
> > Not sure how this weird not-really-a-percent-symbol turned up.
> > ---
> > src/modules/module-combine-sink.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/modules/module-combine-sink.c
> > b/src/modules/module-combine-sink.c
> > index 71d1fba..1aad234 100644
> > --- a/src/modules/module-combine-sink.c
> > +++ b/src/modules/module-combine-sink.c
> > @@ -252,9 +252,9 @@ static void adjust_rates(struct userdata *u) {
> > } else {
> > if (base_rate < new_rate + 20 && new_rate < base_rate + 20)
> > new_rate = base_rate;
> > -/* Do the adjustment in small steps; 2‰ can be considered
> > inaudible */
> > +/* Do the adjustment in small steps; 2% can be considered
> > inaudible */
> > if (new_rate < (uint32_t) (current_rate*0.998) || new_rate >
> > (uint32_t) (current_rate*1.002)) {
> > -pa_log_info("[%s] new rate of %u Hz not within 2‰ of %u
> > Hz, forcing smaller adjustment", o->sink_input->sink->name, new_rate,
> > current_rate);
> > +pa_log_info("[%s] new rate of %u Hz not within 2% of %u
> > Hz, forcing smaller adjustment", o->sink_input->sink->name, new_rate,
> > current_rate);
> > new_rate = PA_CLAMP(new_rate, (uint32_t)
> > (current_rate*0.998), (uint32_t) (current_rate*1.002));
> > }
> > pa_log_info("[%s] new rate is %u Hz; ratio is %0.3f; latency
> > is %0.2f msec.", o->sink_input->sink->name, new_rate, (double) new_rate /
> > base_rate, (double) o->total_latency / PA_USEC_PER_MSEC);
>
> This was correct, it is really per mill and not percent, see the factors
> in the estimation of new_rate and it is the correct symbol for per mill.
Ah, I see now.
-- Arun
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 6/8] core: Document behaviour of pa_shared_remove() in case name does not exist
On 08.03.2017 16:09, Peter Meerwald-Stadler wrote:
ignore pa_shared_remove() return value
Coverity ID: #1380672
I think the commit title and commit message should be swapped,
because the real reason for the patch is to silence a warning about
the unused return value.
Signed-off-by: Peter Meerwald-Stadler
---
src/pulsecore/shared.c | 2 +-
src/pulsecore/shared.h | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/pulsecore/shared.c b/src/pulsecore/shared.c
index 1b5eea9..9bc7eb5 100644
--- a/src/pulsecore/shared.c
+++ b/src/pulsecore/shared.c
@@ -111,6 +111,6 @@ int pa_shared_replace(pa_core *c, const char *name, void
*data) {
pa_assert(c);
pa_assert(name);
-pa_shared_remove(c, name);
+(void) pa_shared_remove(c, name);
return pa_shared_set(c, name, data);
}
diff --git a/src/pulsecore/shared.h b/src/pulsecore/shared.h
index 49e8738..19ac462 100644
--- a/src/pulsecore/shared.h
+++ b/src/pulsecore/shared.h
@@ -44,7 +44,9 @@ int pa_shared_set(pa_core *c, const char *name, void *data);
/* Remove the specified shared property. Return non-zero on failure */
int pa_shared_remove(pa_core *c, const char *name);
-/* A combination of pa_shared_remove() and pa_shared_set() */
+/* A combination of pa_shared_remove() and pa_shared_set(); this function
+ * first tries to remove the property by this name and then sets the
+ * property. Return non-zero on failure. */
int pa_shared_replace(pa_core *c, const char *name, void *data);
/* Dump the current set of shared properties */
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 1/2] combine-sink: Use PA_MAX instead of ternary operator for clarity
On 09.03.2017 05:36, Arun Raghavan wrote:
---
src/modules/module-combine-sink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/modules/module-combine-sink.c
b/src/modules/module-combine-sink.c
index 250240a..71d1fba 100644
--- a/src/modules/module-combine-sink.c
+++ b/src/modules/module-combine-sink.c
@@ -227,7 +227,7 @@ static void adjust_rates(struct userdata *u) {
avg_total_latency /= n;
-target_latency = max_sink_latency > min_total_latency ? max_sink_latency : min_total_latency;
+target_latency = PA_MAX(max_sink_latency, min_total_latency);
pa_log_info("[%s] avg total latency is %0.2f msec.", u->sink->name, (double) avg_total_latency / PA_USEC_PER_MSEC);
pa_log_info("[%s] target latency is %0.2f msec.", u->sink->name, (double)
target_latency / PA_USEC_PER_MSEC);
Looks good.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] combine-sink: Use the correct percent symbol in comment/log
On 09.03.2017 05:36, Arun Raghavan wrote:
Not sure how this weird not-really-a-percent-symbol turned up.
---
src/modules/module-combine-sink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/modules/module-combine-sink.c
b/src/modules/module-combine-sink.c
index 71d1fba..1aad234 100644
--- a/src/modules/module-combine-sink.c
+++ b/src/modules/module-combine-sink.c
@@ -252,9 +252,9 @@ static void adjust_rates(struct userdata *u) {
} else {
if (base_rate < new_rate + 20 && new_rate < base_rate + 20)
new_rate = base_rate;
-/* Do the adjustment in small steps; 2‰ can be considered
inaudible */
+/* Do the adjustment in small steps; 2% can be considered
inaudible */
if (new_rate < (uint32_t) (current_rate*0.998) || new_rate >
(uint32_t) (current_rate*1.002)) {
-pa_log_info("[%s] new rate of %u Hz not within 2‰ of %u Hz, forcing smaller
adjustment", o->sink_input->sink->name, new_rate, current_rate);
+pa_log_info("[%s] new rate of %u Hz not within 2% of %u Hz, forcing smaller
adjustment", o->sink_input->sink->name, new_rate, current_rate);
new_rate = PA_CLAMP(new_rate, (uint32_t)
(current_rate*0.998), (uint32_t) (current_rate*1.002));
}
pa_log_info("[%s] new rate is %u Hz; ratio is %0.3f; latency is %0.2f msec.",
o->sink_input->sink->name, new_rate, (double) new_rate / base_rate, (double)
o->total_latency / PA_USEC_PER_MSEC);
This was correct, it is really per mill and not percent, see the factors
in the estimation of new_rate and it is the correct symbol for per mill.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] echo-cancel: Limit the maximum sink/source latency
On Thu, 9 Mar 2017, at 11:23 AM, Arun Raghavan wrote: > On systems with constrained CPUs, we might run into a situation where > the master source/sink is configured to have too high a latency. Forgot to note that I'm seeing this on a Raspberry Pi 3 and large ALSA buffers (so ~64k chunk being pushed from the source at one shot). -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] echo-cancel: Limit the maximum sink/source latency
On systems with constrained CPUs, we might run into a situation where
the master source/sink is configured to have too high a latency.
On the source side, this would cause us to wake up with a large chunk of
data to process, which might cause us to exhust our RT limit and thus be
killed.
So it makes sense to limit the overall latency that we request from the
source (and correspondingly, the sink, so we don't starve for playback
data on the source side).
The 10 blocks maximum is somewhat arbitrary (I'm assuming the system has
enough headroom to process 10 chunks through the canceller without
getting close to the RT limit). This might make sense to make tunable in
the future.
---
src/modules/echo-cancel/module-echo-cancel.c | 32 +---
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/src/modules/echo-cancel/module-echo-cancel.c
b/src/modules/echo-cancel/module-echo-cancel.c
index ed75e0c..0a9f290 100644
--- a/src/modules/echo-cancel/module-echo-cancel.c
+++ b/src/modules/echo-cancel/module-echo-cancel.c
@@ -145,6 +145,8 @@ static const pa_echo_canceller ec_table[] = {
#define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
+#define MAX_LATENCY_BLOCKS 10
+
/* Can only be used in main context */
#define IS_ACTIVE(u) ((pa_source_get_state((u)->source) == PA_SOURCE_RUNNING)
&& \
(pa_sink_get_state((u)->sink) == PA_SINK_RUNNING))
@@ -515,6 +517,7 @@ static int sink_set_state_cb(pa_sink *s, pa_sink_state_t
state) {
/* Called from source I/O thread context */
static void source_update_requested_latency_cb(pa_source *s) {
struct userdata *u;
+pa_usec_t latency;
pa_source_assert_ref(s);
pa_assert_se(u = s->userdata);
@@ -525,15 +528,17 @@ static void source_update_requested_latency_cb(pa_source
*s) {
pa_log_debug("Source update requested latency");
-/* Just hand this one over to the master source */
-pa_source_output_set_requested_latency_within_thread(
-u->source_output,
-pa_source_get_requested_latency_within_thread(s));
+/* Cap the maximum latency so we don't have to process too large chunks */
+latency = PA_MIN(pa_source_get_requested_latency_within_thread(s),
+ pa_bytes_to_usec(u->source_blocksize, &s->sample_spec) *
MAX_LATENCY_BLOCKS);
+
+pa_source_output_set_requested_latency_within_thread(u->source_output,
latency);
}
/* Called from sink I/O thread context */
static void sink_update_requested_latency_cb(pa_sink *s) {
struct userdata *u;
+pa_usec_t latency;
pa_sink_assert_ref(s);
pa_assert_se(u = s->userdata);
@@ -544,10 +549,11 @@ static void sink_update_requested_latency_cb(pa_sink *s) {
pa_log_debug("Sink update requested latency");
-/* Just hand this one over to the master sink */
-pa_sink_input_set_requested_latency_within_thread(
-u->sink_input,
-pa_sink_get_requested_latency_within_thread(s));
+/* Cap the maximum latency so we don't have to process too large chunks */
+latency = PA_MIN(pa_sink_get_requested_latency_within_thread(s),
+ pa_bytes_to_usec(u->sink_blocksize, &s->sample_spec) *
MAX_LATENCY_BLOCKS);
+
+pa_sink_input_set_requested_latency_within_thread(u->sink_input, latency);
}
/* Called from sink I/O thread context */
@@ -1658,6 +1664,7 @@ int pa__init(pa_module*m) {
uint32_t temp;
uint32_t nframes = 0;
bool use_master_format;
+pa_usec_t blocksize_usec;
pa_assert(m);
@@ -2020,6 +2027,15 @@ int pa__init(pa_module*m) {
u->thread_info.current_volume = u->source->reference_volume;
+/* We don't want to deal with too many chunks at a time */
+blocksize_usec = pa_bytes_to_usec(u->source_blocksize,
&u->source->sample_spec);
+pa_source_set_latency_range(u->source, blocksize_usec, blocksize_usec *
MAX_LATENCY_BLOCKS);
+pa_source_output_set_requested_latency(u->source_output, blocksize_usec *
MAX_LATENCY_BLOCKS);
+
+blocksize_usec = pa_bytes_to_usec(u->sink_blocksize,
&u->sink->sample_spec);
+pa_sink_set_latency_range(u->sink, blocksize_usec, blocksize_usec *
MAX_LATENCY_BLOCKS);
+pa_sink_input_set_requested_latency(u->sink_input, blocksize_usec *
MAX_LATENCY_BLOCKS);
+
pa_sink_put(u->sink);
pa_source_put(u->source);
--
2.9.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] udev-detect: don't use readdir_r(), it's deprecated
On Thu, 13 Oct 2016, at 07:52 PM, Tanu Kaskinen wrote:
> readdir_r() was supposed to be a thread-safe version of readdir(), but
> the interface turned out to be problematic. Due to the problems and the
> fact that readdir() is safe enough on modern libc implementations, glibc
> deprecated readdir_r() in version 2.24.
>
> The man page contains more information about what's wrong with
> readdir_r(): http://man7.org/linux/man-pages/man3/readdir_r.3.html
> ---
> src/modules/module-udev-detect.c | 22 +-
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/src/modules/module-udev-detect.c
> b/src/modules/module-udev-detect.c
> index bb41a96..3d7064f 100644
> --- a/src/modules/module-udev-detect.c
> +++ b/src/modules/module-udev-detect.c
> @@ -177,10 +177,8 @@ static bool is_card_busy(const char *id) {
> char *card_path = NULL, *pcm_path = NULL, *sub_status = NULL;
> DIR *card_dir = NULL, *pcm_dir = NULL;
> FILE *status_file = NULL;
> -size_t len;
> -struct dirent *space = NULL, *de;
> +struct dirent *de;
> bool busy = false;
> -int r;
>
> pa_assert(id);
>
> @@ -194,14 +192,11 @@ static bool is_card_busy(const char *id) {
> goto fail;
> }
>
> -len = offsetof(struct dirent, d_name) + fpathconf(dirfd(card_dir),
> _PC_NAME_MAX) + 1;
> -space = pa_xmalloc(len);
> -
> for (;;) {
> -de = NULL;
> -
> -if ((r = readdir_r(card_dir, space, &de)) != 0) {
> -pa_log_warn("readdir_r() failed: %s", pa_cstrerror(r));
> +errno = 0;
> +de = readdir(card_dir);
> +if (!de && errno) {
> +pa_log_warn("readdir() failed: %s", pa_cstrerror(errno));
> goto fail;
> }
>
> @@ -228,8 +223,10 @@ static bool is_card_busy(const char *id) {
> for (;;) {
> char line[32];
>
> -if ((r = readdir_r(pcm_dir, space, &de)) != 0) {
> -pa_log_warn("readdir_r() failed: %s", pa_cstrerror(r));
> +errno = 0;
> +de = readdir(pcm_dir);
> +if (!de && errno) {
> +pa_log_warn("readdir() failed: %s",
> pa_cstrerror(errno));
> goto fail;
> }
>
> @@ -267,7 +264,6 @@ fail:
> pa_xfree(card_path);
> pa_xfree(pcm_path);
> pa_xfree(sub_status);
> -pa_xfree(space);
>
> if (card_dir)
> closedir(card_dir);
> --
LGTM.
-- Arun
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] alsa: Avoid creating tiny memchunks on write iterations
If the ALSA device supports granular pointer reporting, we end up in a
situation where we write out a bunch of data, iterate, and then find a
small amount of data available in the buffer (consumed while we were
writing data into the available buffer space). We do this 10 times
before quitting the write loop.
This is inefficient in itself, but can also have wider consequences. For
example, with module-combine-sink, this will end up pushing the same
small chunks to all other devices too.
Given both of these, it just makes sense to not try to write out data
unless a minimum threshold is available. This could potentially be a
fragment, but it's likely most robust to just work with a fraction of
the total available buffer size.
---
src/modules/alsa/alsa-sink.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 886c735..43cd965 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -88,6 +88,8 @@
#define DEFAULT_REWIND_SAFEGUARD_BYTES (256U) /* 1.33ms @48kHz, we'll never
rewind less than this */
#define DEFAULT_REWIND_SAFEGUARD_USEC (1330) /* 1.33ms, depending on
channels/rate/sample we may rewind more than 256 above */
+#define DEFAULT_WRITE_ITERATION_THRESHOLD 0.03 /* don't iterate write if < 3%
of the buffer is available */
+
struct userdata {
pa_core *core;
pa_module *module;
@@ -580,12 +582,19 @@ static int mmap_write(struct userdata *u, pa_usec_t
*sleep_usec, bool polled, bo
break;
}
-if (++j > 10) {
+j++;
+
+if (j > 10) {
#ifdef DEBUG_TIMING
pa_log_debug("Not filling up, because already too many
iterations.");
#endif
break;
+} else if (j >= 1 && (n_bytes < (DEFAULT_WRITE_ITERATION_THRESHOLD *
(u->hwbuf_size - u->hwbuf_unused {
+#ifdef DEBUG_TIMING
+pa_log_debug("Not filling up, because <%g%% available.",
DEFAULT_WRITE_ITERATION_THRESHOLD * 100);
+#endif
+break;
}
n_bytes -= u->hwbuf_unused;
@@ -754,12 +763,19 @@ static int unix_write(struct userdata *u, pa_usec_t
*sleep_usec, bool polled, bo
break;
}
-if (++j > 10) {
+j++;
+
+if (j > 10) {
#ifdef DEBUG_TIMING
pa_log_debug("Not filling up, because already too many
iterations.");
#endif
break;
+} else if (j >= 1 && (n_bytes < (DEFAULT_WRITE_ITERATION_THRESHOLD *
(u->hwbuf_size - u->hwbuf_unused {
+#ifdef DEBUG_TIMING
+pa_log_debug("Not filling up, because <%g%% available.",
DEFAULT_WRITE_ITERATION_THRESHOLD * 100);
+#endif
+break;
}
n_bytes -= u->hwbuf_unused;
--
2.9.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] echo-cancel: Try to minimise in-flight chunks in snapshot latency
We don't always know whether the in-flight memory chunks will be
rendered or skipped (if the source is not in RUNNING). This can cause us
to have an erroneous estimate of drift, particularly when the canceller
starts.
To avoid this, we explicitly flush out the send and receive sides of the
message queue of audio chunks going from the sink to the source before
trying to perform a resync.
---
src/modules/echo-cancel/module-echo-cancel.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/modules/echo-cancel/module-echo-cancel.c
b/src/modules/echo-cancel/module-echo-cancel.c
index dfd05b6..ed75e0c 100644
--- a/src/modules/echo-cancel/module-echo-cancel.c
+++ b/src/modules/echo-cancel/module-echo-cancel.c
@@ -683,8 +683,13 @@ static void do_resync(struct userdata *u) {
pa_log("Doing resync");
/* update our snapshot */
-source_output_snapshot_within_thread(u, &latency_snapshot);
+/* 1. Get sink input latency snapshot, might cause buffers to be sent to
source thread */
pa_asyncmsgq_send(u->sink_input->sink->asyncmsgq,
PA_MSGOBJECT(u->sink_input), SINK_INPUT_MESSAGE_LATENCY_SNAPSHOT,
&latency_snapshot, 0, NULL);
+/* 2. Pick up any in-flight buffers (and discard if needed) */
+while (pa_asyncmsgq_process_one(u->asyncmsgq))
+;
+/* 3. Now get the source output latency snapshot */
+source_output_snapshot_within_thread(u, &latency_snapshot);
/* calculate drift between capture and playback */
diff_time = calc_diff(u, &latency_snapshot);
--
2.9.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Trivial combine-sink fixes
Hello, Here's a couple of trivial fix-ups that should be no-brainers. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 2/2] combine-sink: Use the correct percent symbol in comment/log
Not sure how this weird not-really-a-percent-symbol turned up.
---
src/modules/module-combine-sink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/modules/module-combine-sink.c
b/src/modules/module-combine-sink.c
index 71d1fba..1aad234 100644
--- a/src/modules/module-combine-sink.c
+++ b/src/modules/module-combine-sink.c
@@ -252,9 +252,9 @@ static void adjust_rates(struct userdata *u) {
} else {
if (base_rate < new_rate + 20 && new_rate < base_rate + 20)
new_rate = base_rate;
-/* Do the adjustment in small steps; 2‰ can be considered
inaudible */
+/* Do the adjustment in small steps; 2% can be considered
inaudible */
if (new_rate < (uint32_t) (current_rate*0.998) || new_rate >
(uint32_t) (current_rate*1.002)) {
-pa_log_info("[%s] new rate of %u Hz not within 2‰ of %u Hz,
forcing smaller adjustment", o->sink_input->sink->name, new_rate, current_rate);
+pa_log_info("[%s] new rate of %u Hz not within 2% of %u Hz,
forcing smaller adjustment", o->sink_input->sink->name, new_rate, current_rate);
new_rate = PA_CLAMP(new_rate, (uint32_t) (current_rate*0.998),
(uint32_t) (current_rate*1.002));
}
pa_log_info("[%s] new rate is %u Hz; ratio is %0.3f; latency is
%0.2f msec.", o->sink_input->sink->name, new_rate, (double) new_rate /
base_rate, (double) o->total_latency / PA_USEC_PER_MSEC);
--
2.9.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 1/2] combine-sink: Use PA_MAX instead of ternary operator for clarity
---
src/modules/module-combine-sink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/modules/module-combine-sink.c
b/src/modules/module-combine-sink.c
index 250240a..71d1fba 100644
--- a/src/modules/module-combine-sink.c
+++ b/src/modules/module-combine-sink.c
@@ -227,7 +227,7 @@ static void adjust_rates(struct userdata *u) {
avg_total_latency /= n;
-target_latency = max_sink_latency > min_total_latency ? max_sink_latency :
min_total_latency;
+target_latency = PA_MAX(max_sink_latency, min_total_latency);
pa_log_info("[%s] avg total latency is %0.2f msec.", u->sink->name,
(double) avg_total_latency / PA_USEC_PER_MSEC);
pa_log_info("[%s] target latency is %0.2f msec.", u->sink->name, (double)
target_latency / PA_USEC_PER_MSEC);
--
2.9.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/8] more Coverity fixes and cleanups
Hi Peter, Thank you for the fix; raop patches look good to me. Best, Hajime > On Mar 8, 2017, at 9:09 AM, Peter Meerwald-Stadler wrote: > > some more Coverity fixes and cleanups > also addresses Hajime Fujita's earlier review comments > > Peter Meerwald-Stadler (8): > raop: Fix potential dereference after NULL check > raop: Fix potential NULL dereference > raop: Log if pa_atoi() fails, latency is not used anyway > raop: Error out on parsing server port component > core: Assert return value of pa_shared_set/_remove() in dbus-shared > core: Document behaviour of pa_shared_remove() in case name does not >exist > oss: Fix dead code > pulse: Explicitly ignore pa_mainloop_run() return value in thread >function > > src/modules/oss/module-oss.c | 2 +- > src/modules/raop/raop-client.c | 50 +- > src/modules/raop/raop-sink.c | 10 + > src/pulse/thread-mainloop.c| 2 +- > src/pulsecore/dbus-shared.c| 4 ++-- > src/pulsecore/shared.c | 2 +- > src/pulsecore/shared.h | 4 +++- > 7 files changed, 44 insertions(+), 30 deletions(-) > > -- > 2.7.4 > > ___ > pulseaudio-discuss mailing list > pulseaudio-discuss@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 10/10] raop: Fix potential NULL dereference
> On Mar 8, 2017, at 9:11 AM, Peter Meerwald-Stadler wrote:
>
>
>>> diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
>>> index d329a09..5248691 100644
>>> --- a/src/modules/raop/raop-client.c
>>> +++ b/src/modules/raop/raop-client.c
>>> @@ -1254,13 +1254,13 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp,
>>> pa_rtsp_state_t state, pa_rtsp_st
>>>pa_xfree(token);
>>>}
>>>
>>> -if (pa_safe_streq(mth, "Basic")) {
>>> +if (pa_safe_streq(mth, "Basic") && realm) {
>>>rtrim_char(realm, '\"’);
>>
>> I would remove `rtrim_char(realm, '\"’);` from this block and keep the if
>> condition as-is, since realm is not used later.
>
> even though realm is not referenced in the code, RFC2617 requires it to be
> present; so I think we should check for it (before we assumed it is
> present)
Okay, then it makes sense.
>
>>>
>>>pa_raop_basic_response(DEFAULT_USER_NAME, c->password,
>>> &response);
>>>ath = pa_sprintf_malloc("Basic %s",
>>>response);
>>> -} else if (pa_safe_streq(mth, "Digest")) {
>>> +} else if (pa_safe_streq(mth, "Digest") && realm && nonce)
>>> {
>>
>> Why don’t we do like this:
>> +if (realm == NULL) {
>> +pa_log_error("realm not provided");
>> +goto error;
>> +} else if (nonce == NULL) {
>> +pa_log_error("nonce not provided");
>> +goto error;
>> +}
>
> ok, this adds more code; my goal was addressing the NULL dereference in
> case realm and/or nonce was missing
>
> p.
>
> --
>
> Peter Meerwald-Stadler
> Mobile: +43 664 24 44 418
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulse audio's UNIX sockets and other questions about pulse and containers
Hello, thank you for the detailed response. I'm trying to test this now,
but without success. I have given my container access to the sockets,
the pulse audio config directory in `~/.config/pulse`. I have given it
the same UID and username as my normal user. I have given it the same
`/etc/machine-id`. And all I get when I try to interact with pulse audio
in my container is:
timothy@dcd7aa1c2b23de93306d:~/Downloads$ paplay 0916.ogg
Connection failure: Connection refused
pa_context_connect() failed: Connection refused
You mention that I should set auth-anonymous, so I added that to
/etc/default.pa and restarted my computer. No luck. One thing I did
notice was that used=-1 when I type `info` in pacmd on the host machine:
index: 8
name:
argument:
used: -1
load once: no
properties:
module.author = "Lennart Poettering"
module.description = "Native protocol (UNIX sockets)"
module.version = "5.0"
I'm not really sure how much I should be investing in the auth-anonymous
approach. I'd like this to work on computers that I don't control, and
I'm not sure to what extent it is acceptable for me to be reconfiguring
pulse under other people's feet.
Regards and thanks for your help so far,
Timothy Hobbs
On 02/16/2017 12:53 PM, Tanu Kaskinen wrote:
On Fri, 2017-02-10 at 19:08 +0100, Timothy Hobbs wrote:
Hello,
I'm trying to discover the best way to share pulse audio between linux
containers. I have found a great deal of answers to this problem online.
Some of them involve sharing UNIX sockets, while others suggest
connecting via localhost using port tunneling or otherwise. I want to
use UNIX sockets and take advantage of POSIX's existing and excelent
naming, namespacing, and security mechanisms. I guess that shouldn't be
a problem, because pulse audio seems to support unix sockets. But I'm
having trouble understanding their layout or finding any documentation
on what data gets transfered via which mechanism and which APIs are
exposed via what protocol. So far I've found that there are unix sockets
in the following places:
- $HOME/.config/pulse/-runtime
This is just a link to:
- /tmp/-pulse/
But how does this differ from the one in run?
- /run/user/$UID/pulse/
Those two directories serve the exact same purpose.
/run/user/$UID/pulse is used when $XDG_RUNTIME_DIR is set, otherwise
pulseaudio creates the directory under /tmp.
I seem to recall that there is also system wide socket, not associated
with any single user of the system. However, I do not remember its location.
That socket only exists when pulseaudio runs in the system wide mode
(i.e. it doesn't exist by default). The location is
"${localstatedir}/run/pulse/native" (I think that usually gets expanded
to /var/run/pulse/native).
There is also discussion of PULSE_AUDIO_COOKIE. Is this really needed
when connecting to UNIX sockets? Are these sockets stand alone or does
pulse audio also communicate over dbus or x11 or something?
In the most common setups the cookie is not used with unix sockets. In
the per-user setup the user who runs pulseaudio is always allowed to
connect, and other users aren't expected to connect anyway. In the
system-wide setup access is controlled via group membership (users not
in the "pulse-access" group are denied access).
It's possible to use the cookie authorization with unix sockets, but I
can't think of any situation where that would be the best solution.
Pulseaudio uses dbus and x11, but not for anything really important, so
I expect that you can live without them.
For my usecase, the localhost option is a non-starter. It involves
configuring networking in the container to allow for the tunneling of
some ports which seems fiddly and like a can of worms from a security
perspective. Even if I weren't useing containers, I don't like the idea
of accessing services through localhost. Numbered ports with no human
readable names? No namespacing between users? Security from the outside
world ensured through conventions, manual checks, and hacks? Good greif!
If you have other suggestions that don't involve networking, I'd be
happy to hear them. I do hope, though, that if there isn't one already,
we'll be able to create a page as informative as this one for using pulse audio with containers.
Sharing the native protocol unix socket should work. If you want to use
a non-standard socket location, pass the "socket" option to module-
native-protocol-unix and set "default-server = /path/to/socket" in
client.conf so that clients find the socket.
I'd guess a container trying to connect to a socket from a different
container will get blocked by default. If you put the socket in a place
that only trusted users can access, then you can safely pass the auth-
anonymous option to module-native-protocol-unix.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinf
Re: [pulseaudio-discuss] [PATCH v2 10/10] raop: Fix potential NULL dereference
> > diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
> > index d329a09..5248691 100644
> > --- a/src/modules/raop/raop-client.c
> > +++ b/src/modules/raop/raop-client.c
> > @@ -1254,13 +1254,13 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp,
> > pa_rtsp_state_t state, pa_rtsp_st
> > pa_xfree(token);
> > }
> >
> > -if (pa_safe_streq(mth, "Basic")) {
> > +if (pa_safe_streq(mth, "Basic") && realm) {
> > rtrim_char(realm, '\"’);
>
> I would remove `rtrim_char(realm, '\"’);` from this block and keep the if
> condition as-is, since realm is not used later.
even though realm is not referenced in the code, RFC2617 requires it to be
present; so I think we should check for it (before we assumed it is
present)
> >
> > pa_raop_basic_response(DEFAULT_USER_NAME, c->password,
> > &response);
> > ath = pa_sprintf_malloc("Basic %s",
> > response);
> > -} else if (pa_safe_streq(mth, "Digest")) {
> > +} else if (pa_safe_streq(mth, "Digest") && realm && nonce)
> > {
>
> Why don’t we do like this:
> +if (realm == NULL) {
> +pa_log_error("realm not provided");
> +goto error;
> +} else if (nonce == NULL) {
> +pa_log_error("nonce not provided");
> +goto error;
> +}
ok, this adds more code; my goal was addressing the NULL dereference in
case realm and/or nonce was missing
p.
--
Peter Meerwald-Stadler
Mobile: +43 664 24 44 418___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 05/10] raop: Fix resource leaks
> > diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
> > index 4c3083e..03558f6 100644
> > --- a/src/modules/raop/raop-client.c
> > +++ b/src/modules/raop/raop-client.c
> > @@ -1223,7 +1223,7 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp,
> > pa_rtsp_state_t state, pa_rtsp_st
> > static bool waiting = false;
> > const char *current = NULL;
> > char space[] = " ";
> > -char *token,*ath = NULL;
> > +char *token, *ath = NULL;
> > char *publ, *wath, *mth, *val;
>
> Should we initialize mth to NULL here to prevent freeing a random address
> later?
> This could happen when `wath = pa_xstrdup(pa_headerlist_gets(headers,
> "WWW-Authenticate"));` becomes NULL.
>
> > char *realm = NULL, *nonce = NULL, *response = NULL;
> > char comma[] = ",";
> > @@ -1260,9 +1260,6 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp,
> > pa_rtsp_state_t state, pa_rtsp_st
> > pa_raop_basic_response(DEFAULT_USER_NAME, c->password,
> > &response);
> > ath = pa_sprintf_malloc("Basic %s",
> > response);
> > -
> > -pa_xfree(response);
> > -pa_xfree(realm);
> > } else if (pa_safe_streq(mth, "Digest")) {
> > rtrim_char(realm, '\"');
> > rtrim_char(nonce, '\"');
yes, this is addressed in a followup patch
thanks, regards, p.
--
Peter Meerwald-Stadler
Mobile: +43 664 24 44 418
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 4/8] raop: Error out on parsing server port component
don't ignore server port parsing errors as suggested by Hajime Fujita
Signed-off-by: Peter Meerwald-Stadler
Cc: Hajime Fujita
---
src/modules/raop/raop-client.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
index 78f86a4..ae950f7 100644
--- a/src/modules/raop/raop-client.c
+++ b/src/modules/raop/raop-client.c
@@ -988,7 +988,7 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_
pa_socket_client *sc = NULL;
uint32_t sport = DEFAULT_UDP_AUDIO_PORT;
uint32_t cport =0, tport = 0;
-char *ajs, *token, *pc;
+char *ajs, *token, *pc, *trs;
const char *token_state = NULL;
char delimiters[] = ";";
@@ -1031,17 +1031,21 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_
pa_socket_client_unref(sc);
sc = NULL;
} else if (c->protocol == PA_RAOP_PROTOCOL_UDP) {
-char *trs = pa_xstrdup(pa_headerlist_gets(headers,
"Transport"));
+trs = pa_xstrdup(pa_headerlist_gets(headers, "Transport"));
if (trs) {
/* Now parse out the server port component of the
response. */
while ((token = pa_split(trs, delimiters, &token_state))) {
if ((pc = strstr(token, "="))) {
*pc = 0;
-if (pa_streq(token, "control_port"))
-(void) pa_atou(pc + 1, &cport);
-if (pa_streq(token, "timing_port"))
-(void) pa_atou(pc + 1, &tport);
+if (pa_streq(token, "control_port")) {
+if (pa_atou(pc + 1, &cport) < 0)
+goto setup_error_parse;
+}
+if (pa_streq(token, "timing_port")) {
+if (pa_atou(pc + 1, &tport) < 0)
+goto setup_error_parse;
+}
*pc = '=';
}
pa_xfree(token);
@@ -1072,6 +1076,11 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_
pa_xfree(ajs);
break;
+setup_error_parse:
+pa_log("Failed parsing server port components");
+pa_xfree(token);
+pa_xfree(trs);
+/* fall-thru */
setup_error:
if (c->tcp_sfd >= 0)
pa_close(c->tcp_sfd);
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 6/8] core: Document behaviour of pa_shared_remove() in case name does not exist
ignore pa_shared_remove() return value
Coverity ID: #1380672
Signed-off-by: Peter Meerwald-Stadler
---
src/pulsecore/shared.c | 2 +-
src/pulsecore/shared.h | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/pulsecore/shared.c b/src/pulsecore/shared.c
index 1b5eea9..9bc7eb5 100644
--- a/src/pulsecore/shared.c
+++ b/src/pulsecore/shared.c
@@ -111,6 +111,6 @@ int pa_shared_replace(pa_core *c, const char *name, void
*data) {
pa_assert(c);
pa_assert(name);
-pa_shared_remove(c, name);
+(void) pa_shared_remove(c, name);
return pa_shared_set(c, name, data);
}
diff --git a/src/pulsecore/shared.h b/src/pulsecore/shared.h
index 49e8738..19ac462 100644
--- a/src/pulsecore/shared.h
+++ b/src/pulsecore/shared.h
@@ -44,7 +44,9 @@ int pa_shared_set(pa_core *c, const char *name, void *data);
/* Remove the specified shared property. Return non-zero on failure */
int pa_shared_remove(pa_core *c, const char *name);
-/* A combination of pa_shared_remove() and pa_shared_set() */
+/* A combination of pa_shared_remove() and pa_shared_set(); this function
+ * first tries to remove the property by this name and then sets the
+ * property. Return non-zero on failure. */
int pa_shared_replace(pa_core *c, const char *name, void *data);
/* Dump the current set of shared properties */
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 08/10] raop: Silence unchecked return value warnings
> > --- a/src/modules/raop/raop-client.c
> > +++ b/src/modules/raop/raop-client.c
> > @@ -1034,9 +1034,9 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
> > pa_rtsp_state_t state, pa_rtsp_
> > if ((pc = strstr(token, "="))) {
> > *pc = 0;
> > if (pa_streq(token, "control_port"))
> > -pa_atou(pc + 1, &cport);
> > +(void) pa_atou(pc + 1, &cport);
> > if (pa_streq(token, "timing_port"))
> > -pa_atou(pc + 1, &tport);
> > +(void) pa_atou(pc + 1, &tport);
>
> I think these should jump to `setup_error` in case of error instead of
> ignoring.
I've posted a followup patch implementing your suggestion; strictly, this
is adding functionality to the code, so maybe better to do it in two
patches anyway :)
regards, p.
--
Peter Meerwald-Stadler
Mobile: +43 664 24 44 418
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 7/8] oss: Fix dead code
mode cannot be 0
Coverity ID: #1137964
Signed-off-by: Peter Meerwald-Stadler
---
src/modules/oss/module-oss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/modules/oss/module-oss.c b/src/modules/oss/module-oss.c
index 8a5a692..5ad765d 100644
--- a/src/modules/oss/module-oss.c
+++ b/src/modules/oss/module-oss.c
@@ -1195,7 +1195,7 @@ int pa__init(pa_module*m) {
goto fail;
}
-mode = (playback && record) ? O_RDWR : (playback ? O_WRONLY : (record ?
O_RDONLY : 0));
+mode = (playback && record) ? O_RDWR : (playback ? O_WRONLY : O_RDONLY);
ss = m->core->default_sample_spec;
map = m->core->default_channel_map;
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 5/8] core: Assert return value of pa_shared_set/_remove() in dbus-shared
it must succeed, or we are leaking memory
Coverity ID: #1380674, #1380673
Signed-off-by: Peter Meerwald-Stadler
---
src/pulsecore/dbus-shared.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/pulsecore/dbus-shared.c b/src/pulsecore/dbus-shared.c
index 935b068..3422c29 100644
--- a/src/pulsecore/dbus-shared.c
+++ b/src/pulsecore/dbus-shared.c
@@ -45,7 +45,7 @@ static pa_dbus_connection* dbus_connection_new(pa_core *c,
pa_dbus_wrap_connecti
pconn->property_name = name;
pconn->connection = conn;
-pa_shared_set(c, name, pconn);
+pa_assert_se(pa_shared_set(c, name, pconn) >= 0);
return pconn;
}
@@ -88,7 +88,7 @@ void pa_dbus_connection_unref(pa_dbus_connection *c) {
pa_dbus_wrap_connection_free(c->connection);
-pa_shared_remove(c->core, c->property_name);
+pa_assert_se(pa_shared_remove(c->core, c->property_name) >= 0);
pa_xfree(c);
}
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 2/8] raop: Fix potential NULL dereference
wath may be NULL, as suggested by Hajime Fujita
Coverity ID: #1398156
setting val = NULL is not needed
Signed-off-by: Peter Meerwald-Stadler
Cc: Hajime Fujita
---
src/modules/raop/raop-client.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
index f23cf93..e2cf04e 100644
--- a/src/modules/raop/raop-client.c
+++ b/src/modules/raop/raop-client.c
@@ -1229,7 +1229,7 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_st
const char *current = NULL;
char space[] = " ";
char *token, *ath = NULL;
-char *publ, *wath, *mth, *val;
+char *publ, *wath, *mth = NULL, *val;
char *realm = NULL, *nonce = NULL, *response = NULL;
char comma[] = ",";
@@ -1244,19 +1244,18 @@ static void rtsp_auth_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_st
goto fail;
}
-if (wath)
+if (wath) {
mth = pa_split(wath, space, ¤t);
-while ((token = pa_split(wath, comma, ¤t))) {
-val = NULL;
-if ((val = strstr(token, "="))) {
-if (NULL == realm && val > strstr(token, "realm"))
-realm = pa_xstrdup(val + 2);
-else if (NULL == nonce && val > strstr(token, "nonce"))
-nonce = pa_xstrdup(val + 2);
-val = NULL;
-}
+while ((token = pa_split(wath, comma, ¤t))) {
+if ((val = strstr(token, "="))) {
+if (NULL == realm && val > strstr(token, "realm"))
+realm = pa_xstrdup(val + 2);
+else if (NULL == nonce && val > strstr(token,
"nonce"))
+nonce = pa_xstrdup(val + 2);
+}
-pa_xfree(token);
+pa_xfree(token);
+}
}
if (pa_safe_streq(mth, "Basic") && realm) {
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 8/8] pulse: Explicitly ignore pa_mainloop_run() return value in thread function
Coverity ID: #1137975
Signed-off-by: Peter Meerwald-Stadler
---
src/pulse/thread-mainloop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/pulse/thread-mainloop.c b/src/pulse/thread-mainloop.c
index cbfc474..993b7ba 100644
--- a/src/pulse/thread-mainloop.c
+++ b/src/pulse/thread-mainloop.c
@@ -97,7 +97,7 @@ static void thread(void *userdata) {
pa_mutex_lock(m->mutex);
-pa_mainloop_run(m->real_mainloop, NULL);
+(void) pa_mainloop_run(m->real_mainloop, NULL);
pa_mutex_unlock(m->mutex);
}
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 3/8] raop: Log if pa_atoi() fails, latency is not used anyway
Coverity ID: #1398152
Signed-off-by: Peter Meerwald-Stadler
---
src/modules/raop/raop-client.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
index e2cf04e..78f86a4 100644
--- a/src/modules/raop/raop-client.c
+++ b/src/modules/raop/raop-client.c
@@ -1102,8 +1102,10 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp,
pa_rtsp_state_t state, pa_rtsp_
pa_log_debug("RAOP: RECORD");
alt = pa_xstrdup(pa_headerlist_gets(headers, "Audio-Latency"));
-if (alt)
-pa_atoi(alt, &latency);
+if (alt) {
+if (pa_atoi(alt, &latency) < 0)
+pa_log("Failed to parse audio latency");
+}
pa_raop_packet_buffer_reset(c->pbuf, c->seq);
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 0/8] more Coverity fixes and cleanups
some more Coverity fixes and cleanups also addresses Hajime Fujita's earlier review comments Peter Meerwald-Stadler (8): raop: Fix potential dereference after NULL check raop: Fix potential NULL dereference raop: Log if pa_atoi() fails, latency is not used anyway raop: Error out on parsing server port component core: Assert return value of pa_shared_set/_remove() in dbus-shared core: Document behaviour of pa_shared_remove() in case name does not exist oss: Fix dead code pulse: Explicitly ignore pa_mainloop_run() return value in thread function src/modules/oss/module-oss.c | 2 +- src/modules/raop/raop-client.c | 50 +- src/modules/raop/raop-sink.c | 10 + src/pulse/thread-mainloop.c| 2 +- src/pulsecore/dbus-shared.c| 4 ++-- src/pulsecore/shared.c | 2 +- src/pulsecore/shared.h | 4 +++- 7 files changed, 44 insertions(+), 30 deletions(-) -- 2.7.4 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 1/8] raop: Fix potential dereference after NULL check
Coverity ID: #1398157
Signed-off-by: Peter Meerwald-Stadler
---
src/modules/raop/raop-sink.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/modules/raop/raop-sink.c b/src/modules/raop/raop-sink.c
index c5ff8b9..4ca625f 100644
--- a/src/modules/raop/raop-sink.c
+++ b/src/modules/raop/raop-sink.c
@@ -246,10 +246,12 @@ static int sink_process_msg(pa_msgobject *o, int code,
void *data, int64_t offse
if (u->rtpoll_item) {
pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item,
&nbfds);
-for (i = 0; i < nbfds; i++) {
-if (pollfd && pollfd->fd >= 0)
- pa_close(pollfd->fd);
-pollfd++;
+if (pollfd) {
+for (i = 0; i < nbfds; i++) {
+if (pollfd->fd >= 0)
+ pa_close(pollfd->fd);
+pollfd++;
+}
}
pa_rtpoll_item_free(u->rtpoll_item);
u->rtpoll_item = NULL;
--
2.7.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/3] misc cleanups
> On 07.03.2017 16:56, Peter Meerwald-Stadler wrote: > > Peter Meerwald-Stadler (3): > >build: Use #ifdef to check for #defines > >core-util: Fix description of pa_split() > >raop: Fix check for invalid file descriptor > > > > src/modules/dbus/module-dbus-protocol.c | 4 ++-- > > src/modules/raop/raop-client.c | 30 > +++--- > > src/modules/raop/raop-sink.c| 2 +- > > src/pulsecore/core-util.c | 4 ++-- > > src/pulsecore/dbus-util.c | 4 ++-- > > src/pulsecore/shm.c | 2 +- > > src/utils/padsp.c | 6 +++--- > > 7 files changed, 26 insertions(+), 26 deletions(-) > > > All three look good to me. applied, thanks! -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/3] misc cleanups
On 07.03.2017 16:56, Peter Meerwald-Stadler wrote: Peter Meerwald-Stadler (3): build: Use #ifdef to check for #defines core-util: Fix description of pa_split() raop: Fix check for invalid file descriptor src/modules/dbus/module-dbus-protocol.c | 4 ++-- src/modules/raop/raop-client.c | 30 +++--- src/modules/raop/raop-sink.c| 2 +- src/pulsecore/core-util.c | 4 ++-- src/pulsecore/dbus-util.c | 4 ++-- src/pulsecore/shm.c | 2 +- src/utils/padsp.c | 6 +++--- 7 files changed, 26 insertions(+), 26 deletions(-) All three look good to me. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
