On 04/01/2013 03:03 PM, Tanu Kaskinen wrote:
> 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.
Or actually I don't mind either way, so no need to do changes if you
don't want to.
--
Tanu
