[pulseaudio-discuss] [PATCH 2/2] modules, pulse, core: React to failing pa_memblockq_push calls.

Jarkko Suontausta jarkko.suontausta at digia.com
Thu May 24 00:38:23 PDT 2012


This adds an assertion to pa_memblockq_push() and pa_memblockq_push_align()
calls, the return value of which was mostly ignored in the core and modules.
The calls return a negative value in case the maximum memblock queue size
would be exceeded.

The maximum queue sizes are currently set to be at least 16 megabytes, which
corresponds to about 22 seconds of 16-bit 8-channel audio at 48 kHz, or well
over a minute of stereo audio. If the queue gets full, it should be a sign
of an internal error.

In some cases, mostly when the output memblock queue gets full due to
external circumstances, asserting is not feasible. Those parts of the
code are marked with a FIXME comment instead.
---
 src/modules/echo-cancel/module-echo-cancel.c |    4 ++--
 src/modules/module-combine-sink.c            |    2 ++
 src/modules/module-equalizer-sink.c          |    4 ++--
 src/modules/module-ladspa-sink.c             |    2 +-
 src/modules/module-loopback.c                |    1 +
 src/modules/module-virtual-sink.c            |    2 +-
 src/modules/module-virtual-source.c          |    2 +-
 src/modules/module-virtual-surround-sink.c   |    2 +-
 src/pulse/context.c                          |    1 +
 src/pulsecore/protocol-esound.c              |    3 ++-
 src/pulsecore/protocol-http.c                |    2 ++
 src/pulsecore/protocol-simple.c              |    3 ++-
 src/pulsecore/sink-input.c                   |    4 ++--
 src/pulsecore/sound-file-stream.c            |    2 +-
 14 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
index 4510277..edcc526 100644
--- a/src/modules/echo-cancel/module-echo-cancel.c
+++ b/src/modules/echo-cancel/module-echo-cancel.c
@@ -890,7 +890,7 @@ static void source_output_push_cb(pa_source_output *o, const pa_memchunk *chunk)
     while (pa_asyncmsgq_process_one(u->asyncmsgq) > 0)
         ;
 
-    pa_memblockq_push_align(u->source_memblockq, chunk);
+    pa_assert_se(pa_memblockq_push_align(u->source_memblockq, chunk) >= 0);
 
     rlen = pa_memblockq_get_length(u->source_memblockq);
     plen = pa_memblockq_get_length(u->sink_memblockq);
@@ -1043,7 +1043,7 @@ static int source_output_process_msg_cb(pa_msgobject *obj, int code, void *data,
             pa_source_output_assert_io_context(u->source_output);
 
             if (u->source_output->source->thread_info.state == PA_SOURCE_RUNNING)
-                pa_memblockq_push_align(u->sink_memblockq, chunk);
+                pa_assert_se(pa_memblockq_push_align(u->sink_memblockq, chunk) >= 0);
             else
                 pa_memblockq_flush_write(u->sink_memblockq, TRUE);
 
diff --git a/src/modules/module-combine-sink.c b/src/modules/module-combine-sink.c
index dec2279..6d36968 100644
--- a/src/modules/module-combine-sink.c
+++ b/src/modules/module-combine-sink.c
@@ -384,6 +384,7 @@ static void render_memblock(struct userdata *u, struct output *o, size_t length)
         }
 
         /* And place it directly into the requesting output's queue */
+        /* FIXME: pa_memblockq_push_align()'s return value is not checked */
         pa_memblockq_push_align(o->memblockq, &chunk);
         pa_memblock_unref(chunk.memblock);
     }
