[pulseaudio-discuss] [PATCH] Add a configure option to change 'udevrulesdir'
This patch serves two purposes: 1) Allows something other than the de-facto standard udev rules dir or /lib/udev/rules.d to be used (the udev build system allows you to customise this) 2) Allows a prefixed, non-root install (right now, the /lib/... path is hard-coded into the build system --- configure.ac|7 +++ src/Makefile.am |1 - 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index dfbd9bc..af60fff 100644 --- a/configure.ac +++ b/configure.ac @@ -1395,6 +1395,13 @@ AC_ARG_WITH( AC_SUBST(modlibexecdir) +AC_ARG_WITH( +[udev-rules-dir], +AS_HELP_STRING([--with-udev-rules-dir],[Directory where to install udev rules to (defaults to /lib/udev/rules.d)]), +[udevrulesdir=$withval], [udevrulesdir=/lib/udev/rules.d]) + +AC_SUBST(udevrulesdir) + AC_ARG_ENABLE( [force-preopen], AS_HELP_STRING([--enable-force-preopen],[Preopen modules, even when dlopen() is supported.]), diff --git a/src/Makefile.am b/src/Makefile.am index 3be2869..11826a4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -31,7 +31,6 @@ pulselibexecdir=$(libexecdir)/pulse xdgautostartdir=$(sysconfdir)/xdg/autostart alsaprofilesetsdir=$(datadir)/pulseaudio/alsa-mixer/profile-sets alsapathsdir=$(datadir)/pulseaudio/alsa-mixer/paths -udevrulesdir=/lib/udev/rules.d dbuspolicydir=$(sysconfdir)/dbus-1/system.d ### -- 1.6.4.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulseaudio in general - does it make sense?
On Tue, 2009-12-22 at 12:47 -0500, Gene Heskett wrote: [...] Coding is kewl, I do it myself on smaller systems, but where are the man pages that should allow us to make it Just Work(TM)? [...] I think the majority of us are interested. And contrary to rumors extant all over the web, the majority of us _can_ read and follow directions. We just don't have a thing to read yet. [...] Have you seen this: http://www.pulseaudio.org/wiki/PerfectSetup ? Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Metronome
On Mon, 2010-01-11 at 13:33 +0100, Stéphane List wrote: [...] It works well if my PC is already playing a MP3. But if my PC is silent, the 3 first ticks don't look very accurate. Any hint ? Sounds like this might be your problem: https://tango.0pointer.de/pipermail/pulseaudio-discuss/2009-December/005893.html Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Stream volumes as the universal volume adjustment method
On Thu, 2010-01-14 at 23:25 +0100, Lennart Poettering wrote: [...] 4 Put a bigger focus on automatically managed stream volumes. i.e. volume-follows-focus (Arun!), Whoops - kind of got off-tracked on this particular project. Last place I was stuck was how to get the window-state tracking goodness of libwnck without the GLib mainloop. Any ideas? I thinking a small standalone app which uses libwnck and throws out DBus events on the changes we'd like to see could do the trick. Will get on this soon. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] system-wide daemon
On Wed, 2010-02-10 at 11:16 -0500, Bill Cox wrote: Ok, the new glitchless code sounds cool. Reducing the interrupts seems close to pointless from a power savings view, unless we're in an embedded environment where we slow down the CPU and lower it's power on sub-second intervals. Otherwise, copying the data the data to the sound buffer will heavily dominate power. Why would the power concerns not apply on laptops and netbooks? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Crash/Underruns observed randomly when system alert is mixed with audio stream in Pulse Audio
Hello, On Wed, 2010-02-10 at 20:28 +0530, Lakshmi N V wrote: Hi, I'm using the Pulse Audio v0.9.21 with Moblin 2.1 OS on an embedded system. I've observed that when a audio stream is in progress through Pulse Audio and a system alert is generated through libcanberra-gtk-play, the system alert is mixed successfully for the first time, but in the second iteration, Pulse Audio either crashes or goes into continuous underruns. Similar issue is observed, when system alerts are generated continuously using libcanberra. As I understand, the underruns could be occuring because of latency added by software mixing in Pulse Audio. Is this a possibility? Has this behaviour been observed on any other platform? Are there any known issues with playback of short streams like alerts through Pulse Audio? Doesn't happen on my desktop with PA 0.9.21.1 and libcanberra 0.22. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] echocnzelation and feedback killer
On Sat, 2010-09-11 at 17:49 +0200, Alexey Fisher wrote: Hallo all, i see in git you working on echo cancelation. Will it work for audio feedback/Larsen effect too? I mean sort of feedback killer? It will not. The echo cancellation module is meant for things like voip calls. For example, if you're using speakers and a mic, whatever the other end says goes back through the mic, and they will be hear themselves after a short gap. That's the echo we're trying to cancel. So the goal is for the *far end* to not hear echo. It will not make a difference locally. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 0/5] volume: Decrease PA_VOLUME_MAX to be 2^31
Hey folks, Here's a patchset to change PA_VOLUME_MAX to (2^31-1), which is about half its current value. This really should not impact anything significantly, since the maximum gain we can apply decreases from ~289 dB to ~271 dB. Why this change is good is that we can significantly simplify the software volume scaling arithmetic, since the volume can always be treated as a signed number. I am working on rewriting the volume scaling in Orc, and this would make that considerably simpler (and more fruitful, since we'd have to jump through hoops to deal with volumes = 2^31 while doing signed multiplication). If we choose to retain the old hand-optimised assembly, that should also benefit from this change. I have intentionally not revised the protocol version since I think it is fair to consider the actual value of PA_VOLUME_MAX as internal, but I'm open to being corrected if this is not true. I would actually like to decrease the maximum volume to a much saner value, but I will start that discussion in a separate thread. Comments/brickbats welcome. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 1/5] volume: Use a macro to check if a volume is valid
This adds a PA_VOLUME_IS_VALID() macro for checking if a given pa_volume_t is valid. This makes changes to the volume ranges simpler (just change PA_VOLUME_MAX, for example, without needing to modify any other code). --- src/modules/dbus/iface-core.c |2 +- src/modules/dbus/iface-device.c |2 +- src/modules/dbus/iface-sample.c |4 ++-- src/modules/dbus/iface-stream.c |2 +- src/modules/module-stream-restore.c |2 +- src/pulse/scache.c |4 ++-- src/pulse/volume.c | 34 +- src/pulse/volume.h |3 +++ src/pulsecore/core-scache.c |4 ++-- 9 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c index e7c00a7..268a87e 100644 --- a/src/modules/dbus/iface-core.c +++ b/src/modules/dbus/iface-core.c @@ -1310,7 +1310,7 @@ static void handle_upload_sample(DBusConnection *conn, DBusMessage *msg, void *u } for (i = 0; i n_volume_entries; ++i) { -if (default_volume[i] PA_VOLUME_MAX) { +if (!PA_VOLUME_IS_VALID(default_volume[i])) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume: %u., default_volume[i]); goto finish; } diff --git a/src/modules/dbus/iface-device.c b/src/modules/dbus/iface-device.c index bb91d71..a8652df 100644 --- a/src/modules/dbus/iface-device.c +++ b/src/modules/dbus/iface-device.c @@ -438,7 +438,7 @@ static void handle_set_volume(DBusConnection *conn, DBusMessage *msg, DBusMessag } for (i = 0; i n_volume_entries; ++i) { -if (volume[i] PA_VOLUME_MAX) { +if (!PA_VOLUME_IS_VALID(volume[i])) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Too large volume value: %u, volume[i]); return; } diff --git a/src/modules/dbus/iface-sample.c b/src/modules/dbus/iface-sample.c index c1fa193..2381079 100644 --- a/src/modules/dbus/iface-sample.c +++ b/src/modules/dbus/iface-sample.c @@ -366,7 +366,7 @@ static void handle_play(DBusConnection *conn, DBusMessage *msg, void *userdata) if (!(property_list = pa_dbus_get_proplist_arg(conn, msg, msg_iter))) return; -if (volume PA_VOLUME_MAX) { +if (PA_VOLUME_IS_VALID(volume)) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume.); goto finish; } @@ -416,7 +416,7 @@ static void handle_play_to_sink(DBusConnection *conn, DBusMessage *msg, void *us goto finish; } -if (volume PA_VOLUME_MAX) { +if (PA_VOLUME_IS_VALID(volume)) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume.); goto finish; } diff --git a/src/modules/dbus/iface-stream.c b/src/modules/dbus/iface-stream.c index 0255be4..364572b 100644 --- a/src/modules/dbus/iface-stream.c +++ b/src/modules/dbus/iface-stream.c @@ -378,7 +378,7 @@ static void handle_set_volume(DBusConnection *conn, DBusMessage *msg, DBusMessag } for (i = 0; i n_volume_entries; ++i) { -if (volume[i] PA_VOLUME_MAX) { +if (PA_VOLUME_IS_VALID(volume[i])) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Too large volume value: %u, volume[i]); return; } diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index 346b6ad..5ce1c41 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -367,7 +367,7 @@ static int get_volume_arg(DBusConnection *conn, DBusMessage *msg, DBusMessageIte pa_assert_se(dbus_message_iter_next(struct_iter)); dbus_message_iter_get_basic(struct_iter, chan_vol); -if (chan_vol PA_VOLUME_MAX) { +if (PA_VOLUME_IS_VALID(chan_vol)) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume: %u, chan_vol); return -1; } diff --git a/src/pulse/scache.c b/src/pulse/scache.c index b2169b6..cb8d7c5 100644 --- a/src/pulse/scache.c +++ b/src/pulse/scache.c @@ -191,7 +191,7 @@ pa_operation *pa_context_play_sample(pa_context *c, const char *name, const char pa_tagstruct_putu32(t, PA_INVALID_INDEX); pa_tagstruct_puts(t, dev); -if (volume == PA_VOLUME_INVALID c-version 15) +if (!PA_VOLUME_IS_VALID(volume) c-version 15) volume = PA_VOLUME_NORM; pa_tagstruct_putu32(t, volume); @@ -232,7 +232,7 @@ pa_operation *pa_context_play_sample_with_proplist(pa_context *c, const char *na pa_tagstruct_putu32(t, PA_INVALID_INDEX); pa_tagstruct_puts(t, dev); -if (volume == PA_VOLUME_INVALID c-version 15) +if (!PA_VOLUME_IS_VALID(volume) c-version 15) volume = PA_VOLUME_NORM; pa_tagstruct_putu32(t, volume); diff --git a/src/pulse/volume.c b/src/pulse/volume.c index 59e9a18..7d4951b 100644 --- a/src/pulse/volume.c +++ b/src/pulse/volume.c @@ -201,8 +201,8
[pulseaudio-discuss] [PATCH 2/5] volume: Clamp volume to PA_VOLUME_MAX
This ensures that we always clamp the volume to PA_VOLUME_MAX. While this currently has no effect, it will be required for making sure we don't exceed PA_VOLUME_MAX when its value changes in the future. --- src/modules/module-lirc.c|6 +++--- src/modules/module-match.c |2 +- src/modules/module-mmkbd-evdev.c |4 ++-- src/modules/module-waveout.c |4 ++-- src/modules/oss/oss-util.c |4 ++-- src/pulse/volume.c | 20 +++- src/pulse/volume.h |3 +++ 7 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/modules/module-lirc.c b/src/modules/module-lirc.c index e977862..15f3442 100644 --- a/src/modules/module-lirc.c +++ b/src/modules/module-lirc.c @@ -172,7 +172,7 @@ fail: int pa__init(pa_module*m) { pa_modargs *ma = NULL; struct userdata *u; -pa_volume_t volume_limit = PA_VOLUME_NORM*3/2; +pa_volume_t volume_limit = PA_CLAMP_VOLUME(PA_VOLUME_NORM*3/2); pa_volume_t volume_step = PA_VOLUME_NORM/20; pa_assert(m); @@ -199,8 +199,8 @@ int pa__init(pa_module*m) { u-sink_name = pa_xstrdup(pa_modargs_get_value(ma, sink, NULL)); u-lirc_fd = -1; u-mute_toggle_save = 0; -u-volume_limit = volume_limit; -u-volume_step = volume_step; +u-volume_limit = PA_CLAMP_VOLUME(volume_limit); +u-volume_step = PA_CLAMP_VOLUME(volume_step); if ((u-lirc_fd = lirc_init((char*) pa_modargs_get_value(ma, appname, pulseaudio), 1)) 0) { pa_log(lirc_init() failed.); diff --git a/src/modules/module-match.c b/src/modules/module-match.c index b1693f1..d83c86c 100644 --- a/src/modules/module-match.c +++ b/src/modules/module-match.c @@ -127,7 +127,7 @@ static int load_rules(struct userdata *u, const char *filename) { *d = 0; if (pa_atou(v, k) = 0) { -volume = (pa_volume_t) k; +volume = (pa_volume_t) PA_CLAMP_VOLUME(k); } else if (*v == '') { char *e; diff --git a/src/modules/module-mmkbd-evdev.c b/src/modules/module-mmkbd-evdev.c index 193c1f4..4e89aed 100644 --- a/src/modules/module-mmkbd-evdev.c +++ b/src/modules/module-mmkbd-evdev.c @@ -188,8 +188,8 @@ int pa__init(pa_module*m) { u-sink_name = pa_xstrdup(pa_modargs_get_value(ma, sink, NULL)); u-fd = -1; u-fd_type = 0; -u-volume_limit = volume_limit; -u-volume_step = volume_step; +u-volume_limit = PA_CLAMP_VOLUME(volume_limit); +u-volume_step = PA_CLAMP_VOLUME(volume_step); if ((u-fd = pa_open_cloexec(pa_modargs_get_value(ma, device, DEFAULT_DEVICE), O_RDONLY, 0)) 0) { pa_log(Failed to open evdev device: %s, pa_cstrerror(errno)); diff --git a/src/modules/module-waveout.c b/src/modules/module-waveout.c index d1b9f2f..6fedceb 100644 --- a/src/modules/module-waveout.c +++ b/src/modules/module-waveout.c @@ -359,8 +359,8 @@ static int sink_get_hw_volume_cb(pa_sink *s) { if (waveOutGetVolume(u-hwo, vol) != MMSYSERR_NOERROR) return -1; -left = (vol 0x) * PA_VOLUME_NORM / WAVEOUT_MAX_VOLUME; -right = ((vol 16) 0x) * PA_VOLUME_NORM / WAVEOUT_MAX_VOLUME; +left = PA_CLAMP_VOLUME((vol 0x) * PA_VOLUME_NORM / WAVEOUT_MAX_VOLUME); +right = PA_CLAMP_VOLUME(((vol 16) 0x) * PA_VOLUME_NORM / WAVEOUT_MAX_VOLUME); /* Windows supports 2 channels, except for volume control */ if (s-hw_volume.channels 2) diff --git a/src/modules/oss/oss-util.c b/src/modules/oss/oss-util.c index b95023c..966a6ca 100644 --- a/src/modules/oss/oss-util.c +++ b/src/modules/oss/oss-util.c @@ -271,10 +271,10 @@ int pa_oss_get_volume(int fd, unsigned long mixer, const pa_sample_spec *ss, pa_ pa_cvolume_reset(volume, ss-channels); -volume-values[0] = ((vol 0xFF) * PA_VOLUME_NORM) / 100; +volume-values[0] = PA_CLAMP_VOLUME(((vol 0xFF) * PA_VOLUME_NORM) / 100); if (volume-channels = 2) -volume-values[1] = (((vol 8) 0xFF) * PA_VOLUME_NORM) / 100; +volume-values[1] = PA_CLAMP_VOLUMEvol 8) 0xFF) * PA_VOLUME_NORM) / 100); pa_log_debug(Read mixer settings: %s, pa_cvolume_snprint(cv, sizeof(cv), volume)); return 0; diff --git a/src/pulse/volume.c b/src/pulse/volume.c index 7d4951b..f74d720 100644 --- a/src/pulse/volume.c +++ b/src/pulse/volume.c @@ -79,7 +79,9 @@ pa_cvolume* pa_cvolume_set(pa_cvolume *a, unsigned channels, pa_volume_t v) { a-channels = (uint8_t) channels; for (i = 0; i a-channels; i++) -a-values[i] = v; +/* Clamp in case there is stale data that exceeds the current + * PA_VOLUME_MAX */ +a-values[i] = PA_CLAMP_VOLUME(v); return a; } @@ -206,7 +208,7 @@ pa_volume_t pa_sw_volume_multiply(pa_volume_t a, pa_volume_t b) { /* cbrt((a/PA_VOLUME_NORM)^3*(b/PA_VOLUME_NORM)^3)*PA_VOLUME_NORM = a*b/PA_VOLUME_NORM */ -return (pa_volume_t) (((uint64_t) a * (uint64_t) b + (uint64_t) PA_VOLUME_NORM / 2ULL) / (uint64_t) PA_VOLUME_NORM); +return
[pulseaudio-discuss] [PATCH 3/5] cli: Validate volume before setting
This causes an error to be generated if an invalid volume is provided to commands that set sink/sink-input/source volume. --- src/pulsecore/cli-command.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c index d23331d..a18ebd3 100644 --- a/src/pulsecore/cli-command.c +++ b/src/pulsecore/cli-command.c @@ -527,6 +527,11 @@ static int pa_cli_command_sink_volume(pa_core *c, pa_tokenizer *t, pa_strbuf *bu return -1; } +if (!PA_VOLUME_IS_VALID(volume)) { +pa_strbuf_puts(buf, Volume outside permissible range.\n); +return -1; +} + if (!(sink = pa_namereg_get(c, n, PA_NAMEREG_SINK))) { pa_strbuf_puts(buf, No sink found by this name or index.\n); return -1; @@ -569,6 +574,11 @@ static int pa_cli_command_sink_input_volume(pa_core *c, pa_tokenizer *t, pa_strb return -1; } +if (!PA_VOLUME_IS_VALID(volume)) { +pa_strbuf_puts(buf, Volume outside permissible range.\n); +return -1; +} + if (!(si = pa_idxset_get_by_index(c-sink_inputs, (uint32_t) idx))) { pa_strbuf_puts(buf, No sink input found with this index.\n); return -1; @@ -605,6 +615,11 @@ static int pa_cli_command_source_volume(pa_core *c, pa_tokenizer *t, pa_strbuf * return -1; } +if (!PA_VOLUME_IS_VALID(volume)) { +pa_strbuf_puts(buf, Volume outside permissible range.\n); +return -1; +} + if (!(source = pa_namereg_get(c, n, PA_NAMEREG_SOURCE))) { pa_strbuf_puts(buf, No source found by this name or index.\n); return -1; -- 1.7.3.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 4/5] pactl: Validate volume before setting
This makes sure that a valid volume is provided before setting on sink/sink-input/source. --- src/utils/pactl.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/src/utils/pactl.c b/src/utils/pactl.c index cae96f2..98c4d45 100644 --- a/src/utils/pactl.c +++ b/src/utils/pactl.c @@ -1248,6 +1248,11 @@ int main(int argc, char *argv[]) { goto quit; } +if (!PA_VOLUME_IS_VALID(v)) { +pa_log(_(Volume outside permissible range.\n)); +goto quit; +} + sink_name = pa_xstrdup(argv[optind+1]); volume = (pa_volume_t) v; @@ -1265,6 +1270,11 @@ int main(int argc, char *argv[]) { goto quit; } +if (!PA_VOLUME_IS_VALID(v)) { +pa_log(_(Volume outside permissible range.\n)); +goto quit; +} + source_name = pa_xstrdup(argv[optind+1]); volume = (pa_volume_t) v; @@ -1287,6 +1297,11 @@ int main(int argc, char *argv[]) { goto quit; } +if (!PA_VOLUME_IS_VALID(v)) { +pa_log(_(Volume outside permissible range.\n)); +goto quit; +} + volume = (pa_volume_t) v; } else if (pa_streq(argv[optind], set-sink-mute)) { -- 1.7.3.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 5/5] volume: Decrease PA_VOLUME_MAX to be 2^31
This decrease PA_VOLUME_MAX to be less than 2^31. We want to do this in order to simplify signed arithmetic when applying software volume scaling (since the volume can now always be safely treated as a signed number). --- src/pulse/volume.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/pulse/volume.h b/src/pulse/volume.h index 5c5e7d2..8f959c2 100644 --- a/src/pulse/volume.h +++ b/src/pulse/volume.h @@ -110,7 +110,7 @@ typedef uint32_t pa_volume_t; #define PA_VOLUME_MUTED ((pa_volume_t) 0U) /** Maximum valid volume we can store. \since 0.9.15 */ -#define PA_VOLUME_MAX ((pa_volume_t) UINT32_MAX-1) +#define PA_VOLUME_MAX ((pa_volume_t) UINT32_MAX/2) /** Special 'invalid' volume. \since 0.9.16 */ #define PA_VOLUME_INVALID ((pa_volume_t) UINT32_MAX) -- 1.7.3.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] volume: Decrease PA_VOLUME_MAX to be 2^31
On Sun, 2010-10-10 at 10:19 +0200, David Henningsson wrote: On 2010-10-09 20:27, Arun Raghavan wrote: Hey folks, Here's a patchset to change PA_VOLUME_MAX to (2^31-1), which is about half its current value. This really should not impact anything significantly, since the maximum gain we can apply decreases from ~289 dB to ~271 dB. Why this change is good is that we can significantly simplify the software volume scaling arithmetic, since the volume can always be treated as a signed number. I am working on rewriting the volume scaling in Orc, Could you elaborate on this? Is Orc a programming language? Are you saying that native PulseAudio should only compile on compilers that have Orc support? Ah, sorry, I should've provided more background. Orc is a programming language which allows you to write simple programs to optimise inner loops that you might otherwise hand-optimise with MMX/SSE/NEON. The idea is that you write an Orc program for the inner loop, and the Orc library will, at runtime, generate assembly for the architecture you're running on. More information at http://code.entropywave.com/projects/orc/ I've actually already added some Orc optimisations to the echo cancellation module (see src/modules/echo-cancel/adrian-aec-orc.orc) and this would make that considerably simpler (and more fruitful, since we'd have to jump through hoops to deal with volumes= 2^31 while doing signed multiplication). If we choose to retain the old hand-optimised assembly, that should also benefit from this change. I'm not exactly sure where and for what PA_VOLUME_MAX is used, but does it correspond to 0 dB in any way? Thinking assembly, could it be that we have some e g fixed-point arithmetic that we must compensate? Colin covered this already. Only thing I have to add is that the volume is, in fact, treated as a fixed-point number with the lower 16-bits being the fractional part. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] volume: Decrease PA_VOLUME_MAX to be 2^31
On Sun, 2010-10-10 at 10:46 +0100, Colin Guthrie wrote: [...] I'd also like to expose a new constant in the headers called PA_VOLUME_OVERDRIVE (or similarly named) that represents +11dB and should be our generally recommended overdrive (aka software amplification 100%) amount for volume control UIs. Currently gnome-volume-control allows up to PA_VOLUME_NORM*1.5 which is ~= +11dB via current mappings, which is one reason why we previously decided on +11dB as our overdrive amount. That said, other volume control UIs (pavucontrol, kmix and, surprisingly, gnome-volume-control-applet) only allow volumes up to PA_VOLUME_NORM, so the idea would be to allow them to go right up to PA_VOLUME_OVERDRIVE, and hopefully implement an appropriate use of colours and markers etc. to make it clear to the user that they may not want to go 100% or risk clipping. http://pulseaudio.org/wiki/WritingVolumeControlUIs#Colouredvolumesliders Sounds reasonable. Although, independent of the rest of these changes, I'd also like to propose that the max. s/w gain (i.e. PA_VOLUME_MAX) be decreased from its current value (~289 dB now, ~271 dB with my change) to something more reasonable, like ~20 dB. I came to this conclusion the hard way - while testing these patches, I managed to turn up the volume high enough to temporarily damage my hearing (it was a sudden burst, and I got to the off switch in time, so no permanent damage done, I hope). I understand that some overdrive is useful in general, but are there any objections to capping the max. to something like ~20 dB? Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] volume: Decrease PA_VOLUME_MAX to be 2^31
On Sun, 2010-10-10 at 13:03 +0100, Colin Guthrie wrote: 'Twas brillig, and Arun Raghavan at 10/10/10 11:30 did gyre and gimble: On Sun, 2010-10-10 at 10:46 +0100, Colin Guthrie wrote: [...] I'd also like to expose a new constant in the headers called PA_VOLUME_OVERDRIVE (or similarly named) that represents +11dB and should be our generally recommended overdrive (aka software amplification 100%) amount for volume control UIs. Currently gnome-volume-control allows up to PA_VOLUME_NORM*1.5 which is ~= +11dB via current mappings, which is one reason why we previously decided on +11dB as our overdrive amount. That said, other volume control UIs (pavucontrol, kmix and, surprisingly, gnome-volume-control-applet) only allow volumes up to PA_VOLUME_NORM, so the idea would be to allow them to go right up to PA_VOLUME_OVERDRIVE, and hopefully implement an appropriate use of colours and markers etc. to make it clear to the user that they may not want to go 100% or risk clipping. http://pulseaudio.org/wiki/WritingVolumeControlUIs#Colouredvolumesliders Sounds reasonable. Although, independent of the rest of these changes, I'd also like to propose that the max. s/w gain (i.e. PA_VOLUME_MAX) be decreased from its current value (~289 dB now, ~271 dB with my change) to something more reasonable, like ~20 dB. I came to this conclusion the hard way - while testing these patches, I managed to turn up the volume high enough to temporarily damage my hearing (it was a sudden burst, and I got to the off switch in time, so no permanent damage done, I hope). Urk! I'll try and type quietly... :D I understand that some overdrive is useful in general, but are there any objections to capping the max. to something like ~20 dB? I think that imposing a more sensible limit probably makes sense. I can't think of any practical reasons to push things to dB's 20 ish so on the whole I'd be in favour of a more sensible upper limit. Quite what that limit should be however is perhaps debatable so other opinions should maybe be canvassed to get the final 20dB upper limit. Right, I'm pretty open to any sane limit - just want to establish what that limit is. :) Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Broken Volume Control
On Mon, 2010-10-18 at 16:43 -0500, pl bossart wrote: Looks like the volume control logic has taken a big hit, when I move the volume up and down with the slider on the Output Devices tab in pavucontrol, I have _huge_ pops and clicks, the volume goes to zero in the playback tab. I don't get the pops and clicks here but I do see the volume going to zero. Git bisect flags the following patch and I manually confirmed that this patch introduces the noises. Arun, can you look into this? I noticed that there are some inconsistencies on the use of !PA_VOLUME_IS_VALID, sometimes the ! is missing. I'm attaching a patch for this. It solved the volume-to-zero problem for me. Could you see if the pops and clicks go away too? Thanks, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] volume: Fix incorrect usage of PA_VOLUME_IS_VALID
The commit that introduced this macro was incorrect in some places. This patch fixes these. Thanks to Pierre-Louis Bossart for pointing this out. --- src/modules/dbus/iface-sample.c |4 ++-- src/modules/dbus/iface-stream.c |2 +- src/modules/module-stream-restore.c |2 +- src/pulsecore/core-scache.c |4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/modules/dbus/iface-sample.c b/src/modules/dbus/iface-sample.c index 2381079..93d4fc8 100644 --- a/src/modules/dbus/iface-sample.c +++ b/src/modules/dbus/iface-sample.c @@ -366,7 +366,7 @@ static void handle_play(DBusConnection *conn, DBusMessage *msg, void *userdata) if (!(property_list = pa_dbus_get_proplist_arg(conn, msg, msg_iter))) return; -if (PA_VOLUME_IS_VALID(volume)) { +if (!PA_VOLUME_IS_VALID(volume)) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume.); goto finish; } @@ -416,7 +416,7 @@ static void handle_play_to_sink(DBusConnection *conn, DBusMessage *msg, void *us goto finish; } -if (PA_VOLUME_IS_VALID(volume)) { +if (!PA_VOLUME_IS_VALID(volume)) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume.); goto finish; } diff --git a/src/modules/dbus/iface-stream.c b/src/modules/dbus/iface-stream.c index 364572b..df00f0e 100644 --- a/src/modules/dbus/iface-stream.c +++ b/src/modules/dbus/iface-stream.c @@ -378,7 +378,7 @@ static void handle_set_volume(DBusConnection *conn, DBusMessage *msg, DBusMessag } for (i = 0; i n_volume_entries; ++i) { -if (PA_VOLUME_IS_VALID(volume[i])) { +if (!PA_VOLUME_IS_VALID(volume[i])) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Too large volume value: %u, volume[i]); return; } diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index 5ce1c41..37ab306 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -367,7 +367,7 @@ static int get_volume_arg(DBusConnection *conn, DBusMessage *msg, DBusMessageIte pa_assert_se(dbus_message_iter_next(struct_iter)); dbus_message_iter_get_basic(struct_iter, chan_vol); -if (PA_VOLUME_IS_VALID(chan_vol)) { +if (!PA_VOLUME_IS_VALID(chan_vol)) { pa_dbus_send_error(conn, msg, DBUS_ERROR_INVALID_ARGS, Invalid volume: %u, chan_vol); return -1; } diff --git a/src/pulsecore/core-scache.c b/src/pulsecore/core-scache.c index cd388ef..5ec6159 100644 --- a/src/pulsecore/core-scache.c +++ b/src/pulsecore/core-scache.c @@ -336,12 +336,12 @@ int pa_scache_play_item(pa_core *c, const char *name, pa_sink *sink, pa_volume_t pass_volume = TRUE; -if (e-volume_is_set !PA_VOLUME_IS_VALID(volume)) { +if (e-volume_is_set PA_VOLUME_IS_VALID(volume)) { pa_cvolume_set(r, e-sample_spec.channels, volume); pa_sw_cvolume_multiply(r, r, e-volume); } else if (e-volume_is_set) r = e-volume; -else if (!PA_VOLUME_IS_VALID(volume)) +else if (PA_VOLUME_IS_VALID(volume)) pa_cvolume_set(r, e-sample_spec.channels, volume); else pass_volume = FALSE; -- 1.7.3.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] New dependency: Orc
Hi folks, I've been doing some work optimising the software volume scaling code, and along with my previous changes to decrease the maximum volume to 2^31-1, there seems to be a pretty good performance increase (almost 2x on my Core2 processor). The actual optimisations have been written in Orc[1], which is a language to write simple functions that get translated to SIMD instructions at runtime. I should have sent this out a while back, since we're actually using Orc for one of the echo-cancellation modules that was merged to master, but now that there could be core code using this, I thought I'd get more thoughts on making Orc an optional dependency of PulseAudio. The way I've written things right now, the old C and hand-rolled assembly is still there. Only when Orc support is enabled, and we're on a CPU where the Orc code is known to be faster, we use the Orc code. I've only written the mono and stereo S16NE functions so far, so for other formats, the old code is used. If you don't have or don't want to use Orc, it can be disabled at configure time (--disable-orc). If you do enable it, there are a couple of generated files generate for each Orc source program. These actually even contain C fallback for when the system you're on doesn't have Orc or that Orc doesn't have a backend for. At some point, if the fallback C code and the Orc functions become good enough to replace everything else, we can look at just using these to replace all the other implementations. That day isn't today, though. :) The code is at: http://git.collabora.co.uk/?p=user/arun/pulseaudio.git - there are also some fixes to the various volume scaling test code. Comments/brickbats solicited :) Cheers, Arun [1] http://code.entropywave.com/projects/orc/ p.s.: I've not tried out the Orc code on ARM (with or without NEON), so if anyone wants to give that a whirl before I get around to it, please do posts the results here. The hand-rolled should be faster for now since it uses a single instruction for the multiply+shift operation. ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] New dependency: Orc
On Wed, 2010-10-27 at 15:14 -0500, pl bossart wrote: I've been doing some work optimising the software volume scaling code, and along with my previous changes to decrease the maximum volume to 2^31-1, there seems to be a pretty good performance increase (almost 2x on my Core2 processor). Are you saying you have a 2x performance gain over sse assembly? That would most likely mean we need to fix the assembly for x86 and have an even better performance than with orc and its intermediate step of SIMD code generation... That is what I got even when I replaced the 32x16-bit volume multiplication code with the same logic that I'm using in Orc. I don't claim to be any good with SSE/MMX-fu, so it's likely we can do better with hand-rolled code in most cases. The SIMD code-generation happens on the first call (and when Orc supports it, will only happen at init time), so that should relly not be a concern. Howeve, IMO, it makes sense to switch if the performance gain of the hand-rolled code isn't very significant (because the Orc code really is far more maintainable), and with time Orc should be able to do a better job of worrying about minor differences between the various x86/x86_64 architectures, instruction scheduling differences, etc. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] New dependency: Orc
On Thu, 2010-10-28 at 01:47 +0100, Arun Raghavan wrote: On Wed, 2010-10-27 at 15:14 -0500, pl bossart wrote: I've been doing some work optimising the software volume scaling code, and along with my previous changes to decrease the maximum volume to 2^31-1, there seems to be a pretty good performance increase (almost 2x on my Core2 processor). Are you saying you have a 2x performance gain over sse assembly? That would most likely mean we need to fix the assembly for x86 and have an even better performance than with orc and its intermediate step of SIMD code generation... That is what I got even when I replaced the 32x16-bit volume multiplication code with the same logic that I'm using in Orc. I don't I forgot to mention that even the Orc MMX backend provides the same kind of perf gain over the current hand-rolled code (I didn't try to rewrite that like I did the SSE). Also, we don't have any NEON optimisations for the s/w volume stuff in PA, so the Orc NEON backend might be interesting to try there. I don't know if there's a SIMD version of the ARM 32x16-bit mul, but if there is, it's possible to get Orc to use that as well. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] New dependency: Orc
On Wed, 2010-10-27 at 15:10 +0100, Arun Raghavan wrote: [...] Comments/brickbats solicited :) Hopefully I've addressed Pierre's concerns. Does other objections? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] New dependency: Orc
On Thu, 2010-11-18 at 20:53 +0530, Arun Raghavan wrote: On Wed, 2010-10-27 at 15:10 +0100, Arun Raghavan wrote: [...] Comments/brickbats solicited :) Hopefully I've addressed Pierre's concerns. Does other objections? D'oh! s/Does/Any/ -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [ANNOUNCE] New Version Naming Scheme
On Sun, 2010-11-28 at 15:31 +, Colin Guthrie wrote: [...] So if we only had a single version number, the same problems would be apparent with our current version conundrum. As a compromise for simplifying things and for getting a sensible version tag pushed to master, we decided to adopt a major.minor version scheme. By following this principle, this means that master will eventually become our v1.0. This means that master will be tagged as v1.0-dev version soon and the necessary changes to the build system will be made to suit this. To keep backwards compatibility, PA_MICRO will still be defined in version.h, but will always be 0. When a new major release is rolled, the tag is pushed to the developers local git repository and the tarballs are generated (the tag needs to be in place for this to work). When all is tested, the tag will be pushed and the tarballs published. A new branch will also be created called $MAJOR.x-stable (the name is up for discussion, but I think it's sensible). Whenever the next commit is made to master, it will be tagged as $(($MAJOR+1)).0-dev. This allows the correct version number to be represented in builds made from the git tree. Likewise, when a commit is pushed to $MAJOR.x-stable branch it will be tagged as v$MAJOR.1-dev, again to indicate the version more accurately in builds. If/when we release a bugfix update from the $MAJOR.x-stable branch, then we will tag it as v$MAJOR.1 with the next commit after release getting the v$MAJOR.2-dev tag and so on for any bugfix releases we make. So with the above policy, after each official version tags, the next commit should always have a new version tag with the -dev suffix. I believe this method of working will be quite clear and also follows quite closely the general approach we had with stable-queue, but with a simplified naming scheme and version tag policy on top. I might be missing something, but this scheme seems to preclude intermediate releases off the -dev branch (since the generated tarball will always have a $MAJOR.0 version). Or would that be solvable by pushing a $MAJOR.0_beta/_rc tag to git? I guess we /would/ want to make some pre-release(s) off the 1.0-dev before doing the final 1.0 release. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] volume: 20 dB should be enough for anyone
This limits the maximum gain we perform in software to approximately 20 dB. There does not appear to be a good reason for greater gain, and clipping to 20 dB can prevent accidentally setting an absurdly large volume and potential damage to the ears. --- src/pulse/volume.h |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/pulse/volume.h b/src/pulse/volume.h index 35128b6..d646a7b 100644 --- a/src/pulse/volume.h +++ b/src/pulse/volume.h @@ -113,7 +113,8 @@ typedef uint32_t pa_volume_t; #define PA_VOLUME_MUTED ((pa_volume_t) 0U) /** Maximum valid volume we can store. \since 0.9.15 */ -#define PA_VOLUME_MAX ((pa_volume_t) UINT32_MAX/2) +/* This value corresponds to ~20 dB (of software amplification) */ +#define PA_VOLUME_MAX ((pa_volume_t) 141193) /** Special 'invalid' volume. \since 0.9.16 */ #define PA_VOLUME_INVALID ((pa_volume_t) UINT32_MAX) -- 1.7.3.2 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] volume: 20 dB should be enough for anyone
On Sun, 2010-12-05 at 19:47 +0530, Arun Raghavan wrote: This limits the maximum gain we perform in software to approximately 20 dB. There does not appear to be a good reason for greater gain, and clipping to 20 dB can prevent accidentally setting an absurdly large volume and potential damage to the ears. This was discussed earlier when, but just to reiterate, I'm not too specific about what the limit is as long as there is a sane limit. So if you have any objections, please speak up now. :) Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Resampling and format conversion question
On Thu, 2010-12-09 at 16:02 -0600, abraham duenas wrote: Hello, I have one question regarding resampler and working format. I have a card which has 48kHz working sampling frequency and s16le format. When I play any file at that frequency my understanding is that no resampling should take place. I can see on PA logs these: resampler.c: Forcing resampler 'copy', because of fixed, identical sample rates. resampler.c: Using resampler 'copy' resampler.c: Using float32le as working format. sink-input.c: Created input 3 Playback Stream on input with sample spec s16le 2ch 48000Hz and channel map front-left,front-right In this 'copy' case what is the resampler.c: Using float32le as working format for? Is it changing my s16le to float32le? Any help would be really appreciated :) A lot of cards do 44100 and 48000 Hz, and PulseAudio selects 44100 Hz by default (configurable in /etc/pulse/daemon.conf). Are you sure this is not the case for you? Also, is your card set up for stereo output? Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] trivial: Mention speex as a resampler reference in pulse-daemon.conf
On Sat, 2010-12-18 at 00:03 -0500, Daniel Chen wrote: Hi, While triaging the Debian bugs for PulseAudio, I noticed this trivially-fixed item. Here's a patch against stable-queue/master. I think this is incorrect - the sentence basically said the src-* resampling methods are provided by libsamplerate - for more details, look at the libsamplerate documentation. We could rephrase as: See the documentation of libsamplerate and speex for explanations of the different src- and speex- methods. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Dynamic reconfiguration of sampling rate
On Mon, 2011-01-17 at 14:24 -0600, pl bossart wrote: [...] I can see cases where you have 1 compressed stream and 1 PCM, and you mix the two in hardware, but I am having a really hard time finding a use case where you would have multiple (more than 2) PCM streams at different rates. Maybe automotive cases, where the infotainment unit might send multiple streams to a head unit were the mixing/routing is actually done? Did anyone request hardware mixing on the mailing list? This is something we often hear about from people with Creative SoundBlaster Live/Audigy cards and their ilk, which support hardware mixing. To some extent, the complaint is reasonable, since we're forcing resampling on CPU even though this can be off-loaded to the soundcard. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Coding style question regarding curly braces
On Thu, 2011-01-20 at 15:03 +0200, Tanu Kaskinen wrote: Hi, When reviewing code, should I complain about excessive curly brackets in code like this? if (foo) { bar = 0; } Pulseaudio source code follows quite consistently the unwritten rule that you shouldn't use curly braces unless necessary, so I was surprised to notice that the unwritten rule was really unwritten (ie. not mentioned in http://pulseaudio.org/wiki/CodingStyle). So, should I add the rule to that page, or should I stop caring about this? I vote for making the convention explicit, but then I'm biased since I prefer not putting braces around one-line conditionals. :) Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Orc Question
Hey Colin, On Sun, 2011-01-23 at 14:17 +, Colin Guthrie wrote: Hi, This is likely directed to Arun, but others may know too! When I run a make distcheck and probably other builds too, I get this in my checkout: # Changed but not updated: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: src/modules/echo-cancel/adrian-aec-orc-dist.c # modified: src/modules/echo-cancel/adrian-aec-orc-dist.h # If these files are auto-generated, should they be shipped in git or left to be generated as part of the build process? You're right, those shouldn't be in git. This slipped my mind in the rest of the Orc + svolume discussion earlier. I've already cleaned up the build bits for Orc quite a bit - it's in my orc branch [1], so even if the volume bits need to wait to go in, the 3 build cleanups before that could go in now. Cheers, Arun [1] http://git.collabora.co.uk/?p=user/arun/pulseaudio.git;a=shortlog;h=refs/heads/orc ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Dynamic reconfiguration of sampling rate
On Tue, 2011-01-18 at 16:53 -0600, pl bossart wrote: In my normal usage, there is a potential (solvable) problem with this approach, though. I use Rhythmbox and most of my songs are at 44100 Hz, but there are some at 48000 Hz. If I start playing one of the 48000 Hz songs, all subsequent streams will be upsampled to 48000 Hz till I pause for 5 seconds. As you suggest, this will become less of a problem if we decrease the idle time required before suspend, and IMO this enough to make this a non-blocker. I wonder what happens if we set the timeout to zero for ALSA devices? Arun, I just tried with load-module module-suspend-on-idle timeout=0, and it seems to work fine on the HDAudio output. I was able to switch back and forth between 44.1 and 48kHz tracks without audible issues. I am not sure what happens with Rhythmbox if you enabled the cross-fade between tracks, most likely you would remain at the rate defined by the first song. The xfade backend always resamples the stream to 44100 Hz (since it does the mixing in GStreamer). Not much we can do there, but I am trying to see if we can push the mixing to PulseAudio (we also need this to maintain reasonable response time when setting a large tlength in pulsesink to save power). Even if this is done, though, we're stuck with the rate defined by the first song till a pause, but this is still not so bad. FWIW, the zero-length wait seems to work fine for me as well (also HDAudio). A couple of things to consider if we do decide to go ahead with this are are that: * This will cause a pause on every seek in GStreamer as well (should be fine in general since a seek causes a flush as well, but just something to keep in mind while testing) * We probably need to be more intelligent about resetting the alsa-sink smoother since it tends to swing wildly for a while after resume (right now it gets reset on every suspend/resume - probably makes more sense to only reset if some minimum amount of time has passed) Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Dynamic reconfiguration of sampling rate
On Mon, 2011-01-24 at 14:14 -0800, pl bossart wrote: FWIW, the zero-length wait seems to work fine for me as well (also HDAudio). A couple of things to consider if we do decide to go ahead with this are are that: Any ideas on how we can make this timeout sink-specific? Everything I know of (default.pa, module-udev-detect) will set this timeout for all possible sinks. Not clear to me how I can make this configuratble. Properties maybe, but I don't know how to set them with a command or configuration file. I can't think if anything simple to handle this. However. if we're going to have to apply some policy on a per-profile basis, perhaps we could let modules set a can-quick-suspend property or some such on the sink which module-suspend-on-idle can look for. This leaves the decision to the module that exposes the sink which, in theory, knows best about this anyway. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Properties to suppress save/restore of stream volumes
On Thu, 2011-02-03 at 08:34 +0200, Tanu Kaskinen wrote: On Wed, 2011-02-02 at 13:29 +0530, Arun Raghavan wrote: Currently Rhythmbox does fading using the GStreamer 'volume' element, so this problem doesn't exist with Rhythmbox as it stands now. Some background ... To play audio, Rhythmbox/Totem/Banshee and others use gconfaudiosink using the music profile. In most systems, this is just directly mapped to pulsesink. The pulsesink default buffer-time value is pretty low and thus not power-efficient (~15 wakeups/s just from the alsa-sink thread). While trying to make the default values for this larger, I ran across a problem with the RB xfade backend. Having a long buffer means that you introduce a buffer-time-sized latency. Obviously, a 1+ second delay between hitting next and the actual fading is not acceptable. Since GStreamer doesn't support rewinding the stream in any simple fashion. there doesn't seem to be a direct approach to fixing this. Would implementing rewind support in gstreamer count as a direct approach? There's clearly a need for it. Since GStreamer is a generic framework, we'd need to be able to rewind any given media, and for video, this is probably not going to be a feasible option. Instead, I've been writing a new xfade backend that creates a new playbin2 pipeline (and thus new pulsesink) for each stream, and then directly controlling the volume on the pulsesink element for fades. Hope this clarifies the situation. Yes, thank you for the clear explanation. Even with this patch, how can fade-in work? If Rhythmbox disables volume restoring when creating a new stream and fading it in, how does it know what should be the target volume? Basically, we allow the volume to be restored for the first stream, and then maintain the current volume till the end. Thus, when we know we're fading a stream in from an existing stream, we don't need to restore the volume. I now realize that your patch actually breaks things. If the xfade pipeline disables volume restoring, then volume control through stream-restore doesn't work anymore for that particular stream. If the volume of the entry that is being applied to RB is updated by some external program, and the external program chooses to apply updates immediately (which is the usual case), then RB volume is not updated. I think this particular problem can be solved simply by removing the should_restore_volume() check from apply_entry(). The check in sink_input_fixate_hook_callback() should cover the documented functionality (prevent restoring at stream creation time). This makes sense. Uh, actually, for sink_input_fixate_hook_callback(), isn't it already enough that module-stream-restore checks whether the volume is set for the new stream? You initialize the volume to zero when starting fade-in, right? Is the restore_volume property completely redundant? Also correct. The question is do we want to have a symmetric interface (set properties for both), or prevent having multiple ways of doing the same thing (set volume vs. set property). I don't see one as being clearly better than the other, so I'll go with the majority (if we decide to include these properties). Ditto for the property name. Replying to the rest in a separate mail. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Echo cancellation in PulseAudio
Hello, On Tue, 2011-02-08 at 15:35 +0530, ssrk wrote: Hi all, I wish to know is there support for echo cancellation in PulseAudio? when I was searching for it I found this link http://nlnet.nl/project/pulseaudio/ Do we have echo cancellation support in PulseAudio already? You can find module-echo-cancel in the master branch in git. It's not in a release yet. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Echo cancellation in PulseAudio
On Tue, 2011-02-08 at 16:31 +0530, ssrk wrote: Hi Arun, [...] Thanks for that reply, Let me try to download and compile it. Have you tried loading that module and checking that feature? if so, please send those commands also. Once you've run the daemon, use pactl load-module module-echo-cancel to load the module. You'll see a new sink and source that adds echo cancellation when used. If your application marks its streams' intended role is marked as 'phone' (Empathy does this for voip calls already), the echo-cancelling sink and source will automatically be used. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] MP3 passthrough over A2DP (2nd version)
On Fri, 2010-11-19 at 18:11 -0200, João Paulo Rechi Vita wrote: Hello Pierre, On Wed, Nov 10, 2010 at 17:00, pl bossart bossart.nos...@gmail.com wrote: Here's the corrected path with most comments from Tanu implemented. I attached some additional gstreamer patches so that people can test the actual functionality. As Tanu said previously, it seems the gstreamer patches didn't make to the list. Could you please make them available somehow? I'm also curious to test this. Hey Colin, are these still stuck in moderation somehow? Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Build assembler errors on ARM OMAP3
On Mon, 2011-02-07 at 12:52 -0600, Kurt Taylor wrote: I am seeing the following when trying to build pulseaudio on an ARM-based Beagleboard (OMAP3): CC libpulsecore_1.0_la-svolume_arm.lo ../libtool: line 975: warning: setlocale: LC_MESSAGES: cannot change locale (en_GB.utf8) {standard input}: Assembler messages: {standard input}:82: Error: thumb conditional instruction should be in IT block -- `addcs r0,r8' {standard input}:83: Error: thumb conditional instruction should be in IT block -- `movcs r6,r0' {standard input}:98: Error: thumb conditional instruction should be in IT block -- `addcs r0,r8' {standard input}:99: Error: thumb conditional instruction should be in IT block -- `movcs r6,r0' {standard input}:119: Error: thumb conditional instruction should be in IT block -- `addcs r0,r8' {standard input}:120: Error: thumb conditional instruction should be in IT block -- `movcs r6,r0' I am using the normal build (bootstrap.sh, configure, make) on Linaro ALIP on the Beagleboard. The build worked fine on a Pandaboard with Ubuntu 10.10. It looks like something is different and not being detected via bootstrap/configure on ALIP. I believe configure has identified the architecture correctly. Also, I have searched and seen commits in the pulseaudio/ubuntu maillist archive for adding -Wa, -mimplicit-it=thumb. I have added this to CFLAGS without success. Any thoughts on what else I could try? What CFLAGS are you using? This should only turn up if you're compiling that assembly in Thumb-2 mode. I'm not sure if it would actually be desirable to compile in Thumb-2 mode for the OMAP3, but the correct fix for this would probably be to add an ite cs before the addcs in svolume_arm.c. Could you test this out (or let me know if you'd like a patch that does this)? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [RFC] Passthrough support
Hey folks, I've put up a draft proposed API changes to get passthrough support to the point where things Just Work™ for at least the common cases, based on previous discussion on-/off-list: http://pulseaudio.org/wiki/PassthroughSupport Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. Thanks Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-15 at 17:47 +0100, Xavier Bestel wrote: Hi, On Tue, 2011-02-15 at 22:02 +0530, Arun Raghavan wrote: Hey folks, I've put up a draft proposed API changes to get passthrough support to the point where things Just Work™ for at least the common cases, based on previous discussion on-/off-list: http://pulseaudio.org/wiki/PassthroughSupport Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. How about reusing gstreamer formats, so that adding a new format to PA makes it appear automagically in apps using GST (no need to recompile GST) ? Tempting as it is ;), we want to keep PA framework-agnostic. Also, in most cases, you will likely need to update pulsesink on the GStreamer side anyway to do some sort of payloading (IEC 61937 in the formats we're talking about now). Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-15 at 12:33 -0600, pl bossart wrote: Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. Thanks Arun, this is a good start. Couple of comments: Thanks for the detailed review. Responses inline ... - change raw to PCM, it's less ambiguous. Ack. - I am not sure I understand how/when the query would be used. Seems to me like a notification with the formats exposed by the sink currently selected would be more usable. And if a change in routing happens (new accessory, audio policy, etc), the client is informed and can reconfigure to PCM if need be. In this scheme, how would the client first determine what formats are available? The notification will also be required - we can either piggyback in sink, sink-input and card change notifications, or introduce a new one for a change in available formats (I prefer the latter). - I think we need to look at the 'start_move' and 'finish_move' as well. If a reconfiguration to PCM is needed, we may need to wait until the client sends PCM again? Ah, this isn't clear in the proposal at all. As Tanu pointed out, when moving, the client needs to disconnect the existing stream and connect with a new stream anyway. That said, it would be nice to give the client fair warning instead of pulling the stream out from under it's feet. I'll dig in a bit and try to find a more elegant approach here. - I am not sure the channel map matters for compressed data. The receiver will adjust anyway to the actual number of speakers, e.g. downmix if you use a headset. I presume you mean in the query API? I've kept all the arguments we pass to pa_stream_new() and pa_stream_connect_playback() to be future-proof. In most cases, everything but the stream flags and proplist will be NULL, but if, at some point in the future, any of the other parameters start to be used in routing. (not very practical example: if the receiver expects input as a particular channel map, but the compressed data has a different mapping, we would claim to not support that format so the client would have to decode and then remap in PA) - you want to add E-AC3 as a basic format. It uses a different IEC syntax Ack. - The receiver may support the same format at different sampling frequencies (eg MPEG at 32, 44.1 and 48kHz but not any other for BT). We will need to list explicitly which sampling frequencies are supported for each format. [taking this up in a separate thread] - Changing the monitor is tricky, we may want to keep it but either tag the data as IEC as well, or just zero out. Right. - On deprecating the PA_SINK_INPUT_PASSTHROUGH: yes it can be removed as long as you have some means to signal that the data isn't your ordinary PCM. Extending the pa_sample_spec would break the API, I vote for a proplist here. I think we might need to break the API here in order to keep it clean. IMHO it is somewhat hackish to expect clients to set their sample format to s16 stereo + passthrough flag to signal compressed data. Besides UIs will all need to check this flag and the sink input's properties to be able to tell what format is being played. The only real API break is that PA_SAMPLE_MAX will change. I was thinking of something along the lines of: PA_SAMPLE_U8 = 0 ... other PCM formats ... PA_SAMPLE_PCM_MAX ... compressed formats here ... PA_SAMPLE_MAX PA_SAMPLE_INVALID = -1 PA_SAMPLE_PCM_ANY = -2 The last addition (PCM_ANY) is for convenience in the query API (you don't need one pa_format_info for each PCM format, then). -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-15 at 12:11 -0600, pl bossart wrote: How about reusing gstreamer formats, so that adding a new format to PA makes it appear automagically in apps using GST (no need to recompile GST) ? Tempting as it is ;), we want to keep PA framework-agnostic. Also, in most cases, you will likely need to update pulsesink on the GStreamer side anyway to do some sort of payloading (IEC 61937 in the formats we're talking about now). Actually Gstreamer would need to add a set of IEC61937 formats as well. The current IEC968 format exposed in gstreamer can mean either PCM or compressed but doesn't differentiate between, say, AC3 and DTS. We were thinking of doing the payloading in pulsesink since there aren't really any other elements that use this format. In that case, we don't need a new format. If it turns out that a dedicated iec61937 payloader might be generally useful, then we /will/ need to add those new formats. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support (extending proplists)
On Tue, 2011-02-15 at 12:33 -0600, pl bossart wrote: Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. [...] - The receiver may support the same format at different sampling frequencies (eg MPEG at 32, 44.1 and 48kHz but not any other for BT). We will need to list explicitly which sampling frequencies are supported for each format. This one will probably be somewhat contentious. I was going to put this information in the format_info structure's proplist so clients can read and match as they please. There is one stumbling block here in that proplists do not allow list or range types. We can either support these by abusing the proplists a bit (for ranges have a prop.min and prop.max, for lists have prop.num_elements, prop.elem_0, prop.elem_1, etc.), using simple strings (the way GstCaps are done) or add first-class list and range types. Listed in increasing order of effort. IMO the first is too hacky. The second is quite flexible, but potentially too generic? The third is the most disruptive, but also most exactly suited to what we need. I'm leaning towards the second (string representation like (type) [min, max] for range and (type) { val1, val2, val3 } for list). In the current context, the type might be implicit in the key, but since this literally becomes part of the API, I'd like to plan ahead. Thoughts? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-15 at 22:14 +0200, Tanu Kaskinen wrote: On Tue, 2011-02-15 at 22:02 +0530, Arun Raghavan wrote: Hey folks, I've put up a draft proposed API changes to get passthrough support to the point where things Just Work™ for at least the common cases, based on previous discussion on-/off-list: http://pulseaudio.org/wiki/PassthroughSupport Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. Good initiative! My comments: Thanks for the review! I'm not sure the query function is needed. If its purpose is just to enable stream format negotiation, I'd do it so that when connecting the stream, some representation of the set of formats that the client supports for the new stream would be provided by the client. The representation might be a proplist, or something else. It's then up to the server to select the exact format. The format set representation might support ordering by preference, so that the client would have more control. The final format could then be queried from the pa_stream object. If the representation would be something else than a proplist, then I guess it would be ok to create a special function for creating a passthrough stream. The point of this proposal is that this would avoid two round-trips to the server: first the _get_actual_sink() call and then the _get_sink_info_by_index() call. This is doable, and not mutually exclusive with the query API. We could offer this as a simple API for clients that want to offload the negotiation to PA if the need is felt - it would require either breaking API in pa_stream_connect_playback() or adding a variant that takes an array of pa_format_info structs. Having a query makes for a more flexible API, though. For example, it fits more more neatly with the way GStreamer works - you query the sink's format and then build your pipeline from source to sink accordingly. Also, it would probably be good to be able to make this query for UIs. Does the assumption, that all sinks support PCM streams, cause some actual consequences in terms of API? I can't immediately see any consequences - it seems like an implementation detail that can be changed later. (I'd like to reserve the possibility to have passthrough-only sinks in the future, just in case.) Fair enough. The only niggling API change would be adding a PA_FORMAT_PCM_ANY (it would be unwieldy to return one pa_format_info for each PCM format). Pierre had a comment about moving streams between compressed and PCM sinks. It seems that we understood the proposal differently. Pierre talks about waiting the client to start sending the correctly formatted data before finishing the move, but isn't the proposal such that there doesn't need to be any waiting logic? When moving is initiated, the stream is actually killed and the client is notified that it should create another stream. You're right, and I think this still needs some fleshing out though, as I mentioned in another mail. I vote for disabling monitor sources altogether for passthrough sinks, at least in the first phase, unless it's trivial to send the compressed data in some way that is actually useful. Sending silence doesn't sound useful to me. The only potential use I see is possibly for debugging, so I'm fine with disabling it in general. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Wed, 2011-02-16 at 10:10 +0200, Tanu Kaskinen wrote: [...] What if the stream would be created at the time when GStreamer queries the sink format? The query apparently already contains information about the preferred format, otherwise your proposal makes it impossible to do routing decisions based on the sample spec parameter. pulsesink would respond to the query by checking the parameters of the created pa_stream object. If the pipeline building is a heavy process, the stream could be created in corked state. At pipeline setup time, we might not actually have all the information required to set the sample format. With a manual, non-autoplugged pipeline, we just set the elements to READY (that's the bit where we create the context and will do the query) starting from sink to source. TBH, in most cases, I expect the query to be performed with all parameters except maybe stream flags and props as NULL. All those other parameters are only there so that the API doesn't need to change for use-cases that we don't foresee at the moment. So in theory, this means that if you determine routing with some information, but make the connection with more parameters, things might break and you might need to renegotiate (with a limit on how many times you do this). In practice, this shouldn't bite us until there is some significant change to the routing logic, by which time I hope all the pieces for triggering upstream renegotiation from pulsesink will be in place. :) Can you elaborate on what you had in mind regarding UIs? As for UIs, it would probably be nice to have some sort of (hidden by default) way to show supported format in a sink. Of course, pactl/pacmd would expose this data too. Other than informative purposes, it would also make debugging simpler (Hey, my BT headset isn't playing MP3s automatically .. Can you check if it's actually supported using these simple steps?) Does the assumption, that all sinks support PCM streams, cause some actual consequences in terms of API? I can't immediately see any consequences - it seems like an implementation detail that can be changed later. (I'd like to reserve the possibility to have passthrough-only sinks in the future, just in case.) Fair enough. The only niggling API change would be adding a PA_FORMAT_PCM_ANY (it would be unwieldy to return one pa_format_info for each PCM format). I think the PA_SAMPLE_* enumeration should be left alone. Let it be a legacy thing for clients that aren't interested in anything else than PCM. pa_format_info doesn't need the pa_sample_spec member. The encoding type can be provided with a new enumeration with PA_ENCODING_PCM, PA_ENCODING_MP3 etc. Then the proplist, or whatever is chosen for representing the format set, can contain information saying that everything is supported. I didn't get the last bit about what you'd add to the proplist. The separate enum would work well for the format_info struct, but would you also suggest extending the API to use the encoding type while setting up/connecting streams? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
Hello, On Wed, 2011-02-16 at 09:56 -0600, pl bossart wrote: Fair enough. The only niggling API change would be adding a PA_FORMAT_PCM_ANY (it would be unwieldy to return one pa_format_info for each PCM format). I think the PA_SAMPLE_* enumeration should be left alone. Let it be a legacy thing for clients that aren't interested in anything else than PCM. pa_format_info doesn't need the pa_sample_spec member. The encoding type can be provided with a new enumeration with PA_ENCODING_PCM, PA_ENCODING_MP3 etc. Then the proplist, or whatever is chosen for representing the format set, can contain information saying that everything is supported. I didn't get the last bit about what you'd add to the proplist. The separate enum would work well for the format_info struct, but would you also suggest extending the API to use the encoding type while setting up/connecting streams? I agree with Tanu. Changin the PA_SAMPLE_ enum will open a can of worms since it will create an issue with the byte-to-ms conversion. I tried in the past and it became a nightmare, I had to change code everywhere and had to lie on the results. For the passthrough to work, you want the IEC bistream to look like PCM 16/32 bit stereo so that these conversions are well handled. You would use a tag/flag/proplist to make sure the binary data isn't modified/resampled/etc. For a quick proof-of-concept, I hacked up a patch to do this by changing PA_SAMPLE_*. The bytes_to_ms conversion is handled correctly, and I disabled monitors to start with, but we'll probably need to make this more intelligent once we squash the passthrough profiles with the non-passthrough ones and attach multiple formats to the sink. http://people.collabora.co.uk/~arun/0001-pulse-Add-sample-formats-for-compressed-formats.patch It's possible I'm missing something, but I did some quick testing on top of your BT passthrough sink as well as with normal ALSA and A2DP+SBC without triggering any asserts. It's certainly not complete (we need to handle things like the Rygel sink that depend on the monitor, but that's not a problem related to this approach). I'm open to going with another approach if that's the consensus, but I thought it would be easiest to just try to code it to prove feasibility one way or the other. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Wed, 2011-02-16 at 10:39 -0600, pl bossart wrote: - I am not sure I understand how/when the query would be used. Seems to me like a notification with the formats exposed by the sink currently selected would be more usable. And if a change in routing happens (new accessory, audio policy, etc), the client is informed and can reconfigure to PCM if need be. In this scheme, how would the client first determine what formats are available? The notification will also be required - we can either piggyback in sink, sink-input and card change notifications, or introduce a new one for a change in available formats (I prefer the latter). The problem is that you don't know on what sink you will play until you have actually created the pa_stream. The audio policy and routing rules may kick in and if you query before you connect you would end-up with a broken configuration. But the query API includes all the information that we can provide at stream creation/connect time, so things would only break if the query and connect are done with different parameters, or if the sink changed in the period between the two calls. It should be possible for clients to gracefully handle this and renegotiate. One possibly is to connect as PCM, then get a notification from the sink that it can actually support compressed data and then configure the stream. This does solve the problem of connecting and finding the routing changed, but I went with the query API since I think it makes more sense to design around the normal case (assuming that the sink disappearing between query and connect is going to be uncommon). Another possibility is to connect but ask the sink to provide its capabilities in return. We would then have another call to refine the stream configuration. Since the query basically includes all the parameters from pa_stream_new() and pa_stream_connect_playback(), doesn't this amount to the same thing as the current proposal? Maybe we should have a short call on this. Yes, ff we're not able to agree on a solution tomorrow, we could do this over phone/IRC. FWIW, I should be around most of the time except between ~2130 and ~0430 UTC. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Thu, 2011-02-17 at 10:26 +0200, Tanu Kaskinen wrote: On Wed, 2011-02-16 at 19:17 +0530, Arun Raghavan wrote: Can you elaborate on what you had in mind regarding UIs? As for UIs, it would probably be nice to have some sort of (hidden by default) way to show supported format in a sink. Of course, pactl/pacmd would expose this data too. Other than informative purposes, it would also make debugging simpler (Hey, my BT headset isn't playing MP3s automatically .. Can you check if it's actually supported using these simple steps?) Yes, the supported formats should be provided in the sink introspection data. This isn't necessarily relevant when creating streams, though. Ah, right. I was squashing things that aren't really related. [...] I would suggest that the sinks and the clients use the same presentation for describing the supported formats. It may be a proplist or something else. In a purely proplist-based solution there may be no need to extend the stream creation API. However, a purely proplist-based solution would look hacky to me, but that's a matter of taste. If I had to decide the format set representation now, I might do it as follows: typedef enum pa_encoding { PA_ENCODING_PCM, PA_ENCODING_MP3, ... } typedef struct pa_format_desc { pa_encoding_t encoding; pa_proplist *params; /* string - string: parameter name * to parameter value. The parameter * value has syntax for representing * also lists and ranges. Missing * parameter means that anything is * allowed, unknown parameters are * ignored. */ } pa_format_desc; Uh, that ended up pretty identical to your pa_format_info proposal, even though I didn't mean to :) I tried to replace the proplist with a solution that doesn't require any string parsing, but that ended up very complicated. Good to see we converged on the same structure. :) bikeshed Building on the pa_sample_spec naming, pa_stream_spec, perhaps? /bikeshed I'm proposing that a new function is added for creating streams that takes a list of pa_format_desc structs (or maybe pa_format_info is a better name) instead of sample spec and channel map. Rethinking this, I'm starting to agree with you and Pierre on this. For the PA_ENCODING_PCM, the proplist would basically just encode the same data that's in pa_sample_spec. For other formats, we can add the data we need. I'm researching you other proposal and will update the proposal on the wiki with both these in a while. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
Hi Tanu, On Thu, 2011-02-17 at 09:45 +0200, Tanu Kaskinen wrote: On Thu, 2011-02-17 at 02:54 +0530, Arun Raghavan wrote: On Wed, 2011-02-16 at 10:39 -0600, pl bossart wrote: - I am not sure I understand how/when the query would be used. Seems to me like a notification with the formats exposed by the sink currently selected would be more usable. And if a change in routing happens (new accessory, audio policy, etc), the client is informed and can reconfigure to PCM if need be. In this scheme, how would the client first determine what formats are available? The notification will also be required - we can either piggyback in sink, sink-input and card change notifications, or introduce a new one for a change in available formats (I prefer the latter). The problem is that you don't know on what sink you will play until you have actually created the pa_stream. The audio policy and routing rules may kick in and if you query before you connect you would end-up with a broken configuration. But the query API includes all the information that we can provide at stream creation/connect time, so things would only break if the query and connect are done with different parameters, or if the sink changed in the period between the two calls. It should be possible for clients to gracefully handle this and renegotiate. Your proposal would probably work well enough in practice, but I still would like more an approach where you create a stream and after it gets routed you finalize the stream format. If routing rules change before the format is decided, the daemon can tear down the stream and inform the client that it happened because of routing change. The client knows now that it should retry. In your proposal the client isn't aware that routing rules have changed, so it doesn't know why the connection fails. Also, even if connecting succeeds, but with different routing, the stream format that the client chooses may be suboptimal. So, my proposal is the following: Thanks for the detailed proposal! Sinks have a list of format descriptions. One format description is a tuple consisting of the encoding type and some parameters that are characteristic for that encoding type. Depending on the parameter, a parameter value can be a single value, a range, a list or the parameter may be unset, meaning that anything is allowed. There should also be a special encoding type any, that means that the client supports anything (useful for recording applications that just dump the stream to a file). When a client creates a new stream, it gives a similar list of formats as described above for sinks. The list must cover all formats that the client can support (usually the list contains only one tuple with only fixed parameters). The daemon routes the stream to some sink, and then the daemon takes an intersection of the sink formats and the stream formats. If the resulting set contains exactly one fixed format, then that is used for the stream. If the set contains more options than one fixed format, then the daemon decides the best format using some unspecified algorithm. If the set is empty, then the stream creation fails. The client can also choose not to specify any formats. In that case the routing logic can't use the format as input for decisions, but currently there's no such routing logic anyway. After routing is done, the stream enters to a new state, PA_STREAM_ROUTED. In this state the client has to query the set of formats that the sink supports, and decide the final format. When the client sets the format, the stream changes state to PA_STREAM_READY. This sounds pretty clean overall, and after talking to Wim on IRC for a bit, I think we might even be able to do without the no-formats variant of the API. That said, it would be nice to have the last bit as well, so I'll keep it put it down as something to add after the basic bits are done. I've updated the wiki with this version now. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Thu, 2011-02-17 at 09:13 +, Colin Guthrie wrote: 'Twas brillig, and Tanu Kaskinen at 17/02/11 07:45 did gyre and gimble: Your proposal would probably work well enough in practice, but I still would like more an approach where you create a stream and after it gets routed you finalize the stream format. Yeah, I agree with this statement. It just feels cleaner (my comment about the format being used in the routing would still be an issue here but I agree that it was probably an extreme example that wouldn't really be needed in practice anyway). If routing rules change before the format is decided, the daemon can tear down the stream and inform the client that it happened because of routing change. The client knows now that it should retry. In your proposal the client isn't aware that routing rules have changed, so it doesn't know why the connection fails. Also, even if connecting succeeds, but with different routing, the stream format that the client chooses may be suboptimal. So, my proposal is the following: Sinks have a list of format descriptions. One format description is a tuple consisting of the encoding type and some parameters that are characteristic for that encoding type. Depending on the parameter, a parameter value can be a single value, a range, a list or the parameter may be unset, meaning that anything is allowed. There should also be a special encoding type any, that means that the client supports anything (useful for recording applications that just dump the stream to a file). When a client creates a new stream, it gives a similar list of formats as described above for sinks. The list must cover all formats that the client can support (usually the list contains only one tuple with only fixed parameters). The daemon routes the stream to some sink, and then the daemon takes an intersection of the sink formats and the stream formats. If the resulting set contains exactly one fixed format, then that is used for the stream. If the set contains more options than one fixed format, then the daemon decides the best format using some unspecified algorithm. If the set is empty, then the stream creation fails. When this fails, should we go back to the routing system and ask to be routed again, but not to this failing sink? I can see this being a) useful, but b) complicated (not so much complicated on initial connection but complicated when a stream may get re-routed (i.e. when a new, totally unrelated sink comes along, the routing system may re-evaluate it's priority lists and try to move the stream to a higher priority sink (i.e. the one that failed the first time). I probably didn't understand this correctly, but wouldn't this only be helpful if a new sink comes in exactly between when the routing fails and when you retry? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support (extending proplists)
On Wed, 2011-02-16 at 12:33 +, Colin Guthrie wrote: 'Twas brillig, and Arun Raghavan at 16/02/11 04:42 did gyre and gimble: On Tue, 2011-02-15 at 12:33 -0600, pl bossart wrote: Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. [...] - The receiver may support the same format at different sampling frequencies (eg MPEG at 32, 44.1 and 48kHz but not any other for BT). We will need to list explicitly which sampling frequencies are supported for each format. This one will probably be somewhat contentious. I was going to put this information in the format_info structure's proplist so clients can read and match as they please. There is one stumbling block here in that proplists do not allow list or range types. We can either support these by abusing the proplists a bit (for ranges have a prop.min and prop.max, for lists have prop.num_elements, prop.elem_0, prop.elem_1, etc.), using simple strings (the way GstCaps are done) or add first-class list and range types. Listed in increasing order of effort. IMO the first is too hacky. The second is quite flexible, but potentially too generic? The third is the most disruptive, but also most exactly suited to what we need. I'm leaning towards the second (string representation like (type) [min, max] for range and (type) { val1, val2, val3 } for list). In the current context, the type might be implicit in the key, but since this literally becomes part of the API, I'd like to plan ahead. Thoughts? Random suggestion: 1. The range thing seems simple enough the prop.min and prop.max seems like an easy enough thing to list and support (perhaps even with a wrapper function if it saves a bit of sanity checking). 2. The lists are slightly harder, so how about we just support JSON style syntax here for simple array storage? i.e. ['val1','val2','val\'s beyond'] ? Again a wrapper function or two can help create/decode these? Using a format like JSON should be pretty simple and we could easily enough push out the whole proplist as a single JSON encoded thing at some point in the future if needed too. We really should decide on what we need first, I guess :) I assume that for a given property, the data type is implicitly understood (this is how things work now). The other thing to worry about is whether a given property should be expected to only be either single-valued, a range or a list. This might not be true - for example when querying a sink, you might get the supported sample rates as a list, but when connecting, sample rate is single-valued (we might not use properties to set the sample rate, but as a generic data structure, I hope you see my point). From the API perspective, perhaps we could add an enum to represent the type (single-valued, multi-valued, range), and then have an API to query this (like pa_proplist_get_type(proplist, key)) that clients would use if multiple possibilities exist. We can then have API for each type. Implementation wise, I think it's nicer to just have one key and do some string parsing for the range case as well. One thing to note here is that we cannot support lists of strings since, aiui, JSON requires that strings be enclosed in quotes and we do not currently do this for single-valued strings. The json.org site is down, so if I'm wrong, I blame Wikipedia. :) -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
Hello! On Tue, 2011-02-15 at 22:02 +0530, Arun Raghavan wrote: Hey folks, I've put up a draft proposed API changes to get passthrough support to the point where things Just Work™ for at least the common cases, based on previous discussion on-/off-list: http://pulseaudio.org/wiki/PassthroughSupport Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. I've updated this page with the changes discussed so far (and translated them into the actual API changes that we need). Please take a look and let me know if this looks acceptable. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Fri, 2011-02-18 at 03:42 -0700, Kelly Anderson wrote: [...] The alsa driver has creates a /proc entry that lists the coding types available for a particular device. It'd be nice if you would use that for the list of supported coding types (at least for a default). I'm not sure the list would be automatically updated if you unplugged a device and plugged in a new device. The comprehensive list of supported coding types is in alsa-kernel/pci/hda/hda-eld.c. I don't know how it works for HDMI, but if it does update that list based on the receiver, that would be ideal for us, since we should be able to probe that while routing. For S/PDIF, we don't have that option so I guess we'll need to add some additional checkboxes to GUIs so users can select what formats their receiver supports. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Fri, 2011-02-18 at 11:04 +, Colin Guthrie wrote: 'Twas brillig, and Arun Raghavan at 18/02/11 09:41 did gyre and gimble: Hello! On Tue, 2011-02-15 at 22:02 +0530, Arun Raghavan wrote: Hey folks, I've put up a draft proposed API changes to get passthrough support to the point where things Just Work™ for at least the common cases, based on previous discussion on-/off-list: http://pulseaudio.org/wiki/PassthroughSupport Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. I've updated this page with the changes discussed so far (and translated them into the actual API changes that we need). Please take a look and let me know if this looks acceptable. I've maybe missed the discussion, but is it worth having both: pa_stream_new_extended and pa_stream_new_with_proplist_extended Doesn't the former just call the latter with a NULL proplist anyway? While I appreciate the symmetry with the existing API, I'd say avoid the cruft and just call it pa_stream_new_extended() which implies both the format and the proplist. WDYT? Good point - I agree with this. Also, fixed option c in the wiki. :) Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] PA Roadmap Planning and LAC Conference
On Fri, 2011-02-18 at 23:00 +0200, Tanu Kaskinen wrote: On Fri, 2011-02-18 at 11:15 -0600, Kurt Taylor wrote: Thanks for organizing this Colin! I will do my best to attend, but that is 5am (UTC-6) for me. It sounds like a few hours later would be better than the suggested 11:00 UTC. Arun would probably be the first person to be annoyed by the time being moved too much forward, but he's going to sleep at 21:30 UTC, so there's plenty of headroom :) :) Indeed. If it helps, I created a little webthingummy to get everyone's preferences (we can add more options, but it seemed to me 24th was acceptable to all): http://www.doodle.com/ivx28di2kzztm2dt Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Sat, 2011-02-19 at 13:28 +0200, Tanu Kaskinen wrote: On Fri, 2011-02-18 at 15:11 +0530, Arun Raghavan wrote: Hello! On Tue, 2011-02-15 at 22:02 +0530, Arun Raghavan wrote: Hey folks, I've put up a draft proposed API changes to get passthrough support to the point where things Just Work™ for at least the common cases, based on previous discussion on-/off-list: http://pulseaudio.org/wiki/PassthroughSupport Please review/comment. Once we have some consensus, I'll send in patches to actually get this done. I've updated this page with the changes discussed so far (and translated them into the actual API changes that we need). Please take a look and let me know if this looks acceptable. I think pa_encoding_t should include PA_ENCODING_ANY for the case where a recording application doesn't care at all about the format. Check. Maybe it would be better to leave out also the channel map from pa_stream_new_extended()? The channel map is relevant only for PCM streams, so putting it in the format info proplist would make sense in my opinion. Agreed. I believe there's no need to alter the pa_stream_connect_* functions in any way. Returning the final format is not possible at the time when the function returns - the API is asynchronous, so the function only sends a request to the server, it doesn't get any response back. When the response eventually comes from the server, the stream state changes to PA_STREAM_READY, which triggers a callback in the client code. That callback can then query the final format. A new function is needed for that: Ah, I /knew/ I was forgetting something when I added that ... const pa_format_info * const *pa_stream_get_format_info(pa_stream *s) My proposal is that the return value is a NULL-terminated list, for supporting sink format querying in the future. For now the list will always contain only one item. Another possibility would be returning just a single format info instance here, and defining a separate function for getting the format list in sink format query cases. Sounds good - I think the NULL-terminated list is preferable. Thanks Tanu, I've updated the page now. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sbc_math.h: add explicit check for ARMv6 instructions
On Sun, 2011-02-20 at 19:20 +0100, Paul Menzel wrote: Date: Sun, 20 Feb 2011 15:57:55 +0100 Building PulseAudio 051d8213 [1] using OpenEmbedded with distribution `minimal-uclibc` for `MACHINE=om-gta01` having an ARMv4T architecture (armv4t-oe-linux-uclibceabi) compilation fails with the following error. [...] {standard input}:4099: Error: selected processor does not support Thumb mode `mla r3,r2,r6,r3' {standard input}:4114: Error: selected processor does not support Thumb mode `mla r3,r2,ip,r3' {standard input}:4125: Error: selected processor does not support Thumb mode `mla r3,r6,r2,r3' make[3]: *** [libbluetooth_sbc_la-sbc.lo] Error 1 Apply the same fix as in [2], which is similar to the patch applied in OpenEmbedded since commit 976ab4b5 [3]. [1] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=051d82133f0ae6a57bf66fd200bc8e3591a7d5ca [2] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=12b900858ae82d435c100d6eb94cb7bb22fe5e29 [3] http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=976ab4b4587d548c0483a274058c5359cb72bf1b Signed-off-by: Paul Menzel paulepan...@users.sourceforge.net CC: Marcel Holtmann mar...@holtmann.org --- I do not know if these files are just copied from linux-bluetooth upstream, so I am adding Marcel to CC anyway because he is mentioned in the copyright section. It does appear that the files are a copy of the bluez package. If we're fixing this on the PulseAudio side, I think we should include something like diff below configure.ac, so that we can actually verify that the mla instruction is available for the arch being used. -- Arun diff --git a/configure.ac b/configure.ac index 08c947a..77e9823 100644 --- a/configure.ac +++ b/configure.ac @@ -257,13 +257,14 @@ case $host in asm volatile (ldr r0, %2 \n ldr r2, %3 \n ldr r3, %4 \n + mla r4, r1, r2, r3 \n ssat r1, #8, r0 \n str r1, %0 \n pkhbt r1, r3, r2, LSL #8 \n str r1, %1 \n : =m (a), =m (b) : m (a), m (b), m (c) - : r0, r1, r2, r3, cc); + : r0, r1, r2, r3, r4, cc); return (a == -128 b == 0xaabb) ? 0 : -1; ]]), [pulseaudio_cv_support_armv6=yes], ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sbc_math.h: add explicit check for ARMv6 instructions
On Tue, 2011-02-22 at 09:48 +, Colin Guthrie wrote: 'Twas brillig, and Arun Raghavan at 21/02/11 20:31 did gyre and gimble: On Sun, 2011-02-20 at 19:20 +0100, Paul Menzel wrote: Date: Sun, 20 Feb 2011 15:57:55 +0100 Building PulseAudio 051d8213 [1] using OpenEmbedded with distribution `minimal-uclibc` for `MACHINE=om-gta01` having an ARMv4T architecture (armv4t-oe-linux-uclibceabi) compilation fails with the following error. [...] {standard input}:4099: Error: selected processor does not support Thumb mode `mla r3,r2,r6,r3' {standard input}:4114: Error: selected processor does not support Thumb mode `mla r3,r2,ip,r3' {standard input}:4125: Error: selected processor does not support Thumb mode `mla r3,r6,r2,r3' make[3]: *** [libbluetooth_sbc_la-sbc.lo] Error 1 Apply the same fix as in [2], which is similar to the patch applied in OpenEmbedded since commit 976ab4b5 [3]. [1] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=051d82133f0ae6a57bf66fd200bc8e3591a7d5ca [2] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=12b900858ae82d435c100d6eb94cb7bb22fe5e29 [3] http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=976ab4b4587d548c0483a274058c5359cb72bf1b Signed-off-by: Paul Menzel paulepan...@users.sourceforge.net CC: Marcel Holtmann mar...@holtmann.org --- I do not know if these files are just copied from linux-bluetooth upstream, so I am adding Marcel to CC anyway because he is mentioned in the copyright section. Looking at src/Makefile.am, it looks like at some point of time, we pulled bluez files directly from git (the update-sbc rule). Does anyone know what the plan was with regards to keeping these files up-to-date? There seem to be a bunch of armv6 updates upstream as well. [...] Paul, can you confirm if this avoids the problem for you? If so, then Arun, can you make a proper patch? You can? Awesome, thanks :) :D Once we've resolved what the situation is with bluez files, I'm happy to help with this. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-22 at 10:03 -0600, pl bossart wrote: Hi Arun, I've updated this page with the changes discussed so far (and translated them into the actual API changes that we need). Please take a look and let me know if this looks acceptable. Couple of comments: - There is a miss on the pa_format_info. the sampling frequency is required. The IEC format can be used at various sampling frequencies. Passing just IEC isn't enough to configure the link with the receiver. The number of channels could be 2 by default, but we may need to change this as well for formats like TrueHD or DTS Master. I figured it'd be best to leave everything but the format in the proplist and then let the sink pick out the properties it needs/expects. This allows us to keep the structure generic and lets sinks specify what/how much information they need to be able to decide whether a format is supported or not. - in case no IEC formats are supported by the sink, what is the returned value? If this is NULL, meaning there is no support, how do we fall back to plain PCM? I assume that the every sink that supports PCM will have one format with PA_ENCODING_PCM in its list of supported formats. With the extended API, I'd like to treat PCM and compressed formats as being equal as much as possible. - in case the routing changes, for example if you plug HDMI or BT, how do we change the connection? The plan is to have clients disconnect and recreate the stream. We could eventually look at a convenience API to do this. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-22 at 13:27 -0600, pl bossart wrote: Still, I am not clear about the negociation. the client would need to use pa_stream_new_extended with multiple formats, and then get the actual format with get_format_info. If my understanding is correct, then maybe you would need a list of formats instead of just one? Oh, indeed! That was the intention, but I guess I missed putting that in the wiki. Fixed now. ok. Looks good. One last comment on proplists. If we don't define a set of mandatory elements, then it's going to be really hard to use this pa_stream_new_extended() routine. How will the client figure out what exactly it needs to send for each format? I was thinking that his would basically develop as convention, though the few basic properties (rate, channels) could be documented along with the pa_encoding_t structure. We can document additional properties (for example bitrate, if some sink ever needs it) as being optional but recommended if available. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sbc_math.h: add explicit check for ARMv6 instructions
Hi Paul, On Tue, 2011-02-22 at 12:33 +0100, Paul Menzel wrote: Am Dienstag, den 22.02.2011, 09:48 + schrieb Colin Guthrie: 'Twas brillig, and Arun Raghavan at 21/02/11 20:31 did gyre and gimble: On Sun, 2011-02-20 at 19:20 +0100, Paul Menzel wrote: Date: Sun, 20 Feb 2011 15:57:55 +0100 Building PulseAudio 051d8213 [1] using OpenEmbedded with distribution `minimal-uclibc` for `MACHINE=om-gta01` having an ARMv4T architecture (armv4t-oe-linux-uclibceabi) compilation fails with the following error. [...] {standard input}:4099: Error: selected processor does not support Thumb mode `mla r3,r2,r6,r3' {standard input}:4114: Error: selected processor does not support Thumb mode `mla r3,r2,ip,r3' {standard input}:4125: Error: selected processor does not support Thumb mode `mla r3,r6,r2,r3' make[3]: *** [libbluetooth_sbc_la-sbc.lo] Error 1 Apply the same fix as in [2], which is similar to the patch applied in OpenEmbedded since commit 976ab4b5 [3]. [1] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=051d82133f0ae6a57bf66fd200bc8e3591a7d5ca [2] http://git.0pointer.de/?p=pulseaudio.git;a=commit;h=12b900858ae82d435c100d6eb94cb7bb22fe5e29 [3] http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=976ab4b4587d548c0483a274058c5359cb72bf1b Signed-off-by: Paul Menzel paulepan...@users.sourceforge.net CC: Marcel Holtmann mar...@holtmann.org --- I do not know if these files are just copied from linux-bluetooth upstream, so I am adding Marcel to CC anyway because he is mentioned in the copyright section. It does appear that the files are a copy of the bluez package. If we're fixing this on the PulseAudio side, I think we should include something like diff below configure.ac, so that we can actually verify that the mla instruction is available for the arch being used. -- Arun diff --git a/configure.ac b/configure.ac index 08c947a..77e9823 100644 --- a/configure.ac +++ b/configure.ac @@ -257,13 +257,14 @@ case $host in asm volatile (ldr r0, %2 \n ldr r2, %3 \n ldr r3, %4 \n + mla r4, r1, r2, r3 \n ssat r1, #8, r0 \n str r1, %0 \n pkhbt r1, r3, r2, LSL #8 \n str r1, %1 \n : =m (a), =m (b) : m (a), m (b), m (c) - : r0, r1, r2, r3, cc); + : r0, r1, r2, r3, r4, cc); return (a == -128 b == 0xaabb) ? 0 : -1; ]]), [pulseaudio_cv_support_armv6=yes], Paul, can you confirm if this avoids the problem for you? If so, then Arun, can you make a proper patch? You can? Awesome, thanks :) Does not Arun’s patch just improve the check for `HAVE_ARMV6`? My patch just adds the check `if defined(HAVE_ARMV6)` to `sbc_math.h`. Without that addition all the configure checks are not evaluated at all in `sbc_math.h`. For bits other than bluez, we need ARMv6 or above. For the MLA instruction that's triggering the error you see, we need a version of ARMv6 or above that also supports Thumb2 [1]. So your patch is incorrect in that it will fail on (e.g.) ARMv6K. My patch above, on the other hand, will make PA not use v6 asm on ARMv6K even if bluez support is disabled, which is also incorrect. The correct fix for this, imo, is in bluez (there is a new sbc_primitives_armv6.h that can probably be used at least as a template). We need to do an sbc-udpate on the PA side anyway, and can pull this when we do. Cheers, Arun [1] http://infocenter.arm.com/help/topic/com.arm.doc.qrc0001m/QRC0001_UAL.pdf ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Properties to suppress save/restore of stream volumes
On Fri, 2011-02-25 at 09:42 +, Colin Guthrie wrote: 'Twas brillig, and Arun Raghavan at 08/02/11 15:34 did gyre and gimble: If there are no objections to this approach, we could expose the fading API via a property (set it to trigger a fade) and a signal (for the fade-done callback). For the completion notification, I realised we would just pass the callback to the _set_volume_with_ramping() function like the rest of the API, so nothing new needs to be done for that it seems. Now all that remains is to figure out whether this belongs in core or in an extension. I think it fits in core (a _with_ramping variant of existing core API), but I'm not a religious about it. :) Just to revive this thread since our meeting Now that we know the ramping stuff will be ripped out for 1.0 release (by your good self Arun!), I guess we can put this on the back burner for now? Oh, the irony! :) Yep, this will need to fade (:p) to the background. Pity, since the gst side power optimisation also gets postponed as a result, but hopefully it'll be something we can get back to soon after 1.0 (I'm an optimist! :D). Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] Add src/*-symdef.h to .gitignore.
On Sun, 2011-02-27 at 12:50 +0200, Tanu Kaskinen wrote: --- src/.gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/.gitignore b/src/.gitignore index c93974b..1380e26 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -66,3 +66,4 @@ voltest start-pulseaudio-x11 start-pulseaudio-kde vector-test +*-symdef.h Had this on my list for this week - you beat me to it! :) We should remove the corresponding entries from the .gitignore files in module subdirs as well since the files aren't created there any more. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Tue, 2011-02-22 at 22:52 +0200, Tanu Kaskinen wrote: [...] The set of parameters can potentially grow forever - we don't know what parameters may become relevant in the future. So, I think we should start by documenting the parameters that we know are important today, and define the approach for adding new parameters in the future. When thinking about handling unknown parameters, there are four cases that need to be considered: 1) A sink has a parameter that a client doesn't understand. 2) A source has a parameter that a client doesn't understand. 3) A playback stream has a parameter that a server doesn't understand. 4) A record stream has a parameter that a server doesn't understand. The cases 2 and 3 should be easy - if the receiver of the data doesn't understand a parameter, it means that it doesn't care. The unknown parameter can be just ignored by the negotiation logic, and the final format should have that parameter removed from the proplist. Cases 1 and 4 are more difficult. One possibility would be to fail the negotiation in such case. That would require that the sender always specifies explicitly all parameters that it knows about, even if it can support anything (IIRC I suggested earlier that a missing parameter would mean that everything is supported). Another possibility would be to try anyway, and the receiver would then kill the stream if the actual payload wasn't satisfactory. What should the negotiation logic do in this case? If the unknown parameter is a fixed value, the parameter should probably be left in the final format proplist. What if the unknown parameter is a range or a list? Maybe the negotiation logic should in this case leave the parameter untouched in the final format proplist. I don't know which approach I like more. The first approach (failing) would be cleaner, but the second approach (trying anyway) would work in more cases. I prefer failing if the pa_stream's properties are not a subset of the properties specified by the sink/source. As Pierre mentions, in the foreseeable we're going to be dealing with a limited number of properties (rate, channels, 2-3 codec parameters), so this approach will work until the requirements change in some fundamental way. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [Patch] Infinite loop in mainloop
Hi Marcel, On Sat, 2011-01-01 at 16:50 +0100, Marcel wrote: I am porting pulseaudio to OS/2. During this I run into trouble with the mainloop occasionally eating up all CPU resources. It turned out to be an inconsistency in the internal state of the mainloop. m-wakeup_requested was 0 while the wakup pipe was ready. In fact most probably there is still a race condition somewhere in the code. However, the following patch will help to recover from this inconsistencies more gracefully, especially if the mainloop is running at high priority. This came up in the meeting last Thursday [1]. Could you provide more context on this? It does appear that this is some sort of OS/2 bug rather than a bug in PA. If no other information is available, then I think this is okay to go in with an OS/2-specific conditional. Cheers, Arun [1] http://colin.guthr.ie/meetings/pulseaudio-meeting/2011/pulseaudio-meeting.2011-02-24-21.02.log.html#l-379 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC] Passthrough support
On Sun, 2011-02-27 at 23:20 -0800, Christ Schlacta wrote: [...] mark settings as optional or required for negotiation. if one or the other can't support an Optional parameter, then it gets replied to as unsupported but continuing. a warning is logged. a mandatory option will cause it to fail with an error message logged (Warning, Remote sink doesn't support required option bitrate. either upgrade Remote Sink, or set bitrate as optional by using optional bitrate foo) or similar. We can do this, but what does it really get us? On the sink side, we know what restrictions we want to place on the input, and it's reasonable to assume anything unspecified is fine (if it's not, it should be specified as a restriction anyway). On the stream side, we're not providing the information as a restriction - we're saying I have this stream, it can be with one of these formats/properties, find me a sink to plug into. What would an optional parameter achieve here? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] introspect: Client-side implementation for has_volume/read_only_volume
I got bitten by this (stream aborts because client iterates on the introspection data till all data is consumed), so hacked up a quick patch in case Tanu hasn't had the time to get to this yet. ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] introspect: Client-side implementation for has_volume/read_only_volume
This completes the client-side changes to the protocol extension introduced by commit 99ddca89cdca9b0b92ab9870764f9211e6a82e31 --- src/pulse/introspect.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/pulse/introspect.c b/src/pulse/introspect.c index 2a81788..575ca8d 100644 --- a/src/pulse/introspect.c +++ b/src/pulse/introspect.c @@ -996,7 +996,7 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm while (!pa_tagstruct_eof(t)) { pa_sink_input_info i; -pa_bool_t mute = FALSE, corked = FALSE; +pa_bool_t mute = FALSE, corked = FALSE, has_volume, read_only_volume; pa_zero(i); i.proplist = pa_proplist_new(); @@ -1015,7 +1015,9 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm pa_tagstruct_gets(t, i.driver) 0 || (o-context-version = 11 pa_tagstruct_get_boolean(t, mute) 0) || (o-context-version = 13 pa_tagstruct_get_proplist(t, i.proplist) 0) || -(o-context-version = 19 pa_tagstruct_get_boolean(t, corked) 0)) { +(o-context-version = 19 pa_tagstruct_get_boolean(t, corked) 0) || +(o-context-version = 20 (pa_tagstruct_get_boolean(t, has_volume) 0 || + pa_tagstruct_get_boolean(t, read_only_volume) 0))) { pa_context_fail(o-context, PA_ERR_PROTOCOL); pa_proplist_free(i.proplist); @@ -1024,6 +1026,8 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm i.mute = (int) mute; i.corked = (int) corked; +i.has_volume = (int) has_volume; +i.read_only_volume = (int) read_only_volume; if (o-callback) { pa_sink_input_info_cb_t cb = (pa_sink_input_info_cb_t) o-callback; -- 1.7.4.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] introspect: Client-side implementation for has_volume/read_only_volume
This completes the client-side changes to the protocol extension introduced by commit 99ddca89cdca9b0b92ab9870764f9211e6a82e31 --- configure.ac |2 +- src/pulse/introspect.c |8 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 21dae30..2171089 100644 --- a/configure.ac +++ b/configure.ac @@ -37,7 +37,7 @@ AC_SUBST(PA_MAJORMINOR, pa_major.pa_minor) AC_SUBST(PACKAGE_URL, [http://pulseaudio.org/]) AC_SUBST(PA_API_VERSION, 12) -AC_SUBST(PA_PROTOCOL_VERSION, 19) +AC_SUBST(PA_PROTOCOL_VERSION, 20) # The stable ABI for client applications, for the version info x:y:z # always will hold y=z diff --git a/src/pulse/introspect.c b/src/pulse/introspect.c index 2a81788..35e091a 100644 --- a/src/pulse/introspect.c +++ b/src/pulse/introspect.c @@ -996,7 +996,7 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm while (!pa_tagstruct_eof(t)) { pa_sink_input_info i; -pa_bool_t mute = FALSE, corked = FALSE; +pa_bool_t mute = FALSE, corked = FALSE, has_volume = FALSE, read_only_volume = FALSE; pa_zero(i); i.proplist = pa_proplist_new(); @@ -1015,7 +1015,9 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm pa_tagstruct_gets(t, i.driver) 0 || (o-context-version = 11 pa_tagstruct_get_boolean(t, mute) 0) || (o-context-version = 13 pa_tagstruct_get_proplist(t, i.proplist) 0) || -(o-context-version = 19 pa_tagstruct_get_boolean(t, corked) 0)) { +(o-context-version = 19 pa_tagstruct_get_boolean(t, corked) 0) || +(o-context-version = 20 (pa_tagstruct_get_boolean(t, has_volume) 0 || + pa_tagstruct_get_boolean(t, read_only_volume) 0))) { pa_context_fail(o-context, PA_ERR_PROTOCOL); pa_proplist_free(i.proplist); @@ -1024,6 +1026,8 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm i.mute = (int) mute; i.corked = (int) corked; +i.has_volume = (int) has_volume; +i.read_only_volume = (int) read_only_volume; if (o-callback) { pa_sink_input_info_cb_t cb = (pa_sink_input_info_cb_t) o-callback; -- 1.7.4.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] introspect: Client-side implementation for has_volume/read_only_volume
On Tue, 2011-03-01 at 21:38 +0200, Tanu Kaskinen wrote: On Wed, 2011-03-02 at 00:43 +0530, Arun Raghavan wrote: This completes the client-side changes to the protocol extension Thanks for fixing this! I forgot also incrementing PA_PROTOCOL_VERSION in my patch, so you should fix that too. Missed that because of my local changes. introduced by commit 99ddca89cdca9b0b92ab9870764f9211e6a82e31 --- src/pulse/introspect.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/pulse/introspect.c b/src/pulse/introspect.c index 2a81788..575ca8d 100644 --- a/src/pulse/introspect.c +++ b/src/pulse/introspect.c @@ -996,7 +996,7 @@ static void context_get_sink_input_info_callback(pa_pdispatch *pd, uint32_t comm while (!pa_tagstruct_eof(t)) { pa_sink_input_info i; -pa_bool_t mute = FALSE, corked = FALSE; +pa_bool_t mute = FALSE, corked = FALSE, has_volume, read_only_volume; The new variables should be initialized - if the server uses a protocol version less than 20, you assign undefined values to i.has_volume and i.read_only_volume. D'oh! Agreed and sent fixed patch. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Nvidia hdmi passthrough
On Fri, 2011-03-04 at 20:47 -0700, Kelly Anderson wrote: diff --git a/src/Makefile.am b/src/Makefile.am index 24e2f82..bb501fb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1131,7 +1131,9 @@ dist_alsapaths_DATA = \ modules/alsa/mixer/paths/analog-output-lfe-on-mono.conf \ modules/alsa/mixer/paths/analog-output-mono.conf \ modules/alsa/mixer/paths/iec958-stereo-output.conf \ - modules/alsa/mixer/paths/iec958-passthrough-output.conf + modules/alsa/mixer/paths/iec958-passthrough-output.conf \ + modules/alsa/mixer/paths/hdmi-stereo-output.conf \ + modules/alsa/mixer/paths/hdmi-passthrough-output.conf The additional passthrough profile will become moot once my branch is ready to merge. Please hold off merging this for now. I should have something for you to rebase on before long. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] Reconfigure sample rates on resume
On Sun, 2011-03-06 at 16:30 -0600, Pierre-Louis Bossart wrote: This is the second version of my endeavors to remove or make SRC simpler when possible. An idle non-network, local or passthrough devices will be suspended immediately when there aren't any inputs. When a new input is connected, we try to resume with the best match between requested sample rate and sink sample rate. To make things simple, the sink can only support a default and an alternate rate, typically 44.1 and 48kHz. Of course the feature is disabled when both rates are identical (if the sink cannot be reconfigured). Tests with HDAudio and USB show a negligible added delay but a nice reduction of CPU activity. An alternative idea: At sink-input trigger a reconfiguration if possible/supported. Sinks expose a reconfigure_rate method (I'm sure we can find a better name) if they can do a quick switch. In the ALSA sink, this function would check if there are no sink-inputs attached and the rate is supported, and then send a message to the I/O thread to do the actual reconfiguration. We would cache the rate if suspended and apply on unsuspend, or do a suspend-reconfigure-unsuspend if it's running. With this, we can replace default-sample-rate with a minimum-sample-rate (anything below this gets upsampled), and use the sink-input sample rate if supported (or an integer multiple as your code does). The advantage of course is that we're no longer restricted to two sample rates. I did a quick walk over the code and this seems feasible. I'm doing something similar (but far simpler) for passthrough mode in alsa-sink - when a passthrough input is added, I do the suspend-update_rate-unsuspend if required and it seems to work fine. Does this make sense or am I missing something basic here? Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] Reconfigure sample rates on resume
On Tue, 2011-03-08 at 07:03 -0600, pl bossart wrote: At sink-input trigger a reconfiguration if possible/supported. Sinks expose a reconfigure_rate method (I'm sure we can find a better name) if they can do a quick switch. So far it's similar to the update_rate() method I added on the sink, In the ALSA sink, this function would check if there are no sink-inputs attached and the rate is supported, and then send a message to the I/O thread to do the actual reconfiguration. We would cache the rate if suspended and apply on unsuspend, or do a suspend-reconfigure-unsuspend if it's running. It's interesting. The only problems I see is a potential duplication of code, since you force the suspend in the sink, and it's already part of module-suspend-on-idle. Meaning you will need to implement this message in the BT sink as well. I wonder if there would be potential I assumed we'd only be implementing this in ALSA for now since we're not allowing reconfiguration on BT in your patches either. Either way, I don't expect there to be too much duplication. races if the suspend-on-idle timeout happens right after you enable the new stream. This should get serialised in the mainloop as far as I can tell. With this, we can replace default-sample-rate with a minimum-sample-rate (anything below this gets upsampled), and use the sink-input sample rate if supported (or an integer multiple as your code does). The advantage of course is that we're no longer restricted to two sample rates. I don't really see a benefit of having more than 2 sample rates. The only exception is passthrough, where you want to forward the sink-input frequency as is. And reading again I think it's the same behavior I implemented? The benefit is just that if you have ultra-high-quality 88.4/96 kHz music, it doesn't get resampled. I honestly don't know how many people out there care about this, but if the hardware supports higher sample rates, I see no reason to not expose the ability to play that. I did a quick walk over the code and this seems feasible. I'm doing something similar (but far simpler) for passthrough mode in alsa-sink - when a passthrough input is added, I do the suspend-update_rate-unsuspend if required and it seems to work fine. Does this make sense or am I missing something basic here? Couldn't you add the same behavior with my patches? If the sink-input is passthrough then the desired rate is the sink-input rate. Overall I am still not clear on what your alternative implementation brings? But I am badly jet-lagged, I may have missed something as well. Yes, whatever the final solution is, I'll definitely be reusing this API in my passthrough work. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 1/2] Log feature:Add a new log target to a file descriptor
Hi, I haven't tested your patch, but some comments on the code below. On Tue, 2011-03-08 at 17:15 +0100, Vincent Becker wrote: This patch enables logging of text debug messages (pa_log feature) into a file or a device driver. Example : pulseaudio --log-target=file:./mylog.txt Signed-off-by: Vincent Becker vincentx.bec...@intel.com --- src/daemon/cmdline.c |5 +++-- src/daemon/daemon-conf.c | 34 -- src/pulsecore/log.c | 24 src/pulsecore/log.h |5 + 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c index f6cdcdc..2c3eb67 100644 --- a/src/daemon/cmdline.c +++ b/src/daemon/cmdline.c @@ -145,7 +145,8 @@ void pa_cmdline_help(const char *argv0) { this time passed\n --log-level[=LEVEL] Increase or set verbosity level\n -vIncrease the verbosity level\n - --log-target={auto,syslog,stderr} Specify the log target\n + --log-target={auto,syslog,stderr,\n + file:PATH} Specify the log target\n --log-meta[=BOOL] Include code location in log messages\n --log-time[=BOOL] Include timestamps in log messages\n --log-backtrace=FRAMESInclude a backtrace in log messages\n @@ -318,7 +319,7 @@ int pa_cmdline_parse(pa_daemon_conf *conf, int argc, char *const argv [], int *d case ARG_LOG_TARGET: if (pa_daemon_conf_set_log_target(conf, optarg) 0) { -pa_log(_(Invalid log target: use either 'syslog', 'stderr' or 'auto'.)); +pa_log(_(Invalid log target: use either 'syslog', 'stderr', 'auto' or a valid file name 'file:path'.)); goto fail; } break; diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c index e38e67a..5696f00 100644 --- a/src/daemon/daemon-conf.c +++ b/src/daemon/daemon-conf.c @@ -28,6 +28,8 @@ #include stdio.h #include string.h #include unistd.h +#include sys/stat.h +#include fcntl.h #ifdef HAVE_SCHED_H #include sched.h @@ -141,6 +143,9 @@ static const pa_daemon_conf default_conf = { #endif }; +static int log_fd = -1; + + pa_daemon_conf* pa_daemon_conf_new(void) { pa_daemon_conf *c; @@ -166,6 +171,9 @@ pa_daemon_conf* pa_daemon_conf_new(void) { void pa_daemon_conf_free(pa_daemon_conf *c) { pa_assert(c); +if (log_fd = 0) +pa_close(log_fd); + pa_xfree(c-script_commands); pa_xfree(c-dl_search_path); pa_xfree(c-default_script_file); @@ -185,8 +193,30 @@ int pa_daemon_conf_set_log_target(pa_daemon_conf *c, const char *string) { } else if (!strcmp(string, stderr)) { c-auto_log_target = 0; c-log_target = PA_LOG_STDERR; -} else -return -1; +} else if (pa_startswith(string, file:)) { +char *file_path; +const char *state = NULL; +unsigned i; + +/* After second pa_split call, file_path will contain the right part of file:path */ +for (i = 0; i 2; i++) +file_path = pa_split(string, :, state); I believe this would leak. You could just use file_path = string[5] here. +/* Check if the file is regular */ +if ((log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT, 0777)) = 0) { Definitely don't want to give execute permissions for that file. For paranoia's sake, I'd use S_IRWXU. +c-auto_log_target = 0; +c-log_target = PA_LOG_FD; +pa_log_set_fd(log_fd); +} +else { +printf(Failed to open target file %s, error : %s\n, file_path, pa_cstrerror(errno)); +pa_xfree(file_path); +return -1; +} +pa_xfree(file_path); +} +else + return -1; return 0; } diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c index 2c0e267..a5e26c8 100644 --- a/src/pulsecore/log.c +++ b/src/pulsecore/log.c @@ -71,6 +71,8 @@ static unsigned show_backtrace = 0, show_backtrace_override = 0, skip_backtrace static pa_log_flags_t flags = 0, flags_override = 0; static pa_bool_t no_rate_limit = FALSE; +static int fdlog = -1; + #ifdef HAVE_SYSLOG_H static const int level_to_syslog[] = { [PA_LOG_ERROR] = LOG_ERR, @@ -128,6 +130,12 @@ void pa_log_set_flags(pa_log_flags_t _flags, pa_log_merge_t merge) { flags = _flags; } +void pa_log_set_fd(int fd) { +pa_assert(fd = 0); + +fdlog = fd; +} + void pa_log_set_show_backtrace(unsigned nlevels) { show_backtrace = nlevels; } @@ -399,6 +407,22 @@ void pa_log_levelv_meta( }
[pulseaudio-discuss] [WIP] Passthrough support
Hello all, Based on the RFC I posted earlier, I've implemented basic passthrough support in some usable shape. It's up on the passthrough branch of each of: PulseAudio: http://git.collabora.co.uk/?p=user/arun/pulseaudio.git;a=shortlog;h=refs/heads/passthrough gst-plugins-base: http://git.collabora.co.uk/?p=user/arun/gst-plugins-base.git;a=shortlog;h=refs/heads/passthrough gst-plugins-good: http://git.collabora.co.uk/?p=user/arun/gst-plugins-good.git;a=shortlog;h=refs/heads/passthrough What's here? All the basic plumbing for clients to signal a compresed format, have PA pick the best supported one, actually play that format, and have pactl list show these. The ALSA sink takes AC3 at the moment, and I should be pushing DTS support today/tomorrow after some more testing. It was a lot simpler for me to test this with pulsesink, so I've made the required changes there (I realise this is a bother for those who don't have local builds of gstreamer handy - sorry!). Eventually, I'd like to have a paplay-type interface for compressed formats (pre-payloaded). What's left? * Triggering renegotiation - I should be getting to this soon * Better proplists - We have one proplist per format+rate pair, which is fine for now, but eventually, we need to freeze the string format for lists/ranges * Proper UI for selecting accepted formats - I'm thinking that we need to add bits to the various UIs to provide a checklist of formats that the user's receiver accepts, set that as a property on the sink, and have a module save this (m-d-m, perhaps, or just a new module) * HDMI - in theory this should be easy to support, but I've not tested this yet * Bluetooth - See postscript * Sink-input passthrough flag to go away - I'll do this after paplay properly supports compressed formats * Disabling monitors - I suspend them for now, which seems to work okay, but we probably want to have way to disable these properly * Disabling volume - I'm planning on piggybacking on Tanu's work here :) So that's that. Comments and feedback appreciated. :) Cheers, Arun p.s.: BT support is being a major pita, mostly because my headset refuses to cooperate. There's a passthrough-bt branch in my PA tree which adapts the a2dp sink to the new API, and Pierre's patch for an IEC61937 payloader that never made it to the mailing list is at http://people.collabora.co.uk/~arun/plbossart-gst-plugins-ugly.patch (just needs a small change to pulseutil.c to set make the appropriate gst-pa type translation). ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Wed, 2011-03-09 at 15:40 +0530, Arun Raghavan wrote: Hello all, Based on the RFC I posted earlier, I've implemented basic passthrough support in some usable shape. It's up on the passthrough branch of each of: PulseAudio: http://git.collabora.co.uk/?p=user/arun/pulseaudio.git;a=shortlog;h=refs/heads/passthrough gst-plugins-base: http://git.collabora.co.uk/?p=user/arun/gst-plugins-base.git;a=shortlog;h=refs/heads/passthrough gst-plugins-good: http://git.collabora.co.uk/?p=user/arun/gst-plugins-good.git;a=shortlog;h=refs/heads/passthrough What's here? All the basic plumbing for clients to signal a compresed format, have PA pick the best supported one, actually play that format, and have pactl list show these. The ALSA sink takes AC3 at the moment, and I should be pushing DTS support today/tomorrow after some more testing. FWIW, this has now been pushed too. Needed some changes to the DTS parser in GStreamer, which can be found at: http://git.collabora.co.uk/?p=user/arun/gst-plugins-bad.git;a=shortlog;h=refs/heads/passthrough -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] What tests on AMD and ARM are needed for Orc-based optimised volume scaling?
Hi Paul, On Thu, 2011-03-10 at 13:02 +0100, Paul Menzel wrote: Dear Arun, your commit messages of commit 4cd90d9e [1] says the following. … Since I haven't been able to test on other architectures, the Orc code is only used when MMX/SSE* is present. This can be changed in the future after testing on AMD and ARM machines. What tests need to be performed or what tests did you run to figure out that it works. Sorry, I should've cleaned up that comment. By AMD, I meant CPUs with 3DNow! but no SSE/MMX support. I don't actually see the 3DNOW flags used at any point other than detection, so there shouldn't be anything to worry about here. On ARM, I actually did a quick test and the Orc performance was significantly worse [1]. I don't think I tested the NEON backend, though. The test is simple - I #define RUN_TEST in each of the svolume_* files that I want to check, bump up the number of iterations to 1 or more, and then load pulseaudio a few times to get a fair measurement (the test generates N random samples and runs the scaling function on them). If you want to try this, you'll also need to adjust the conditional orc initialisation in src/pulsecore/cpu-orc.c. Cheers, Arun [1] The hand-rolled ARM code is faster than the Orc ARM backend because the former uses a single instruction that is available on ARM to do what the Orc function does using multiple instructions. I'd spoken to Orc upstream (David Schleef) about the possibility of having this scaling operation as a basic Orc operation, so that we could generate that instruction on ARM and fallback to the multiple instructions on other architectures. He was amenable to the idea, but I haven't had time to actually hack this together. ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Sat, 2011-03-12 at 15:08 -0600, pl bossart wrote: Hi Arun, I looked at the passthrough branch. Looks pretty good. I wrote down some notes: gstaudioiec61937: - need to check that frame is valid and synchronized on AC3 header before pushing iec payload I'm expecting valid data from the parser - don't think it makes sense to duplicate the code from the parser. Now that you mention it though, I'm only using framed=true for the incoming caps, and this might not actually force the parser to be plugged. Will look into this. - no code for mp3 and dts? DTS is present now. MP3 should be there tomorrow. pulsesink: - do we really need two steps for payload and commit? Make things complicated and creates memory copies As the comment there says, we can avoid the duplicate copy by doing the alloc in the payloader. The base class for pulsesink works at sample granularity (and not byte granularity), so it became necessary to do the payloading near the beginning of the base class render function. - commit needs to prevent toy resampler (maybe this means the sink cannot work iwth a live source since no adjustment is possible. Must've dropped your check somehow. Will put it back. - spec.latency_time = GST_BASE_AUDIO_SINK (psink)-latency_time; if (!gst_ring_buffer_parse_caps (spec, caps)) goto out; Does this work? spec is not initialized? Yes, _parse_caps() fills the spec. - change of routing? Working on this. - formats /* FIXME: Eventually, we want the list of supported formats to be set + * as properties by the GUI based on what the user says is supported by + * the receiver */ +/* FIXME: How do we figure out supported rates? :( */ +unsigned int i, rates[] = { 32000, 44100, 48000 }; For HDMI and SPDIF, 32,44.1 and 48 are mandatory. I think you can query the alsa layer to give a range of supported frequencies since this is known in the driver (either hard-coded or obtained with EDID/eld). I remember seeing some ALSA patches on this for the NVIDIA cards. Great, I'll look this up. sink.c - how do we notify client that volume is disabled? Tanu's started some work to make this possible. Will coordinate with him and reuse the work already being done there. I wasn't too clear on the BT support. You mentioned that it was a different branch and that it stilll nneds my patches to gst-ugly. We'd need to have the payloader code in gsraudioiec61937 instead? I am also not sure how we handle the transition from A2DP w/ SBC to A2DP w/ MP3. In the current code for iec958, there's no need to reconfigure the link. Last, I had 3 conflicts with git master that I had to solve by hand. That's odd. I'll do a rebase and push tomorrow. Thanks for the review! Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Sat, 2011-03-12 at 16:01 -0600, pl bossart wrote: Looked at the BT modifications, doesn't look too bad. I would try first with MP3 support only, so that you don't have reconfiguration issues. If you support MP3 only from the beginning, it should be easier to make progress. Yep, I did that locally. No luck, unfortunately. If you can work on integrating the payloader in gstaudioiec61937 I will be able to help you then. That would be great - I should have the payloader done and pushed tomorrow. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Mon, 2011-03-14 at 11:06 -0500, pl bossart wrote: I've pushed updates to passthrough-bt branches on my trees for pulseaudio (some core changes, rebased to current master), gst-plugins-base (MPEG audio payloader), and gst-plugins-good (pulsesink MPEG support). With all this, you should be able to test on your BT headset. I've got all these hanges, but still no luck. [ume@plb GST]$ gst-launch filesrc location=~/AURAL/Audio/theTest-320.mp3 ! pulsesink device=bluez_sink.00_0B_E4_94_31_9D Setting pipeline to PAUSED ... Pipeline is PREROLLING ... ERROR: from element /GstPipeline:pipeline0/GstPulseSink:pulsesink0: The stream is in the wrong format. Additional debug info: gstbaseaudiosink.c(914): gst_base_audio_sink_preroll (): /GstPipeline:pipeline0/GstPulseSink:pulsesink0: sink not negotiated. ERROR: pipeline doesn't want to preroll. Setting pipeline to NULL ... Freeing pipeline ... [ume@plb GST]$ gst-launch filesrc location=~/AURAL/Audio/theTest-320.mp3 ! mp3parse ! pulsesink device=bluez_sink.00_0B_E4_94_31_9D 0:00:00.022098216 30380 0x8add070 ERROR GST_PIPELINE ../grammar.y:614:gst_parse_perform_link: could not link mpegaudioparse0 to pulsesink0 WARNING: erroneous pipeline: could not link mpegaudioparse0 to pulsesink0 what I am missing in the setup ? PCM playback seems to work fine. There was one bit from pulsesink in gst-plugins-good that I'd missed pushing till a couple of hours ago which actually enables MP3 support on the pulsesink side. I'm guessing that is missing in your tree? If not, running gst-launch with --gst-debug=baseaudiosink:5,pulse*:5 should produce some more verbose logs that I can look at. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Mon, 2011-03-14 at 13:43 -0500, pl bossart wrote: I forced support for pulse1.0 it by hand in config.h and found the same problem with the headset. Trying to figure out the differences with my initial patches. there must be a step missing. ok, found a solution, see diff attached. For some reason the setup_ad2p() did both the SBC and MPEG configuration. Adding a test for the mode solves the issue, probably there was a buffer overflow or something bad. Thanks! I was getting around to this view as well - the set_configuration seems to be failing because we're trying to set MPEG caps on the SBC seid. playing with SBC and MPEG works fine now, meaning that the negotiation works well depending on the payload. but when I try to go back to PCM/SBC the second time I get the following assert in pulseaudio (see gst log below) E: module-bluetooth-device.c: Assertion 'u-write_memchunk.length == u-block_size' failed at modules/bluetooth/module-bluetooth-device.c:1519, function a2dp_process_render(). Aborting. I'll let you debug this one, should be easier :-) :) ack. Other things I noticed: the volume is much higher in passthrough mode, maybe we need to find a way to set the volume on the headset to match the volume used for PCM. Also I heard some high-frequency modulations, typically coding noise, maybe there's still something fishy during the mp3 decode part. I get this sort of thing on, as far as I can tell, one channel as well. I figured the decoder on the CSR chip wasn't that great. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Mon, 2011-03-14 at 14:57 -0500, pl bossart wrote: Other things I noticed: the volume is much higher in passthrough mode, maybe we need to find a way to set the volume on the headset to match the volume used for PCM. Also I heard some high-frequency modulations, typically coding noise, maybe there's still something fishy during the mp3 decode part. I get this sort of thing on, as far as I can tell, one channel as well. I figured the decoder on the CSR chip wasn't that great. Looks to me that the quality is slightly worse than with the initial patches, but it's of course a very subjective assessment since I need to reinstall pulse/gstreamer to check the differences instead of doing an A/B test. Can you check and make sure the payloader doesn't skip/change any bytes? If you dump what is actually sent to the headset and compare to the initial file, you shouldn't have any deltas. Yep, I did that before I pushed out the code (verified a few frames by hand, but I'll do something more extensive in the morning). I'm still seeing the setconf error, but now that it's at least clear it's connecting to the wrong seid, I'm hoping to get this fixed. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Tue, 2011-03-15 at 01:36 +0530, Arun Raghavan wrote: On Mon, 2011-03-14 at 14:57 -0500, pl bossart wrote: Other things I noticed: the volume is much higher in passthrough mode, maybe we need to find a way to set the volume on the headset to match the volume used for PCM. Also I heard some high-frequency modulations, typically coding noise, maybe there's still something fishy during the mp3 decode part. I get this sort of thing on, as far as I can tell, one channel as well. I figured the decoder on the CSR chip wasn't that great. Looks to me that the quality is slightly worse than with the initial patches, but it's of course a very subjective assessment since I need to reinstall pulse/gstreamer to check the differences instead of doing an A/B test. Can you check and make sure the payloader doesn't skip/change any bytes? If you dump what is actually sent to the headset and compare to the initial file, you shouldn't have any deltas. Yep, I did that before I pushed out the code (verified a few frames by hand, but I'll do something more extensive in the morning). I'm still seeing the setconf error, but now that it's at least clear it's connecting to the wrong seid, I'm hoping to get this fixed. Turns out this was very likely a bluez problem. I've sent in a patch [1] that at least gets things working for me. I also see the high-pitched pops and clicks (these very definitely weren't there with your payloader) - should get that pinned down before long. I pushed your change along with a fix for the assert you saw to my tree as well. Cheers, Arun [1] http://thread.gmane.org/gmane.linux.bluez.kernel/11715 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Tue, 2011-03-15 at 18:50 -0600, Kelly Anderson wrote: On 03/09/11 03:10, Arun Raghavan wrote: Hello all, Based on the RFC I posted earlier, I've implemented basic passthrough support in some usable shape. It's up on the passthrough branch of each of: PulseAudio: http://git.collabora.co.uk/?p=user/arun/pulseaudio.git;a=shortlog;h=refs/heads/passthrough gst-plugins-base: http://git.collabora.co.uk/?p=user/arun/gst-plugins-base.git;a=shortlog;h=refs/heads/passthrough gst-plugins-good: http://git.collabora.co.uk/?p=user/arun/gst-plugins-good.git;a=shortlog;h=refs/heads/passthrough Hello, I'm wanting to clone the passthrough head but I haven't been able to figure out which git option I need. So far All I've been able to get is master. git ls-remote git://git.collabora.co.uk/git/user/arun/pulseaudio.git After the clone, you need to do a: git checkout -b passthrough origin/passthrough This will create ans switch to a branch called passthrough which will track the remote branch passthrough from the tree you cloned (origin). You can switch to other branches after this with: git checkout branch Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Tue, 2011-03-15 at 23:13 -0600, Kelly Anderson wrote: On 03/15/11 22:04, Arun Raghavan wrote: On Tue, 2011-03-15 at 18:50 -0600, Kelly Anderson wrote: On 03/09/11 03:10, Arun Raghavan wrote: Hello all, Based on the RFC I posted earlier, I've implemented basic passthrough support in some usable shape. It's up on the passthrough branch of each of: PulseAudio: http://git.collabora.co.uk/?p=user/arun/pulseaudio.git;a=shortlog;h=refs/heads/passthrough gst-plugins-base: http://git.collabora.co.uk/?p=user/arun/gst-plugins-base.git;a=shortlog;h=refs/heads/passthrough gst-plugins-good: http://git.collabora.co.uk/?p=user/arun/gst-plugins-good.git;a=shortlog;h=refs/heads/passthrough Hey Arun, I've built and installed the passthrough branch. Great! Thanks for testing this out. :) Isn't changing sample rates supported yet? So far I can only passthrough sample rates that match the default-sample-rate in daemon.conf. Changeable rates is critical to being able to support for example dts-hd at 192khz, when standard dts will be 48khz. I'm using paplay for basic functionality checks. It will work, but not with paplay, only with the extended API when you create a non-PCM stream. I will rationalise the check for passthrough sink inputs in a bit, but for now, could you change the checks (there are 2) in alsa-sink sink_process_msg() from: if (PA_LIKELY(pa_format_info_is_pcm(i-format))) to: if (PA_LIKELY(!(i-flags PA_SINK_INPUT_PASSTHROUGH))) This should make the sample-rate reconfiguration work for paplay as well. I'll apply your HDMI-related patch and push it to my tree (we should really be probing - I'll look into this once the other bits are done). Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Tue, 2011-03-15 at 17:36 -0500, pl bossart wrote: I pushed your change along with a fix for the assert you saw to my tree as well. I tested the latest batch of patches with AC3/SPDIF this time. Works well in general, however I found some interesting points: - in Rhythmbox, the passthrough mode is only enabled if the first file of playlist is compressed. If you start with PCM/WAV, it always uses the PCM mode. Are you using the crossfading backend? If yes, that won't work since they create their own pipeline with a volume element in between to do the crossfades. With the non-crossfading backend, I see it setting up an MP3 stream when it can and PCM when it can't. - in Totem, the passthrough mode works when the visual effects are disabled. I see a nice DD on my receiver...However the playback stops if you play with the volume slider. I get a pop-up error window saying 'an error occurred: pa_stream_set_sink_input_volume(): invalid argument' Ah, I hadn't tried this out. We should be handling this more gracefully right now - will put this on my TODO list. On a related note, maybe we want to change the approach to volume control. Depending on the sink, there might be ways of using side/in-band channels to send volume control information to the decoder. It's going to be protocol-specific. So maybe we need to provide the client with an information coming from the sink on whether volume control is actually supported or not. Tanu's independently working on flags and notifications for signalling when a sink-input (and sink, I believe) volume control is readable/writeable. We're aiming to have this in 1.0 as well, so we should end up piggybacking on his work for passthrough streams/sinks as well. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Wed, 2011-03-16 at 11:19 +0530, Arun Raghavan wrote: [...] It will work, but not with paplay, only with the extended API when you create a non-PCM stream. I will rationalise the check for passthrough sink inputs in a bit, but for now, could you change the checks (there I've pushed a more comprehensive fix, so you shouldn't need to change that line and using paplay should work for you. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sbc_math.h: add explicit check for ARMv6 instructions
Hi Paul, On Wed, 2011-02-23 at 01:07 +0530, Arun Raghavan wrote: [...] The correct fix for this, imo, is in bluez (there is a new sbc_primitives_armv6.h that can probably be used at least as a template). We need to do an sbc-udpate on the PA side anyway, and can pull this when we do. Could you see if the attached patch works for you? If it does, I can push this to the bluez folks. Cheers, Arun diff --git a/src/modules/bluetooth/sbc/sbc_math.h b/src/modules/bluetooth/sbc/sbc_math.h index b87bc81..35d5dcc 100644 --- a/src/modules/bluetooth/sbc/sbc_math.h +++ b/src/modules/bluetooth/sbc/sbc_math.h @@ -23,6 +23,8 @@ * */ +#include sbc_primitives_armv6.h + #define fabs(x) ((x) 0 ? -(x) : (x)) /* C does not provide an explicit arithmetic shift right but this will always be correct and every compiler *should* generate optimal code */ @@ -47,7 +49,7 @@ typedef int32_t sbc_fixed_t; #define SBC_FIXED_0(val) { val = 0; } #define MUL(a, b)((a) * (b)) -#ifdef __arm__ +#ifdef SBC_HAVE_THUMB2 #define MULA(a, b, res) ({\ int tmp = res; \ __asm__(\ diff --git a/src/modules/bluetooth/sbc/sbc_primitives_armv6.h b/src/modules/bluetooth/sbc/sbc_primitives_armv6.h index 1862aed..e70469a 100644 --- a/src/modules/bluetooth/sbc/sbc_primitives_armv6.h +++ b/src/modules/bluetooth/sbc/sbc_primitives_armv6.h @@ -38,6 +38,12 @@ #define SBC_HAVE_ARMV6 1 #endif +#if defined(__ARM_ARCH_6T2__ ) || defined(__ARM_ARCH_7__) || \ + defined(__ARM_ARCH_7A__) || defined(__ARM_ARCH_7R__) || \ + defined(__ARM_ARCH_7M__) +#define SBC_HAVE_THUMB2 1 +#endif + #if !defined(SBC_HIGH_PRECISION) (SCALE_OUT_BITS == 15) \ defined(__GNUC__) defined(SBC_HAVE_ARMV6) \ defined(__ARM_EABI__) !defined(__thumb__) \ ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Couple of minor warning fixes
Since Maarten's declared war on warnings, thought I'd pitch in with a couple of fixes too. :) The first one is a boo-boo from my Orc work, and the second might be contentious to some (but is right imho). Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 1/2] daemon: Fix missing include - cpu-orc.h
--- src/daemon/main.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/daemon/main.c b/src/daemon/main.c index 533c4c3..f7aed51 100644 --- a/src/daemon/main.c +++ b/src/daemon/main.c @@ -95,6 +95,7 @@ #endif #include pulsecore/cpu-arm.h #include pulsecore/cpu-x86.h +#include pulsecore/cpu-orc.h #include cmdline.h #include cpulimit.h -- 1.7.4.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 2/2] bluetooth: Fix alignment warning
While it seems to be common practice to just work around the cast to integer and assume alignment is fine, let's play it safe and do it right by memcpy'ing. --- src/modules/bluetooth/ipc.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/modules/bluetooth/ipc.c b/src/modules/bluetooth/ipc.c index dcecad8..fdca7a3 100644 --- a/src/modules/bluetooth/ipc.c +++ b/src/modules/bluetooth/ipc.c @@ -106,8 +106,11 @@ int bt_audio_service_get_data_fd(int sk) for (cmsg = CMSG_FIRSTHDR(msgh); cmsg != NULL; cmsg = CMSG_NXTHDR(msgh, cmsg)) { if (cmsg-cmsg_level == SOL_SOCKET -cmsg-cmsg_type == SCM_RIGHTS) - return (*(int *) CMSG_DATA(cmsg)); +cmsg-cmsg_type == SCM_RIGHTS) { + int fd; + memcpy(fd, CMSG_DATA(cmsg), sizeof(int)); + return fd; + } } errno = EINVAL; -- 1.7.4.1 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [WIP] Passthrough support
On Sat, 2011-03-19 at 18:04 -0600, Kelly Anderson wrote: On 03/16/11 10:48, Arun Raghavan wrote: On Wed, 2011-03-16 at 11:19 +0530, Arun Raghavan wrote: [...] It will work, but not with paplay, only with the extended API when you create a non-PCM stream. I will rationalise the check for passthrough sink inputs in a bit, but for now, could you change the checks (there I've pushed a more comprehensive fix, so you shouldn't need to change that line and using paplay should work for you. -- Arun Arun, Hey, I just got around to testing without the following patch. It doesn't work. I also didn't see any recent checkins in the passthrough tree that would seem to affect this. Is it possible the fix didn't get pushed? Yep. :| Pushed now. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] Get rid of some warnings
On Sun, 2011-03-20 at 13:12 +, Colin Guthrie wrote: 'Twas brillig, and Maarten Bosmans at 19/03/11 15:26 did gyre and gimble: Mostly warnings about unused stuff. Furthermore, the first hunk is a fix for the change in 177948a6. Finally, comment in AEC_dtd was translated and the code simplified slightly. Thanks :) CC module_echo_cancel_la-adrian-aec.lo modules/echo-cancel/adrian-aec.h:360:15: warning: ‘AEC_getambient’ defined but not used [-Wunused-function] modules/echo-cancel/adrian-aec.h:368:14: warning: ‘AEC_setgain’ defined but not used [-Wunused-function] modules/echo-cancel/adrian-aec.h:374:14: warning: ‘AEC_setaes’ defined but not used [-Wunused-function] modules/echo-cancel/adrian-aec.h:377:16: warning: ‘AEC_max_dotp_xf_xf’ declared ‘static’ but never defined [-Wunused-function] Is the Adrian code ours or is it external in some capacity. Arun, if it's external and we will update it periodically, can you do the necessary to push where it needs to go? (Obviously the PA_GCC_UNUSED cannot be pushed anywhere). The upstream code isn't actively developed, and if it does, I'm going to have to do a manual merge anyway (since the original code was C++). So all good here. :) Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] Get rid of some warnings
On Sun, 2011-03-20 at 22:56 +0100, Maarten Bosmans wrote: 2011/3/20 Colin Guthrie gm...@colin.guthr.ie: 'Twas brillig, and Maarten Bosmans at 19/03/11 15:26 did gyre and gimble: Mostly warnings about unused stuff. Furthermore, the first hunk is a fix for the change in 177948a6. Finally, comment in AEC_dtd was translated and the code simplified slightly. Thanks :) CC module_echo_cancel_la-adrian-aec.lo modules/echo-cancel/adrian-aec.h:360:15: warning: ‘AEC_getambient’ defined but not used [-Wunused-function] modules/echo-cancel/adrian-aec.h:368:14: warning: ‘AEC_setgain’ defined but not used [-Wunused-function] modules/echo-cancel/adrian-aec.h:374:14: warning: ‘AEC_setaes’ defined but not used [-Wunused-function] modules/echo-cancel/adrian-aec.h:377:16: warning: ‘AEC_max_dotp_xf_xf’ declared ‘static’ but never defined [-Wunused-function] Is the Adrian code ours or is it external in some capacity. Arun, if it's external and we will update it periodically, can you do the necessary to push where it needs to go? (Obviously the PA_GCC_UNUSED cannot be pushed anywhere). I checked that there were already a lot of changes in git after the inital import, and Arun seems to confirm. Arun: what is the license of those files?, it doesn't really say in the header. May be a audit of all files/licenses should be done in general? There's a note about it at the bottom of LICENSE in the top directory. Maybe we should add something similar for the bluez bits as well. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] Move compile-time checks around pa_run_from_build_tree to core-util
On Tue, 2011-03-22 at 16:06 +0100, Maarten Bosmans wrote: Is there a better way than #if defined(__linux__) !defined(__OPTIMIZE__) to check for a debug build? By default the CFLAGS contain -g -O2, so __OPTIMIZE__ will not be defined and running uninstalled does not work. Lennart might be able to answer this better, but to me it sounds like we should just drop the !__OPTIMIZE__ bit. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] svolume_orc.c: error: line 67: unknown directive: .longparam
On Mon, 2011-03-21 at 00:06 +0100, Paul Menzel wrote: Dear PulseAudio folks, [...] I guess it has something to do with commit 4cd90d9e32ca9a23e3c0f7615974ea0c55ff3e49 Author: Arun Raghavan arun.ragha...@collabora.co.uk Date: Mon Oct 25 17:59:08 2010 +0100 volume: Add Orc-based optimised volume scaling This adds volume scaling for 1- and 2-channel software volume scaling using Orc. While testing the MMX and SSE backends on a Core2, I see an ~2x performance benefit over the hand-rolled MMX and SSE code. Since I haven't been able to test on other architectures, the Orc code is only used when MMX/SSE* is present. This can be changed in the future after testing on AMD and ARM machines. but I do not know anything about this. I am using OpenEmbedded with `minimal` or `minimal-uclibc` for `MACHINE = at91sam9260ek`. ORCC 0.4.9 is used on this system. Could you try with Orc 0.4.10? Unfortunately, I don't have a quick way to downgrade my local Orc version to verify? If this is not possible, we can just bump the patch to 0.4.11, which was released a while ago (and is what I'm using). BTW, for 0.9.22 and current stable-queue, the Orc stuff will not get used for anything on ARM. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] Move compile-time checks around pa_run_from_build_tree to core-util
On Thu, 2011-03-24 at 11:46 +0100, Maarten Bosmans wrote: [...] I think only compiling that on developer builds and inlining return FALSE for normal, e.g. distro builds makes sense. However __OPTIMIZE__ is not a good differentiator here. The callers all seem to be initialisation routines only, so why not just let it be called in distro and manual builds? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 1/2] pactl: Accept more volume specification formats
On Thu, 2011-03-24 at 13:07 +, Colin Guthrie wrote: 'Twas brillig, and Maarten Bosmans at 24/03/11 12:44 did gyre and gimble: 2011/3/24 Maarten Bosmans mkbosm...@gmail.com: With this you can specify the volume with 6554, 10%, 0.001 or -60dB, all resulting in the same volume change. I was also going to add relative volumes, such as +3dB and -5%, by detecting a + or - sign in the volume. But that clashes with the absolute dB scale (insofar a dB can ever be absolute) that can also be negative. Any suggestions for graceful handling of this? How about if the first letter of the volume change is an i or a d then this indicated increment or decrement relative volume? It's not as clean as the +/- labelling sadly but such is life. Alternatively your absolute dB volumes could be specified as 60-dB or 7+dB (where 7dB implies 7+dB)... That way the prefix +/- notation could be used for relative adjustments. The only downside there is that setting absolute dB volumes is more confusing (you'd never need to use anything other than XdB for relative adjustments anyway). Personally I'd go for the later as I think relative adjustments are probably more common, so it's syntax should be neatest, but I could be very wrong :D Or maybe just do this as a separate set-volume-step command (or -increment or something better named)? -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] modules/bluetooth/sbc/sbc_primitives.h:41:22: error: expected ':', ',', '; ', '}' or '__attribute__' before 'X'
On Thu, 2011-03-24 at 23:20 +0100, Paul Menzel wrote: Dear PulseAudio folks, I get the following error with latest master (a9c8f904). CC libbluetooth_sbc_la-sbc.lo In file included from modules/bluetooth/sbc/sbc_primitives_armv6.h:30:0, from modules/bluetooth/sbc/sbc_math.h:27, from modules/bluetooth/sbc/sbc.c:46: modules/bluetooth/sbc/sbc_primitives.h:41:22: error: expected ':', ',', ';', '}' or '__attribute__' before 'X' modules/bluetooth/sbc/sbc.c: In function 'sbc_synthesize_four': [...] modules/bluetooth/sbc/sbc.c: In function 'sbc_get_implementation_info': modules/bluetooth/sbc/sbc.c:1216:24: error: 'struct sbc_encoder_state' has no member named 'implementation_info' modules/bluetooth/sbc/sbc.c:1217:1: warning: control reaches end of non-void function [-Wreturn-type] make[3]: *** [libbluetooth_sbc_la-sbc.lo] Error 1 make[3]: Leaving directory `/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/pulseaudio-0.9.22+git-r11.1-r0+a9c8f904b0c4a03e7bff004c103aa5910f8bea3d/git/src' make[2]: *** [all] Error 2 make[2]: Leaving directory `/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/pulseaudio-0.9.22+git-r11.1-r0+a9c8f904b0c4a03e7bff004c103aa5910f8bea3d/git/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/pulseaudio-0.9.22+git-r11.1-r0+a9c8f904b0c4a03e7bff004c103aa5910f8bea3d/git' I am using OpenEmbedded with `minimal` using EGLIBC or `minimal-uclibc` using uClibc for `MACHINE = beagleboard`. Unfortunately I do not know what syntax error is causing this. modules/bluetooth/sbc/sbc_primitives.h:41:22: error: expected ':', ',', ';', '}' or '__attribute__' before 'X' I'm guessing that you've patched sbc_math.h with my earlier patch? If yes, looks like you need to add a #include sbc_tables.h in sbc_primitives.h since it depends on some defines from that file. I guess this ends up happening in all the other instances where that file is used. In the mean time, I'll try to unbreak my cross-compile environment so I can test this stuff locally as well. Cheers, Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [solved, invalid] modules/bluetooth/sbc/sbc_primitives.h:41:22: error: expected ':', ',', '; ', '}' or '__attribute__' before 'X'
On Fri, 2011-03-25 at 08:07 +0100, Paul Menzel wrote: Am Freitag, den 25.03.2011, 11:10 +0530 schrieb Arun Raghavan: On Thu, 2011-03-24 at 23:20 +0100, Paul Menzel wrote: I get the following error with latest master (a9c8f904). CC libbluetooth_sbc_la-sbc.lo In file included from modules/bluetooth/sbc/sbc_primitives_armv6.h:30:0, from modules/bluetooth/sbc/sbc_math.h:27, from modules/bluetooth/sbc/sbc.c:46: modules/bluetooth/sbc/sbc_primitives.h:41:22: error: expected ':', ',', ';', '}' or '__attribute__' before 'X' modules/bluetooth/sbc/sbc.c: In function 'sbc_synthesize_four': [...] modules/bluetooth/sbc/sbc.c: In function 'sbc_get_implementation_info': modules/bluetooth/sbc/sbc.c:1216:24: error: 'struct sbc_encoder_state' has no member named 'implementation_info' modules/bluetooth/sbc/sbc.c:1217:1: warning: control reaches end of non-void function [-Wreturn-type] make[3]: *** [libbluetooth_sbc_la-sbc.lo] Error 1 make[3]: Leaving directory `/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/pulseaudio-0.9.22+git-r11.1-r0+a9c8f904b0c4a03e7bff004c103aa5910f8bea3d/git/src' make[2]: *** [all] Error 2 make[2]: Leaving directory `/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/pulseaudio-0.9.22+git-r11.1-r0+a9c8f904b0c4a03e7bff004c103aa5910f8bea3d/git/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/pulseaudio-0.9.22+git-r11.1-r0+a9c8f904b0c4a03e7bff004c103aa5910f8bea3d/git' I am using OpenEmbedded with `minimal` using EGLIBC or `minimal-uclibc` using uClibc for `MACHINE = beagleboard`. Unfortunately I do not know what syntax error is causing this. modules/bluetooth/sbc/sbc_primitives.h:41:22: error: expected ':', ',', ';', '}' or '__attribute__' before 'X' I'm guessing that you've patched sbc_math.h with my earlier patch? If yes, looks like you need to add a #include sbc_tables.h in sbc_primitives.h since it depends on some defines from that file. I guess this ends up happening in all the other instances where that file is used. I am sorry. I had still two patches applied (your `sbc_math.h` and a revert of 4cd90d9e32ca9a23e3c0f7615974ea0c55ff3e49) which I overlooked. In the mean time, I'll try to unbreak my cross-compile environment so I can test this stuff locally as well. Gentoo crossdev. dbus-glib ebuild is broken while cross-compiling, so I couldn't even get bluez. Found a fix, so hoping to make some progress. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss