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

Tanu Kaskinen tanuk at iki.fi
Mon Apr 1 05:03:30 PDT 2013


On 04/01/2013 02:13 PM, Peter Meerwald wrote:
> 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 :)

It seems that you're right. I would still prefer explicit casts, because 
few people bother to memorize the C promotion rules.

-- 
Tanu


More information about the pulseaudio-discuss mailing list