Re: [pulseaudio-discuss] [PATCH] Add option to disable highpass-filter on non-lfe channels.

2017-08-12 Thread Alexander E. Patrakov
2017-08-12 0:27 GMT+05:00 Markus Ebner :
> On 08/11/2017 06:51 PM, Alexander E. Patrakov wrote:
>>
>> 2017-08-11 21:48 GMT+05:00 Alexander E. Patrakov :
>>>
>>> 2017-08-10 1:27 GMT+05:00 Markus Ebner :

 Added a new configuration option: remix-non-lfe-channels (default: yes),
 which results in the non-lfe channels being highpass-filtered.
 When set to false, the lfe channel is lowpass-filtered and the other
 channels stay untouched.
 Removing the low frequencies from speakers that support low frequencies
 sounds awful.
 The current behavior is the default, since this case is less likely than
 speakers not playing well with low frequencies.
>>>
>>> I tend to reject the patch. In the use case with satellite speakers
>>> that are able to reproduce low frequencies, enabling LFE remixing (and
>>> thus the filter) is a user error.
>>
>> Also, the highpass filter on non-lfe channels was designed to filter
>> out exactly the same frequencies that were sent from the main channels
>> to LFE. I.e. "moved". With your patch, they will be "duplicated",
>> which is wrong.
>>
>>
> I'm not that knowledgeable regarding audio, so my reply represents what I
> think I know of the subject.
>
> I am not quite sure what the problem is. In the Computer Graphics field, the
> simple principle is: All's well that looks well.
> I thought that the same applied to audio. As long as it sounds good, why
> bother with the details?
>
> Most "cheap" 5.1 computer speakers exist of
> - 4 satellites (high to maybe center frequencies)
> - 1 center (center frequencies)
> - 1 subwoofer (low frequenciesa)
>
> Pulseaudio's lfe remix in its current form does exactly what is best for
> this setup, since only the subwoofer is able to represent low frequencies.

Well, except for the scaling factor for the audio moved into the
subwoofer. That's a known bug, but nobody among devs knows what's
correct.

