[FFmpeg-devel] [libav-devel] [PATCH] aacsbr: break infinite loop in sbr_hf_calc_npatches

[Vittorio Giovara]

On Wed, Apr 22, 2015 at 5:31 PM, Claudio Freire <klaussfreire at gmail.com> wrote:
> On Wed, Apr 22, 2015 at 12:54 PM, Andreas Cadhalpun
> <andreas.cadhalpun at googlemail.com> wrote:
>> On 22.04.2015 17:35, Claudio Freire wrote:
>>> On Wed, Apr 22, 2015 at 10:23 AM, Andreas Cadhalpun
>>> <andreas.cadhalpun at googlemail.com> wrote:
>>>> +        if (k == last_k && msb == last_msb) {
>>>> +            av_log(ac->avctx, AV_LOG_ERROR, "patch construction failed\n");
>>>> +            return AVERROR_INVALIDDATA;
>>>> +        }
>>>> +        last_k = k;
>>>> +        last_msb = msb;

Andreas, do you have a sample that triggers the infinite loop?

>>>
>>>
>>> I don't think the INVALIDDATA return will have the desired effect.
>>>
>>> I think you want return -1;
>>
>> This function is only called once and there the check is:
>>     if (sbr_hf_calc_npatches(ac, sbr) < 0)
>>         return -1;
>>
>> Thus returning AVERROR_INVALIDDATA works as well as -1.
>
> The fact that AVERROR_INVALIDDATA < 0 is a close call on 32 bit platforms.
>
> Still, it's not a new assumption in the code, so I'll grant you that.

What do you mean by "A close call"? All AVERROR_* are negative by
design, and they carry more information than a -1, so their increased
usage is certainly welcome.
The fact that it does not get propagated is a separate issue.
-- 
Vittorio