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

[Jai Menon]

Hi,

On Thu, Dec 11, 2008 at 3:05 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
> "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.
>

Well, then its best if 'sum' is made int32_t. I'll post a patch for that.

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

Regards,

Jai