On Tue, 2013-02-05 at 20:55 +0100, Stefan Huber wrote:
> Remixing one channel map to another is (except for special cases) done
> via a linear mapping between channels, whose corresponding matrix is
> computed by calc_map_table(). The k-th row in this matrix corresponds to
> the coefficients of the linear combination of the input channels that
> result in the k-th output channel. In order to avoid clipping of samples
> we require that the sum of these coefficients is (at most) 1. This
> commit ensures this.
>
> Prior to this commit tests/remix-test.c gives 52 of 132 matrices that
> violate this property. For example:
> 'front-left,front-right,front-center,lfe' -> 'front-left,front-right'
> prior this commit after this commit
> I00 I01 I02 I03 I00 I01 I02 I03
> +------------------------ +------------------------
> O00 | 0.750 0.000 0.375 0.375 O00 | 0.560 0.000 0.240 0.200
> O01 | 0.000 0.750 0.375 0.375 O01 | 0.000 0.560 0.240 0.200
>
> Building the matrix is done in several steps. In order to guarantee
> normalized rows, this patch assures that each step preserves a row-sum
> of 1.0 (or leaves it at 0.0). Most notably, if a center channel is mixed
> onto left and right channels then the weights 0.7 and 0.3 (which sum up
> to 1.0) are used rather than 0.75 and 0.375. Similarly, if the LFE
> channel is mixed onto left and right channels then the weights 0.8 and
> 0.2 are used instead of 0.75 and 0.375.
> ---
> src/pulsecore/resampler.c | 74 ++++++++++++++++++++++++---------------------
> 1 file changed, 40 insertions(+), 34 deletions(-)
Sounds like a good change to me. I'll point out a couple of cosmetic
issues below.
> diff --git a/src/pulsecore/resampler.c b/src/pulsecore/resampler.c
> index 0d21158..2022c83 100644
> --- a/src/pulsecore/resampler.c
> +++ b/src/pulsecore/resampler.c
> @@ -742,11 +742,11 @@ static void calc_map_table(pa_resampler *r) {
> * 7) Make sure S:Center, S:LFE is used:
> *
> * S:Center, S:LFE: If not connected, mix into all D:left, all
> - * D:right, all D:center channels, gain is 0.375. The current (as
> - * result of 1..6) factors should be multiplied by 0.75. (Alt.
> - * suggestion: 0.25 vs. 0.5) If C-front is only mixed into
> - * L-front/R-front if available, otherwise into all L/R channels.
> - * Similarly for C-rear.
> + * D:right, all D:center channels, gain is 0.3. The current (as
> + * result of 1..6) factors should be multiplied by 0.7 (in the case
> + * of S:Center) and 0.8 (in the case of S:LFE). If C-front is only
On that last quoted line, the "If" doesn't seem to belong there.
> + * mixed into L-front/R-front if available, otherwise into all L/R
> + * channels. Similarly for C-rear.
> *
> * S: and D: shall relate to the source resp. destination channels.
> *
> @@ -901,13 +901,11 @@ static void calc_map_table(pa_resampler *r) {
>
> for (ic = 0; ic < n_ic; ic++) {
>
> - if (ic_connected[ic]) {
> - m->map_table_f[oc][ic] *= .9f;
> - continue;
> - }
> + m->map_table_f[oc][ic] *= .9f;
>
> - if (on_left(r->i_cm.map[ic]))
> + if (!ic_connected[ic] && on_left(r->i_cm.map[ic])) {
> m->map_table_f[oc][ic] = .1f / (float) ic_unconnected_left;
> + }
No braces for single-line ifs, please.
--
Tanu
