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

[Tanu Kaskinen]

On 04/01/2013 03:28 PM, Thomas Martitz wrote:
> Am 01.04.2013 13:13, schrieb 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.
>>
>> 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 :)
>>
>
>
> Why should int16+int16 be promoted to int32? Pretty sure this code is
> prone to overflow.

I was pretty sure too, until I tried compiling a test program. I think 
this text in the C99 standard[1] (section 6.3.1.1) means that shorts are 
always promoted to ints, even if both operands of a calculation are shorts:

"The following may be used in an expression wherever an int or unsigned 
int may be used:

  - An object or expression with an integer type whose integer 
conversion rank is less than or equal to the rank of int and unsigned int.

  - A bit-field of type _Bool, int, signed int, or unsigned int.

If an int can represent all values of the original type, the value is 
converted to an int; otherwise, it is converted to an unsigned int. 
These are called the integer promotions. All other types are unchanged 
by the integer promotions."

I have trouble understanding that text, but that's the closest thing 
that I found that would explain the observed behavior.

There's also this example in section 5.1.2.3:

"EXAMPLE 2

In executing the fragment

         char c1, c2;
         /* ... */
         c1 = c1 + c2;

the "integer promotions" require that the abstract machine promote the 
value of each variable to int size and then add the two ints and 
truncate the sum. Provided the addition of two chars can be done without 
overflow, or with overflow wrapping silently to produce the correct 
result, the actual execution need only produce the same result, possibly 
omitting the promotions."

[1] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf

-- 
Tanu