Re: [pulseaudio-discuss] How to modify (source) port priorities?

2012-11-07 Thread Raymond Yau
>
> 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?

2012-11-07 Thread Ronan Jouchet

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?

2012-11-07 Thread David Henningsson

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)

2012-11-07 Thread David Henningsson

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

2012-11-07 Thread Pierre-Louis Bossart



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

2012-11-07 Thread Arun Raghavan
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

2012-11-07 Thread Luiz Augusto von Dentz
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.

2012-11-07 Thread Tanu Kaskinen
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.

2012-11-07 Thread Tanu Kaskinen
---
 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.

2012-11-07 Thread Tanu Kaskinen
---
 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().

2012-11-07 Thread Tanu Kaskinen
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.

2012-11-07 Thread Tanu Kaskinen
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().

2012-11-07 Thread Tanu Kaskinen
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().

2012-11-07 Thread David Henningsson

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.

2012-11-07 Thread Tanu Kaskinen
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().

2012-11-07 Thread Tanu Kaskinen
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

2012-11-07 Thread Ian Malone
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