@@ -571,6 +572,7 @@ static int sink_input_process_msg(pa_msgobject *obj, int code, void *data, int64
         case SINK_INPUT_MESSAGE_POST:
 
             if (PA_SINK_IS_OPENED(o->sink_input->sink->thread_info.state))
+                /* FIXME: pa_memblockq_push_align()'s return value is not checked */
                 pa_memblockq_push_align(o->memblockq, chunk);
             else
                 pa_memblockq_flush_write(o->memblockq, TRUE);
diff --git a/src/modules/module-equalizer-sink.c b/src/modules/module-equalizer-sink.c
index adaef69..82e6324 100644
--- a/src/modules/module-equalizer-sink.c
+++ b/src/modules/module-equalizer-sink.c
@@ -516,7 +516,7 @@ static void flatten_to_memblockq(struct userdata *u){
         dst = pa_memblock_acquire(tchunk.memblock);
         memcpy(dst, u->output_buffer + i, tchunk.length);
         pa_memblock_release(tchunk.memblock);
-        pa_memblockq_push(u->output_q, &tchunk);
+        pa_assert_se(pa_memblockq_push(u->output_q, &tchunk) >= 0);
         pa_memblock_unref(tchunk.memblock);
         i += tchunk.length;
     }
@@ -642,7 +642,7 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
         while (pa_memblockq_peek(u->input_q, &tchunk) < 0) {
             //pa_sink_render(u->sink, input_remaining * fs, &tchunk);
             pa_sink_render_full(u->sink, PA_MIN(input_remaining * fs, mbs), &tchunk);
-            pa_memblockq_push(u->input_q, &tchunk);
+            pa_assert_se(pa_memblockq_push(u->input_q, &tchunk) >= 0);
             pa_memblock_unref(tchunk.memblock);
         }
         pa_assert(tchunk.memblock);
diff --git a/src/modules/module-ladspa-sink.c b/src/modules/module-ladspa-sink.c
index be05715..c701465 100644
--- a/src/modules/module-ladspa-sink.c
+++ b/src/modules/module-ladspa-sink.c
@@ -232,7 +232,7 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
         pa_memchunk nchunk;
 
         pa_sink_render(u->sink, nbytes, &nchunk);
-        pa_memblockq_push(u->memblockq, &nchunk);
+        pa_assert_se(pa_memblockq_push(u->memblockq, &nchunk) >= 0);
         pa_memblock_unref(nchunk.memblock);
     }
 
diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
index 1a69445..cde88b6 100644
--- a/src/modules/module-loopback.c
+++ b/src/modules/module-loopback.c
@@ -464,6 +464,7 @@ static int sink_input_process_msg_cb(pa_msgobject *obj, int code, void *data, in
             pa_sink_input_assert_io_context(u->sink_input);
 
             if (PA_SINK_IS_OPENED(u->sink_input->sink->thread_info.state))
+                /* FIXME: pa_memblockq_push_align()'s return value is not checked */
                 pa_memblockq_push_align(u->memblockq, chunk);
             else
                 pa_memblockq_flush_write(u->memblockq, TRUE);
diff --git a/src/modules/module-virtual-sink.c b/src/modules/module-virtual-sink.c
index cf11ffa..432a479 100644
--- a/src/modules/module-virtual-sink.c
+++ b/src/modules/module-virtual-sink.c
@@ -218,7 +218,7 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
         pa_memchunk nchunk;
 
         pa_sink_render(u->sink, nbytes, &nchunk);
-        pa_memblockq_push(u->memblockq, &nchunk);
+        pa_assert_se(pa_memblockq_push(u->memblockq, &nchunk) >= 0);
         pa_memblock_unref(nchunk.memblock);
     }
 
diff --git a/src/modules/module-virtual-source.c b/src/modules/module-virtual-source.c
index bf07580..d800441 100644
--- a/src/modules/module-virtual-source.c
+++ b/src/modules/module-virtual-source.c
@@ -291,7 +291,7 @@ static void source_output_push_cb(pa_source_output *o, const pa_memchunk *chunk)
             /* make sure we get nbytes from the sink with render_full,
                otherwise we cannot mix with the uplink */
             pa_sink_render_full(u->sink, nbytes, &nchunk);
-            pa_memblockq_push(u->sink_memblockq, &nchunk);
+            pa_assert_se(pa_memblockq_push(u->sink_memblockq, &nchunk) >= 0);
             pa_memblock_unref(nchunk.memblock);
         }
         pa_assert(tchunk.length == chunk->length);
diff --git a/src/modules/module-virtual-surround-sink.c b/src/modules/module-virtual-surround-sink.c
index e13d92a..e4698f4 100644
--- a/src/modules/module-virtual-surround-sink.c
+++ b/src/modules/module-virtual-surround-sink.c
@@ -235,7 +235,7 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
         pa_memchunk nchunk;
 
         pa_sink_render(u->sink, nbytes * u->sink_fs / u->fs, &nchunk);
-        pa_memblockq_push(u->memblockq, &nchunk);
+        pa_assert_se(pa_memblockq_push(u->memblockq, &nchunk) >= 0);
         pa_memblock_unref(nchunk.memblock);
     }
 
diff --git a/src/pulse/context.c b/src/pulse/context.c
index 4618635..e5e7cec 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -354,6 +354,7 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o
 
         if (chunk->memblock) {
             pa_memblockq_seek(s->record_memblockq, offset, seek, TRUE);
+            /* FIXME: pa_memblockq_push_align()'s return value is not checked */
             pa_memblockq_push_align(s->record_memblockq, chunk);
         } else
             pa_memblockq_seek(s->record_memblockq, offset+chunk->length, seek, TRUE);
