Re: [pulseaudio-discuss] [PATCH] add module-virtual-surround-sink
On Fri, 2012-02-17 at 15:56 +0100, Niels Ole Salscheider wrote:
I'd put this variable inside the innermost if, because it's not used
elsewhere in this function.
I think that is not allowed in C89/C90...
I think C89 allows variable declarations at the beginning of any block.
And even if it doesn't, variables are declared inside if blocks all over
Pulseaudio's code, so not doing it here won't achieve anything in terms
of complying with C89.
That sounds sensible. I think at some point I got confused by
sink_input being
the input for the master sink that accepts the output signal I
compute.
I hope I got it right now. I have attached a new version of my patch.
Thanks!
One question about the processing: does it add any latency?
From 56ac4d4a95971818657202677bf0646b3626a2a4 Mon Sep 17 00:00:00 2001
From: Niels Ole Salscheider niels_...@salscheider-online.de
Date: Sun, 8 Jan 2012 21:22:35 +0100
Subject: [PATCH] add module-virtual-surround-sink
It provides a virtual surround sound effect
v2: Normalize hrir to avoid clipping, some cleanups
v3: use fabs, not abs
v4: implement changes proposed by Tanu Kaskinen
---
src/Makefile.am|7 +
...rtual-sink.c = module-virtual-surround-sink.c} | 344
2 files changed, 288 insertions(+), 63 deletions(-)
copy src/modules/{module-virtual-sink.c = module-virtual-surround-sink.c}
(63%)
diff --git a/src/Makefile.am b/src/Makefile.am
index 521bf50..8f942f0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1009,6 +1009,7 @@ modlibexec_LTLIBRARIES += \
module-loopback.la \
module-virtual-sink.la \
module-virtual-source.la \
+ module-virtual-surround-sink.la \
module-switch-on-connect.la \
module-filter-apply.la \
module-filter-heuristics.la
@@ -1309,6 +1310,7 @@ SYMDEF_FILES = \
module-loopback-symdef.h \
module-virtual-sink-symdef.h \
module-virtual-source-symdef.h \
+ module-virtual-surround-sink-symdef.h \
module-switch-on-connect-symdef.h \
module-filter-apply-symdef.h \
module-filter-heuristics-symdef.h
@@ -1525,6 +1527,11 @@ module_virtual_source_la_CFLAGS = $(AM_CFLAGS)
$(SERVER_CFLAGS)
module_virtual_source_la_LDFLAGS = $(MODULE_LDFLAGS)
module_virtual_source_la_LIBADD = $(MODULE_LIBADD)
+module_virtual_surround_sink_la_SOURCES =
modules/module-virtual-surround-sink.c
+module_virtual_surround_sink_la_CFLAGS = $(AM_CFLAGS) $(SERVER_CFLAGS)
+module_virtual_surround_sink_la_LDFLAGS = $(MODULE_LDFLAGS)
+module_virtual_surround_sink_la_LIBADD = $(MODULE_LIBADD)
+
# X11
module_x11_bell_la_SOURCES = modules/x11/module-x11-bell.c
diff --git a/src/modules/module-virtual-sink.c
b/src/modules/module-virtual-surround-sink.c
similarity index 63%
copy from src/modules/module-virtual-sink.c
copy to src/modules/module-virtual-surround-sink.c
index cf11ffa..d3dfe6f 100644
--- a/src/modules/module-virtual-sink.c
+++ b/src/modules/module-virtual-surround-sink.c
@@ -3,6 +3,7 @@
Copyright 2010 Intel Corporation
Contributor: Pierre-Louis Bossart pierre-louis.boss...@intel.com
+Copyright 2012 Niels Ole Salscheider niels_...@salscheider-online.de
PulseAudio is free software; you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as published
@@ -37,11 +38,15 @@
#include pulsecore/rtpoll.h
#include pulsecore/sample-util.h
#include pulsecore/ltdl-helper.h
+#include pulsecore/sound-file.h
+#include pulsecore/resampler.h
-#include module-virtual-sink-symdef.h
+#include math.h
-PA_MODULE_AUTHOR(Pierre-Louis Bossart);
-PA_MODULE_DESCRIPTION(_(Virtual sink));
+#include module-virtual-surround-sink-symdef.h
+
+PA_MODULE_AUTHOR(Niels Ole Salscheider);
+PA_MODULE_DESCRIPTION(_(Virtual surround sink));
PA_MODULE_VERSION(PACKAGE_VERSION);
PA_MODULE_LOAD_ONCE(FALSE);
PA_MODULE_USAGE(
@@ -54,6 +59,8 @@ PA_MODULE_USAGE(
channel_map=channel map
use_volume_sharing=yes or no
force_flat_volume=yes or no
+ hrir=/path/to/left_hrir.wav
+ hrir_channel_map=channel map
));
#define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
@@ -71,6 +78,23 @@ struct userdata {
pa_bool_t auto_desc;
unsigned channels;
+unsigned hrir_channels;
+unsigned master_channels;
+
+unsigned fs, sink_fs;
+
+unsigned *mapping_left;
+unsigned *mapping_right;
+
+unsigned output_left, output_right;
+
+unsigned hrir_samples;
+pa_sample_spec hrir_ss;
+pa_channel_map hrir_map;
+pa_memchunk hrir_chunk;
+
+pa_memchunk input_buffer_chunk;
The hrir data and the input buffer don't really need to be stored inside
memchunks. They could be just
Re: [pulseaudio-discuss] [PATCH] add module-virtual-surround-sink
Hello,
One question about the processing: does it add any latency?
That depends on the impulse response. In general, the current output sample is
a weighted sum of the current input sample and some preceding.
Provided that the first values of the impulse response are not equal (or
approximately) zero there is no added latency.
The hrir data and the input buffer don't really need to be stored inside
memchunks. They could be just arrays of floats.
I fixed this for the input buffer but pa_resampler_run stores the hrir data
inside a memchunk.
I guess master_channels is unneeded now that it's hardcoded to be 2. And
this piece of code can be simplified to be just two assignments to dst.
Fixed. Is it ok to assume that the first channel is left and the second is
right or do I need to get this information from the sink input sample spec?
Pulseaudio's convention is one more indentation level for the cases
inside the switch.
You need to check that hrir_file isn't NULL.
The hrir_temp_chunk memblock needs to be unreffed also in the fail
section if it's non-NULL to avoid memory leaks. For that reason the
memblock pointer also has to be set to NULL here and in the beginning of
the function.
All fixed. I have attached an updated patch.
Wouldn't it be better to default to the hrir file channel map? In that
case the file reading needs to be done before initializing the sink
channel map.
Shouldn't the default hrir channel map be hrir_temp_map, i.e. the
channel map of the file? And aren't hrir_ss and hrir_map redundant
anyway - shouldn't the hrir sample spec and channel map be the same as
what the sink has?
That is what I would like to do, too. But the channel map that is returned by
pa_sound_file_load is wrong. I am not even sure if there is a way to store
the channel map in the wav header.
Because of that I provided a way to specify the channel map of the hrir wav.
Is there a better solution?
Regards,
OleFrom c124c11f82051a835e695b94392eba8b63e27254 Mon Sep 17 00:00:00 2001
From: Niels Ole Salscheider niels_...@salscheider-online.de
Date: Sun, 8 Jan 2012 21:22:35 +0100
Subject: [PATCH] add module-virtual-surround-sink
It provides a virtual surround sound effect
v2: Normalize hrir to avoid clipping, some cleanups
v3: use fabs, not abs
v4: implement changes proposed by Tanu Kaskinen
v5: likewise
---
src/Makefile.am|7 +
...rtual-sink.c = module-virtual-surround-sink.c} | 321
2 files changed, 265 insertions(+), 63 deletions(-)
copy src/modules/{module-virtual-sink.c = module-virtual-surround-sink.c} (66%)
diff --git a/src/Makefile.am b/src/Makefile.am
index 603ccc3..9aa31f4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1019,6 +1019,7 @@ modlibexec_LTLIBRARIES += \
module-loopback.la \
module-virtual-sink.la \
module-virtual-source.la \
+ module-virtual-surround-sink.la \
module-switch-on-connect.la \
module-filter-apply.la \
module-filter-heuristics.la
@@ -1319,6 +1320,7 @@ SYMDEF_FILES = \
module-loopback-symdef.h \
module-virtual-sink-symdef.h \
module-virtual-source-symdef.h \
+ module-virtual-surround-sink-symdef.h \
module-switch-on-connect-symdef.h \
module-filter-apply-symdef.h \
module-filter-heuristics-symdef.h
@@ -1535,6 +1537,11 @@ module_virtual_source_la_CFLAGS = $(AM_CFLAGS) $(SERVER_CFLAGS)
module_virtual_source_la_LDFLAGS = $(MODULE_LDFLAGS)
module_virtual_source_la_LIBADD = $(MODULE_LIBADD)
+module_virtual_surround_sink_la_SOURCES = modules/module-virtual-surround-sink.c
+module_virtual_surround_sink_la_CFLAGS = $(AM_CFLAGS) $(SERVER_CFLAGS)
+module_virtual_surround_sink_la_LDFLAGS = $(MODULE_LDFLAGS)
+module_virtual_surround_sink_la_LIBADD = $(MODULE_LIBADD)
+
# X11
module_x11_bell_la_SOURCES = modules/x11/module-x11-bell.c
diff --git a/src/modules/module-virtual-sink.c b/src/modules/module-virtual-surround-sink.c
similarity index 66%
copy from src/modules/module-virtual-sink.c
copy to src/modules/module-virtual-surround-sink.c
index cf11ffa..78c354e 100644
--- a/src/modules/module-virtual-sink.c
+++ b/src/modules/module-virtual-surround-sink.c
@@ -3,6 +3,7 @@
Copyright 2010 Intel Corporation
Contributor: Pierre-Louis Bossart pierre-louis.boss...@intel.com
+Copyright 2012 Niels Ole Salscheider niels_...@salscheider-online.de
PulseAudio is free software; you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as published
@@ -37,11 +38,15 @@
#include pulsecore/rtpoll.h
#include pulsecore/sample-util.h
#include pulsecore/ltdl-helper.h
+#include pulsecore/sound-file.h
+#include pulsecore/resampler.h
-#include module-virtual-sink-symdef.h
+#include math.h
-PA_MODULE_AUTHOR(Pierre-Louis Bossart);
-PA_MODULE_DESCRIPTION(_(Virtual sink));
+#include module-virtual-surround-sink-symdef.h
+
+PA_MODULE_AUTHOR(Niels Ole Salscheider);
+PA_MODULE_DESCRIPTION(_(Virtual surround sink));
Re: [pulseaudio-discuss] [PATCH] UCM patches on ubuntu 1:1.1-0ubuntu4
On 02/21/2012 04:34 AM, Feng Wei wrote: Hi Arun, I'm not clear what should I do to upstream patches. I tested them on ubuntu, so that I must follow what David had done in port structure. In my original mind, I will first upstream to ubuntu, then pulseaudio community. My current patches are maintained in bzr according to ubuntu, I want them to be merged in ubuntu branch. Hi Feng, Sorry I haven't responded earlier, but I'm not sure what to do about these patches either. As I see it there are at least three problems that need to be resolved: * The competing implementation problem: We've had multiple implementations posted to the PulseAudio mailinglist, one by Janos and Jaska, one by Alejandro and Margarita (probably discontinued?), and one by yourself. It would be great if the UCM community could give us a hint on why we should choose one over another. * The patches are based on an older version of PulseAudio - the one that uses the input devices for jack detection. Ubuntu 12.04, as well as PulseAudio 2.0, will release with the new kcontrol jack detection interface [1]. In essence, your patches do not apply, and should we consider these for Ubuntu 12.04 and/or PulseAudio 2.0 we're in quite a hurry. Perhaps it's even too late, I don't know. * Verification and testing is difficult, for a variety of reasons (this point is not a real blocker like the other two, just a little cumbersome): 1) Requires special hardware. I could probably get hold of some hardware if that was the only thing keeping me from reviewing it though. 2) Requires special configuration files, which are usually kept private by companies. I assume that the files you have used for imx53 and omap are public though (where can they be found)? 3) I *was* going to complain about the lack of UCM documentation, but it seems like running make doc in alsa-lib 1.0.25 does create a UCM page (called case interface), and while it does not look perfect everywhere it seems like most things can actually be figured out. -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic [1] Assuming my recently posted patches pass the review of my fellow developers on this list :-) ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] UCM patches on ubuntu 1:1.1-0ubuntu4
Hi David, I'm appreciated for your comments. UCM really has a long way to go. 2012/2/24 David Henningsson david.hennings...@canonical.com: On 02/21/2012 04:34 AM, Feng Wei wrote: Hi Arun, I'm not clear what should I do to upstream patches. I tested them on ubuntu, so that I must follow what David had done in port structure. In my original mind, I will first upstream to ubuntu, then pulseaudio community. My current patches are maintained in bzr according to ubuntu, I want them to be merged in ubuntu branch. Hi Feng, Sorry I haven't responded earlier, but I'm not sure what to do about these patches either. As I see it there are at least three problems that need to be resolved: * The competing implementation problem: We've had multiple implementations posted to the PulseAudio mailinglist, one by Janos and Jaska, one by Alejandro and Margarita (probably discontinued?), and one by yourself. It would be great if the UCM community could give us a hint on why we should choose one over another. Exactly. Maybe one reason is my patch is the only one to implement the agreed concepts mapping between PA and alsa-lib. * The patches are based on an older version of PulseAudio - the one that uses the input devices for jack detection. Ubuntu 12.04, as well as PulseAudio 2.0, will release with the new kcontrol jack detection interface [1]. In essence, your patches do not apply, and should we consider these for Ubuntu 12.04 and/or PulseAudio 2.0 we're in quite a hurry. Perhaps it's even too late, I don't know. It's really too late. I'll follow the new jack detection method and update my patches tho. * Verification and testing is difficult, for a variety of reasons (this point is not a real blocker like the other two, just a little cumbersome): 1) Requires special hardware. I could probably get hold of some hardware if that was the only thing keeping me from reviewing it though. Understandable. 2) Requires special configuration files, which are usually kept private by companies. I assume that the files you have used for imx53 and omap are public though (where can they be found)? I hold the configs for linaro release at https://code.launchpad.net/~b34248/+junk/alsa-lib-1.0.24.1 3) I *was* going to complain about the lack of UCM documentation, but it seems like running make doc in alsa-lib 1.0.25 does create a UCM page (called case interface), and while it does not look perfect everywhere it seems like most things can actually be figured out. -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic [1] Assuming my recently posted patches pass the review of my fellow developers on this list :-) -- Wei.Feng (irc wei_feng) Linaro Multimedia Team Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
