Re: [pulseaudio-discuss] underrun behavior with alsa-plugins
On 2011-04-19 18:12, Maarten Lankhorst wrote: Hi all, For wine I was investigating a bug with pulseaudio, it seems alsa-plugins' pulse driver ignores underruns. Hmm, doesn't wine come with a native pulse driver these days? This is probably because an underrun will force you to call snd_pcm_prepare, this will destroy the stream and set up a similar new one again. ...whereas the proper way to resolve the underrun in the pulse plugin is just to feed it with more data, which is likely what would happen if you do not report the underrun. This causes more susceptibility to underruns, so that code was disabled. Takashi added some kind of option so you can turn the reporting of underruns on again, if that helps. However if I force underruns to occur,the state will stay running and it appears there is still some data left in the buffer, so sound stalls entirely. This is nothing I've heard of. The latency gets updated to something like 0x7bdX which looks suspiciously much like a pointer to me, which may be a bug. If I instead make pulse_prepare call pulse_start on underrun, it's handled properly and sound will continue to work. So my questions are. 1. Is the weird latency value update a bug? I guess so, but what sense would it make to read the latency if you're in an underrun condition - and btw, exactly what latency variable is this? Digging it up I can only assume it's a bug in src/stream.c , but I haven't figured out why yet. Tested with pulseaudio.c 2. Any comments on this patch for alsa-plugins? IIRC, by setting handle_underrun to 1, you're reopening bugs of broken/skipping audio for mpg123 and other programs, which get stuck in an infinite loop of write one sample, force start the stream, write more samples, asynchronusly get an underrun, drop the stream and drop samples already written. There should be threads on at least alsa-devel about this, from a year back or so. -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] filter-apply: Make housekeeping optional
Adds an autoclean option (defaults to TRUE) that controls whether
module-filter-apply cleans up unused modules or not. This is useful in
cases where you know that a filter will be used often and thus can avoid
overhead from repeated module load/unload.
---
src/modules/module-filter-apply.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/modules/module-filter-apply.c
b/src/modules/module-filter-apply.c
index d4bded5..12acc91 100644
--- a/src/modules/module-filter-apply.c
+++ b/src/modules/module-filter-apply.c
@@ -25,6 +25,7 @@
#include pulse/timeval.h
#include pulse/rtclock.h
+#include pulse/i18n.h
#include pulsecore/macro.h
#include pulsecore/hashmap.h
@@ -44,11 +45,14 @@ PA_MODULE_AUTHOR(Colin Guthrie);
PA_MODULE_DESCRIPTION(Load filter sinks automatically when needed);
PA_MODULE_VERSION(PACKAGE_VERSION);
PA_MODULE_LOAD_ONCE(TRUE);
+PA_MODULE_USAGE(_(autoclean=automatically unload unused filters?));
static const char* const valid_modargs[] = {
+autoclean,
NULL
};
+#define DEFAULT_AUTOCLEAN TRUE
#define HOUSEKEEPING_INTERVAL (10 * PA_USEC_PER_SEC)
struct filter {
@@ -66,6 +70,7 @@ struct userdata {
*sink_input_proplist_slot,
*sink_input_unlink_slot,
*sink_unlink_slot;
+pa_bool_t autoclean;
pa_time_event *housekeeping_time_event;
};
@@ -152,6 +157,9 @@ static void housekeeping_time_callback(pa_mainloop_api*a,
pa_time_event* e, cons
static void trigger_housekeeping(struct userdata *u) {
pa_assert(u);
+if (!u-autoclean)
+return;
+
if (u-housekeeping_time_event)
return;
@@ -345,6 +353,12 @@ int pa__init(pa_module *m) {
u-core = m-core;
+u-autoclean = DEFAULT_AUTOCLEAN;
+if (pa_modargs_get_value_boolean(ma, autoclean, u-autoclean) 0) {
+pa_log(Failed to parse autoclean value);
+goto fail;
+}
+
u-filters = pa_hashmap_new(filter_hash, filter_compare);
u-sink_input_put_slot =
pa_hook_connect(m-core-hooks[PA_CORE_HOOK_SINK_INPUT_PUT], PA_HOOK_LATE,
(pa_hook_cb_t) sink_input_put_cb, u);
--
1.7.5.rc1
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
'Twas brillig, and Tanu Kaskinen at 18/04/11 10:31 did gyre and gimble: For publishing APIs: /* Returns a negative error code if the extra api is already registered. */ int pa_extra_api_register(const char *name, void *api); void pa_extra_api_unregister(const_char *name); For consuming APIs: /* Returns the currently registered api object, * or NULL if the api isn't registered right now. */ void *pa_extra_api_get(const char *name); Additionally, there would be a couple of hooks defined for getting notifications about APIs getting registered and unregistered. That seems sensible. The api object would be a struct with function pointers. The API provider would create instances of that struct and provide the function implementation. The API consumer would talk to the provider using those function pointers. OK, that all seems fine IMO. The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. If the API is not dependant on a particular module, what code calls pa_extra_api_register? I'd be happy enough defining this extra API as an IMC (c.f. IPC) system and thus make it very much tied to modules. You could even have a call tracker module that just provides the code you posted earlier with nothing more than a .h file and pa__init and pa__done implementations and make sure this is loaded before any modules that happen to want to use it. I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. Yeah extension is already in use for protocol extensions I guess. How about pa_imc_*? (for inter-module comms) or is that perhaps too cryptic? Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] underrun behavior with alsa-plugins
Hi David, Op 20-04-11 09:33, David Henningsson schreef: On 2011-04-19 18:12, Maarten Lankhorst wrote: Hi all, For wine I was investigating a bug with pulseaudio, it seems alsa-plugins' pulse driver ignores underruns. Hmm, doesn't wine come with a native pulse driver these days? Nope, but distros patch in a buggy winepulse driver, wine is working on rearchitecting its driver model, so that a pulseaudio driver might be added after that, but the current winepulse driver was a bad copy/replace job of the wrong sound driver. :) This is probably because an underrun will force you to call snd_pcm_prepare, this will destroy the stream and set up a similar new one again. ...whereas the proper way to resolve the underrun in the pulse plugin is just to feed it with more data, which is likely what would happen if you do not report the underrun. This causes more susceptibility to underruns, so that code was disabled. Takashi added some kind of option so you can turn the reporting of underruns on again, if that helps. If you read my patch, that's what it does. However if I force underruns to occur,the state will stay running and it appears there is still some data left in the buffer, so sound stalls entirely. This is nothing I've heard of. Sadly all too common here, if I feed data and it underruns, it may appear to be running but is stalled entirely. The latency gets updated to something like 0x7bdX which looks suspiciously much like a pointer to me, which may be a bug. If I instead make pulse_prepare call pulse_start on underrun, it's handled properly and sound will continue to work. So my questions are. 1. Is the weird latency value update a bug? I guess so, but what sense would it make to read the latency if you're in an underrun condition - and btw, exactly what latency variable is this? In a response to a latency event I printf the latency inside the callback, with pa_stream_get_latency. Digging it up I can only assume it's a bug in src/stream.c , but I haven't figured out why yet. Tested with pulseaudio.c 2. Any comments on this patch for alsa-plugins? IIRC, by setting handle_underrun to 1, you're reopening bugs of broken/skipping audio for mpg123 and other programs, which get stuck in an infinite loop of write one sample, force start the stream, write more samples, asynchronusly get an underrun, drop the stream and drop samples already written. There should be threads on at least alsa-devel about this, from a year back or so. This is exactly what my patch was addressing, I was fixing that bug by handling underrun correctly. snd_pcm_prepare() is called when an underrun occurs, so instead I make it only restart the stream if underrun. Not handling underruns at all seems to allow it to stall on xrun, while claiming to be running. This sometimes appears to happen in other programs too though, like mplayer. Could it be because of the weird latency value? Cheers, Maarten ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
On Wed, 2011-04-20 at 11:36 +0300, Colin Guthrie wrote: The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. If the API is not dependant on a particular module, what code calls pa_extra_api_register? I'd be happy enough defining this extra API as an IMC (c.f. IPC) system and thus make it very much tied to modules. Hmm... did you get the impression that these apis would not necessarily be tied to any modules at all? That was not the idea - the apis would be implemented always by modules, but not necessarily by any particular module. For example, if there was an api for getting the current call state, it could be implemented by multiple modules, each of which would use different logic for determining whether a call is active or not. The modules would of course conflict with each other, so they could not be loaded at the same time. This would be handled by pa_extra_api_register() - only one module could have the api registered at any given time, the other modules would get an error when they try to call pa_extra_api_register(). I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. Yeah extension is already in use for protocol extensions I guess. How about pa_imc_*? (for inter-module comms) or is that perhaps too cryptic? Yeah, it's cryptic, but it's also shorter. And extra api isn't an obvious name either. I'm not sure which I like more. Probably imc. Regarding function names, they will actually have form pa_imc_manager_* or pa_extra_api_manager_* if I get to decide... The functions can be methods of either some manager object or pa_core. If they're implemented directly by pa_core, then the prefix will of course be pa_core_, but I'd prefer having a separate manager object. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] match: don't double free in case of missing table file
From: Marc-André Lureau marcandre.lur...@gmail.com
---
src/modules/module-match.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/modules/module-match.c b/src/modules/module-match.c
index c8f2706..3eba149 100644
--- a/src/modules/module-match.c
+++ b/src/modules/module-match.c
@@ -95,7 +95,6 @@ static int load_rules(struct userdata *u, const char
*filename) {
f = pa_open_config_file(DEFAULT_MATCH_TABLE_FILE,
DEFAULT_MATCH_TABLE_FILE_USER, NULL, fn);
if (!f) {
-pa_xfree(fn);
pa_log(Failed to open file config file: %s, pa_cstrerror(errno));
goto finish;
}
--
1.7.4.2
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] match: don't double free in case of missing table file
'Twas brillig, and Tanu Kaskinen at 20/04/11 12:49 did gyre and gimble: From: Marc-André Lureau marcandre.lur...@gmail.com Thanks in my tree now. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
'Twas brillig, and Tanu Kaskinen at 20/04/11 11:54 did gyre and gimble: On Wed, 2011-04-20 at 11:36 +0300, Colin Guthrie wrote: The api object header could be located in src/extra_apis - that would make the api not directly dependent on a particular module. If the API is not dependant on a particular module, what code calls pa_extra_api_register? I'd be happy enough defining this extra API as an IMC (c.f. IPC) system and thus make it very much tied to modules. Hmm... did you get the impression that these apis would not necessarily be tied to any modules at all? That was not the idea - the apis would be implemented always by modules, but not necessarily by any particular module. For example, if there was an api for getting the current call state, it could be implemented by multiple modules, each of which would use different logic for determining whether a call is active or not. The modules would of course conflict with each other, so they could not be loaded at the same time. This would be handled by pa_extra_api_register() - only one module could have the api registered at any given time, the other modules would get an error when they try to call pa_extra_api_register(). Ahh I get what you mean :) Yeah this all makes sense. Thanks for clarifying :) I've been using the term extra api here. I don't think it's the greatest name in the world, but that's the best I could think of. I'd like extension more, but that word is already used for other purposes. Yeah extension is already in use for protocol extensions I guess. How about pa_imc_*? (for inter-module comms) or is that perhaps too cryptic? Yeah, it's cryptic, but it's also shorter. And extra api isn't an obvious name either. I'm not sure which I like more. Probably imc. Regarding function names, they will actually have form pa_imc_manager_* or pa_extra_api_manager_* if I get to decide... The functions can be methods of either some manager object or pa_core. If they're implemented directly by pa_core, then the prefix will of course be pa_core_, but I'd prefer having a separate manager object. OK, so the manager object is separate but kept in the core? Seems fine by me to keep things neatly separated. I thinking of taking a pop at this at some point soon (if you don't beat me to it), so can we decide on the name now? Anyone got any better naming suggestions? pa_imc_manager_* (Pro: short, Con: Inter Module Comms cryptic) pa_extra_api_manager_* (Pro: clear, Con: long, Extension vs. Extra which is which and for what purpose?) pa_xapi_manager_* (Pro: short, Con: might be confused with X11) pa_mxapi_manager_* (Pro: quite short, Con: Module eXtension API but still quite cryptic) I'm not really blown away by any of them :s Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] match: match rule earlier, in SINK_INPUT_NEW
From: Marc-André Lureau marcandre.lur...@gmail.com
---
src/modules/module-match.c | 35 +--
1 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/src/modules/module-match.c b/src/modules/module-match.c
index 3eba149..b3316d4 100644
--- a/src/modules/module-match.c
+++ b/src/modules/module-match.c
@@ -43,7 +43,6 @@
#include pulsecore/core-util.h
#include pulsecore/modargs.h
#include pulsecore/log.h
-#include pulsecore/core-subscribe.h
#include pulsecore/sink-input.h
#include pulsecore/core-util.h
@@ -77,7 +76,7 @@ struct rule {
struct userdata {
struct rule *rules;
char *property_key;
-pa_subscription *subscription;
+pa_hook_slot *sink_input_new_hook_slot;
};
static int load_rules(struct userdata *u, const char *filename) {
@@ -191,23 +190,15 @@ finish:
return ret;
}
-static void callback(pa_core *c, pa_subscription_event_type_t t, uint32_t idx,
void *userdata) {
-struct userdata *u = userdata;
-pa_sink_input *si;
+static pa_hook_result_t sink_input_new_hook_callback(pa_core *c,
pa_sink_input_new_data *si, struct userdata *u) {
struct rule *r;
const char *n;
pa_assert(c);
pa_assert(u);
-if (t != (PA_SUBSCRIPTION_EVENT_SINK_INPUT|PA_SUBSCRIPTION_EVENT_NEW))
-return;
-
-if (!(si = pa_idxset_get_by_index(c-sink_inputs, idx)))
-return;
-
if (!(n = pa_proplist_gets(si-proplist, u-property_key)))
-return;
+return PA_HOOK_OK;
pa_log_debug(Matching with %s, n);
@@ -216,14 +207,17 @@ static void callback(pa_core *c,
pa_subscription_event_type_t t, uint32_t idx, v
if (r-proplist) {
pa_log_debug(updating proplist of sink input '%s', n);
pa_proplist_update(si-proplist, PA_UPDATE_MERGE, r-proplist);
-} else {
+} else if (si-volume_writable) {
pa_cvolume cv;
pa_log_debug(changing volume of sink input '%s' to 0x%03x,
n, r-volume);
pa_cvolume_set(cv, si-sample_spec.channels, r-volume);
-pa_sink_input_set_volume(si, cv, TRUE, FALSE);
-}
+pa_sink_input_new_data_set_volume(si, cv);
+} else
+pa_log_debug(the volume of sink input '%s' is not writable,
can't change it, n);
}
}
+
+return PA_HOOK_OK;
}
int pa__init(pa_module*m) {
@@ -239,7 +233,6 @@ int pa__init(pa_module*m) {
u = pa_xnew(struct userdata, 1);
u-rules = NULL;
-u-subscription = NULL;
m-userdata = u;
u-property_key = pa_xstrdup(pa_modargs_get_value(ma, key,
PA_PROP_MEDIA_NAME));
@@ -247,10 +240,8 @@ int pa__init(pa_module*m) {
if (load_rules(u, pa_modargs_get_value(ma, table, NULL)) 0)
goto fail;
-/* FIXME: Doing this asynchronously is just broken. This needs to
- * use a hook! */
-
-u-subscription = pa_subscription_new(m-core,
PA_SUBSCRIPTION_MASK_SINK_INPUT, callback, u);
+/* hook EARLY - 1, to match before stream-restore */
+u-sink_input_new_hook_slot =
pa_hook_connect(m-core-hooks[PA_CORE_HOOK_SINK_INPUT_NEW], PA_HOOK_EARLY -
1, (pa_hook_cb_t) sink_input_new_hook_callback, u);
pa_modargs_free(ma);
return 0;
@@ -272,8 +263,8 @@ void pa__done(pa_module*m) {
if (!(u = m-userdata))
return;
-if (u-subscription)
-pa_subscription_free(u-subscription);
+if (u-sink_input_new_hook_slot)
+pa_hook_slot_free(u-sink_input_new_hook_slot);
if (u-property_key)
pa_xfree(u-property_key);
--
1.7.4.2
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2] call-state-tracker: New component.
On Wed, 2011-04-20 at 15:19 +0300, Colin Guthrie wrote: OK, so the manager object is separate but kept in the core? Seems fine by me to keep things neatly separated. Yep. I thinking of taking a pop at this at some point soon (if you don't beat me to it), so can we decide on the name now? Anyone got any better naming suggestions? pa_imc_manager_* (Pro: short, Con: Inter Module Comms cryptic) pa_extra_api_manager_* (Pro: clear, Con: long, Extension vs. Extra which is which and for what purpose?) pa_xapi_manager_* (Pro: short, Con: might be confused with X11) pa_mxapi_manager_* (Pro: quite short, Con: Module eXtension API but still quite cryptic) I'm not really blown away by any of them :s I'll vote for pa_imc_manager_*. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] core: Drop empty gaps in the memblockq when playing data from it.
It's possible that the memblockq of a sink input is rewound to a negative read
index if the sink input is moved between sinks shortly after its creation. When
this happens, pa_memblockq_peek() returns a memchunk whose 'memblock' field is
NULL and whose 'length' field indicates the length of the gap caused by the
negative read index. This will trigger an assert in play-memblockq.c.
If the memblockq had a silence memchunk, pa_memblockq_peek() would return
silence for the duration of the gap and the assert would be avoided. However,
this approach would prevent the sink input from being drained and is thus not
possible. Instead, we handle the aforementioned situation by dropping the gap
indicated by the 'length' field of the memchunk and by peeking the actual data
that comes after the gap.
This scenario seems to be quite rare in everyday use, but it causes a severe
bug in the handheld world. The assert can be triggered e.g. by loading two null
sinks, playing a sample from the cache to one of them and then moving the
created sink input between the two sinks. The rewinds done by the null sinks
seem to be quite long (I don't know if this is normal behaviour or something
fishy in module-null-sink).
See also:
6bd34156b130c07b130de10111a12ef6dab18b52
virtual-sink: Fix a crash when moving the sink to a new master right after
setup.
https://tango.0pointer.de/pipermail/pulseaudio-discuss/2011-February/009105.html
---
src/pulsecore/play-memblockq.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/pulsecore/play-memblockq.c b/src/pulsecore/play-memblockq.c
index f075a5b..66e47ea 100644
--- a/src/pulsecore/play-memblockq.c
+++ b/src/pulsecore/play-memblockq.c
@@ -135,11 +135,14 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t
nbytes, pa_memchunk *chunk
return -1;
}
-/* FIXME: u-memblockq doesn't have a silence memchunk set, so
- * pa_memblockq_peek() will return 0 without returning any memblock if the
- * read index points to a hole. If the memblockq is rewound beyond index 0,
- * then there will be a hole. */
-pa_assert(chunk-memblock);
+/* If there's no memblock, there's going to be data in the memblockq after
+ * a gap with length chunk-length. Drop the the gap and peek the actual
+ * data. There should always be some data coming - hence the assert. The
+ * gap will occur if the memblockq is rewound beyond index 0.*/
+if (!chunk-memblock) {
+pa_memblockq_drop(u-memblockq, chunk-length);
+pa_assert_se(pa_memblockq_peek(u-memblockq, chunk) = 0);
+}
chunk-length = PA_MIN(chunk-length, nbytes);
pa_memblockq_drop(u-memblockq, chunk-length);
--
1.7.0.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] underrun behavior with alsa-plugins
On 2011-04-20 12:09, Maarten Lankhorst wrote: Hi David, Op 20-04-11 09:33, David Henningsson schreef: On 2011-04-19 18:12, Maarten Lankhorst wrote: Hi all, For wine I was investigating a bug with pulseaudio, it seems alsa-plugins' pulse driver ignores underruns. Hmm, doesn't wine come with a native pulse driver these days? Nope, but distros patch in a buggy winepulse driver, wine is working on rearchitecting its driver model, so that a pulseaudio driver might be added after that, but the current winepulse driver was a bad copy/replace job of the wrong sound driver. :) This is probably because an underrun will force you to call snd_pcm_prepare, this will destroy the stream and set up a similar new one again. ...whereas the proper way to resolve the underrun in the pulse plugin is just to feed it with more data, which is likely what would happen if you do not report the underrun. This causes more susceptibility to underruns, so that code was disabled. Takashi added some kind of option so you can turn the reporting of underruns on again, if that helps. If you read my patch, that's what it does. However if I force underruns to occur,the state will stay running and it appears there is still some data left in the buffer, so sound stalls entirely. This is nothing I've heard of. Sadly all too common here, if I feed data and it underruns, it may appear to be running but is stalled entirely. Could you provide a test case where this occurs? (Pulseaudio/ALSA version, client applications/libraries, etc) The latency gets updated to something like 0x7bdX which looks suspiciously much like a pointer to me, which may be a bug. If I instead make pulse_prepare call pulse_start on underrun, it's handled properly and sound will continue to work. So my questions are. 1. Is the weird latency value update a bug? I guess so, but what sense would it make to read the latency if you're in an underrun condition - and btw, exactly what latency variable is this? In a response to a latency event I printf the latency inside the callback, with pa_stream_get_latency. Digging it up I can only assume it's a bug in src/stream.c , but I haven't figured out why yet. Tested with pulseaudio.c 2. Any comments on this patch for alsa-plugins? IIRC, by setting handle_underrun to 1, you're reopening bugs of broken/skipping audio for mpg123 and other programs, which get stuck in an infinite loop of write one sample, force start the stream, write more samples, asynchronusly get an underrun, drop the stream and drop samples already written. There should be threads on at least alsa-devel about this, from a year back or so. This is exactly what my patch was addressing, I was fixing that bug by handling underrun correctly. snd_pcm_prepare() is called when an underrun occurs, so instead I make it only restart the stream if underrun. Not handling underruns at all seems to allow it to stall on xrun, while claiming to be running. This sometimes appears to happen in other programs too though, like mplayer. Could it be because of the weird latency value? Ok, thanks for the clarifications. I've taken a closer look at your patch now and have the following comments: When you're calling pulse_start instead of continuing in pulse_prepare - pulse_start will call uncork/trigger, won't that just cause another underrun? Would it perhaps be better to just return without doing anything? mpg123 was used as a test case for the original bug, and mpg123 calls snd_pcm_drop on an underrun, so you will be regressing mpg123 by changing handle_underrun to 1. (Now of course we could fix mpg123, but I don't know if there are a lot of other programs out there doing similar things?) My main point is that the underrun is often obsolete when the message reached the application, because more data has already been written, therefore reporting it to the app does more harm than good. At least until the underrun callback (and PA protocol) supports sending the position of underrun, together with the underrun message. If we had that position, we could compare that with the current write pointer to determine whether the underrun is actually obsolete or not. -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] core: Drop empty gaps in the memblockq when playing data from it.
The assert that this patch fixes can be reproduced with e.g. the following script: SAMPLE_PATH=/usr/share/sounds/alsa/ SAMPLE=Front_Left pactl remove-sample $SAMPLE 2 /dev/null pactl upload-sample $SAMPLE_PATH$SAMPLE.wav mod1=`pactl load-module module-null-sink sink_name=null1` mod2=`pactl load-module module-null-sink sink_name=null2` pactl play-sample $SAMPLE null1 input=`pactl list | grep Sink Input # | tail -n 1 | cut -d# -f2` echo Sample $SAMPLE playing as Sink Input #$input pactl move-sink-input $input null2 pactl move-sink-input $input null1 pactl unload-module $mod1 pactl unload-module $mod2 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] core: Drop empty gaps in the memblockq when playing data from it.
'Twas brillig, and Antti-Ville Jansson at 20/04/11 14:00 did gyre and gimble: The assert that this patch fixes can be reproduced with e.g. the following script: SAMPLE_PATH=/usr/share/sounds/alsa/ SAMPLE=Front_Left pactl remove-sample $SAMPLE 2 /dev/null pactl upload-sample $SAMPLE_PATH$SAMPLE.wav mod1=`pactl load-module module-null-sink sink_name=null1` mod2=`pactl load-module module-null-sink sink_name=null2` pactl play-sample $SAMPLE null1 input=`pactl list | grep Sink Input # | tail -n 1 | cut -d# -f2` echo Sample $SAMPLE playing as Sink Input #$input pactl move-sink-input $input null2 pactl move-sink-input $input null1 pactl unload-module $mod1 pactl unload-module $mod2 Thanks for the awesome test case. I'll take a look at this shortly :) Cheers Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] match: match rule earlier, in SINK_INPUT_NEW
Seems reasonable. In my tree now. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] underrun behavior with alsa-plugins
Hey,
Op 20-04-11 15:18, David Henningsson schreef:
However if I
force underruns to occur,the state will stay running and it appears
there is still some data left in the buffer, so sound stalls entirely.
This is nothing I've heard of.
Sadly all too common here, if I feed data and it underruns, it may
appear to be running but is stalled entirely.
Could you provide a test case where this occurs? (Pulseaudio/ALSA
version, client applications/libraries, etc)
Well this could be a basic test version I suppose, although it's not
fair since it's guaranteed to underrun and doesn't work with dmix since
it specifies period sizes exactly.
Digging it up I can only
assume it's a bug in src/stream.c , but I haven't figured out why yet.
Tested with pulseaudio.c
2. Any comments on this patch for alsa-plugins?
IIRC, by setting handle_underrun to 1, you're reopening bugs of
broken/skipping audio for mpg123 and other programs, which get stuck in
an infinite loop of write one sample, force start the stream, write
more samples, asynchronusly get an underrun, drop the stream and drop
samples already written.
There should be threads on at least alsa-devel about this, from a year
back or so.
This is exactly what my patch was addressing, I was fixing that bug by
handling underrun correctly. snd_pcm_prepare() is called when an
underrun occurs, so instead I make it only restart the stream if
underrun. Not handling underruns at all seems to allow it to stall on
xrun, while claiming to be running. This sometimes appears to happen in
other programs too though, like mplayer. Could it be because of the
weird latency value?
Ok, thanks for the clarifications. I've taken a closer look at your
patch now and have the following comments:
When you're calling pulse_start instead of continuing in pulse_prepare -
pulse_start will call uncork/trigger, won't that just cause another
underrun? Would it perhaps be better to just return without doing anything?
mpg123 was used as a test case for the original bug, and mpg123 calls
snd_pcm_drop on an underrun, so you will be regressing mpg123 by
changing handle_underrun to 1. (Now of course we could fix mpg123, but I
don't know if there are a lot of other programs out there doing similar
things?)
My main point is that the underrun is often obsolete when the message
reached the application, because more data has already been written,
therefore reporting it to the app does more harm than good.
At least until the underrun callback (and PA protocol) supports sending
the position of underrun, together with the underrun message. If we had
that position, we could compare that with the current write pointer to
determine whether the underrun is actually obsolete or not.
Well, this is a dumb unfair test program that forcibly underruns,
probably should work if you change default to hw:0, but I think dmix
doesn't allow you to set random period sizes. If I try this with the
pulseaudio default plugin, it will break.
With my patch:
~$ ./a.out
Written: 2047 - available: 2047
Successfully underrun
Written: 960 - available: 960
Successfully underrun
Written: 1890 - available: 1890
Successfully underrun
Without:
~$ ./a.out 2/dev/null
Written: 2047 - available: 2047
Written: 0 - available: 0
Written: 0 - available: 0
Note: not any form of error checking is done here, just cobbled this
dumb program together for demonstration purposes. :)
Cheers,
Maarten
#include stdio.h
#include unistd.h
#include asoundlib.h
int main() {
unsigned t = 1, rate = 44100;
char silence[4096] = {};
snd_pcm_hw_params_t *hw_parms;
snd_pcm_sw_params_t *sw_parms;
snd_pcm_t *pcm;
snd_pcm_hw_params_alloca(hw_parms);
snd_pcm_sw_params_alloca(sw_parms);
if (snd_pcm_open(pcm, default, SND_PCM_STREAM_PLAYBACK, SND_PCM_NONBLOCK) 0) return 1;
snd_pcm_hw_params_any(pcm, hw_parms);
snd_pcm_hw_params_set_period_size(pcm, hw_parms, 512, 0);
snd_pcm_hw_params_set_periods(pcm, hw_parms, 4, 0);
snd_pcm_hw_params_set_format(pcm, hw_parms, SND_PCM_FORMAT_S16_LE);
snd_pcm_hw_params_set_rate_near(pcm, hw_parms, rate, NULL);
snd_pcm_hw_params_set_channels(pcm, hw_parms, 1);
snd_pcm_hw_params_set_access(pcm, hw_parms, SND_PCM_ACCESS_RW_INTERLEAVED);
snd_pcm_hw_params(pcm, hw_parms);
snd_pcm_prepare(pcm);
snd_pcm_start(pcm);
snd_pcm_writei(pcm, silence, sizeof(silence)/2);
while (1) {
snd_pcm_uframes_t avail = snd_pcm_avail_update(pcm);
int state = snd_pcm_state(pcm);
if (state == SND_PCM_STATE_XRUN) {
printf(Successfully underrun\n);
avail = sizeof(silence)/2;
snd_pcm_prepare(pcm);
snd_pcm_writei(pcm, silence, avail);
} else if (state == SND_PCM_STATE_RUNNING) {
long ret = snd_pcm_writei(pcm, silence, avail);
printf(Written: %li - available: %lu\n, ret, avail);
usleep(10);
}
usleep(1);
}
}
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
Re: [pulseaudio-discuss] review+pull-request: Passthrough support
On 04/19/2011 02:03 PM, pl bossart wrote: I finally managed to figure out why eac3 wouldn't work in passthrough mode. Apparently my HDMI receiver will only lock on eac3 data if the AES0 control is set to 0x06 (non-audio, ie iec61937). ffmpeg -acodec copy -i csi_miami_5.1_256_spx.eac3 -f spdif outfile.pcm aplay -Dhdmi:AES0=0x6 -fs16 -c2 -r192000 outfile.pcm the same file plays well through pulseaudio if I modifed default.conf to [Mapping hdmi-stereo] device-strings = hdmi:%f,AES0=0x06 channel-map = left,right priority = 4 direction = output Somehow we need to find a way to set this AES0 byte when opening the iec958 or hdmi device in the passthrough code, and reset it back to 0x04 for PCM playback (should be fairly easy to do by looking at the code of iecset in alsa-utils). Beats me why this was not needed for plain ac3, but setting the audio bit is actually the correct thing to do. So far I haven't been able to get anything except stereo working with passthrough. It seems to me that selecting multiple channels in passthrough mode would be very similar to the necessary AES0 changes, so maybe we can get that fixed too. I've basically hit a wall by only being able to passthrough 192khz stereo. When the passthrough data rate exceeds the available rate, my code clips the data packets. Which seems to work OK when the excess is maybe 200 bytes or so. On some HD DTS I'm clipping more like 2K per packet which results in some very nasty sounds emanating from the speakers. ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] review+pull-request: Passthrough support
So far I haven't been able to get anything except stereo working with passthrough. It seems to me that selecting multiple channels in passthrough mode would be very similar to the necessary AES0 changes, so maybe we can get that fixed too. I've basically hit a wall by only being able to passthrough 192khz stereo. When the passthrough data rate exceeds the available rate, my code clips the data packets. Which seems to work OK when the excess is maybe 200 bytes or so. On some HD DTS I'm clipping more like 2K per packet which results in some very nasty sounds emanating from the speakers. Looks like a know issue? I remember some comments/patches from Anssi that if you want to passthrough data at a rate of more than 6.144Mbit/s then yes you need to use AES0=0x06 so that the HMDI hardware uses HBR packets. -Pierre ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] review+pull-request: Passthrough support
On Tue, 2011-04-19 at 15:03 -0500, pl bossart wrote: I finally managed to figure out why eac3 wouldn't work in passthrough mode. Apparently my HDMI receiver will only lock on eac3 data if the AES0 control is set to 0x06 (non-audio, ie iec61937). ffmpeg -acodec copy -i csi_miami_5.1_256_spx.eac3 -f spdif outfile.pcm aplay -Dhdmi:AES0=0x6 -fs16 -c2 -r192000 outfile.pcm the same file plays well through pulseaudio if I modifed default.conf to [Mapping hdmi-stereo] device-strings = hdmi:%f,AES0=0x06 channel-map = left,right priority = 4 direction = output Somehow we need to find a way to set this AES0 byte when opening the iec958 or hdmi device in the passthrough code, and reset it back to 0x04 for PCM playback (should be fairly easy to do by looking at the code of iecset in alsa-utils). Beats me why this was not needed for plain ac3, but setting the audio bit is actually the correct thing to do. If my reading is correct, this is something we should do for all formats anyway? So far all the tests seem ok with the ffmpeg examples, with the exception of two files: - 7_pt_1: works well with ffmpeg, aplay but the sound is way too fast with pulseaudio. Lots of rewind messages seen in PulseAudio. - serenity_english_5.1_1536.eac3 file: no sound out, and again lots of rewind messages (the receiver still shows the D+ logo though). The same file converted with ffmpeg and played with aplay seems fine. looks like a buffering issue more than a payloader problem really. Is the ALSA device correctly being configured to 192 kHz? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] glib functions on PA
Hi, The initial implementation of jack detection is using threads and simple file operations like open, close and read currently I'm looking into using pa_threads and glib functions. First step It is simple enough e.g change threads by pa_threads and for file operations I have u-fd = g_open(u-device_id, O_RDONLY) ioch = g_io_channel_unix_new(u-fd); then use g_io_channel_read_chars() instead of read. I'm wonder if it is the beast approach or I'm missing any PA function that fits better for this purpose. Any comments are appreciated. Thanks, Margarita ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] glib functions on PA
On Wed, 2011-04-20 at 23:26 -0500, Margarita Olaya wrote: Hi, The initial implementation of jack detection is using threads and simple file operations like open, close and read currently I'm looking into using pa_threads and glib functions. First step It is simple enough e.g change threads by pa_threads and for file operations I have u-fd = g_open(u-device_id, O_RDONLY) ioch = g_io_channel_unix_new(u-fd); then use g_io_channel_read_chars() instead of read. I'm wonder if it is the beast approach or I'm missing any PA function that fits better for this purpose. I'm not aware of any PA code that uses glib for this. module-pipe-sink seems to do most of the things you need so that might serve as a good starting point. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] glib functions on PA
On Thu, 2011-04-21 at 10:13 +0530, Arun Raghavan wrote: On Wed, 2011-04-20 at 23:26 -0500, Margarita Olaya wrote: Hi, The initial implementation of jack detection is using threads and simple file operations like open, close and read currently I'm looking into using pa_threads and glib functions. First step It is simple enough e.g change threads by pa_threads and for file operations I have u-fd = g_open(u-device_id, O_RDONLY) ioch = g_io_channel_unix_new(u-fd); then use g_io_channel_read_chars() instead of read. I'm wonder if it is the beast approach or I'm missing any PA function that fits better for this purpose. I'm not aware of any PA code that uses glib for this. module-pipe-sink seems to do most of the things you need so that might serve as a good starting point. Actually, module-pipe-source is even closer to what you want to do. :) -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] RFC: Filter module loader
On Fri, 2011-04-15 at 20:55 +0200, Colin Guthrie wrote:
Also, I had to apply this patch to equalizer-sink to prevent it from
crashing the server on unload:
diff --git a/src/modules/module-equalizer-sink.c
b/src/modules/module-equalizer-sink.c
index 0bbb23a..611f7dd 100644
--- a/src/modules/module-equalizer-sink.c
+++ b/src/modules/module-equalizer-sink.c
@@ -1286,7 +1286,7 @@ void pa__done(pa_module*m) {
save_state(u);
-dbus_done(u);
+//dbus_done(u);
for(c = 0; c u-channels; ++c)
pa_xfree(u-base_profiles[c]);
I mentioned this in another thread... hopefully someone more familiar
with dbus can take a look at the underlying issue here :)
I took a look. The issue was that the dbus protocol used a string from
the equalizer module as a key to an internal hashmap, instead a copy of
the string. This caused trouble when the equalizer module freed the
string.
A patch is coming.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] protocol-dbus: Fix some memory management bugs.
There were several memory leaks. In addition to those,
pa_dbus_protocol_add_interface() used a string from the
caller as a key to a hashmap, instead of a copy of the
string. This caused trouble when the caller freed the
string while the key was still in use in the hashmap.
---
src/pulsecore/protocol-dbus.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/pulsecore/protocol-dbus.c b/src/pulsecore/protocol-dbus.c
index c329915..8784c34 100644
--- a/src/pulsecore/protocol-dbus.c
+++ b/src/pulsecore/protocol-dbus.c
@@ -742,7 +742,7 @@ int pa_dbus_protocol_add_interface(pa_dbus_protocol *p,
obj_entry-interfaces = pa_hashmap_new(pa_idxset_string_hash_func,
pa_idxset_string_compare_func);
obj_entry-introspection = NULL;
-pa_hashmap_put(p-objects, path, obj_entry);
+pa_hashmap_put(p-objects, obj_entry-path, obj_entry);
obj_entry_created = TRUE;
}
@@ -770,11 +770,6 @@ int pa_dbus_protocol_add_interface(pa_dbus_protocol *p,
return 0;
fail:
-if (obj_entry_created) {
-pa_hashmap_remove(p-objects, path);
-pa_xfree(obj_entry);
-}
-
return -1;
}
@@ -804,6 +799,7 @@ static void method_handler_free_cb(void *p, void *userdata)
{
}
pa_xfree((pa_dbus_arg_info *) h-arguments);
+pa_xfree(h);
}
static void method_signature_free_cb(void *p, void *userdata) {
@@ -945,6 +941,7 @@ static void signal_paths_entry_free(struct
signal_paths_entry *e) {
while ((path = pa_idxset_steal_first(e-paths, NULL)))
pa_xfree(path);
+pa_idxset_free(e-paths, NULL, NULL);
pa_xfree(e);
}
@@ -966,9 +963,12 @@ int
pa_dbus_protocol_unregister_connection(pa_dbus_protocol *p, DBusConnection *
while ((object_path =
pa_idxset_steal_first(conn_entry-all_signals_objects, NULL)))
pa_xfree(object_path);
+pa_idxset_free(conn_entry-all_signals_objects, NULL, NULL);
+
while ((signal_paths_entry =
pa_hashmap_steal_first(conn_entry-listening_signals)))
signal_paths_entry_free(signal_paths_entry);
+pa_hashmap_free(conn_entry-listening_signals, NULL, NULL);
pa_xfree(conn_entry);
return 0;
--
1.7.4.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] glib functions on PA
On Thu, 2011-04-21 at 10:13 +0530, Arun Raghavan wrote: On Wed, 2011-04-20 at 23:26 -0500, Margarita Olaya wrote: Hi, The initial implementation of jack detection is using threads and simple file operations like open, close and read currently I'm looking into using pa_threads and glib functions. First step It is simple enough e.g change threads by pa_threads and for file operations I have u-fd = g_open(u-device_id, O_RDONLY) ioch = g_io_channel_unix_new(u-fd); then use g_io_channel_read_chars() instead of read. I'm wonder if it is the beast approach or I'm missing any PA function that fits better for this purpose. I'm not aware of any PA code that uses glib for this. module-pipe-sink seems to do most of the things you need so that might serve as a good starting point. Also, the server side code doesn't use glib for anything - the glib dependency has been avoided on purpose. (I don't know or remember the details why Lennart has been avoiding glib - is it only to keep the amount of dependencies low, or are there some other reasons why PA shouldn't use glib.) -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