diff --git a/src/pulsecore/protocol-esound.c b/src/pulsecore/protocol-esound.c
index 00ea000..3c8b4a0 100644
--- a/src/pulsecore/protocol-esound.c
+++ b/src/pulsecore/protocol-esound.c
@@ -1304,6 +1304,7 @@ static int connection_process_msg(pa_msgobject *o, int code, void*userdata, int6
 
         case CONNECTION_MESSAGE_POST_DATA:
 /*             pa_log("got data %u", chunk->length); */
+            /* FIXME: pa_memblockq_push_align()'s return value is not checked */
             pa_memblockq_push_align(c->output_memblockq, chunk);
             do_work(c);
             break;
@@ -1333,7 +1334,7 @@ static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int
             pa_assert(chunk);
 
             /* New data from the main loop */
-            pa_memblockq_push_align(c->input_memblockq, chunk);
+            pa_assert_se(pa_memblockq_push_align(c->input_memblockq, chunk) >= 0);
 
             if (pa_memblockq_is_readable(c->input_memblockq) && c->playback.underrun) {
                 pa_log_debug("Requesting rewind due to end of underrun.");
diff --git a/src/pulsecore/protocol-http.c b/src/pulsecore/protocol-http.c
index d745634..41e7347 100644
--- a/src/pulsecore/protocol-http.c
+++ b/src/pulsecore/protocol-http.c
@@ -207,6 +207,8 @@ static int source_output_process_msg(pa_msgobject *m, int code, void *userdata,
         case SOURCE_OUTPUT_MESSAGE_POST_DATA:
             /* While this function is usually called from IO thread
              * context, this specific command is not! */
+
+            /* FIXME: pa_memblockq_push_align()'s return value is not checked */
             pa_memblockq_push_align(c->output_memblockq, chunk);
             do_work(c);
             break;
diff --git a/src/pulsecore/protocol-simple.c b/src/pulsecore/protocol-simple.c
index 8d8f5b8..29b52ff 100644
--- a/src/pulsecore/protocol-simple.c
+++ b/src/pulsecore/protocol-simple.c
@@ -290,6 +290,7 @@ static int connection_process_msg(pa_msgobject *o, int code, void*userdata, int6
 
         case CONNECTION_MESSAGE_POST_DATA:
 /*             pa_log("got data %u", chunk->length); */
+            /* FIXME: pa_memblockq_push_align()'s return value is not checked */
             pa_memblockq_push_align(c->output_memblockq, chunk);
             do_work(c);
             break;
@@ -319,7 +320,7 @@ static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int
             pa_assert(chunk);
 
             /* New data from the main loop */
-            pa_memblockq_push_align(c->input_memblockq, chunk);
+            pa_assert_se(pa_memblockq_push_align(c->input_memblockq, chunk) >= 0);
 
             if (pa_memblockq_is_readable(c->input_memblockq) && c->playback.underrun) {
                 pa_log_debug("Requesting rewind due to end of underrun.");
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index f6f93b8..6328527 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -894,7 +894,7 @@ void pa_sink_input_peek(pa_sink_input *i, size_t slength /* in sink frames */, p
                     pa_volume_memchunk(&wchunk, &i->sink->sample_spec, &i->volume_factor_sink);
                 }
 
-                pa_memblockq_push_align(i->thread_info.render_memblockq, &wchunk);
+                pa_assert_se(pa_memblockq_push_align(i->thread_info.render_memblockq, &wchunk) >= 0);
             } else {
                 pa_memchunk rchunk;
                 pa_resampler_run(i->thread_info.resampler, &wchunk, &rchunk);
@@ -910,7 +910,7 @@ void pa_sink_input_peek(pa_sink_input *i, size_t slength /* in sink frames */, p
                         pa_volume_memchunk(&rchunk, &i->sink->sample_spec, &i->volume_factor_sink);
                     }
 
-                    pa_memblockq_push_align(i->thread_info.render_memblockq, &rchunk);
+                    pa_assert_se(pa_memblockq_push_align(i->thread_info.render_memblockq, &rchunk) >= 0);
                     pa_memblock_unref(rchunk.memblock);
                 }
             }
diff --git a/src/pulsecore/sound-file-stream.c b/src/pulsecore/sound-file-stream.c
index 24d3314..fdb5f1b 100644
--- a/src/pulsecore/sound-file-stream.c
+++ b/src/pulsecore/sound-file-stream.c
@@ -186,7 +186,7 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t length, pa_memchunk *chunk
 
         tchunk.length = (size_t) n * fs;
 
-        pa_memblockq_push(u->memblockq, &tchunk);
+        pa_assert_se(pa_memblockq_push(u->memblockq, &tchunk) >= 0);
         pa_memblock_unref(tchunk.memblock);
     }
 
-- 
1.7.0.4



More information about the pulseaudio-discuss mailing list