[pulseaudio-discuss] [PATCH 04/10] remap: Add stereo to mono special case remapping

[Peter Meerwald]

Hello Tanu,

thanks for reviewing!

> > > +        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.
> 
> Or the inputs could be cast to uint32_t, which I guess would be better
> than dividing multiple times.

it should be '=' obviously -- will fix and repost shortly

I think (s[0] + s[1])/2 is correct; at least as long as sizeof(int) > 
sizeof(short); short gets promoted to int -- see 'integer promotion in 
C99'; also my compiler thinks above is correct :)

dividing before summation looses precision and should be avoided (the 
generic matrix remapping codes does multiplication and truncation, 
probably for performance reasons; there should be an intermediate (larger) 
datatype and rounding if we cared about precision)

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)