> Whereas the satellites of high-class speaker-setups (like the "Teufel
> Theater 500 Surround 5.1 Set") support the full spectrum of frequencies.
> The satellite of the above mentioned setup ("Tower Speaker T 500 F 16") has
> dedicated woofers inside.
> And if I haven't made a mistake, driving this setup with the lfe-remix
> enabled will result in the towers' woofers to effectively be disabled.
> Why would I pay 1700€ for a high-class setup if I can't properly drive its
> speakers?

Correct. The question is, again: why does one want lfe-remix enabled
in such setup?

You either want to reproduce low frequencies with woofers inside
satellites, or to move them to the subwoofer. But note that there is
no sharp transition between them, and that's important.

> Sure, this particular setup probably has an analog frequency-separation
> filter inside the subwoofer, but there are setups that haven't. In such a
> setup (without filter), disabling the lfe-remixer will lead to the subwoofer
> playing the whole frequency spectrum - which isn't that good of an idea. And
> not all analog filters are better than pulseaudios' (We tested a couple).
>
> Our particular setup is:
> - 2 x the box CL 110 Top MK II (90 - 19000Hz)
> - 1 x custom made subwoofer (without an analog frequency filter inside) (20
> - 140Hz)
> Comparing the unpatched pulseaudio (lfe remix on/off) does not sound good,
> while with the patch, it sounds much better (on).

If this is reproducible with a non-custom subwoofer (in a whole system
that passed the sales person approval and was tested with a non-PC
signal source), I accept the bug report, but not the solution. In this
custom case, I would call it a subwoofer design error. In other words,
if the system cannot be configured to sound well with an off-the-shelf
receiver (such as Onkyo TX-NR 636, which does not support filtering
only the subwoofer), I see no point in supporting it with PulseAudio.

Basically, no matter how good the separation filter is, there is a
frequency band (in PulseAudio's LR4 filter, that's approximately two
octaves) where the signal is reproduced both by the satellites and the
subwoofer. In this overlap area, the satellites and the subwoofer must
sound exactly(*) in-phase. That's why matching filters are required,
so that the delay introduced by them always differs by 360 degrees of
phase. And in this overlap area, in a good system, nether the
satellites nor the subwoofer must display any response roll-off or
changing phase delays. So the woofers inside satellites are indeed
properly driven even if LFE remixing is enabled.

(*) The "exactly" requirement is so that to minimize imbalance caused
by movements of the listener. Details here:
http://www.nextdigital.com.br/Linkwitz_%20AES%201976.pdf

What has happened in your system (with insufficient overlap of speaker
responses) is that the satellites themselves, via their acoustic
properties, took the role of the high-pass filter that you want to
disable in PulseAudio. That's valid, but in this case the subwoofer
must have a matching (and not random

Re: [pulseaudio-discuss] [PATCH] Add option to disable highpass-filter on non-lfe channels.

2017-08-11 Thread Markus Ebner

On 08/11/2017 06:51 PM, Alexander E. Patrakov wrote:

2017-08-11 21:48 GMT+05:00 Alexander E. Patrakov :

2017-08-10 1:27 GMT+05:00 Markus Ebner :

Added a new configuration option: remix-non-lfe-channels (default: yes), which 
results in the non-lfe channels being highpass-filtered.
When set to false, the lfe channel is lowpass-filtered and the other channels 
stay untouched.
Removing the low frequencies from speakers that support low frequencies sounds 
awful.
The current behavior is the default, since this case is less likely than 
speakers not playing well with low frequencies.

I tend to reject the patch. In the use case with satellite speakers
that are able to reproduce low frequencies, enabling LFE remixing (and
thus the filter) is a user error.

Also, the highpass filter on non-lfe channels was designed to filter
out exactly the same frequencies that were sent from the main channels
to LFE. I.e. "moved". With your patch, they will be "duplicated",
which is wrong.


I'm not that knowledgeable regarding audio, so my reply represents what 
I think I know of the subject.


I am not quite sure what the problem is. In the Computer Graphics field, 
the simple principle is: All's well that looks well.
I thought that the same applied to audio. As long as it sounds good, why 
bother with the details?


Most "cheap" 5.1 computer speakers exist of
- 4 satellites (high to maybe center frequencies)
- 1 center (center frequencies)
- 1 subwoofer (low frequenciesa)

Pulseaudio's lfe remix in its current form does exactly what is best for 
this setup, since only the subwoofer is able to represent low frequencies.


Whereas the satellites of high-class speaker-setups (like the "Teufel 
Theater 500 Surround 5.1 Set") support the full spectrum of frequencies.
The satellite of the above mentioned setup ("Tower Speaker T 500 F 16") 
has dedicated woofers inside.
And if I haven't made a mistake, driving this setup with the lfe-remix 
enabled will result in the towers' woofers to effectively be disabled.
Why would I pay 1700€ for a high-class setup if I can't properly drive 
its speakers?


Sure, this particular setup probably has an analog frequency-separation 
filter inside the subwoofer, but there are setups that haven't. In such 
a setup (without filter), disabling the lfe-remixer will lead to the 
subwoofer playing the whole frequency spectrum - which isn't that good 
of an idea. And not all analog filters are better than pulseaudios' (We 
tested a couple).


Our particular setup is:
- 2 x the box CL 110 Top MK II (90 - 19000Hz)
- 1 x custom made subwoofer (without an analog frequency filter inside) 
(20 - 140Hz)
Comparing the unpatched pulseaudio (lfe remix on/off) does not sound 
good, while with the patch, it sounds much better (on).

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


[pulseaudio-discuss] [PATCH] Add option to disable highpass-filter on non-lfe channels.

2017-08-11 Thread Markus Ebner
Added a new configuration option: remix-non-lfe-channels (default: yes), which 
results in the non-lfe channels being highpass-filtered.
When set to false, the lfe channel is lowpass-filtered and the other channels 
stay untouched.
Removing the low frequencies from speakers that support low frequencies sounds 
awful.
The current behavior is the default, since this case is less likely than 
speakers not playing well with low frequencies.
---
 src/daemon/daemon-conf.c   |   2 +
 src/daemon/daemon-conf.h   |   1 +
 src/daemon/daemon.conf.in  |   1 +
 src/daemon/main.c  |   1 +
 src/modules/module-virtual-surround-sink.c |   2 +-
 src/pulsecore/core.c   |   1 +
 src/pulsecore/core.h   |   1 +
 src/pulsecore/filter/biquad.c  |   9 ++
 src/pulsecore/filter/biquad.h  |   2 +
 src/pulsecore/filter/crossover.c   | 128 +++--
 src/pulsecore/filter/lfe-filter.c  |   7 +-
 src/pulsecore/filter/lfe-filter.h  |   2 +-
 src/pulsecore/resampler.c  |   5 +-
 src/pulsecore/resampler.h  |   3 +-
 src/pulsecore/sink-input.c |   2 +
 src/pulsecore/source-output.c  |   2 +
 16 files changed, 100 insertions(+), 69 deletions(-)

diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
index 98831267..cf14d0db 100644
--- a/src/daemon/daemon-conf.c
+++ b/src/daemon/daemon-conf.c
@@ -86,6 +86,7 @@ static const pa_daemon_conf default_conf = {
 .remixing_use_all_sink_channels = true,
 .disable_lfe_remixing = true,
 .lfe_crossover_freq = 0,
+.remix_non_lfe_channels = true,
 .config_file = NULL,
 .use_pid_file = true,
 .system_instance = false,
@@ -562,6 +563,7 @@ int pa_daemon_conf_load(pa_daemon_conf *c, const char 
*filename) {
 { "disable-lfe-remixing",   pa_config_parse_bool, 
&c->disable_lfe_remixing, NULL },
 { "enable-lfe-remixing",pa_config_parse_not_bool, 
&c->disable_lfe_remixing, NULL },
 { "lfe-crossover-freq", pa_config_parse_unsigned, 
&c->lfe_crossover_freq, NULL },
+{ "remix-non-lfe-channels", pa_config_parse_bool, 
&c->remix_non_lfe_channels, NULL },
 { "load-default-script-file",   pa_config_parse_bool, 
&c->load_default_script_file, NULL },
 { "shm-size-bytes", pa_config_parse_size, 
&c->shm_size, NULL },
 { "log-meta",   pa_config_parse_bool, 
&c->log_meta, NULL },
diff --git a/src/daemon/daemon-conf.h b/src/daemon/daemon-conf.h
index 953ea33a..d9ccb8b5 100644
--- a/src/daemon/daemon-conf.h
+++ b/src/daemon/daemon-conf.h
@@ -71,6 +71,7 @@ typedef struct pa_daemon_conf {
 disable_remixing,
 remixing_use_all_sink_channels,
 disable_lfe_remixing,
+remix_non_lfe_channels,
 load_default_script_file,
 disallow_exit,
 log_meta,
diff --git a/src/daemon/daemon.conf.in b/src/daemon/daemon.conf.in
index a9555238..0202a2c9 100644
--- a/src/daemon/daemon.conf.in
+++ b/src/daemon/daemon.conf.in
@@ -58,6 +58,7 @@ ifelse(@HAVE_DBUS@, 1, [dnl
 ; enable-remixing = yes
 ; remixing-use-all-sink-channels = yes
 ; enable-lfe-remixing = no
+; remix-non-lfe-channels = yes
 ; lfe-crossover-freq = 0
 
 ; flat-volumes = yes
diff --git a/src/daemon/main.c b/src/daemon/main.c
index f35252d0..12f27754 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -1040,6 +1040,7 @@ int main(int argc, char *argv[]) {
 c->disable_remixing = conf->disable_remixing;
 c->remixing_use_all_sink_channels = conf->remixing_use_all_sink_channels;
 c->disable_lfe_remixing = conf->disable_lfe_remixing;
+c->remix_non_lfe_channels = conf->remix_non_lfe_channels;
 c->deferred_volume = conf->deferred_volume;
 c->running_as_daemon = conf->daemonize;
 c->disallow_exit = conf->disallow_exit;
diff --git a/src/modules/module-virtual-surround-sink.c 
b/src/modules/module-virtual-surround-sink.c
index 09c5e6dd..a879a06d 100644
--- a/src/modules/module-virtual-surround-sink.c
+++ b/src/modules/module-virtual-surround-sink.c
@@ -787,7 +787,7 @@ int pa__init(pa_module*m) {
 
 /* resample hrir */
 resampler = pa_resampler_new(u->sink->core->mempool, &hrir_temp_ss, 
&hrir_map, &hrir_ss, &hrir_map, u->sink->core->lfe_crossover_freq,
- PA_RESAMPLER_SRC_SINC_BEST_QUALITY, 
PA_RESAMPLER_NO_REMAP);
+ 
u->sink->core->remix_non_lfe_channels,PA_RESAMPLER_SRC_SINC_BEST_QUALITY, 
PA_RESAMPLER_NO_REMAP);
 
 u->hrir_samples = hrir_temp_chunk.length / pa_frame_size(&hrir_temp_ss) * 
hrir_ss.rate / hrir_temp_ss.rate;
 if (u->hrir_samples > 64) {
diff --git a/src/pulsecore/core.c b/src/pulsecore/core.c
index e01677d5..395939ce 100644
--- a/src/pulsecore/core.c
+++ b/src/pulsecore/core.c
@@ -144,6 +144,7 @@ pa_core* pa_core_new(pa