[FFmpeg-devel] [PATCHv2] avcodec/apedec: fix bug introduced in commit d3e5fbb1406995e07fccbff3ca8c1e24f57a1f7b

[Ganesh Ajjanagadde]

On Sat, Oct 3, 2015 at 8:53 AM, Ganesh Ajjanagadde
<gajjanagadde at gmail.com> wrote:
> On Sat, Oct 3, 2015 at 7:58 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> Hi,
>>
>> On Sat, Oct 3, 2015 at 8:31 AM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> wrote:
>>>
>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>> ---
>>>  libavcodec/apedec.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>>> index 7b34d26..05cb17e 100644
>>> --- a/libavcodec/apedec.c
>>> +++ b/libavcodec/apedec.c
>>> @@ -1281,7 +1281,7 @@ static void do_apply_filter(APEContext *ctx, int
>>> version, APEFilter *f,
>>>              /* Update the adaption coefficients */
>>>              absres = FFABS(res);
>>>              if (absres)
>>> -                *f->adaptcoeffs = ((res & (-(1<<31))) ^ (-(1<<30))) >>
>>> +                *f->adaptcoeffs = ((res & 0x80000000) ^ (-(1<<30))) >>
>>>                                    (25 + (absres <= f->avg*3) + (absres <=
>>> f->avg*4/3));
>>>              else
>>>                  *f->adaptcoeffs = 0;
>>> --
>>> 2.6.0
>>
>>
>> I wonder if some compilers will complain that this overflows (strictly
>> speaking it does) and that it should be -0x80000000 instead?
>
> please ignore current patch, breaks fate. Ronald's idea also breaks fate.

Both the -2*1<<30 or the INT32_MIN idea work. I can create a patch
based on either of these depending on what people prefer.

>
>>
>> Ronald