On 22.04.2015 18:31, Claudio Freire wrote:
> On Wed, Apr 22, 2015 at 12:54 PM, Andreas Cadhalpun
> <[email protected]> wrote:
>> On 22.04.2015 17:35, Claudio Freire wrote:
>>> On Wed, Apr 22, 2015 at 10:23 AM, Andreas Cadhalpun
>>> <[email protected]> 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.
I think there is more generally the assumption that all error codes are
negative.
> 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
The condition for increasing num_patches never becomes true:
sbr->patch_num_subbands[sbr->num_patches] = FFMAX(sb - usb, 0);
Here sb = usb ...
sbr->patch_start_subband[sbr->num_patches] = sbr->k[0] - odd -
sbr->patch_num_subbands[sbr->num_patches];
if (sbr->patch_num_subbands[sbr->num_patches] > 0) {
... thus this condition is false.
usb = sb;
msb = sb;
sbr->num_patches++;
} else
msb = sbr->kx[1];
If you have any other idea how to fix this, please let me know.
> rather than a bandaid like this, but aside
> from that preference (which is personal) the patch seems to make
> sense.
OK.
Best regards,
Andreas
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
