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

Jai Menon jmenon86
Thu Dec 11 05:31:04 CET 2008


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.
Well, at this point I think it is better if I just make 'sum' an
int32_t. It started out as an int just because
 I'm on a 32-bit machine. Is this better or the attached patch (which
seems like too much code actually ;-) )?
> --
> M?ns Rullg?rd
> mans at mansr.com

Regards,

Jai
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alacenc_issue636_fix.patch
Type: application/octet-stream
Size: 686 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081211/eeb6589a/attachment.obj>



More information about the ffmpeg-devel mailing list