So to recap a little on the fighting rewinds issue - the one that makes
pulseaudio crash due to spending too much time in RT prio, and with the
pulseaudio log filled with rewinds.
A month ago I provided five patches in total, two in gstreamer[1] and
three in pulseaudio.
GStreamer #1: we have the "don't start the stream until we have written
some samples" - this fix is in gstreamer HEAD, committed by Wim Taymans.
GStreamer #2: send larger packages by removing a strange limit to
gstreamer's "segment size". This fix has been posted to the
gstreamer-devel mailinglist, but not committed AFAIK. I'm attaching a
version of the patch which has proper git headers and against git head.
Both GStreamer fixes have recently been uploaded to Ubuntu Natty, the
development version of Ubuntu.
PulseAudio #1 and #2: I believe these should be applied to PulseAudio
upstream.
PulseAudio #3: A person with alias h31 helped out here, and noticed this
patch was the one causing a regression in Skype. While I haven't exactly
figured out why, I do notice that this patch could cause problems for
very short samples. So let's drop this one for the time being.
For reference I resend the three patches (one in GStreamer, two in
pulseaudio) that I would like to see applied.
--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
[1] Well, Wim Taymans has some credit for the helping out with the first
one.
>From f07d6d2f692a6e51b01dabd5b4ac4a0c8ad3e270 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.hennings...@canonical.com>
Date: Mon, 6 Dec 2010 16:25:25 +0100
Subject: [PATCH 1/3] Fighting rewinds: Seek and write data in the same message
Allow a message in the queue to perform both a seek and a post data.
For clients that do not use PA_SEEK_RELATIVE (e g gstreamer), this
cuts the message count - and sometimes even the rewinds - in half.
Signed-off-by: David Henningsson <david.hennings...@canonical.com>
---
src/pulsecore/protocol-native.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index 337869d..af44126 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -1351,6 +1351,7 @@ static void flush_write_no_account(pa_memblockq *q) {
static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int64_t offset, pa_memchunk *chunk) {
pa_sink_input *i = PA_SINK_INPUT(o);
playback_stream *s;
+ int64_t windex_seek = 0;
pa_sink_input_assert_ref(i);
s = PLAYBACK_STREAM(i->userdata);
@@ -1359,18 +1360,19 @@ static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int
switch (code) {
case SINK_INPUT_MESSAGE_SEEK: {
- int64_t windex;
- windex = pa_memblockq_get_write_index(s->memblockq);
+ windex_seek = pa_memblockq_get_write_index(s->memblockq);
/* The client side is incapable of accounting correctly
* for seeks of a type != PA_SEEK_RELATIVE. We need to be
* able to deal with that. */
pa_memblockq_seek(s->memblockq, offset, PA_PTR_TO_UINT(userdata), PA_PTR_TO_UINT(userdata) == PA_SEEK_RELATIVE);
-
- handle_seek(s, windex);
- return 0;
+ if (!chunk) {
+ handle_seek(s, windex_seek);
+ return 0;
+ }
+ /* else fall through and write some data */
}
case SINK_INPUT_MESSAGE_POST_DATA: {
@@ -1379,6 +1381,8 @@ static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int
pa_assert(chunk);
windex = pa_memblockq_get_write_index(s->memblockq);
+ if (code == SINK_INPUT_MESSAGE_SEEK)
+ windex = PA_MIN(windex, windex_seek);
/* pa_log("sink input post: %lu %lli", (unsigned long) chunk->length, (long long) windex); */
@@ -4440,9 +4444,9 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o
if (chunk->memblock) {
if (seek != PA_SEEK_RELATIVE || offset != 0)
- pa_asyncmsgq_post(ps->sink_input->sink->asyncmsgq, PA_MSGOBJECT(ps->sink_input), SINK_INPUT_MESSAGE_SEEK, PA_UINT_TO_PTR(seek), offset, NULL, NULL);
-
- pa_asyncmsgq_post(ps->sink_input->sink->asyncmsgq, PA_MSGOBJECT(ps->sink_input), SINK_INPUT_MESSAGE_POST_DATA, NULL, 0, chunk, NULL);
+ pa_asyncmsgq_post(ps->sink_input->sink->asyncmsgq, PA_MSGOBJECT(ps->sink_input), SINK_INPUT_MESSAGE_SEEK, PA_UINT_TO_PTR(seek), offset, chunk, NULL);
+ else
+ pa_asyncmsgq_post(ps->sink_input->sink->asyncmsgq, PA_MSGOBJECT(ps->sink_input), SINK_INPUT_MESSAGE_POST_DATA, NULL, 0, chunk, NULL);
} else
pa_asyncmsgq_post(ps->sink_input->sink->asyncmsgq, PA_MSGOBJECT(ps->sink_input), SINK_INPUT_MESSAGE_SEEK, PA_UINT_TO_PTR(seek), offset+chunk->length, NULL, NULL);
--
1.7.1
>From 08bd6b67ee32d6a6747545a0ebf9df4a8a54b9dc Mon Sep 17 00:00:00 2001
From: David Henningsson <david.hennings...@canonical.com>
Date: Thu, 9 Dec 2010 11:08:37 +0100
Subject: [PATCH 2/3] Fighting rewinds: Reduce calls to handle_seek
If many small blocks are in queue, handle_seek is being called
for every one of them, sometimes causing a rewind. Delay the
call until all blocks are handled, then call handle_seek only
once.
Signed-off-by: David Henningsson <david.hennings...@canonical.com>
---
src/pulsecore/protocol-native.c | 56 ++++++++++++++++++---------------------
1 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index af44126..5dab80e 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -125,6 +125,10 @@ typedef struct playback_stream {
uint32_t drain_tag;
uint32_t syncid;
+ /* Optimization to avoid too many rewinds with a lot of small blocks */
+ pa_atomic_t seek_or_post_in_queue;
+ int64_t seek_windex;
+
pa_atomic_t missing;
pa_usec_t configured_sink_latency;
pa_buffer_attr buffer_attr;
@@ -1099,6 +1103,8 @@ static playback_stream* playback_stream_new(
s->buffer_attr = *a;
s->adjust_latency = adjust_latency;
s->early_requests = early_requests;
+ pa_atomic_store(&s->seek_or_post_in_queue, 0);
+ s->seek_windex = -1;
s->sink_input->parent.process_msg = sink_input_process_msg;
s->sink_input->pop = sink_input_pop_cb;
@@ -1351,7 +1357,6 @@ static void flush_write_no_account(pa_memblockq *q) {
static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int64_t offset, pa_memchunk *chunk) {
pa_sink_input *i = PA_SINK_INPUT(o);
playback_stream *s;
- int64_t windex_seek = 0;
pa_sink_input_assert_ref(i);
s = PLAYBACK_STREAM(i->userdata);
@@ -1359,45 +1364,35 @@ static int sink_input_process_msg(pa_msgobject *o, int code, void *userdata, int
switch (code) {
- case SINK_INPUT_MESSAGE_SEEK: {
-
- windex_seek = pa_memblockq_get_write_index(s->memblockq);
-
- /* The client side is incapable of accounting correctly
- * for seeks of a type != PA_SEEK_RELATIVE. We need to be
- * able to deal with that. */
-
- pa_memblockq_seek(s->memblockq, offset, PA_PTR_TO_UINT(userdata), PA_PTR_TO_UINT(userdata) == PA_SEEK_RELATIVE);
- if (!chunk) {
- handle_seek(s, windex_seek);
- return 0;
- }
- /* else fall through and write some data */
- }
-
+ case SINK_INPUT_MESSAGE_SEEK:
case SINK_INPUT_MESSAGE_POST_DATA: {
- int64_t windex;
-
- pa_assert(chunk);
+ int64_t windex = pa_memblockq_get_write_index(s->memblockq);
- windex = pa_memblockq_get_write_index(s->memblockq);
- if (code == SINK_INPUT_MESSAGE_SEEK)
- windex = PA_MIN(windex, windex_seek);
-
-/* pa_log("sink input post: %lu %lli", (unsigned long) chunk->length, (long long) windex); */
+ if (code == SINK_INPUT_MESSAGE_SEEK) {
+ /* The client side is incapable of accounting correctly
+ * for seeks of a type != PA_SEEK_RELATIVE. We need to be
+ * able to deal with that. */
- if (pa_memblockq_push_align(s->memblockq, chunk) < 0) {
+ pa_memblockq_seek(s->memblockq, offset, PA_PTR_TO_UINT(userdata), PA_PTR_TO_UINT(userdata) == PA_SEEK_RELATIVE);
+ windex = PA_MIN(windex, pa_memblockq_get_write_index(s->memblockq));
+ }
+ if (chunk && pa_memblockq_push_align(s->memblockq, chunk) < 0) {
if (pa_log_ratelimit())
pa_log_warn("Failed to push data into queue");
pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(s), PLAYBACK_STREAM_MESSAGE_OVERFLOW, NULL, 0, NULL, NULL);
pa_memblockq_seek(s->memblockq, (int64_t) chunk->length, PA_SEEK_RELATIVE, TRUE);
}
- handle_seek(s, windex);
-
-/* pa_log("sink input post2: %lu", (unsigned long) pa_memblockq_get_length(s->memblockq)); */
-
+ /* If more data is in queue, we rewind later instead. */
+ if (s->seek_windex != -1)
+ windex = PA_MIN(windex, s->seek_windex);
+ if (pa_atomic_dec(&s->seek_or_post_in_queue) > 1)
+ s->seek_windex = windex;
+ else {
+ s->seek_windex = -1;
+ handle_seek(s, windex);
+ }
return 0;
}
@@ -4442,6 +4437,7 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o
if (playback_stream_isinstance(stream)) {
playback_stream *ps = PLAYBACK_STREAM(stream);
+ pa_atomic_inc(&ps->seek_or_post_in_queue);
if (chunk->memblock) {
if (seek != PA_SEEK_RELATIVE || offset != 0)
pa_asyncmsgq_post(ps->sink_input->sink->asyncmsgq, PA_MSGOBJECT(ps->sink_input), SINK_INPUT_MESSAGE_SEEK, PA_UINT_TO_PTR(seek), offset, chunk, NULL);
--
1.7.1
>From 51f56adfc341adb5022b96154fcc390c113d3118 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.hennings...@canonical.com>
Date: Mon, 31 Jan 2011 05:58:36 +0100
Subject: [PATCH] Pulsesink: Allow chunks up to bufsize instead of segsize
By allowing larger chunks to be sent, PulseAudio will have a
lower CPU usage. This is especially important on low-end machines,
where PulseAudio can crash if packets are coming in at a higher
rate than PulseAudio can process them.
Signed-off-by: David Henningsson <david.hennings...@canonical.com>
---
ext/pulse/pulsesink.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/ext/pulse/pulsesink.c b/ext/pulse/pulsesink.c
index 9bebfec..295d93f 100644
--- a/ext/pulse/pulsesink.c
+++ b/ext/pulse/pulsesink.c
@@ -1339,11 +1339,11 @@ gst_pulseringbuffer_commit (GstRingBuffer * buf, guint64 * sample,
towrite = out_samples * bps;
- /* Only ever write segsize bytes at once. This will
- * also limit the PA shm buffer to segsize
+ /* Only ever write bufsize bytes at once. This will
+ * also limit the PA shm buffer to bufsize
*/
- if (towrite > buf->spec.segsize)
- towrite = buf->spec.segsize;
+ if (towrite > bufsize)
+ towrite = bufsize;
if ((pbuf->m_writable < towrite) || (offset != pbuf->m_lastoffset)) {
/* if no room left or discontinuity in offset,
@@ -1392,9 +1392,9 @@ gst_pulseringbuffer_commit (GstRingBuffer * buf, guint64 * sample,
}
/* make sure we only buffer up latency-time samples */
- if (pbuf->m_writable > buf->spec.segsize) {
+ if (pbuf->m_writable > bufsize) {
/* limit buffering to latency-time value */
- pbuf->m_writable = buf->spec.segsize;
+ pbuf->m_writable = bufsize;
GST_LOG_OBJECT (psink, "Limiting buffering to %" G_GSIZE_FORMAT,
pbuf->m_writable);
@@ -1413,9 +1413,9 @@ gst_pulseringbuffer_commit (GstRingBuffer * buf, guint64 * sample,
pbuf->m_writable);
/* Just to make sure that we didn't get more than requested */
- if (pbuf->m_writable > buf->spec.segsize) {
+ if (pbuf->m_writable > bufsize) {
/* limit buffering to latency-time value */
- pbuf->m_writable = buf->spec.segsize;
+ pbuf->m_writable = bufsize;
}
}
--
1.7.1
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
