Re: [FFmpeg-devel] [PATCH 2/3] avcodec/aacenc_is: Assert that minthr is not 0.0, this would lead to division by 0 later

2016-05-18 Thread Claudio Freire
On Mon, May 16, 2016 at 4:10 PM, Michael Niedermayer
 wrote:
> yes,
>
> with the patch fate fails:
>
> Test aac-pred-encode failed. Look at tests/data/fate/aac-pred-encode.err for 
> details.
> make: *** [fate-aac-pred-encode] Error 134
>
>
>>
>> A threshold of 0 would in theory cause a zeroed band (zeroes[i] == 1),
>> and those should be skipped.
>>
>> I think the proper fix would be figuring out why those aren't being
>> skipped, if that is the case.
>
> i never meant this patch to be a proper fix, more a bug report ...

I think I see the problem, the code is measuring minthr on a
per-window basis when it should be on a per-window-group basis.

I'll test some fixes soon, time permitting.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/aacenc_is: Assert that minthr is not 0.0, this would lead to division by 0 later

2016-05-16 Thread Claudio Freire
On Mon, May 16, 2016 at 12:26 PM, Kieran Kunhya  wrote:
>> Testcase is fate-aac-pred-encode
>>
>> Signed-off-by: Michael Niedermayer 
>> ---
>>  libavcodec/aacenc_is.c |3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/libavcodec/aacenc_is.c b/libavcodec/aacenc_is.c
>> index 473897b..e5cfa14 100644
>> --- a/libavcodec/aacenc_is.c
>> +++ b/libavcodec/aacenc_is.c
>> @@ -64,6 +64,9 @@ struct AACISError ff_aac_is_encoding_err(AACEncContext
>> *s, ChannelElement *cpe,
>>  abs_pow34_v(I34, IS,   sce0->ics.swb_sizes[g]);
>>  maxval = find_max_val(1, sce0->ics.swb_sizes[g], I34);
>>  is_band_type = find_min_book(maxval, is_sf_idx);
>> +
>> +av_assert0(minthr != 0.0);
>> +
>>  dist1 += quantize_band_cost(s, [start + (w+w2)*128], L34,
>>  sce0->ics.swb_sizes[g],
>>  sce0->sf_idx[w*16+g],
>> --
>> 1.7.9.5
>>
>>
>>
> Does this assert on actual input data?

A threshold of 0 would in theory cause a zeroed band (zeroes[i] == 1),
and those should be skipped.

I think the proper fix would be figuring out why those aren't being
skipped, if that is the case.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/3] avcodec/aacenc_is: Assert that minthr is not 0.0, this would lead to division by 0 later

2016-05-16 Thread Michael Niedermayer
Testcase is fate-aac-pred-encode

Signed-off-by: Michael Niedermayer 
---
 libavcodec/aacenc_is.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/aacenc_is.c b/libavcodec/aacenc_is.c
index 473897b..e5cfa14 100644
--- a/libavcodec/aacenc_is.c
+++ b/libavcodec/aacenc_is.c
@@ -64,6 +64,9 @@ struct AACISError ff_aac_is_encoding_err(AACEncContext *s, 
ChannelElement *cpe,
 abs_pow34_v(I34, IS,   sce0->ics.swb_sizes[g]);
 maxval = find_max_val(1, sce0->ics.swb_sizes[g], I34);
 is_band_type = find_min_book(maxval, is_sf_idx);
+
+av_assert0(minthr != 0.0);
+
 dist1 += quantize_band_cost(s, [start + (w+w2)*128], L34,
 sce0->ics.swb_sizes[g],
 sce0->sf_idx[w*16+g],
-- 
1.7.9.5

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel