[FFmpeg-devel] [PATCH] Issue 636 : ALAC encoding sometimes fails

[Måns Rullgård]

"Jai Menon" <jmenon86 at gmail.com> writes:

> Hi,
>
> On Wed, Dec 10, 2008 at 4:56 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
>>
>> Michael Niedermayer wrote:
>>> On Wed, Dec 10, 2008 at 10:49:10AM +0530, Jai Menon wrote:
>>>> Hi,
>
> [...]
>
>>>> > >              sum >>= lpc.lpc_quant;
>>>> > >              sum += samples[0];
>>>> > > -            residual[i] = samples[lpc.lpc_order+1] - sum;
>>>> > > +            residual[i] = (samples[lpc.lpc_order+1] - sum) << (32 -
>>>> > s->write_sample_size) >>
>>>> > > +                          (32 - s->write_sample_size);
>>>> > >              res_val = residual[i];
>>>> >
>>>> > you are missing a int32_t cast in there, without it this wont work on
>>>> > systems where int is not exactly 32bit
>>>> >
>>>>
>>>> Oversight on my part. 16 bit systems are a nightmare from which I'm trying
>>>> to awake.
>>>
>>> int could be 64bit, it cannot be 16bit on a POSIX system as its required
>>> by POSIX to be >= 32
>>
>> 36-bit words used to be commonplace once upon a time...
>>
>>>> @@ -253,7 +253,7 @@
>>>>
>>>>              sum >>= lpc.lpc_quant;
>>>>              sum += samples[0];
>>>> -            residual[i] = (samples[lpc.lpc_order+1] - sum) << (32 -
>>>> s->write_sample_size) >>
>>>> +            residual[i] = (int32_t)(samples[lpc.lpc_order+1] - sum) << (32
>>>> - s->write_sample_size) >>
>>>>                            (32 - s->write_sample_size);
>>>
>>> id do the cast after the << because its the >> that differes from where
>>> the sign bit is
>>
>> Put differently, if int is wider than 32 bits the left-hand argument
>> to >> must be sign-extended from 32 bits to this size for the >> to
>> work as intended.
>>
>
> done.
>
>> It would be more efficient if 32 were replaced with CHAR_BIT*sizeof(int)
>> instead.  This could be put in a nicely named macro, making it even more
>> obvious what the intent of the code is.
>
> Also done.

With this variant the cast is wrong.  Think about what happens if int
is wider than 32 bits.

-- 
M?ns Rullg?rd
mans at mansr.com