On 2015-02-01 16:15, Alexander E. Patrakov wrote:
29.01.2015 03:14, David Henningsson wrote:
When enable-lfe-remixing is set, an LFE channel is present in the
resampler's destination channel map but not in the source channel map,
we insert a low-pass filter instead of just averaging the channels.
Other channels will get a high-pass filter.
That's a policy decision. Essentially, your code assumes that the LFE
channel, if present, is correct for the target speakers. Other possible
policies would be to add it to center and then always regenerate it, or
to add the regenerated content to the original LFE content.
But, for the time being, let's proceed with this policy.
In this patch, the crossover frequency is hardcoded to 120Hz (to be fixed
in later patches).
Note that in current state the LFE filter is
- not very optimised
- not rewind friendly (rewinding can cause audible artifacts)
Signed-off-by: David Henningsson <david.hennings...@canonical.com>
---
src/Makefile.am | 3 ++
src/pulsecore/filter/crossover.c | 79 ++++++++++++++++++++++++++++++-
src/pulsecore/filter/crossover.h | 6 +++
src/pulsecore/filter/lfe-filter.c | 97
+++++++++++++++++++++++++++++++++++++++
src/pulsecore/filter/lfe-filter.h | 38 +++++++++++++++
src/pulsecore/resampler.c | 34 ++++++++++++--
src/pulsecore/resampler.h | 3 ++
7 files changed, 255 insertions(+), 5 deletions(-)
create mode 100644 src/pulsecore/filter/lfe-filter.c
create mode 100644 src/pulsecore/filter/lfe-filter.h
diff --git a/src/Makefile.am b/src/Makefile.am
index e37a22a..d200271 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -911,6 +911,9 @@ lib_LTLIBRARIES += libpulsecore-@PA_MAJORMINOR@.la
# Pure core stuff
libpulsecore_@PA_MAJORMINOR@_la_SOURCES = \
+ pulsecore/filter/lfe-filter.c pulsecore/filter/lfe-filter.h \
+ pulsecore/filter/biquad.c pulsecore/filter/biquad.h \
+ pulsecore/filter/crossover.c pulsecore/filter/crossover.h \
pulsecore/asyncmsgq.c pulsecore/asyncmsgq.h \
pulsecore/asyncq.c pulsecore/asyncq.h \
pulsecore/auth-cookie.c pulsecore/auth-cookie.h \
diff --git a/src/pulsecore/filter/crossover.c
b/src/pulsecore/filter/crossover.c
index 11a8c6e..c902099 100644
--- a/src/pulsecore/filter/crossover.c
+++ b/src/pulsecore/filter/crossover.c
@@ -3,10 +3,10 @@
* found in the LICENSE file.
*/
-#include "crossover.h"
#include "biquad.h"
+#include "crossover.h"
-static void lr4_set(struct lr4 *lr4, enum biquad_type type, float freq)
+void lr4_set(struct lr4 *lr4, enum biquad_type type, float freq)
{
struct biquad q;
biquad_set(&q, type, freq, 0, 0);
@@ -23,6 +23,81 @@ static void lr4_set(struct lr4 *lr4, enum
biquad_type type, float freq)
lr4->z2 = 0;
}
+void lr4_process(struct lr4 *lr4, int samples, int channels, float
*data0)
+{
+ float lx1 = lr4->x1;
+ float lx2 = lr4->x2;
+ float ly1 = lr4->y1;
+ float ly2 = lr4->y2;
+ float lz1 = lr4->z1;
+ float lz2 = lr4->z2;
+ float lb0 = lr4->b0;
+ float lb1 = lr4->b1;
+ float lb2 = lr4->b2;
+ float la1 = lr4->a1;
+ float la2 = lr4->a2;
+
+ int i;
+ for (i = 0; i < samples; i += channels) {
+ float x, y, z;
+ x = data0[i];
+ y = lb0*x + lb1*lx1 + lb2*lx2 - la1*ly1 - la2*ly2;
+ z = lb0*y + lb1*ly1 + lb2*ly2 - la1*lz1 - la2*lz2;
+ lx2 = lx1;
+ lx1 = x;
+ ly2 = ly1;
+ ly1 = y;
+ lz2 = lz1;
+ lz1 = z;
+ data0[i] = z;
In-place modification. Why?
+ }
+
+ lr4->x1 = lx1;
+ lr4->x2 = lx2;
+ lr4->y1 = ly1;
+ lr4->y2 = ly2;
+ lr4->z1 = lz1;
+ lr4->z2 = lz2;
+}
+
+void lr4_process_s16(struct lr4 *lr4, int samples, int channels,
short *data0)
+{
+ float lx1 = lr4->x1;
+ float lx2 = lr4->x2;
+ float ly1 = lr4->y1;
+ float ly2 = lr4->y2;
+ float lz1 = lr4->z1;
+ float lz2 = lr4->z2;
+ float lb0 = lr4->b0;
+ float lb1 = lr4->b1;
+ float lb2 = lr4->b2;
+ float la1 = lr4->a1;
+ float la2 = lr4->a2;
+
+ int i;
+ for (i = 0; i < samples; i += channels) {
+ float x, y, z;
+ x = data0[i];
+ y = lb0*x + lb1*lx1 + lb2*lx2 - la1*ly1 - la2*ly2;
+ z = lb0*y + lb1*ly1 + lb2*ly2 - la1*lz1 - la2*lz2;
+ lx2 = lx1;
+ lx1 = x;
+ ly2 = ly1;
+ ly1 = y;
+ lz2 = lz1;
+ lz1 = z;
+ data0[i] = z;
Filters can overshoot. Please clamp to the range of short.
Is this really true here - as Butterworth and LR4 filters never boost
any frequency, only attenuate, can this filter still overshoot?
+ }
+
+ lr4->x1 = lx1;
+ lr4->x2 = lx2;
+ lr4->y1 = ly1;
+ lr4->y2 = ly2;
+ lr4->z1 = lz1;
+ lr4->z2 = lz2;
+}
+
+
/* Split input data using two LR4 filters, put the result into the
input array
* and another array.
*
diff --git a/src/pulsecore/filter/crossover.h
b/src/pulsecore/filter/crossover.h
index 99a601c..aa1140f 100644
--- a/src/pulsecore/filter/crossover.h
+++ b/src/pulsecore/filter/crossover.h
@@ -25,6 +25,12 @@ struct lr4 {
float z1, z2;
};
+void lr4_set(struct lr4 *lr4, enum biquad_type type, float freq);
+
+void lr4_process(struct lr4 *lr4, int samples, int channels, float
*data0);
+void lr4_process_s16(struct lr4 *lr4, int samples, int channels,
short *data0);
+
+
/* Three bands crossover filter:
*
* INPUT --+-- lp0 --+-- lp1 --+---> LOW (0)
diff --git a/src/pulsecore/filter/lfe-filter.c
b/src/pulsecore/filter/lfe-filter.c
new file mode 100644
index 0000000..1597eca
--- /dev/null
+++ b/src/pulsecore/filter/lfe-filter.c
@@ -0,0 +1,97 @@
+/***
+ This file is part of PulseAudio.
+
+ Copyright 2014 David Henningsson, Canonical Ltd.
+
+ PulseAudio is free software; you can redistribute it and/or modify
+ it under the terms of the GNU Lesser General Public License as
published
+ by the Free Software Foundation; either version 2.1 of the License,
+ or (at your option) any later version.
+
+ PulseAudio is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
License
+ along with PulseAudio; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ USA.
+***/
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include "lfe-filter.h"
+#include <pulse/xmalloc.h>
+#include <pulsecore/filter/biquad.h>
+#include <pulsecore/filter/crossover.h>
+
+// An LR4 filter, implemented as a chain of two LR2 filters.
As a chain of two Butterworth filters. LR2 is not Butterworth.
Butterworth filter has Q == 0.707, LR2 has Q == 0.5.
Ack
+
+struct pa_lfe_filter {
+ float crossover;
+ pa_channel_map cm;
+ pa_sample_spec ss;
+ bool active;
+ struct lr4 lr4[PA_CHANNELS_MAX];
The allocation (and the fact that there is one filter per output
channel) is non-obvious here and should be commented better. For each
non-LFE channel, a trivial implementation would have one struct lr4 for
highpass filter and one struct lr4 for lowpass filter. Here you reduced
the number of the needed filters by pre-averaging non-LFE input channels
and running them through a single filter.
The existing remap algorithm - or policy, if you prefer - is to average
all channels and put it on the LFE channel.
It seems to me that if you wanted another remap algorithm that should be
a comment in setup_remap(), not here. I wouldn't say that this filter
"expects" anything specific, it just removes frequencies according to
the crossover, and it's the job of setup_remap() to set things up correctly.
I could even add a channel bitmask to make it more generic and allow
other channels to be high- or lowpass than the current fixed "LFE is
lowpass everything else is highpass", but I thought it was unnecessary
complexity at this point.
+};
+
+pa_lfe_filter_t * pa_lfe_filter_new(const pa_sample_spec* ss, const
pa_channel_map* cm, float crossover_freq) {
+
+ pa_lfe_filter_t *f = pa_xnew0(struct pa_lfe_filter, 1);
+ f->crossover = crossover_freq;
+ f->cm = *cm;
+ f->ss = *ss;
+ pa_lfe_filter_update_rate(f, ss->rate);
+ return f;
+}
+
+void pa_lfe_filter_free(pa_lfe_filter_t *f) {
+ pa_xfree(f);
+}
+
+void pa_lfe_filter_reset(pa_lfe_filter_t *f) {
+ pa_lfe_filter_update_rate(f, f->ss.rate);
+}
+
+pa_memchunk * pa_lfe_filter_process(pa_lfe_filter_t *f, pa_memchunk
*buf) {
Maybe a comment is needed that this function expects an average of all
non-LFE channels in the would-be-LFE channel? Not sure how this would
mix with the alternative LFE channel generation policies.
See above.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
