Re: [pulseaudio-discuss] How to modify (source) port priorities?
> > Hello. > > I'm using PulseAudio on a laptop, and there's a long-time annoyance I'm unable to solve with Google's help, could you help me? > > In the "Input" tab of my "Sound" GNOME system panel, the "Connector" keeps being changed from "Microphone" to "Internal Microphone" at each restart (I'd like the contrary: "Internal Microphone" is awfully noisy, and "Microphone" works fine). I can see why in pacmd-list-sources: > {analog-input-microphone-internal: Internal Microphone (priority 8900, available: unknown)} > ... versus ... > {analog-input-microphone: Microphone (priority 8700, available: no)} the available state of mic jack should be yes after the mic is plugged > > --> How can I modify the priorities so that "Microphone" gets automatically a higher priority than "Input Microphone"? > --> Should I just fix that locally with your help, or should I file a bug? > > System info: Dell XPS1635, under Ubuntu 12.10 x64 / pulseaudio 2.1. > Thanks for the help. > are your model xps1645 ? post the output of alsa-info.sh ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] How to modify (source) port priorities?
On 11/07/2012 01:49 AM, David Henningsson wrote: On 11/07/2012 04:51 AM, Ronan Jouchet wrote: In the "Input" tab of my "Sound" GNOME system panel, the "Connector" keeps being changed from "Microphone" to "Internal Microphone" at each restart (I'd like the contrary: "Internal Microphone" is awfully noisy, and "Microphone" works fine). I can see why in pacmd-list-sources: {analog-input-microphone-internal: *Internal Microphone* (*priority 8900*, available: unknown)} ... versus ... {analog-input-microphone: *Microphone* (*priority 8700*, available: no)} --> How can I modify the priorities so that "Microphone" gets automatically a higher priority than "Input Microphone"? Modify the files in /usr/share/pulseaudio/alsa-mixer/paths/analog-input-*.conf Grrreat. I found the "priority" values in {analog-input-mic.conf, analog-input-internal-mic-always.conf, analog-input-internal-mic.conf}, and switched the "87" and "89" values. Now pacmd-list-sources shows priorities that suit me: analog-input-microphone-internal: *Internal Microphone* (priority *8700* , available: *unknown* ) analog-input-microphone: *Microphone* (priority *8900* , available: *no* ) However, even though the priorities look fine, the behavior at startup is still the same: "Internal Microphone" is still preferred to "Microphone". --> Could it be due to the fact that "Microphone" is flagged "available: no"? --> If yes, how can I work around this? If no, any other idea? Any solution putting "Microphone" first will do, I don't care the least about "Internal Microphone". Thanks for the help! -- Ronan Jouchet ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] How to modify (source) port priorities?
On 11/08/2012 12:34 AM, Ronan Jouchet wrote: On 11/07/2012 01:49 AM, David Henningsson wrote: On 11/07/2012 04:51 AM, Ronan Jouchet wrote: In the "Input" tab of my "Sound" GNOME system panel, the "Connector" keeps being changed from "Microphone" to "Internal Microphone" at each restart (I'd like the contrary: "Internal Microphone" is awfully noisy, and "Microphone" works fine). I can see why in pacmd-list-sources: {analog-input-microphone-internal: *Internal Microphone* (*priority 8900*, available: unknown)} ... versus ... {analog-input-microphone: *Microphone* (*priority 8700*, available: no)} --> How can I modify the priorities so that "Microphone" gets automatically a higher priority than "Input Microphone"? Modify the files in /usr/share/pulseaudio/alsa-mixer/paths/analog-input-*.conf Grrreat. I found the "priority" values in {analog-input-mic.conf, analog-input-internal-mic-always.conf, analog-input-internal-mic.conf}, and switched the "87" and "89" values. Now pacmd-list-sources shows priorities that suit me: analog-input-microphone-internal: *Internal Microphone* (priority *8700* , available: *unknown* ) analog-input-microphone: *Microphone* (priority *8900* , available: *no* ) However, even though the priorities look fine, the behavior at startup is still the same: "Internal Microphone" is still preferred to "Microphone". --> Could it be due to the fact that "Microphone" is flagged "available: no"? This means that you have not plugged your microphone in. If it's still available: no when your mic is plugged in, it is very likely an ALSA bug. module-switch-on-port-available is the module that switches away from unavailable ports. --> If yes, how can I work around this? If no, any other idea? Any solution putting "Microphone" first will do, I don't care the least about "Internal Microphone". In the same files you just edited, you can also comment out the "Jack" sections to stop PulseAudio from picking up any detection. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Low latency (was: PulseConf report)
On 11/07/2012 09:59 PM, Pierre-Louis Bossart wrote: For those who aren't following the planet, thought I'd like you know that I've put up notes from PulseConf up at: http://arunraghavan.net/2012/11/pulseconf-2012-report/ One comment on the low-latency case for desktop gaming with a 16ms latency. I imagine this means trouble when sending data to the HDaudio driver. With the PulseAudio sink architecture, you need the sink and ring buffer to be of equal size (feature, not bug), which means you need a ring buffer size of 8ms tops (neglecting the client-server buffer), The ring buffer is still large (to be able to dynamically change to higher latency), but it is being almost empty. But maybe that is not a practical difference here. Not sure what you mean with "sink" and "ring buffer". When mixing, data goes from the sink-input / "client-server" buffer into the DMA buffer directly. I know Arun is experimenting with the latency calculations to improve low-latency scenarios, so better not go into details as they might be about to change :-) and events up to 4ms apart. Has anyone tried the changes we pushed recently at the kernel level to properly handle the ring buffer pointer and delay? I believe some of the underruns may be due to the ~1ms inaccuracy that we had before these changes. If your driver is already giving you a 25% precision error no wonder things are broken? Right now we have bigger issues, such as why nobody is responding to messages such as this one [1] :-( -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [1] https://lkml.org/lkml/2012/11/5/74 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] PulseConf report
For those who aren't following the planet, thought I'd like you know that I've put up notes from PulseConf up at: http://arunraghavan.net/2012/11/pulseconf-2012-report/ One comment on the low-latency case for desktop gaming with a 16ms latency. I imagine this means trouble when sending data to the HDaudio driver. With the PulseAudio sink architecture, you need the sink and ring buffer to be of equal size (feature, not bug), which means you need a ring buffer size of 8ms tops (neglecting the client-server buffer), and events up to 4ms apart. Has anyone tried the changes we pushed recently at the kernel level to properly handle the ring buffer pointer and delay? I believe some of the underruns may be due to the ~1ms inaccuracy that we had before these changes. If your driver is already giving you a 25% precision error no wonder things are broken? ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] PulseConf report
Hi folks, For those who aren't following the planet, thought I'd like you know that I've put up notes from PulseConf up at: http://arunraghavan.net/2012/11/pulseconf-2012-report/ Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] bluetooth: Add support for transport created by external profile
From: Luiz Augusto von Dentz With BlueZ 5 it is possible to have profile registered by a third party process which does not share the same bus id as bluetoothd so it is necessary to store the sender of the transport to be able to talk to it. Note that this is backward compatible. --- src/modules/bluetooth/bluetooth-util.c | 14 +- src/modules/bluetooth/bluetooth-util.h | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c index 272b6ce..7404db2 100644 --- a/src/modules/bluetooth/bluetooth-util.c +++ b/src/modules/bluetooth/bluetooth-util.c @@ -150,6 +150,7 @@ static void transport_free(pa_bluetooth_transport *t) { for (i = 0; i < PA_BLUETOOTH_TRANSPORT_HOOK_MAX; i++) pa_hook_done(&t->hooks[i]); +pa_xfree(t->sender); pa_xfree(t->path); pa_xfree(t->config); pa_xfree(t); @@ -1012,7 +1013,7 @@ int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, const char *access dbus_error_init(&err); -pa_assert_se(m = dbus_message_new_method_call("org.bluez", t->path, "org.bluez.MediaTransport", "Acquire")); +pa_assert_se(m = dbus_message_new_method_call(t->sender, t->path, "org.bluez.MediaTransport", "Acquire")); pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING, &accesstype, DBUS_TYPE_INVALID)); r = dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->y->connection), m, -1, &err); @@ -1048,7 +1049,7 @@ void pa_bluetooth_transport_release(pa_bluetooth_transport *t, const char *acces dbus_error_init(&err); -pa_assert_se(m = dbus_message_new_method_call("org.bluez", t->path, "org.bluez.MediaTransport", "Release")); +pa_assert_se(m = dbus_message_new_method_call(t->sender, t->path, "org.bluez.MediaTransport", "Release")); pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING, &accesstype, DBUS_TYPE_INVALID)); dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->y->connection), m, -1, &err); @@ -1075,12 +1076,13 @@ static int setup_dbus(pa_bluetooth_discovery *y) { return 0; } -static pa_bluetooth_transport *transport_new(pa_bluetooth_discovery *y, const char *path, enum profile p, const uint8_t *config, int size) { +static pa_bluetooth_transport *transport_new(pa_bluetooth_discovery *y, const char *sender, const char *path, enum profile p, const uint8_t *config, int size) { pa_bluetooth_transport *t; unsigned i; t = pa_xnew0(pa_bluetooth_transport, 1); t->y = y; +t->sender = pa_xstrdup(sender); t->path = pa_xstrdup(path); t->profile = p; t->config_size = size; @@ -1100,7 +1102,7 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage pa_bluetooth_discovery *y = userdata; pa_bluetooth_device *d; pa_bluetooth_transport *t; -const char *path, *dev_path = NULL, *uuid = NULL; +const char *sender, *path, *dev_path = NULL, *uuid = NULL; uint8_t *config = NULL; int size = 0; pa_bool_t nrec = FALSE; @@ -1169,7 +1171,9 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage else p = PROFILE_A2DP_SOURCE; -t = transport_new(y, path, p, config, size); +sender = dbus_message_get_sender(m); + +t = transport_new(y, sender, path, p, config, size); if (nrec) t->nrec = nrec; pa_hashmap_put(d->transports, t->path, t); diff --git a/src/modules/bluetooth/bluetooth-util.h b/src/modules/bluetooth/bluetooth-util.h index 8a3f2ad..1773bef 100644 --- a/src/modules/bluetooth/bluetooth-util.h +++ b/src/modules/bluetooth/bluetooth-util.h @@ -72,6 +72,7 @@ typedef enum pa_bluetooth_transport_hook { struct pa_bluetooth_transport { pa_bluetooth_discovery *y; +char *sender; char *path; enum profile profile; uint8_t codec; -- 1.7.11.7 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v2 4/4] pacat: Handle holes in recording streams.
pa_silence_memory() pulls sample-util as a dependency, so it had to be moved from libpulsecore to libpulsecommon. sample-util in turn pulls some more stuff. --- src/Makefile.am |8 src/utils/pacat.c | 37 ++--- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 155f908..f8a0230 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -586,6 +586,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/dynarray.c pulsecore/dynarray.h \ pulsecore/endianmacros.h \ pulsecore/flist.c pulsecore/flist.h \ + pulsecore/g711.c pulsecore/g711.h \ pulsecore/hashmap.c pulsecore/hashmap.h \ pulsecore/i18n.c pulsecore/i18n.h \ pulsecore/idxset.c pulsecore/idxset.h \ @@ -617,6 +618,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/queue.c pulsecore/queue.h \ pulsecore/random.c pulsecore/random.h \ pulsecore/refcnt.h \ + pulsecore/sample-util.c pulsecore/sample-util.h \ pulsecore/shm.c pulsecore/shm.h \ pulsecore/bitset.c pulsecore/bitset.h \ pulsecore/socket-client.c pulsecore/socket-client.h \ @@ -624,6 +626,8 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/socket-util.c pulsecore/socket-util.h \ pulsecore/strbuf.c pulsecore/strbuf.h \ pulsecore/strlist.c pulsecore/strlist.h \ + pulsecore/svolume_c.c pulsecore/svolume_arm.c \ + pulsecore/svolume_mmx.c pulsecore/svolume_sse.c \ pulsecore/tagstruct.c pulsecore/tagstruct.h \ pulsecore/time-smoother.c pulsecore/time-smoother.h \ pulsecore/tokenizer.c pulsecore/tokenizer.h \ @@ -838,7 +842,6 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/core-subscribe.c pulsecore/core-subscribe.h \ pulsecore/core.c pulsecore/core.h \ pulsecore/fdsem.c pulsecore/fdsem.h \ - pulsecore/g711.c pulsecore/g711.h \ pulsecore/hook-list.c pulsecore/hook-list.h \ pulsecore/ltdl-helper.c pulsecore/ltdl-helper.h \ pulsecore/modargs.c pulsecore/modargs.h \ @@ -853,13 +856,10 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/remap_mmx.c pulsecore/remap_sse.c \ pulsecore/resampler.c pulsecore/resampler.h \ pulsecore/rtpoll.c pulsecore/rtpoll.h \ - pulsecore/sample-util.c pulsecore/sample-util.h \ pulsecore/cpu.h \ pulsecore/cpu-arm.c pulsecore/cpu-arm.h \ pulsecore/cpu-x86.c pulsecore/cpu-x86.h \ pulsecore/cpu-orc.c pulsecore/cpu-orc.h \ - pulsecore/svolume_c.c pulsecore/svolume_arm.c \ - pulsecore/svolume_mmx.c pulsecore/svolume_sse.c \ pulsecore/sconv-s16be.c pulsecore/sconv-s16be.h \ pulsecore/sconv-s16le.c pulsecore/sconv-s16le.h \ pulsecore/sconv_sse.c \ diff --git a/src/utils/pacat.c b/src/utils/pacat.c index 2cd8aa5..e7a4255 100644 --- a/src/utils/pacat.c +++ b/src/utils/pacat.c @@ -45,6 +45,7 @@ #include #include #include +#include #define TIME_EVENT_USEC 5 @@ -59,6 +60,9 @@ static pa_mainloop_api *mainloop_api = NULL; static void *buffer = NULL; static size_t buffer_length = 0, buffer_index = 0; +static void *silence_buffer = NULL; +static size_t silence_buffer_length = 0; + static pa_io_event* stdio_event = NULL; static pa_proplist *proplist = NULL; @@ -257,18 +261,18 @@ static void stream_read_callback(pa_stream *s, size_t length, void *userdata) { return; } -pa_assert(data); pa_assert(length > 0); -if (buffer) { +/* If there is a hole in the stream, we generate silence, except + * if it's a passthrough stream in which case we skip the hole. */ +if (data || !(flags & PA_STREAM_PASSTHROUGH)) { buffer = pa_xrealloc(buffer, buffer_length + length); -memcpy((uint8_t*) buffer + buffer_length, data, length); +if (data) +memcpy((uint8_t *) buffer + buffer_length, data, length); +else +pa_silence_memory((uint8_t *) buffer + buffer_length, length, &sample_spec); + buffer_length += length; -} else { -buffer = pa_xmalloc(length); -memcpy(buffer, data, length); -buffer_length = length; -buffer_index = 0; } pa_stream_drop(s); @@ -287,17 +291,27 @@ static void stream_read_callback(pa_stream *s, size_t length, void *userdata) { return; } -
[pulseaudio-discuss] [PATCH v2 3/4] padsp: Handle holes in recording streams.
--- src/utils/padsp.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/utils/padsp.c b/src/utils/padsp.c index f6a3520..858cec8 100644 --- a/src/utils/padsp.c +++ b/src/utils/padsp.c @@ -922,9 +922,22 @@ static int fd_info_copy_data(fd_info *i, int force) { return -1; } -if (!data) +if (len <= 0) break; +if (!data) { +/* Maybe we should generate silence here, but I'm lazy and + * I'll just skip any holes in the stream. */ +if (pa_stream_drop(i->rec_stream) < 0) { +debug(DEBUG_LEVEL_NORMAL, __FILE__": pa_stream_drop(): %s\n", pa_strerror(pa_context_errno(i->context))); +return -1; +} + +assert(n >= len); +n -= len; +continue; +} + buf = (const char*)data + i->rec_offset; if ((r = write(i->thread_fd, buf, len - i->rec_offset)) <= 0) { -- 1.7.10.4 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v2 2/4] simple: Handle holes in recording streams.
--- src/pulse/simple.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/pulse/simple.c b/src/pulse/simple.c index 3524296..860cd18 100644 --- a/src/pulse/simple.c +++ b/src/pulse/simple.c @@ -331,9 +331,14 @@ int pa_simple_read(pa_simple *p, void*data, size_t length, int *rerror) { r = pa_stream_peek(p->stream, &p->read_data, &p->read_length); CHECK_SUCCESS_GOTO(p, rerror, r == 0, unlock_and_fail); -if (!p->read_data) { +if (p->read_length <= 0) { pa_threaded_mainloop_wait(p->mainloop); CHECK_DEAD_GOTO(p, rerror, unlock_and_fail); +} else if (!p->read_data) { +/* There's a hole in the stream, skip it. We could generate + * silence, but that wouldn't work for compressed streams. */ +r = pa_stream_drop(p->stream); +CHECK_SUCCESS_GOTO(p, rerror, r == 0, unlock_and_fail); } else p->read_index = 0; } -- 1.7.10.4 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v2 1/4] pulse: Fix hole handling in pa_stream_peek().
Previously, if there was a hole in a recording stream, pa_stream_peek() would crash. Holes could be handled silently inside pa_stream_peek() by generating silence (wouldn't work for compressed streams, though) or by skipping any holes. However, I think it's better to let the caller decide how the holes should be handled, so in case of holes, pa_stream_peek() will return NULL data pointer and the length of the hole in the nbytes argument. This change is technically an interface break, because previously the documentation didn't mention the possibility of holes that need special handling. However, since holes caused crashing anyway in the past, it's not a regression if applications keep misbehaving due to not handing holes properly. Some words about when holes can appear in recording streams: I think it would be reasonable behavior if overruns due to the application reading data too slowly would cause holes. Currently that's not the case - overruns will just cause audio to be skipped. But the point is that this might change some day. I'm not sure how holes can occur with the current code, but as the linked bug shows, they can happen. It's most likely due to recording from a monitor source where the thing being monitored has holes in its playback stream. BugLink: http://bugs.launchpad.net/bugs/1058200 --- src/pulse/stream.c | 20 src/pulse/stream.h | 20 +++- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/pulse/stream.c b/src/pulse/stream.c index 2b6d306..f692d37 100644 --- a/src/pulse/stream.c +++ b/src/pulse/stream.c @@ -1593,9 +1593,17 @@ int pa_stream_peek(pa_stream *s, const void **data, size_t *length) { if (!s->peek_memchunk.memblock) { if (pa_memblockq_peek(s->record_memblockq, &s->peek_memchunk) < 0) { +/* record_memblockq is empty. */ *data = NULL; *length = 0; return 0; + +} else if (!s->peek_memchunk.memblock) { +/* record_memblockq isn't empty, but it doesn't have any data at + * the current read index. */ +*data = NULL; +*length = s->peek_memchunk.length; +return 0; } s->peek_data = pa_memblock_acquire(s->peek_memchunk.memblock); @@ -1614,7 +1622,7 @@ int pa_stream_drop(pa_stream *s) { PA_CHECK_VALIDITY(s->context, !pa_detect_fork(), PA_ERR_FORKED); PA_CHECK_VALIDITY(s->context, s->state == PA_STREAM_READY, PA_ERR_BADSTATE); PA_CHECK_VALIDITY(s->context, s->direction == PA_STREAM_RECORD, PA_ERR_BADSTATE); -PA_CHECK_VALIDITY(s->context, s->peek_memchunk.memblock, PA_ERR_BADSTATE); +PA_CHECK_VALIDITY(s->context, s->peek_memchunk.length > 0, PA_ERR_BADSTATE); pa_memblockq_drop(s->record_memblockq, s->peek_memchunk.length); @@ -1622,9 +1630,13 @@ int pa_stream_drop(pa_stream *s) { if (s->timing_info_valid && !s->timing_info.read_index_corrupt) s->timing_info.read_index += (int64_t) s->peek_memchunk.length; -pa_assert(s->peek_data); -pa_memblock_release(s->peek_memchunk.memblock); -pa_memblock_unref(s->peek_memchunk.memblock); +if (s->peek_memchunk.memblock) { +pa_assert(s->peek_data); +s->peek_data = NULL; +pa_memblock_release(s->peek_memchunk.memblock); +pa_memblock_unref(s->peek_memchunk.memblock); +} + pa_memchunk_reset(&s->peek_memchunk); return 0; diff --git a/src/pulse/stream.h b/src/pulse/stream.h index b4464fa..a6785ec 100644 --- a/src/pulse/stream.h +++ b/src/pulse/stream.h @@ -534,11 +534,21 @@ int pa_stream_write( pa_seek_mode_t seek /**< Seek mode, must be PA_SEEK_RELATIVE for upload streams */); /** Read the next fragment from the buffer (for recording streams). - * \a data will point to the actual data and \a nbytes will contain the size - * of the data in bytes (which can be less or more than a complete - * fragment). Use pa_stream_drop() to actually remove the data from - * the buffer. If no data is available this will return a NULL - * pointer. */ + * If there is data at the current read index, \a data will point to + * the actual data and \a nbytes will contain the size of the data in + * bytes (which can be less or more than a complete fragment). + * + * If there is no data at the current read index, it means that either + * the buffer is empty or it contains a hole (that is, the write index + * is ahead of the read index but there's no data where the read index + * points at). If the buffer is empty, \a data will be NULL and + * \a nbytes will be 0. If there is a hole, \a data will be NULL and + * \a nbytes will contain the length of the hole. + * + * Use pa_stream_drop() to actually remove the data from the buffer + * and move the read index forward. pa_stream_drop() should not be + * called if the buffer is empty, but it should be called if there is + * a hole. */ int pa_stream_peek( pa_stream *p /**< Th
[pulseaudio-discuss] [PATCH v2 0/4] Handling holes in recording streams.
Changes in v2: - Split the patch into four parts. - Simplified buffer allocation in pacat by taking advantage of realloc's behavior when the input pointer is null. - Made the silence buffer in pacat a global variable to reduce repeated allocation and freeing of the buffer. Tanu Kaskinen (4): pulse: Fix hole handling in pa_stream_peek(). simple: Handle holes in recording streams. padsp: Handle holes in recording streams. pacat: Handle holes in recording streams. src/Makefile.am|8 src/pulse/simple.c |7 ++- src/pulse/stream.c | 20 src/pulse/stream.h | 20 +++- src/utils/pacat.c | 37 ++--- src/utils/padsp.c | 15 ++- 6 files changed, 81 insertions(+), 26 deletions(-) -- 1.7.10.4 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] pulse: Fix hole handling in pa_stream_peek().
On Wed, 2012-11-07 at 14:21 +0100, David Henningsson wrote: > On 11/07/2012 09:36 AM, Tanu Kaskinen wrote: > > Previously, if there was a hole in a recording stream, > > pa_stream_peek() would crash. Holes could be handled silently inside > > pa_stream_peek() by generating silence (wouldn't work for compressed > > streams, though) or by skipping any holes. However, I think it's > > better to let the caller decide how the holes should be handled, so > > in case of holes, pa_stream_peek() will return NULL data pointer and > > the length of the hole in the nbytes argument. > > > > This change is technically an interface break, because previously the > > documentation didn't mention the possibility of holes that need > > special handling. However, since holes caused crashing anyway in the > > past, it's not a regression if applications keep misbehaving due to > > not handing holes properly. > > > > When adapting pacat to this change, a dependency to sample-util was > > added, and that required moving some code from libpulsecore to > > libpulsecommon. > > It was a little confusing to see both things in one. Maybe split the > patch in one to add what's necessary to improve pa_stream_peek, and one > to deal with the utils? Yes, that's definitely better. > Btw, exactly what did you need from sample-util? Maybe it's smarter to > move that particular thing to pulse/something.h rather than moving > sample-util to libpulsecommon? I need pa_silence_memory(). It might be a useful utility function for clients also, so moving it to libpulse might make sense. On the other hand, pa_silence_memblock() and pa_silence_memchunk() are sort of related, so it makes sense to have them in the same place as pa_silence_memory(), and they don't belong in the public API... I won't do changes for now. If you have a good concrete plan about what to move and where, I can definitely consider it, but I don't see moving stuff from libpulsecore to libpulsecommon as a very significant issue, so I don't want to spend too much time thinking about it. > > @@ -257,24 +258,36 @@ static void stream_read_callback(pa_stream *s, size_t > > length, void *userdata) { > > return; > > } > > > > -pa_assert(data); > > pa_assert(length > 0); > > > > -if (buffer) { > > -buffer = pa_xrealloc(buffer, buffer_length + length); > > -memcpy((uint8_t*) buffer + buffer_length, data, length); > > -buffer_length += length; > > -} else { > > -buffer = pa_xmalloc(length); > > -memcpy(buffer, data, length); > > -buffer_length = length; > > -buffer_index = 0; > > +/* If there is a hole in the stream, we generate silence, > > except > > + * if it's a passthrough stream in which case we skip the > > hole. */ > > +if (data || !(flags & PA_STREAM_PASSTHROUGH)) { > > It looks like this can be more elegantly rewritten as > > if (!buffer) >buffer_length = 0; > as reallocs can take null pointers. > > > +if (buffer) { > > +buffer = pa_xrealloc(buffer, buffer_length + length); > > +if (data) > > +memcpy((uint8_t *) buffer + buffer_length, data, > > length); > > +else > > +pa_silence_memory((uint8_t *) buffer + > > buffer_length, length, &sample_spec); > > +buffer_length += length; > > +} else { > > ...and then the entire section handling this case can be removed. Very good suggestion. I didn't know/remember the exact realloc behavior. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] pulse: Fix hole handling in pa_stream_peek().
On 11/07/2012 09:36 AM, Tanu Kaskinen wrote: Previously, if there was a hole in a recording stream, pa_stream_peek() would crash. Holes could be handled silently inside pa_stream_peek() by generating silence (wouldn't work for compressed streams, though) or by skipping any holes. However, I think it's better to let the caller decide how the holes should be handled, so in case of holes, pa_stream_peek() will return NULL data pointer and the length of the hole in the nbytes argument. This change is technically an interface break, because previously the documentation didn't mention the possibility of holes that need special handling. However, since holes caused crashing anyway in the past, it's not a regression if applications keep misbehaving due to not handing holes properly. When adapting pacat to this change, a dependency to sample-util was added, and that required moving some code from libpulsecore to libpulsecommon. It was a little confusing to see both things in one. Maybe split the patch in one to add what's necessary to improve pa_stream_peek, and one to deal with the utils? Btw, exactly what did you need from sample-util? Maybe it's smarter to move that particular thing to pulse/something.h rather than moving sample-util to libpulsecommon? Some words about when holes can appear in recording streams: I think it would be reasonable behavior if overruns due to the application reading data too slowly would cause holes. Currently that's not the case - overruns will just cause audio to be skipped. But the point is that this might change some day. I'm not sure how holes can occur with the current code, but as the linked bug shows, they can happen. It's most likely due to recording from a monitor source where the thing being monitored has holes in its playback stream. BugLink: http://bugs.launchpad.net/bugs/1058200 --- src/Makefile.am|8 src/pulse/simple.c |7 ++- src/pulse/stream.c | 20 +++ src/pulse/stream.h | 20 ++- src/utils/pacat.c | 55 +++- src/utils/padsp.c | 15 +- 6 files changed, 97 insertions(+), 28 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 155f908..f8a0230 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -586,6 +586,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/dynarray.c pulsecore/dynarray.h \ pulsecore/endianmacros.h \ pulsecore/flist.c pulsecore/flist.h \ + pulsecore/g711.c pulsecore/g711.h \ pulsecore/hashmap.c pulsecore/hashmap.h \ pulsecore/i18n.c pulsecore/i18n.h \ pulsecore/idxset.c pulsecore/idxset.h \ @@ -617,6 +618,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/queue.c pulsecore/queue.h \ pulsecore/random.c pulsecore/random.h \ pulsecore/refcnt.h \ + pulsecore/sample-util.c pulsecore/sample-util.h \ pulsecore/shm.c pulsecore/shm.h \ pulsecore/bitset.c pulsecore/bitset.h \ pulsecore/socket-client.c pulsecore/socket-client.h \ @@ -624,6 +626,8 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/socket-util.c pulsecore/socket-util.h \ pulsecore/strbuf.c pulsecore/strbuf.h \ pulsecore/strlist.c pulsecore/strlist.h \ + pulsecore/svolume_c.c pulsecore/svolume_arm.c \ + pulsecore/svolume_mmx.c pulsecore/svolume_sse.c \ pulsecore/tagstruct.c pulsecore/tagstruct.h \ pulsecore/time-smoother.c pulsecore/time-smoother.h \ pulsecore/tokenizer.c pulsecore/tokenizer.h \ @@ -838,7 +842,6 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/core-subscribe.c pulsecore/core-subscribe.h \ pulsecore/core.c pulsecore/core.h \ pulsecore/fdsem.c pulsecore/fdsem.h \ - pulsecore/g711.c pulsecore/g711.h \ pulsecore/hook-list.c pulsecore/hook-list.h \ pulsecore/ltdl-helper.c pulsecore/ltdl-helper.h \ pulsecore/modargs.c pulsecore/modargs.h \ @@ -853,13 +856,10 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/remap_mmx.c pulsecore/remap_sse.c \ pulsecore/resampler.c pulsecore/resampler.h \ pulsecore/rtpoll.c pulsecore/rtpoll.h \ - pulsecore/sample-util.c pulsecore/sample-util.h \ pulsecore/cpu.h \ pulsecore/cpu-arm.c pulsecore/cpu-arm.h \ pulsecore/cpu-x86.c pulsecore/cpu-x86.h \ pulsecore/cpu-orc.c pulsecore/cpu-orc.h \ - pulsecore/svolume_c.c pulsecore/svolume_arm.c \ - pulsecore/svolume_mmx.c pulsecore/svolume_sse.c \ pulsecore/sconv-s16be.c pulsecore/sconv-s16be.h \
Re: [pulseaudio-discuss] [PATCH 4/5] mainloop: Write to the wakeup pipe unconditionally when waking up the mainloop.
Ping! This old patch should fix a recently reported bug: https://bugs.freedesktop.org/show_bug.cgi?id=56735 This patch seems to depend on patches 1 and 3 in the series -- Tanu On Mon, 2012-04-02 at 15:01 +0300, Tanu Kaskinen wrote: > If the mainloop is just about to enter polling, but m->state > is not POLLING yet when some other thread calls > pa_mainloop_wakeup(), the mainloop will not be woken up. > It's safe to write to the wakeup pipe at any time, so let's > just remove the check. > --- > src/pulse/mainloop.c |6 ++ > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/pulse/mainloop.c b/src/pulse/mainloop.c > index 160ba9c..8ac8f06 100644 > --- a/src/pulse/mainloop.c > +++ b/src/pulse/mainloop.c > @@ -774,10 +774,8 @@ void pa_mainloop_wakeup(pa_mainloop *m) { > char c = 'W'; > pa_assert(m); > > -if (m->state == STATE_POLLING) { > -pa_write(m->wakeup_pipe[1], &c, sizeof(c), &m->wakeup_pipe_type); > -pa_atomic_store(&m->wakeup_requested, TRUE); > -} > +pa_write(m->wakeup_pipe[1], &c, sizeof(c), &m->wakeup_pipe_type); > +pa_atomic_store(&m->wakeup_requested, TRUE); > } > > static void clear_wakeup(pa_mainloop *m) { ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] pulse: Fix hole handling in pa_stream_peek().
Previously, if there was a hole in a recording stream, pa_stream_peek() would crash. Holes could be handled silently inside pa_stream_peek() by generating silence (wouldn't work for compressed streams, though) or by skipping any holes. However, I think it's better to let the caller decide how the holes should be handled, so in case of holes, pa_stream_peek() will return NULL data pointer and the length of the hole in the nbytes argument. This change is technically an interface break, because previously the documentation didn't mention the possibility of holes that need special handling. However, since holes caused crashing anyway in the past, it's not a regression if applications keep misbehaving due to not handing holes properly. When adapting pacat to this change, a dependency to sample-util was added, and that required moving some code from libpulsecore to libpulsecommon. Some words about when holes can appear in recording streams: I think it would be reasonable behavior if overruns due to the application reading data too slowly would cause holes. Currently that's not the case - overruns will just cause audio to be skipped. But the point is that this might change some day. I'm not sure how holes can occur with the current code, but as the linked bug shows, they can happen. It's most likely due to recording from a monitor source where the thing being monitored has holes in its playback stream. BugLink: http://bugs.launchpad.net/bugs/1058200 --- src/Makefile.am|8 src/pulse/simple.c |7 ++- src/pulse/stream.c | 20 +++ src/pulse/stream.h | 20 ++- src/utils/pacat.c | 55 +++- src/utils/padsp.c | 15 +- 6 files changed, 97 insertions(+), 28 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 155f908..f8a0230 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -586,6 +586,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/dynarray.c pulsecore/dynarray.h \ pulsecore/endianmacros.h \ pulsecore/flist.c pulsecore/flist.h \ + pulsecore/g711.c pulsecore/g711.h \ pulsecore/hashmap.c pulsecore/hashmap.h \ pulsecore/i18n.c pulsecore/i18n.h \ pulsecore/idxset.c pulsecore/idxset.h \ @@ -617,6 +618,7 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/queue.c pulsecore/queue.h \ pulsecore/random.c pulsecore/random.h \ pulsecore/refcnt.h \ + pulsecore/sample-util.c pulsecore/sample-util.h \ pulsecore/shm.c pulsecore/shm.h \ pulsecore/bitset.c pulsecore/bitset.h \ pulsecore/socket-client.c pulsecore/socket-client.h \ @@ -624,6 +626,8 @@ libpulsecommon_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/socket-util.c pulsecore/socket-util.h \ pulsecore/strbuf.c pulsecore/strbuf.h \ pulsecore/strlist.c pulsecore/strlist.h \ + pulsecore/svolume_c.c pulsecore/svolume_arm.c \ + pulsecore/svolume_mmx.c pulsecore/svolume_sse.c \ pulsecore/tagstruct.c pulsecore/tagstruct.h \ pulsecore/time-smoother.c pulsecore/time-smoother.h \ pulsecore/tokenizer.c pulsecore/tokenizer.h \ @@ -838,7 +842,6 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/core-subscribe.c pulsecore/core-subscribe.h \ pulsecore/core.c pulsecore/core.h \ pulsecore/fdsem.c pulsecore/fdsem.h \ - pulsecore/g711.c pulsecore/g711.h \ pulsecore/hook-list.c pulsecore/hook-list.h \ pulsecore/ltdl-helper.c pulsecore/ltdl-helper.h \ pulsecore/modargs.c pulsecore/modargs.h \ @@ -853,13 +856,10 @@ libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \ pulsecore/remap_mmx.c pulsecore/remap_sse.c \ pulsecore/resampler.c pulsecore/resampler.h \ pulsecore/rtpoll.c pulsecore/rtpoll.h \ - pulsecore/sample-util.c pulsecore/sample-util.h \ pulsecore/cpu.h \ pulsecore/cpu-arm.c pulsecore/cpu-arm.h \ pulsecore/cpu-x86.c pulsecore/cpu-x86.h \ pulsecore/cpu-orc.c pulsecore/cpu-orc.h \ - pulsecore/svolume_c.c pulsecore/svolume_arm.c \ - pulsecore/svolume_mmx.c pulsecore/svolume_sse.c \ pulsecore/sconv-s16be.c pulsecore/sconv-s16be.h \ pulsecore/sconv-s16le.c pulsecore/sconv-s16le.h \ pulsecore/sconv_sse.c \ diff --git a/src/pulse/simple.c b/src/pulse/simple.c index 3524296..860cd18 100644 --- a/src/pulse/simple.c +++ b/src/pulse/simple.c @@ -331,9 +331,14 @@ int pa_simple_read(pa_simple *p, void*data, size_t length, int *rerror) { r = pa_stream_peek(p->stream, &p->read_data, &p->read
Re: [pulseaudio-discuss] jackdbus module, pulse fails to conform on device reservation API
On 7 November 2012 06:46, David Henningsson wrote: > On 11/07/2012 12:52 AM, Ian Malone wrote: >> >> On 6 November 2012 15:58, Ian Malone wrote: >> etc. >> >> >>> >>> I'll speculate that something somewhere is confused by the presence of >>> two devices and either Audio1 isn't being provided correctly by pulse >>> (though it does create it) or requested properly by Jack (though with >>> only one parameter that's difficult to believe). >>> >> >> I now know enough about dbus to confirm this, the second interface is >> not created properly for some reason. >> >> Here's Audio0: >> [liveuser@localhost ~]$ dbus-send --session --print-reply >> --reply-timeout=2000 --type=method_call >> --dest=org.freedesktop.ReserveDevice1.Audio0 >> /org/freedesktop/ReserveDevice1/Audio0 >> org.freedesktop.DBus.Introspectable.Introspect >> method return sender=:1.119 -> dest=:1.154 reply_serial=2 >> string "> Introspection 1.0//EN" >> "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd";> >> >> > name="RequestRelease"> > direction="in"/> >> >> > name="ApplicationDeviceName" type="s" access="read"/> >> > name="Get"> > name="property" direction="in" type="s"/> > direction="out" type="v"/>> name="org.freedesktop.DBus.Introspectable"> > name="Introspect"> >> " >> >> Here's Audio1: >> [liveuser@localhost ~]$ dbus-send --session --print-reply >> --reply-timeout=2000 --type=method_call >> --dest=org.freedesktop.ReserveDevice1.Audio1 >> /org/freedesktop/ReserveDevice1/Audio1 >> org.freedesktop.DBus.Introspectable.Introspect >> method return sender=:1.119 -> dest=:1.155 reply_serial=2 >> string "> Introspection 1.0//EN" >> "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd";> >> >> >> " >> >> At this point I don't think I have any doubt there's a bug in the >> pulse dbus handling. > > > Hi, > > FWIW, I tried to reproduce this last evening under Ubuntu 12.10 (which uses > PulseAudio 2.1), but it seems to create properly interfaces for the two card > scenario here. I used the "d-feet" introspect program to verify. > (Jackd2-dbus seemed to crash on startup for some reason I have not tracked > down.) > > Thanks for trying. I've had a look at d-feet and noticed that org.freedesktop.ReserveDevice1.Audio0 and org.freedesktop.ReserveDevice1.Audio1 both look okay,ReserveDevice1 is there. EXCEPT that the object path for both is org.freedesktop.ReserveDevice1.Audio0. In other words, N.B. different dest and path: $ dbus-send --session --print-reply --reply-timeout=2000 --type=method_call --dest=org.freedesktop.ReserveDevice1.Audio1 /org/freedesktop/ReserveDevice1/Audio0 org.freedesktop.DBus.Introspectable.Introspect method return sender=:1.35 -> dest=:1.113 reply_serial=2 string "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd";> " I don't know very much about dbus, but I haven't seen that before. The is pulseaudio-2.1-4.fc18.x86_64, but from the dates in git I don't think the dbus code has been touched in a while. -- imalone http://ibmalone.blogspot.co.uk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss