On Fri, 2013-03-29 at 16:56 +0100, Peter Meerwald wrote:
> From: Peter Meerwald <p.meerwald at bct-electronic.com>
>
> add special-case C code for stereo-to-mone remapping
>
> v2:
> * use consistent array addressing
>
> Signed-off-by: Peter Meerwald <p.meerwald at bct-electronic.com>
> ---
> src/pulsecore/remap.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/src/pulsecore/remap.c b/src/pulsecore/remap.c
> index 5d0cd62..9300ab0 100644
> --- a/src/pulsecore/remap.c
> +++ b/src/pulsecore/remap.c
> @@ -86,6 +86,53 @@ static void remap_mono_to_stereo_c(pa_remap_t *m, void *dst, const void *src, un
> }
> }
>
> +static void remap_stereo_to_mono_c(pa_remap_t *m, void *dst, const void *src, unsigned n) {
> + unsigned i;
> +
> + switch (*m->format) {
> + case PA_SAMPLE_FLOAT32NE:
> + {
> + float *d = (float *) dst, *s = (float *) src;
> +
> + for (i = n >> 2; i > 0; i--) {
> + d[0] = (s[0] + s[1])*0.5f;
> + d[1] = (s[2] + s[3])*0.5f;
> + d[2] = (s[4] + s[5])*0.5f;
> + d[3] = (s[6] + s[7])*0.5f;
> + s += 8;
> + d += 4;
> + }
> + for (i = n & 3; i; i--) {
> + d[0] = (s[0] + s[1])*0.5f;
> + s += 2;
> + d += 1;
> + }
> + break;
> + }
> + case PA_SAMPLE_S16NE:
> + {
> + int16_t *d = (int16_t *) dst, *s = (int16_t *) src;
> +
> + for (i = n >> 2; i > 0; i--) {
> + d[0] += (s[0] + s[1])/2;
> + d[1] += (s[2] + s[3])/2;
> + d[2] += (s[4] + s[5])/2;
> + d[3] += (s[6] + s[7])/2;
You probably meant '=', not '+='?
Also, s[0] + s[1] can overflow, so the inputs should be divided
individually before summing.
> + s += 8;
> + d += 4;
> + }
> + for (i = n & 3; i; i--) {
> + d[0] += (s[0] + s[1])/2;
> + s += 2;
> + d += 1;
> + }
> + break;
> + }
> + default:
> + pa_assert_not_reached();
> + }
> +}
> +
> static void remap_channels_matrix_c(pa_remap_t *m, void *dst, const void *src, unsigned n) {
> unsigned oc, ic, i;
> unsigned n_ic, n_oc;
> @@ -172,13 +219,16 @@ static void init_remap_c(pa_remap_t *m) {
> m->map_table_i[0][0] == PA_VOLUME_NORM && m->map_table_i[1][0] == PA_VOLUME_NORM) {
> m->do_remap = (pa_do_remap_func_t) remap_mono_to_stereo_c;
> pa_log_info("Using mono to stereo remapping");
> + } else if (n_ic == 2 && n_oc == 1 &&
> + m->map_table_i[0][0] == PA_VOLUME_HALF && m->map_table_i[0][1] == PA_VOLUME_HALF) {
While this happens to work, I don't think it's correct to use
PA_VOLUME_HALF here. PA_VOLUME_NORM is a pa_volume_t value, and
pa_volume_t is not meant to be used as a multiplication factor type. I'd
expect PA_VOLUME_HALF to be a pa_volume_t constant too, not a
multiplication factor constant.
I should have complained about the use of PA_VOLUME_NORM in commit
578d2ce5, but I didn't realize at that time that map_table_i stored
multiplication factors, not pa_volume_t values.
I think plain numbers would be OK here, although adding new constants
for 16:16 fixed-point values might work too.
--
Tanu
