Hi Tanu,
thanks for the comments!
>> + if (r->flags & PA_RESAMPLER_NO_REMAP) {
>> + pa_assert(!remix);
>> + assert(n_oc == n_ic);
>This assertion is not correct. The number of channels can be different.
>In that case, only the first min(n_ic, n_oc) channels should be
>connected.
Ok, I changed it. (I thought, if there is no remapping then the
remapping matrix needs to be the identity matrix, hence, a square
matrix, hence n_oc == n_ic.)
>> + else if (b == PA_CHANNEL_POSITION_MONO) {
>> + if (n_ic)
>Redundant check.
Yes, indeed :)
>> - /* Try to find matching input ports for this output port */
>> + for (oc = 0; oc < n_oc; oc++) {
>> + pa_bool_t oc_connected = FALSE;
>While you're rewriting this function, you could also change every
>pa_bool_t to bool, every TRUE to true and every FALSE to false.
Hm. Consequently, the helper functions like on_left, front_rear_side,
etc. should be adopted, too. So if we make changes for a few functions
we should do that for the entire file. Shouldn't this be better done in
a separate commit?
Actually, I found an additional simplification and added minor fixes.
This is a diff of resampler.c after the previous version of this commit
and after the current commit (remix-test still gives the same result):
--- ../../pulseaudio-B/src/pulsecore/resampler.c 2013-02-07 09:51:13.745431092 +0100
+++ ../src/pulsecore/resampler.c 2013-02-07 10:27:00.557435564 +0100
@@ -653,14 +653,6 @@
pa_strbuf *s;
char *t;
pa_remap_t *m;
- unsigned
- ic_left = 0,
- ic_right = 0,
- ic_center = 0,
- ic_unconnected_left = 0,
- ic_unconnected_right = 0,
- ic_unconnected_center = 0,
- ic_unconnected_lfe = 0;
pa_assert(r);
@@ -680,10 +672,9 @@
if (r->flags & PA_RESAMPLER_NO_REMAP) {
pa_assert(!remix);
- assert(n_oc == n_ic);
- for (oc = 0; oc < n_oc; oc++)
- m->map_table_f[oc][oc] = 1.0;
+ for (oc = 0; oc < PA_MIN(n_ic, n_oc); oc++)
+ m->map_table_f[oc][oc] = 1.0f;
} else if (r->flags & PA_RESAMPLER_NO_REMIX) {
pa_assert(!remix);
@@ -695,11 +686,10 @@
/* We shall not do any remixing. Hence, just check by name */
if (a == b)
- m->map_table_f[oc][ic] = 1.0;
+ m->map_table_f[oc][ic] = 1.0f;
}
}
} else {
- pa_assert(remix);
/* OK, we shall do the full monty: upmixing and downmixing. Our
* algorithm is relatively simple, does not do spacialization, delay
@@ -761,6 +751,17 @@
* to do our best to pass it to L+R.
*/
+ unsigned
+ ic_left = 0,
+ ic_right = 0,
+ ic_center = 0,
+ ic_unconnected_left = 0,
+ ic_unconnected_right = 0,
+ ic_unconnected_center = 0,
+ ic_unconnected_lfe = 0;
+
+ pa_assert(remix);
+
for (ic = 0; ic < n_ic; ic++) {
if (on_left(r->i_cm.map[ic]))
ic_left++;
@@ -778,14 +779,13 @@
pa_channel_position_t a = r->i_cm.map[ic];
if (a == b || a == PA_CHANNEL_POSITION_MONO) {
- m->map_table_f[oc][ic] = 1.0;
+ m->map_table_f[oc][ic] = 1.0f;
oc_connected = TRUE;
ic_connected[ic] = TRUE;
}
else if (b == PA_CHANNEL_POSITION_MONO) {
- if (n_ic)
- m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
+ m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
oc_connected = TRUE;
ic_connected[ic] = TRUE;
@@ -828,15 +828,18 @@
} else if (on_center(b)) {
- /* We are not connected and at the center. Let's average
- * all center input channels. */
if (ic_center > 0) {
+
+ /* We are not connected and at the center. Let's average
+ * all center input channels. */
+
for (ic = 0; ic < n_ic; ic++)
if (on_center(r->i_cm.map[ic])) {
m->map_table_f[oc][ic] = 1.0f / (float) ic_center;
ic_connected[ic] = TRUE;
}
+
} else if (ic_left + ic_right > 0) {
/* Hmm, no center channel around, let's synthesize it
@@ -853,21 +856,16 @@
* right input channel. Something is really wrong in this
* case anyway. */
- } else if (on_lfe(b)) {
+ } else if (on_lfe(b) && !(r->flags & PA_RESAMPLER_NO_LFE)) {
/* We are not connected and an LFE. Let's average all
* channels for LFE. */
- for (ic = 0; ic < n_ic; ic++) {
-
- if (!(r->flags & PA_RESAMPLER_NO_LFE))
- m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
- else
- m->map_table_f[oc][ic] = 0;
+ for (ic = 0; ic < n_ic; ic++)
+ m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
- /* Please note that a channel connected to LFE doesn't
- * really count as connected. */
- }
+ /* Please note that a channel connected to LFE doesn't
+ * really count as connected. */
}
}
}
-- >8 --
- Separate the cases with PA_RESAMPLER_NO_REMAP or PA_RESAMPLER_NO_REMIX
set and remove redundant if-conditions.
- Fix C90 compiler warning due to mixing code and variable declaration.
- Do not repeatedly count number of left, right and center channels in
the input channel map.
The logic of calc_map_table() remains unaltered.
---
src/pulsecore/resampler.c | 366 +++++++++++++++++++++------------------------
1 file changed, 171 insertions(+), 195 deletions(-)
diff --git a/src/pulsecore/resampler.c b/src/pulsecore/resampler.c
index f0b3fd4..5eaa943 100644
--- a/src/pulsecore/resampler.c
+++ b/src/pulsecore/resampler.c
@@ -668,228 +668,207 @@ static void calc_map_table(pa_resampler *r) {
memset(m->map_table_i, 0, sizeof(m->map_table_i));
memset(ic_connected, 0, sizeof(ic_connected));
- remix = (r->flags & (PA_RESAMPLER_NO_REMAP|PA_RESAMPLER_NO_REMIX)) == 0;
+ remix = (r->flags & (PA_RESAMPLER_NO_REMAP | PA_RESAMPLER_NO_REMIX)) == 0;
- for (oc = 0; oc < n_oc; oc++) {
- pa_bool_t oc_connected = FALSE;
- pa_channel_position_t b = r->o_cm.map[oc];
-
- for (ic = 0; ic < n_ic; ic++) {
- pa_channel_position_t a = r->i_cm.map[ic];
+ if (r->flags & PA_RESAMPLER_NO_REMAP) {
+ pa_assert(!remix);
- if (r->flags & PA_RESAMPLER_NO_REMAP) {
- /* We shall not do any remapping. Hence, just check by index */
+ for (oc = 0; oc < PA_MIN(n_ic, n_oc); oc++)
+ m->map_table_f[oc][oc] = 1.0f;
- if (ic == oc)
- m->map_table_f[oc][ic] = 1.0;
+ } else if (r->flags & PA_RESAMPLER_NO_REMIX) {
+ pa_assert(!remix);
+ for (oc = 0; oc < n_oc; oc++) {
+ pa_channel_position_t b = r->o_cm.map[oc];
- continue;
- }
+ for (ic = 0; ic < n_ic; ic++) {
+ pa_channel_position_t a = r->i_cm.map[ic];
- if (r->flags & PA_RESAMPLER_NO_REMIX) {
/* We shall not do any remixing. Hence, just check by name */
-
if (a == b)
- m->map_table_f[oc][ic] = 1.0;
-
- continue;
+ m->map_table_f[oc][ic] = 1.0f;
}
+ }
+ } else {
- pa_assert(remix);
-
- /* OK, we shall do the full monty: upmixing and
- * downmixing. Our algorithm is relatively simple, does
- * not do spacialization, delay elements or apply lowpass
- * filters for LFE. Patches are always welcome,
- * though. Oh, and it doesn't do any matrix
- * decoding. (Which probably wouldn't make any sense
- * anyway.)
- *
- * This code is not idempotent: downmixing an upmixed
- * stereo stream is not identical to the original. The
- * volume will not match, and the two channels will be a
- * linear combination of both.
- *
- * This is loosely based on random suggestions found on the
- * Internet, such as this:
- * http://www.halfgaar.net/surround-sound-in-linux and the
- * alsa upmix plugin.
- *
- * The algorithm works basically like this:
- *
- * 1) Connect all channels with matching names.
- *
- * 2) Mono Handling:
- * S:Mono: Copy into all D:channels
- * D:Mono: Avg all S:channels
- *
- * 3) Mix D:Left, D:Right:
- * D:Left: If not connected, avg all S:Left
- * D:Right: If not connected, avg all S:Right
- *
- * 4) Mix D:Center
- * If not connected, avg all S:Center
- * If still not connected, avg all S:Left, S:Right
- *
- * 5) Mix D:LFE
- * If not connected, avg all S:*
- *
- * 6) Make sure S:Left/S:Right is used: S:Left/S:Right: If
- * not connected, mix into all D:left and all D:right
- * channels. Gain is 0.1, the current left and right
- * should be multiplied by 0.9.
- *
- * 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.
- *
- * S: and D: shall relate to the source resp. destination channels.
- *
- * Rationale: 1, 2 are probably obvious. For 3: this
- * copies front to rear if needed. For 4: we try to find
- * some suitable C source for C, if we don't find any, we
- * avg L and R. For 5: LFE is mixed from all channels. For
- * 6: the rear channels should not be dropped entirely,
- * however have only minimal impact. For 7: movies usually
- * encode speech on the center channel. Thus we have to
- * make sure this channel is distributed to L and R if not
- * available in the output. Also, LFE is used to achieve a
- * greater dynamic range, and thus we should try to do our
- * best to pass it to L+R.
- */
-
- if (a == b || a == PA_CHANNEL_POSITION_MONO) {
- m->map_table_f[oc][ic] = 1.0;
-
- oc_connected = TRUE;
- ic_connected[ic] = TRUE;
- }
- else if (b == PA_CHANNEL_POSITION_MONO) {
- if (n_ic)
- m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
+ /* OK, we shall do the full monty: upmixing and downmixing. Our
+ * algorithm is relatively simple, does not do spacialization, delay
+ * elements or apply lowpass filters for LFE. Patches are always
+ * welcome, though. Oh, and it doesn't do any matrix decoding. (Which
+ * probably wouldn't make any sense anyway.)
+ *
+ * This code is not idempotent: downmixing an upmixed stereo stream is
+ * not identical to the original. The volume will not match, and the
+ * two channels will be a linear combination of both.
+ *
+ * This is loosely based on random suggestions found on the Internet,
+ * such as this:
+ * http://www.halfgaar.net/surround-sound-in-linux and the alsa upmix
+ * plugin.
+ *
+ * The algorithm works basically like this:
+ *
+ * 1) Connect all channels with matching names.
+ *
+ * 2) Mono Handling:
+ * S:Mono: Copy into all D:channels
+ * D:Mono: Avg all S:channels
+ *
+ * 3) Mix D:Left, D:Right:
+ * D:Left: If not connected, avg all S:Left
+ * D:Right: If not connected, avg all S:Right
+ *
+ * 4) Mix D:Center
+ * If not connected, avg all S:Center
+ * If still not connected, avg all S:Left, S:Right
+ *
+ * 5) Mix D:LFE
+ * If not connected, avg all S:*
+ *
+ * 6) Make sure S:Left/S:Right is used: S:Left/S:Right: If not
+ * connected, mix into all D:left and all D:right channels. Gain is
+ * 0.1, the current left and right should be multiplied by 0.9.
+ *
+ * 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.
+ *
+ * S: and D: shall relate to the source resp. destination channels.
+ *
+ * Rationale: 1, 2 are probably obvious. For 3: this copies front to
+ * rear if needed. For 4: we try to find some suitable C source for C,
+ * if we don't find any, we avg L and R. For 5: LFE is mixed from all
+ * channels. For 6: the rear channels should not be dropped entirely,
+ * however have only minimal impact. For 7: movies usually encode
+ * speech on the center channel. Thus we have to make sure this channel
+ * is distributed to L and R if not available in the output. Also, LFE
+ * is used to achieve a greater dynamic range, and thus we should try
+ * to do our best to pass it to L+R.
+ */
- oc_connected = TRUE;
- ic_connected[ic] = TRUE;
- }
+ unsigned
+ ic_left = 0,
+ ic_right = 0,
+ ic_center = 0,
+ ic_unconnected_left = 0,
+ ic_unconnected_right = 0,
+ ic_unconnected_center = 0,
+ ic_unconnected_lfe = 0;
+
+ pa_assert(remix);
+
+ for (ic = 0; ic < n_ic; ic++) {
+ if (on_left(r->i_cm.map[ic]))
+ ic_left++;
+ if (on_right(r->i_cm.map[ic]))
+ ic_right++;
+ if (on_center(r->i_cm.map[ic]))
+ ic_center++;
}
- if (!oc_connected && remix) {
- /* OK, we shall remix */
+ for (oc = 0; oc < n_oc; oc++) {
+ pa_bool_t oc_connected = FALSE;
+ pa_channel_position_t b = r->o_cm.map[oc];
- /* Try to find matching input ports for this output port */
+ for (ic = 0; ic < n_ic; ic++) {
+ pa_channel_position_t a = r->i_cm.map[ic];
- if (on_left(b)) {
- unsigned n = 0;
+ if (a == b || a == PA_CHANNEL_POSITION_MONO) {
+ m->map_table_f[oc][ic] = 1.0f;
- /* We are not connected and on the left side, let's
- * average all left side input channels. */
+ oc_connected = TRUE;
+ ic_connected[ic] = TRUE;
+ }
+ else if (b == PA_CHANNEL_POSITION_MONO) {
+ m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
- for (ic = 0; ic < n_ic; ic++)
- if (on_left(r->i_cm.map[ic]))
- n++;
+ oc_connected = TRUE;
+ ic_connected[ic] = TRUE;
+ }
+ }
- if (n > 0)
- for (ic = 0; ic < n_ic; ic++)
- if (on_left(r->i_cm.map[ic])) {
- m->map_table_f[oc][ic] = 1.0f / (float) n;
- ic_connected[ic] = TRUE;
- }
+ if (!oc_connected) {
+ /* Try to find matching input ports for this output port */
- /* We ignore the case where there is no left input
- * channel. Something is really wrong in this case
- * anyway. */
+ if (on_left(b)) {
- } else if (on_right(b)) {
- unsigned n = 0;
+ /* We are not connected and on the left side, let's
+ * average all left side input channels. */
- /* We are not connected and on the right side, let's
- * average all right side input channels. */
+ if (ic_left > 0)
+ for (ic = 0; ic < n_ic; ic++)
+ if (on_left(r->i_cm.map[ic])) {
+ m->map_table_f[oc][ic] = 1.0f / (float) ic_left;
+ ic_connected[ic] = TRUE;
+ }
- for (ic = 0; ic < n_ic; ic++)
- if (on_right(r->i_cm.map[ic]))
- n++;
+ /* We ignore the case where there is no left input channel.
+ * Something is really wrong in this case anyway. */
- if (n > 0)
- for (ic = 0; ic < n_ic; ic++)
- if (on_right(r->i_cm.map[ic])) {
- m->map_table_f[oc][ic] = 1.0f / (float) n;
- ic_connected[ic] = TRUE;
- }
+ } else if (on_right(b)) {
- /* We ignore the case where there is no right input
- * channel. Something is really wrong in this case
- * anyway. */
+ /* We are not connected and on the right side, let's
+ * average all right side input channels. */
- } else if (on_center(b)) {
- unsigned n = 0;
+ if (ic_right > 0)
+ for (ic = 0; ic < n_ic; ic++)
+ if (on_right(r->i_cm.map[ic])) {
+ m->map_table_f[oc][ic] = 1.0f / (float) ic_right;
+ ic_connected[ic] = TRUE;
+ }
- /* We are not connected and at the center. Let's
- * average all center input channels. */
+ /* We ignore the case where there is no right input
+ * channel. Something is really wrong in this case anyway.
+ * */
- for (ic = 0; ic < n_ic; ic++)
- if (on_center(r->i_cm.map[ic]))
- n++;
+ } else if (on_center(b)) {
- if (n > 0) {
- for (ic = 0; ic < n_ic; ic++)
- if (on_center(r->i_cm.map[ic])) {
- m->map_table_f[oc][ic] = 1.0f / (float) n;
- ic_connected[ic] = TRUE;
- }
- } else {
- /* Hmm, no center channel around, let's synthesize
- * it by mixing L and R.*/
+ if (ic_center > 0) {
- n = 0;
+ /* We are not connected and at the center. Let's average
+ * all center input channels. */
- for (ic = 0; ic < n_ic; ic++)
- if (on_left(r->i_cm.map[ic]) || on_right(r->i_cm.map[ic]))
- n++;
+ for (ic = 0; ic < n_ic; ic++)
+ if (on_center(r->i_cm.map[ic])) {
+ m->map_table_f[oc][ic] = 1.0f / (float) ic_center;
+ ic_connected[ic] = TRUE;
+ }
+
+ } else if (ic_left + ic_right > 0) {
+
+ /* Hmm, no center channel around, let's synthesize it
+ * by mixing L and R.*/
- if (n > 0)
for (ic = 0; ic < n_ic; ic++)
if (on_left(r->i_cm.map[ic]) || on_right(r->i_cm.map[ic])) {
- m->map_table_f[oc][ic] = 1.0f / (float) n;
+ m->map_table_f[oc][ic] = 1.0f / (float) (ic_left + ic_right);
ic_connected[ic] = TRUE;
}
+ }
- /* We ignore the case where there is not even a
- * left or right input channel. Something is
- * really wrong in this case anyway. */
- }
-
- } else if (on_lfe(b)) {
+ /* We ignore the case where there is not even a left or
+ * right input channel. Something is really wrong in this
+ * case anyway. */
- /* We are not connected and an LFE. Let's average all
- * channels for LFE. */
+ } else if (on_lfe(b) && !(r->flags & PA_RESAMPLER_NO_LFE)) {
- for (ic = 0; ic < n_ic; ic++) {
+ /* We are not connected and an LFE. Let's average all
+ * channels for LFE. */
- if (!(r->flags & PA_RESAMPLER_NO_LFE))
+ for (ic = 0; ic < n_ic; ic++)
m->map_table_f[oc][ic] = 1.0f / (float) n_ic;
- else
- m->map_table_f[oc][ic] = 0;
- /* Please note that a channel connected to LFE
- * doesn't really count as connected. */
+ /* Please note that a channel connected to LFE doesn't
+ * really count as connected. */
}
}
}
- }
-
- if (remix) {
- unsigned
- ic_unconnected_left = 0,
- ic_unconnected_right = 0,
- ic_unconnected_center = 0,
- ic_unconnected_lfe = 0;
for (ic = 0; ic < n_ic; ic++) {
pa_channel_position_t a = r->i_cm.map[ic];
@@ -909,10 +888,9 @@ static void calc_map_table(pa_resampler *r) {
if (ic_unconnected_left > 0) {
- /* OK, so there are unconnected input channels on the
- * left. Let's multiply all already connected channels on
- * the left side by .9 and add in our averaged unconnected
- * channels multiplied by .1 */
+ /* OK, so there are unconnected input channels on the left. Let's
+ * multiply all already connected channels on the left side by .9
+ * and add in our averaged unconnected channels multiplied by .1 */
for (oc = 0; oc < n_oc; oc++) {
@@ -934,10 +912,9 @@ static void calc_map_table(pa_resampler *r) {
if (ic_unconnected_right > 0) {
- /* OK, so there are unconnected input channels on the
- * right. Let's multiply all already connected channels on
- * the right side by .9 and add in our averaged unconnected
- * channels multiplied by .1 */
+ /* OK, so there are unconnected input channels on the right. Let's
+ * multiply all already connected channels on the right side by .9
+ * and add in our averaged unconnected channels multiplied by .1 */
for (oc = 0; oc < n_oc; oc++) {
@@ -960,10 +937,9 @@ static void calc_map_table(pa_resampler *r) {
if (ic_unconnected_center > 0) {
pa_bool_t mixed_in = FALSE;
- /* OK, so there are unconnected input channels on the
- * center. Let's multiply all already connected channels on
- * the center side by .9 and add in our averaged unconnected
- * channels multiplied by .1 */
+ /* OK, so there are unconnected input channels on the center. Let's
+ * multiply all already connected channels on the center side by .9
+ * and add in our averaged unconnected channels multiplied by .1 */
for (oc = 0; oc < n_oc; oc++) {
@@ -992,9 +968,8 @@ static void calc_map_table(pa_resampler *r) {
memset(found_frs, 0, sizeof(found_frs));
/* Hmm, as it appears there was no center channel we
- could mix our center channel in. In this case, mix
- it into left and right. Using .375 and 0.75 as
- factors. */
+ could mix our center channel in. In this case, mix it into
+ left and right. Using .375 and 0.75 as factors. */
for (ic = 0; ic < n_ic; ic++) {
@@ -1052,8 +1027,8 @@ static void calc_map_table(pa_resampler *r) {
if (ic_unconnected_lfe > 0 && !(r->flags & PA_RESAMPLER_NO_LFE)) {
- /* OK, so there is an unconnected LFE channel. Let's mix
- * it into all channels, with factor 0.375 */
+ /* OK, so there is an unconnected LFE channel. Let's mix it into
+ * all channels, with factor 0.375 */
for (ic = 0; ic < n_ic; ic++) {
@@ -1065,6 +1040,7 @@ static void calc_map_table(pa_resampler *r) {
}
}
}
+
/* make an 16:16 int version of the matrix */
for (oc = 0; oc < n_oc; oc++)
for (ic = 0; ic < n_ic; ic++)
--
1.7.9.5
