On Wed, Apr 22, 2015 at 12:54 PM, Andreas Cadhalpun
<andreas.cadhal...@googlemail.com> wrote:
> On 22.04.2015 17:35, Claudio Freire wrote:
>> On Wed, Apr 22, 2015 at 10:23 AM, Andreas Cadhalpun
>> <andreas.cadhal...@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;
>>
>>
>> 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.

With the disclaimer that I'm not familiar with this code said, it
looks like it would be better to attack the reason why it loops
without increasing npatches rather than a bandaid like this, but aside
from that preference (which is personal) the patch seems to make
sense.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to