Re: [FFmpeg-devel] [PATCH] avcodec/aaccoder: Limit sf_idx difference for all cases

[Claudio Freire]

On Wed, Nov 16, 2016 at 8:09 PM, Michael Niedermayer
 wrote:
>> Sorry for the delay, it turned out to be more complex than that.
>>
>> There were a few potential violations that I had already identified in
>> a WIP patch but they did not apply to the fuzzed sample. That sample
>> triggered an interaction with TNS and trellis band type coding that
>> resulted in zeroed bands reappearing and thus invalidating all delta
>> scalefactor validations.
>>
>> The attached patch series fixes most of the delta scalefactor
>> violation risks I could find, including that one.
>>
>> It hasn't been thoroughly tested for quality regressions/improvements.
>> It's possible that it does change quality since it changes key
>> decision points that conduce to the violations but also to lots of
>> audible artifacts. So I believe it should improve quality, but one
>> never knows without proper ABX testing, which I'll be conducting, at
>> least in a limited way, in the following days.
>>
>> In the meantime, I'm attaching the patch series for review.
>
> ping

Is that ping for me?

It's a significant patch so I'd rather have a little review before
pushing, waiting for that.

Well, not really waiting. I did some tests too and found some not so
desirable side effects that I hadn't fully fixed when RL decided to
rob me of most of my free time, so I haven't pushed it yet to prevent
even more serious regressions, but still I'd rather not push without
some form of review.

So, ccing Rostislav in case he just missed the original patch (he's
the most likely to review anyway)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] aacenc: use the decoder's lcg PRNG

[Claudio Freire]

LGTM.

However,

On Sat, Oct 8, 2016 at 12:20 PM, Rostislav Pehlivanov
 wrote:
> +/**
> + * linear congruential pseudorandom number generator
> + *
> + * @param   previous_valpointer to the current state of the generator
> + *
> + * @return  Returns a 32-bit pseudorandom integer
> + */
> +static av_always_inline int lcg_random(unsigned previous_val)
> +{
> +union { unsigned u; int s; } v = { previous_val * 1664525u + 1013904223 
> };
> +return v.s;
> +}

This could use a comment on where the parameters come from.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aacenc: WIP support for PCEs

[Claudio Freire]

On Mon, Oct 3, 2016 at 3:53 PM, Rostislav Pehlivanov
 wrote:
> Hopefully whoever wants to have support for crazy formats can help.
> The table in aacenc.h (temporary position) tells the encoder what
> to put in the bitstream and how to encode. Problem is, the specifications
> dont specify anything. Thats because I've not been able to find any bloody
> specifications and had to work with what the decoder does. And there was
> plenty of guessing there because the decoder does some magic on layout_map
> which I can't even figure out nor even know if its correct (it seems to be
> for the formats I've tested).
> Then there's the problem with the exact order that the channels have to be in.
> Again a guessing game, since you essentially have no idea what the index part
> of the map is supposed to be, whether it has to be incremented starting from
> the first channel or reset upon every front/side/back channel groups. At
> least the map to instruct the encoder's straightforward.
>
> Anyway, help appreciated.

Will take a deeper look later, but on a shallow review, shouldn't it
have some tests?

(seems fairly easily testable)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/aaccoder: Limit sf_idx difference for all cases

[Claudio Freire]

On Sat, Sep 10, 2016 at 3:37 AM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Thu, Aug 25, 2016 at 8:57 AM, Rostislav Pehlivanov
> <atomnu...@gmail.com> wrote:
>>> 64ed96a710787ba5d0666746a8562e7d.dee
>>>
>>> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
>>> ---
>>>  libavcodec/aaccoder.c | 8 +++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
>>> index 284b401..995724b 100644
>>> --- a/libavcodec/aaccoder.c
>>> +++ b/libavcodec/aaccoder.c
>>> @@ -196,7 +196,7 @@ typedef struct TrellisPath {
>>>  static void set_special_band_scalefactors(AACEncContext *s,
>>> SingleChannelElement *sce)
>>>  {
>>>  int w, g;
>>> -int prevscaler_n = -255, prevscaler_i = 0;
>>> +int prevscaler_n = -255, prevscaler_i = 0, prevscaler_d = -255;
>>>  int bands = 0;
>>>
>>>  for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
>>> @@ -211,6 +211,10 @@ static void set_special_band_scalefactors(AACEncContext
>>> *s, SingleChannelElement
>>>  if (prevscaler_n == -255)
>>>  prevscaler_n = sce->sf_idx[w*16+g];
>>>  bands++;
>>> +} else {
>>> +if (prevscaler_d == -255)
>>> +prevscaler_d = sce->sf_idx[w*16+g];
>>> +bands++;
>>>  }
>>>  }
>>>  }
>>> @@ -227,6 +231,8 @@ static void set_special_band_scalefactors(AACEncContext
>>> *s, SingleChannelElement
>>>  sce->sf_idx[w*16+g] = prevscaler_i =
>>> av_clip(sce->sf_idx[w*16+g], prevscaler_i - SCALE_MAX_DIFF, prevscaler_i +
>>> SCALE_MAX_DIFF);
>>>  } else if (sce->band_type[w*16+g] == NOISE_BT) {
>>>  sce->sf_idx[w*16+g] = prevscaler_n =
>>> av_clip(sce->sf_idx[w*16+g], prevscaler_n - SCALE_MAX_DIFF, prevscaler_n +
>>> SCALE_MAX_DIFF);
>>> +} else {
>>> +sce->sf_idx[w*16+g] = prevscaler_d =
>>> av_clip(sce->sf_idx[w*16+g], prevscaler_d - SCALE_MAX_DIFF, prevscaler_d +
>>> SCALE_MAX_DIFF);
>>>  }
>>>  }
>>>  }
>>> --
>>> 2.9.3
>>>
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>>
>> That fuzzed sample seems to be causing the algorithm which does SF
>> difference normalization between normal and PNS bands to fail. This commit
>> masks the problem downstream. IMO that's not the correct way to solve this,
>> as there's no guarantee that another sample won't trigger the same assert
>> even when limiting all scalefactors. Fixing a single fuzzed sample with a
>> hack which doesn't stop other fuzzed samples from triggering the same bug
>> isn't justified.
>> I have the time right now and I'll try to fix this properly, but it might
>> take me a day or two. I think the problem is that when the twoloop coder
>> does the the normalization it doesn't take into account the fact that IS
>> and PNS have their scalefactors modified by set_special_band_scalefactors()
>> later on before encoding.
>
> It seems the root of the issue is that the two stages of PNS don't
> agree on when they can apply PNS or not.
>
> I have a WIP that eliminates the issue by just making the two agree,
> but I've got unrelated changes so I'll try to distill the patch to the
> minimum necessary to fix this during the weekend.

Sorry for the delay, it turned out to be more complex than that.

There were a few potential violations that I had already identified in
a WIP patch but they did not apply to the fuzzed sample. That sample
triggered an interaction with TNS and trellis band type coding that
resulted in zeroed bands reappearing and thus invalidating all delta
scalefactor validations.

The attached patch series fixes most of the delta scalefactor
violation risks I could find, including that one.

It hasn't been thoroughly tested for quality regressions/improvements.
It's possible that it does change quality since it changes key
decision points that conduce to the violations but also to lots of
audible artifacts. So I believe it should improve quality, but one
never knows without proper ABX testing, whic

Re: [FFmpeg-devel] [PATCH] avcodec/aaccoder: Limit sf_idx difference for all cases

[Claudio Freire]

On Thu, Aug 25, 2016 at 8:57 AM, Rostislav Pehlivanov
 wrote:
>> 64ed96a710787ba5d0666746a8562e7d.dee
>>
>> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>> Signed-off-by: Michael Niedermayer 
>> ---
>>  libavcodec/aaccoder.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
>> index 284b401..995724b 100644
>> --- a/libavcodec/aaccoder.c
>> +++ b/libavcodec/aaccoder.c
>> @@ -196,7 +196,7 @@ typedef struct TrellisPath {
>>  static void set_special_band_scalefactors(AACEncContext *s,
>> SingleChannelElement *sce)
>>  {
>>  int w, g;
>> -int prevscaler_n = -255, prevscaler_i = 0;
>> +int prevscaler_n = -255, prevscaler_i = 0, prevscaler_d = -255;
>>  int bands = 0;
>>
>>  for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
>> @@ -211,6 +211,10 @@ static void set_special_band_scalefactors(AACEncContext
>> *s, SingleChannelElement
>>  if (prevscaler_n == -255)
>>  prevscaler_n = sce->sf_idx[w*16+g];
>>  bands++;
>> +} else {
>> +if (prevscaler_d == -255)
>> +prevscaler_d = sce->sf_idx[w*16+g];
>> +bands++;
>>  }
>>  }
>>  }
>> @@ -227,6 +231,8 @@ static void set_special_band_scalefactors(AACEncContext
>> *s, SingleChannelElement
>>  sce->sf_idx[w*16+g] = prevscaler_i =
>> av_clip(sce->sf_idx[w*16+g], prevscaler_i - SCALE_MAX_DIFF, prevscaler_i +
>> SCALE_MAX_DIFF);
>>  } else if (sce->band_type[w*16+g] == NOISE_BT) {
>>  sce->sf_idx[w*16+g] = prevscaler_n =
>> av_clip(sce->sf_idx[w*16+g], prevscaler_n - SCALE_MAX_DIFF, prevscaler_n +
>> SCALE_MAX_DIFF);
>> +} else {
>> +sce->sf_idx[w*16+g] = prevscaler_d =
>> av_clip(sce->sf_idx[w*16+g], prevscaler_d - SCALE_MAX_DIFF, prevscaler_d +
>> SCALE_MAX_DIFF);
>>  }
>>  }
>>  }
>> --
>> 2.9.3
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
> That fuzzed sample seems to be causing the algorithm which does SF
> difference normalization between normal and PNS bands to fail. This commit
> masks the problem downstream. IMO that's not the correct way to solve this,
> as there's no guarantee that another sample won't trigger the same assert
> even when limiting all scalefactors. Fixing a single fuzzed sample with a
> hack which doesn't stop other fuzzed samples from triggering the same bug
> isn't justified.
> I have the time right now and I'll try to fix this properly, but it might
> take me a day or two. I think the problem is that when the twoloop coder
> does the the normalization it doesn't take into account the fact that IS
> and PNS have their scalefactors modified by set_special_band_scalefactors()
> later on before encoding.

It seems the root of the issue is that the two stages of PNS don't
agree on when they can apply PNS or not.

I have a WIP that eliminates the issue by just making the two agree,
but I've got unrelated changes so I'll try to distill the patch to the
minimum necessary to fix this during the weekend.
___
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

[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

[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

Re: [FFmpeg-devel] [PATCH] avcodec: Remove libfaac, the internal AAC encoder is better

[Claudio Freire]

On Sun, Apr 10, 2016 at 4:13 PM, Michael Niedermayer
 wrote:
> that said, to be blunt, make your encoder be capable to encode as fast
> and as good quality as libfaac and after that remove libfaac support
> if you want.


I could make a small patch that could do just that, I believe.

I've always meant to do that and make it be "-aac_coder fast".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fate: add aac test which occasionally crashes (Ticket 1784)

[Claudio Freire]

On Mon, Apr 4, 2016 at 9:00 AM, Claudio Freire <klaussfre...@gmail.com> wrote:
>> http://fate.ffmpeg.org/report.cgi?time=20160403222945=x86_64-archlinux-gcc-valgrindundef
>>
>> No memleaks, but a handful of "Conditional jump or move depends on 
>> uninitialised value(s)" errors.
>
>
> Strange. This shouldn't be new or related to this commit.
>
> I'll take a look nonetheless.

Should be fixed now
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fate: add aac test which occasionally crashes (Ticket 1784)

[Claudio Freire]

On Sun, Apr 3, 2016 at 10:41 PM, James Almer <jamr...@gmail.com> wrote:
> On 4/3/2016 3:29 PM, Claudio Freire wrote:
>> On Sun, Apr 3, 2016 at 3:05 PM, Claudio Freire <klaussfre...@gmail.com> 
>> wrote:
>>> On Sat, Apr 2, 2016 at 4:00 PM, Michael Niedermayer
>>> <mich...@niedermayer.cc> wrote:
>>>> Ideally that should be fixed before its pushed
>>>>
>>>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
>>>> ---
>>>>  tests/fate/aac.mak |   11 +++
>>>>  1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/tests/fate/aac.mak b/tests/fate/aac.mak
>>>> index 324b05d..3d64031 100644
>>>> --- a/tests/fate/aac.mak
>>>> +++ b/tests/fate/aac.mak
>>>> @@ -213,6 +213,17 @@ fate-aac-ltp-encode: CMP_TARGET = 1270
>>>>  fate-aac-ltp-encode: SIZE_TOLERANCE = 3560
>>>>  fate-aac-ltp-encode: FUZZ = 17
>>>>
>>>> +#Ticket1784
>>>> +FATE_AAC_ENCODE += fate-aac-yoraw-encode
>>>> +fate-aac-yoraw-encode: CMD = enc_dec_pcm adts wav s16le 
>>>> $(TARGET_SAMPLES)/audio-reference/yo.raw-short.wav -c:a aac -fflags 
>>>> +bitexact -flags +bitexact
>>>> +fate-aac-yoraw-encode: CMP = stddev
>>>> +fate-aac-yoraw-encode: REF = $(SAMPLES)/audio-reference/yo.raw-short.wav
>>>> +fate-aac-yoraw-encode: CMP_SHIFT = -12288
>>>> +fate-aac-yoraw-encode: CMP_TARGET = 259
>>>> +fate-aac-yoraw-encode: SIZE_TOLERANCE = 3560
>>>> +fate-aac-yoraw-encode: FUZZ = 17
>>>> +
>>>> +
>>>>  FATE_AAC_ENCODE += fate-aac-pred-encode
>>>>  fate-aac-pred-encode: CMD = enc_dec_pcm adts wav s16le 
>>>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -profile:a 
>>>> aac_main -c:a aac -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k 
>>>> -cutoff 22050
>>>>  fate-aac-pred-encode: CMP = stddev
>>>
>>>
>>> Fixed now, feel free to push this
>>
>>
>> Never mind, pushed it myself, kept your signoff.
>
> http://fate.ffmpeg.org/report.cgi?time=20160403222945=x86_64-archlinux-gcc-valgrindundef
>
> No memleaks, but a handful of "Conditional jump or move depends on 
> uninitialised value(s)" errors.


Strange. This shouldn't be new or related to this commit.

I'll take a look nonetheless.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fate: add aac test which occasionally crashes (Ticket 1784)

[Claudio Freire]

On Sun, Apr 3, 2016 at 3:05 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Sat, Apr 2, 2016 at 4:00 PM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> Ideally that should be fixed before its pushed
>>
>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
>> ---
>>  tests/fate/aac.mak |   11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/tests/fate/aac.mak b/tests/fate/aac.mak
>> index 324b05d..3d64031 100644
>> --- a/tests/fate/aac.mak
>> +++ b/tests/fate/aac.mak
>> @@ -213,6 +213,17 @@ fate-aac-ltp-encode: CMP_TARGET = 1270
>>  fate-aac-ltp-encode: SIZE_TOLERANCE = 3560
>>  fate-aac-ltp-encode: FUZZ = 17
>>
>> +#Ticket1784
>> +FATE_AAC_ENCODE += fate-aac-yoraw-encode
>> +fate-aac-yoraw-encode: CMD = enc_dec_pcm adts wav s16le 
>> $(TARGET_SAMPLES)/audio-reference/yo.raw-short.wav -c:a aac -fflags 
>> +bitexact -flags +bitexact
>> +fate-aac-yoraw-encode: CMP = stddev
>> +fate-aac-yoraw-encode: REF = $(SAMPLES)/audio-reference/yo.raw-short.wav
>> +fate-aac-yoraw-encode: CMP_SHIFT = -12288
>> +fate-aac-yoraw-encode: CMP_TARGET = 259
>> +fate-aac-yoraw-encode: SIZE_TOLERANCE = 3560
>> +fate-aac-yoraw-encode: FUZZ = 17
>> +
>> +
>>  FATE_AAC_ENCODE += fate-aac-pred-encode
>>  fate-aac-pred-encode: CMD = enc_dec_pcm adts wav s16le 
>> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -profile:a 
>> aac_main -c:a aac -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k 
>> -cutoff 22050
>>  fate-aac-pred-encode: CMP = stddev
>
>
> Fixed now, feel free to push this


Never mind, pushed it myself, kept your signoff.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fate: add aac test which occasionally crashes (Ticket 1784)

[Claudio Freire]

On Sat, Apr 2, 2016 at 4:00 PM, Michael Niedermayer
 wrote:
> Ideally that should be fixed before its pushed
>
> Signed-off-by: Michael Niedermayer 
> ---
>  tests/fate/aac.mak |   11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tests/fate/aac.mak b/tests/fate/aac.mak
> index 324b05d..3d64031 100644
> --- a/tests/fate/aac.mak
> +++ b/tests/fate/aac.mak
> @@ -213,6 +213,17 @@ fate-aac-ltp-encode: CMP_TARGET = 1270
>  fate-aac-ltp-encode: SIZE_TOLERANCE = 3560
>  fate-aac-ltp-encode: FUZZ = 17
>
> +#Ticket1784
> +FATE_AAC_ENCODE += fate-aac-yoraw-encode
> +fate-aac-yoraw-encode: CMD = enc_dec_pcm adts wav s16le 
> $(TARGET_SAMPLES)/audio-reference/yo.raw-short.wav -c:a aac -fflags +bitexact 
> -flags +bitexact
> +fate-aac-yoraw-encode: CMP = stddev
> +fate-aac-yoraw-encode: REF = $(SAMPLES)/audio-reference/yo.raw-short.wav
> +fate-aac-yoraw-encode: CMP_SHIFT = -12288
> +fate-aac-yoraw-encode: CMP_TARGET = 259
> +fate-aac-yoraw-encode: SIZE_TOLERANCE = 3560
> +fate-aac-yoraw-encode: FUZZ = 17
> +
> +
>  FATE_AAC_ENCODE += fate-aac-pred-encode
>  fate-aac-pred-encode: CMD = enc_dec_pcm adts wav s16le 
> $(TARGET_SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav -profile:a 
> aac_main -c:a aac -aac_is 0 -aac_pns 0 -aac_ms 0 -aac_tns 0 -b:a 128k -cutoff 
> 22050
>  fate-aac-pred-encode: CMP = stddev


Fixed now, feel free to push this
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/aacenc_quantization: Fix undefined behavior and instead detect and print an error

[Claudio Freire]

On Wed, Mar 30, 2016 at 10:15 AM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Wed, Mar 30, 2016 at 8:33 AM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> On Wed, Mar 30, 2016 at 02:04:20AM -0300, Claudio Freire wrote:
>>> On Wed, Mar 30, 2016 at 1:18 AM, Claudio Freire <klaussfre...@gmail.com> 
>>> wrote:
>>> > On Tue, Mar 29, 2016 at 10:51 PM, Michael Niedermayer
>>> > <mich...@niedermayer.cc> wrote:
>>> >> This is a hotfix and not a real fix of the underlaying bug
>>> >> The underlaying bug is ATM not fully understood
>>> >>
>>> >> iam not sure if we should apply this or not
>>> >>
>>> >> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
>>> >> ---
>>> >>  libavcodec/aacenc_quantization.h |   13 +++--
>>> >>  1 file changed, 11 insertions(+), 2 deletions(-)
>>> >>
>>> >> diff --git a/libavcodec/aacenc_quantization.h 
>>> >> b/libavcodec/aacenc_quantization.h
>>> >> index 4250407..d367be0 100644
>>> >> --- a/libavcodec/aacenc_quantization.h
>>> >> +++ b/libavcodec/aacenc_quantization.h
>>> >> @@ -141,8 +141,17 @@ static av_always_inline float 
>>> >> quantize_and_encode_band_cost_template(
>>> >>  if (BT_ESC) {
>>> >>  for (j = 0; j < 2; j++) {
>>> >>  if (ff_aac_codebook_vectors[cb-1][curidx*2+j] == 
>>> >> 64.0f) {
>>> >> -int coef = av_clip_uintp2(quant(fabsf(in[i+j]), 
>>> >> Q, ROUNDING), 13);
>>> >> -int len = av_log2(coef);
>>> >> +float a = fabsf(in[i+j]) * Q;
>>> >> +double f = sqrtf(a * sqrtf(a)) + ROUNDING;
>>> >> +int coef, len;
>>> >> +
>>> >> +if (f > INT_MAX || f < 16) {
>>> >> +av_log(NULL, AV_LOG_ERROR, "f %f is out of 
>>> >> range this is a internal error\n", f);
>>> >> +f = INT_MAX;
>>> >> +}
>>> >> +
>>> >> +coef = av_clip_uintp2(f, 13);
>>> >> +len = av_log2(coef);
>>> >>
>>> >>  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) 
>>> >> - 2);
>>> >>  put_sbits(pb, len, coef);
>>> >
>>> > Actually I just understood the underlying bug and am testing a fix.
>>> >
>>> > Basically, scalefactors need to be bound by (roughly)
>>> > coef2minsf(maxval), which isn't being done atm, and some signals
>>> > prompt the encoder to pick lower and lower scalefactors trying to
>>> > consume unspent bits that cannot really be consumed.
>>>
>>> Try the attached diff instead (I'm not able to reproduce the issue with gcc)
>>
>> seems to fix it (with gcc didnt try clang but fate will once its
>> pushed)
>>
>> Thanks
>
>
> Pushed

In case anyone is worrying about the latest related breakage, don't.
I've got a fix for that one too, I just cannot push it from where I am
ATM. Will do ASAP when I get to my computer.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/aacenc_quantization: Fix undefined behavior and instead detect and print an error

[Claudio Freire]

On Wed, Mar 30, 2016 at 8:33 AM, Michael Niedermayer
<mich...@niedermayer.cc> wrote:
> On Wed, Mar 30, 2016 at 02:04:20AM -0300, Claudio Freire wrote:
>> On Wed, Mar 30, 2016 at 1:18 AM, Claudio Freire <klaussfre...@gmail.com> 
>> wrote:
>> > On Tue, Mar 29, 2016 at 10:51 PM, Michael Niedermayer
>> > <mich...@niedermayer.cc> wrote:
>> >> This is a hotfix and not a real fix of the underlaying bug
>> >> The underlaying bug is ATM not fully understood
>> >>
>> >> iam not sure if we should apply this or not
>> >>
>> >> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
>> >> ---
>> >>  libavcodec/aacenc_quantization.h |   13 +++--
>> >>  1 file changed, 11 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/libavcodec/aacenc_quantization.h 
>> >> b/libavcodec/aacenc_quantization.h
>> >> index 4250407..d367be0 100644
>> >> --- a/libavcodec/aacenc_quantization.h
>> >> +++ b/libavcodec/aacenc_quantization.h
>> >> @@ -141,8 +141,17 @@ static av_always_inline float 
>> >> quantize_and_encode_band_cost_template(
>> >>  if (BT_ESC) {
>> >>  for (j = 0; j < 2; j++) {
>> >>  if (ff_aac_codebook_vectors[cb-1][curidx*2+j] == 
>> >> 64.0f) {
>> >> -int coef = av_clip_uintp2(quant(fabsf(in[i+j]), 
>> >> Q, ROUNDING), 13);
>> >> -int len = av_log2(coef);
>> >> +float a = fabsf(in[i+j]) * Q;
>> >> +double f = sqrtf(a * sqrtf(a)) + ROUNDING;
>> >> +int coef, len;
>> >> +
>> >> +if (f > INT_MAX || f < 16) {
>> >> +av_log(NULL, AV_LOG_ERROR, "f %f is out of 
>> >> range this is a internal error\n", f);
>> >> +f = INT_MAX;
>> >> +}
>> >> +
>> >> +coef = av_clip_uintp2(f, 13);
>> >> +len = av_log2(coef);
>> >>
>> >>  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 
>> >> 2);
>> >>  put_sbits(pb, len, coef);
>> >
>> > Actually I just understood the underlying bug and am testing a fix.
>> >
>> > Basically, scalefactors need to be bound by (roughly)
>> > coef2minsf(maxval), which isn't being done atm, and some signals
>> > prompt the encoder to pick lower and lower scalefactors trying to
>> > consume unspent bits that cannot really be consumed.
>>
>> Try the attached diff instead (I'm not able to reproduce the issue with gcc)
>
> seems to fix it (with gcc didnt try clang but fate will once its
> pushed)
>
> Thanks


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

Re: [FFmpeg-devel] [PATCH] avcodec/aacenc_quantization: Fix undefined behavior and instead detect and print an error

[Claudio Freire]

On Wed, Mar 30, 2016 at 1:18 AM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Tue, Mar 29, 2016 at 10:51 PM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> This is a hotfix and not a real fix of the underlaying bug
>> The underlaying bug is ATM not fully understood
>>
>> iam not sure if we should apply this or not
>>
>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
>> ---
>>  libavcodec/aacenc_quantization.h |   13 +++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/aacenc_quantization.h 
>> b/libavcodec/aacenc_quantization.h
>> index 4250407..d367be0 100644
>> --- a/libavcodec/aacenc_quantization.h
>> +++ b/libavcodec/aacenc_quantization.h
>> @@ -141,8 +141,17 @@ static av_always_inline float 
>> quantize_and_encode_band_cost_template(
>>  if (BT_ESC) {
>>  for (j = 0; j < 2; j++) {
>>  if (ff_aac_codebook_vectors[cb-1][curidx*2+j] == 64.0f) 
>> {
>> -int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, 
>> ROUNDING), 13);
>> -int len = av_log2(coef);
>> +float a = fabsf(in[i+j]) * Q;
>> +double f = sqrtf(a * sqrtf(a)) + ROUNDING;
>> +int coef, len;
>> +
>> +if (f > INT_MAX || f < 16) {
>> +av_log(NULL, AV_LOG_ERROR, "f %f is out of 
>> range this is a internal error\n", f);
>> +f = INT_MAX;
>> +}
>> +
>> +coef = av_clip_uintp2(f, 13);
>> +len = av_log2(coef);
>>
>>  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
>>  put_sbits(pb, len, coef);
>
> Actually I just understood the underlying bug and am testing a fix.
>
> Basically, scalefactors need to be bound by (roughly)
> coef2minsf(maxval), which isn't being done atm, and some signals
> prompt the encoder to pick lower and lower scalefactors trying to
> consume unspent bits that cannot really be consumed.

Try the attached diff instead (I'm not able to reproduce the issue with gcc)
diff --git a/libavcodec/aaccoder_twoloop.h b/libavcodec/aaccoder_twoloop.h
index 397a4db..4747c79 100644
--- a/libavcodec/aaccoder_twoloop.h
+++ b/libavcodec/aaccoder_twoloop.h
@@ -77,7 +77,7 @@ static void search_for_quantizers_twoloop(AVCodecContext 
*avctx,
 int toomanybits, toofewbits;
 char nzs[128];
 uint8_t nextband[128];
-int maxsf[128];
+int maxsf[128], minsf[128];
 float dists[128] = { 0 }, qenergies[128] = { 0 }, uplims[128], 
euplims[128], energies[128];
 float maxvals[128], spread_thr_r[128];
 float min_spread_thr_r, max_spread_thr_r;
@@ -294,11 +294,14 @@ static void search_for_quantizers_twoloop(AVCodecContext 
*avctx,
 abs_pow34_v(s->scoefs, sce->coeffs, 1024);
 ff_quantize_band_cost_cache_init(s);
 
+for (i = 0; i < sizeof(minsf) / sizeof(minsf[0]); ++i)
+minsf[i] = 0;
 for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
 start = w*128;
 for (g = 0;  g < sce->ics.num_swb; g++) {
 const float *scaled = s->scoefs + start;
 maxvals[w*16+g] = find_max_val(sce->ics.group_len[w], 
sce->ics.swb_sizes[g], scaled);
+minsf[w*16+g] = coef2minsf(maxvals[w*16+g]);
 start += sce->ics.swb_sizes[g];
 }
 }
@@ -425,7 +428,7 @@ static void search_for_quantizers_twoloop(AVCodecContext 
*avctx,
 recomprd = 1;
 for (i = 0; i < 128; i++) {
 if (sce->sf_idx[i] > SCALE_ONE_POS) {
-int new_sf = FFMAX(SCALE_ONE_POS, sce->sf_idx[i] - 
qstep);
+int new_sf = FFMAX3(minsf[i], SCALE_ONE_POS, 
sce->sf_idx[i] - qstep);
 if (new_sf != sce->sf_idx[i]) {
 sce->sf_idx[i] = new_sf;
 changed = 1;
@@ -595,7 +598,7 @@ static void search_for_quantizers_twoloop(AVCodecContext 
*avctx,
 int cmb = find_min_book(maxvals[w*16+g], 
sce->sf_idx[w*16+g]);
 int mindeltasf = FFMAX(0, prev - SCALE_MAX_DIFF);
 int maxdeltasf = FFMIN(SCALE_MAX_POS - SCALE_DIV_512, prev 
+ SCALE_MAX_DIFF);
-if ((!cmb || dists[w*16+g] > uplims[w*16+g]) && 
sce->sf_idx[w*16+g] > mindeltasf) {
+if ((!cmb || dists[w*16+g] > uplims[w*16+g]) && 
sce->sf_idx[w*16+g] > FFMAX(mindelta

Re: [FFmpeg-devel] [PATCH] avcodec/aacenc_quantization: Fix undefined behavior and instead detect and print an error

[Claudio Freire]

On Tue, Mar 29, 2016 at 10:51 PM, Michael Niedermayer
 wrote:
> This is a hotfix and not a real fix of the underlaying bug
> The underlaying bug is ATM not fully understood
>
> iam not sure if we should apply this or not
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/aacenc_quantization.h |   13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/aacenc_quantization.h 
> b/libavcodec/aacenc_quantization.h
> index 4250407..d367be0 100644
> --- a/libavcodec/aacenc_quantization.h
> +++ b/libavcodec/aacenc_quantization.h
> @@ -141,8 +141,17 @@ static av_always_inline float 
> quantize_and_encode_band_cost_template(
>  if (BT_ESC) {
>  for (j = 0; j < 2; j++) {
>  if (ff_aac_codebook_vectors[cb-1][curidx*2+j] == 64.0f) {
> -int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, 
> ROUNDING), 13);
> -int len = av_log2(coef);
> +float a = fabsf(in[i+j]) * Q;
> +double f = sqrtf(a * sqrtf(a)) + ROUNDING;
> +int coef, len;
> +
> +if (f > INT_MAX || f < 16) {
> +av_log(NULL, AV_LOG_ERROR, "f %f is out of range 
> this is a internal error\n", f);
> +f = INT_MAX;
> +}
> +
> +coef = av_clip_uintp2(f, 13);
> +len = av_log2(coef);
>
>  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
>  put_sbits(pb, len, coef);

Actually I just understood the underlying bug and am testing a fix.

Basically, scalefactors need to be bound by (roughly)
coef2minsf(maxval), which isn't being done atm, and some signals
prompt the encoder to pick lower and lower scalefactors trying to
consume unspent bits that cannot really be consumed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/aacenc_quantization: use cbrt table

[Claudio Freire]

On Sun, Mar 13, 2016 at 10:30 PM, Ganesh Ajjanagadde  wrote:
>  /**
>   * Calculate rate distortion cost for quantizing with given codebook
> @@ -105,7 +106,7 @@ static av_always_inline float 
> quantize_and_encode_band_cost_template(
>  curbits += 21;
>  } else {
>  int c = av_clip_uintp2(quant(t, Q, ROUNDING), 13);
> -quantized = c*cbrtf(c)*IQ;
> +quantized = av_int2float(ff_cbrt_tab[c])*IQ;
>  curbits += av_log2(c)*2 - 4 + 1;
>  }
>  } else {

Shouldn't that be c*av_int2float(ff_cbrt_tab[c])*IQ ?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] AAC optimizations and 3.0

[Claudio Freire]

On Thu, Mar 10, 2016 at 8:34 PM, Michael Niedermayer
 wrote:
> Hi all
>
> if you fix speed regressions of the AAC encoder please backport them
> also to release/3.0
>
> Thanks

Even if that implies much bigger changes? (like the one I commented
about aacpsy allocation)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] AAC encoder 3x performance drop in 3.0 since Oct 2014

[Claudio Freire]

On Wed, Mar 9, 2016 at 3:52 PM, Reimar Döffinger
<reimar.doeffin...@gmx.de> wrote:
> On Wed, Mar 09, 2016 at 02:20:35PM -0300, Claudio Freire wrote:
>> On Mon, Feb 29, 2016 at 11:30 PM, Ganesh Ajjanagadde <gajja...@gmail.com> 
>> wrote:
>> > On Mon, Feb 22, 2016 at 11:34 PM, Andrey Utkin
>> > <andrey_ut...@fastmail.com> wrote:
>> > No idea about this. However, here is some info.
>> > The regression in speed dates to: 01ecb7172b684f1c4b3e748f95c5a9a494ca36ec.
>> > At this commit, there was a bad speed regression (11.475 s, vs 2.525 s
>> > before vs 6.565 s current).
>> > As can be judged from this, since the main commit bringing in the
>> > revamped encoder, there have been efforts that have shaved off some
>> > time, some that increase it slightly, etc.
>> > However, the chief one that brought it down from 11.x to 6.y was
>> > b629c67ddfceb7026e407685f04d1bb09cb08d31. Since then, performance has
>> > been generally stable at ~ 6.5 s +/- 10%.
>> >
>> > Generally speaking though, it is indeed true that speed is still
>> > somewhat lacking:
>> > https://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184631.html.
>> >
>> > Ideally, FATE should have some basic plotting/performance
>> > infrastructure, e.g a client can submit perf figures so that evolution
>> > over time can be viewed. No idea why this can't be done.
>>
>> From my experience, the slowness is mostly due to the fact that
>> twoloop gets stuck trying to meet the bandwidth limit with an initial
>> allocation (by aacpsy) that makes it hard. That is, it gives off an
>> initial solution that is far off the bandwidth limit, and makes
>> twoloop work extra hard to adjust the solution.
>>
>> I've been attempting to fix that by making psy's initial allocation a
>> closer match to the target bitrate, but that's another huge change in
>> the bit allocator that shows lots of regressions, so it isn't at all
>> near completion. But I can confirm it does speed up the encoder a
>> great deal when psy gives a better initial solution, so that's
>> probably the path to a speedier encoder.
>
> You mean besides fixing the obvious performance bugs in the code :)
> After all we manged to extract about 25% or more of extra performance
> already with no algorithm changes, so there was (is?) a surprising
> amount of low-hanging fruit.

Sure, but the speedup I see is like 4x or something like that (I
haven't properly measured it yet, but it was noticeable)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] AAC encoder 3x performance drop in 3.0 since Oct 2014

[Claudio Freire]

On Mon, Feb 29, 2016 at 11:30 PM, Ganesh Ajjanagadde  wrote:
> On Mon, Feb 22, 2016 at 11:34 PM, Andrey Utkin
>  wrote:
>> Hi!
>> I am aware of news that AAC encoder got stable status recently.
>>
>> But you could find this interesting. We've got an ffmpeg build from
>> October 2014, and it performs three times faster on AAC encoding than
>> recent 3.0 release. There is no complaints about audio quality on old
>> version, and I can honestly say the audio quality is really
>> satisfiable on old version. The performance is paramount in our
>> particular usecase, so it is silly to deploy a new version which
>> performs so noticeably worse. Still deploying new release is needed due
>> to other particular bugfixes.
>>
>> Obvious things like lowering bitrace, setting "-aac_coder fast" don't
>> help.
>>
>> You can check this yourself with this script (it is also inlined below):
>> https://gist.github.com/andrey-utkin/c60cd4070eb962d58075
>>
>> On my workstation, the old version finishes the transcoding in 2.5s,
>> the new one in 6.6s.
>>
>> Is there any workaround? Or is the old times speed is buried by
>> correctness and stability?
>
> No idea about this. However, here is some info.
> The regression in speed dates to: 01ecb7172b684f1c4b3e748f95c5a9a494ca36ec.
> At this commit, there was a bad speed regression (11.475 s, vs 2.525 s
> before vs 6.565 s current).
> As can be judged from this, since the main commit bringing in the
> revamped encoder, there have been efforts that have shaved off some
> time, some that increase it slightly, etc.
> However, the chief one that brought it down from 11.x to 6.y was
> b629c67ddfceb7026e407685f04d1bb09cb08d31. Since then, performance has
> been generally stable at ~ 6.5 s +/- 10%.
>
> Generally speaking though, it is indeed true that speed is still
> somewhat lacking:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184631.html.
>
> Ideally, FATE should have some basic plotting/performance
> infrastructure, e.g a client can submit perf figures so that evolution
> over time can be viewed. No idea why this can't be done.

From my experience, the slowness is mostly due to the fact that
twoloop gets stuck trying to meet the bandwidth limit with an initial
allocation (by aacpsy) that makes it hard. That is, it gives off an
initial solution that is far off the bandwidth limit, and makes
twoloop work extra hard to adjust the solution.

I've been attempting to fix that by making psy's initial allocation a
closer match to the target bitrate, but that's another huge change in
the bit allocator that shows lots of regressions, so it isn't at all
near completion. But I can confirm it does speed up the encoder a
great deal when psy gives a better initial solution, so that's
probably the path to a speedier encoder.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec: Remove libvo-aacenc support.

[Claudio Freire]

On Fri, Jan 29, 2016 at 1:08 AM, Timothy Gu  wrote:
> On Sun, Jan 24, 2016 at 04:33:31PM +, Kieran Kunhya wrote:
>> The internal encoder is superior to libvo-aacenc.
>> ---
>>  configure |   6 --
>>  doc/encoders.texi |  25 --
>>  doc/general.texi  |   8 --
>>  libavcodec/Makefile   |   1 -
>>  libavcodec/allcodecs.c|   1 -
>>  libavcodec/libvo-aacenc.c | 200 
>> --
>>  libavcodec/version.h  |   2 +-
>>  7 files changed, 1 insertion(+), 242 deletions(-)
>>  delete mode 100644 libavcodec/libvo-aacenc.c
>
> Ping.

Since it LGTM and there seems to be consensus, I'll push it when I get
a chance if nobody beats me to it.

But ATM my hands are tied. I have lots of changes already staged and
almost ready for pushing, doing some final AB testing, so it'll take a
while.

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

Re: [FFmpeg-devel] [PATCH] avcodec/aacenc: Check all coefficients for finiteness

[Claudio Freire]

On Wed, Jan 20, 2016 at 11:05 AM, Michael Niedermayer  wrote:
> From: Michael Niedermayer 
>
> This is needed as near infinite values on the input side result in only some
> output to be non finite.
> Also it may still be insufficient if subsequent computations overflow
>
> Fixes null pointer dereference
> Fixes: 
> ae66c0f6c12ac1cd5c2c237031240f57/signal_sigsegv_2618c99_9516_6007026f2185a26d7afea895fbed6e38.ogg
>
> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/aacenc.c |   17 ++---
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> index 688b131..0dcf404 100644
> --- a/libavcodec/aacenc.c
> +++ b/libavcodec/aacenc.c
> @@ -544,6 +544,7 @@ static int aac_encode_frame(AVCodecContext *avctx, 
> AVPacket *avpkt,
>  chans= tag == TYPE_CPE ? 2 : 1;
>  cpe  = >cpe[i];
>  for (ch = 0; ch < chans; ch++) {
> +int k;
>  float clip_avoidance_factor;
>  sce = >ch[ch];
>  ics = >ics;
> @@ -607,17 +608,11 @@ static int aac_encode_frame(AVCodecContext *avctx, 
> AVPacket *avpkt,
>  s->mdct1024.mdct_calc(>mdct1024, sce->lcoeffs, 
> sce->ret_buf);
>  }
>
> -if (!(isfinite(cpe->ch[ch].coeffs[0]) &&
> -  isfinite(cpe->ch[ch].coeffs[  128]) &&
> -  isfinite(cpe->ch[ch].coeffs[2*128]) &&
> -  isfinite(cpe->ch[ch].coeffs[3*128]) &&
> -  isfinite(cpe->ch[ch].coeffs[4*128]) &&
> -  isfinite(cpe->ch[ch].coeffs[5*128]) &&
> -  isfinite(cpe->ch[ch].coeffs[6*128]) &&
> -  isfinite(cpe->ch[ch].coeffs[7*128]))
> -) {
> -av_log(avctx, AV_LOG_ERROR, "Input contains NaN/+-Inf\n");
> -return AVERROR(EINVAL);
> +for (k = 0; k < 1024; k++) {
> +if (!isfinite(cpe->ch[ch].coeffs[k])) {
> +av_log(avctx, AV_LOG_ERROR, "Input contains 
> NaN/+-Inf\n");
> +return AVERROR(EINVAL);
> +}
>  }
>  avoid_clipping(s, sce);
>  }


LGTM.

I was actually wondering whether it would be needed to do exactly this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/aacenc: use isfinite to simplify isnan/isinf logic

[Claudio Freire]

On Thu, Jan 14, 2016 at 7:57 PM, Ganesh Ajjanagadde
 wrote:
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavcodec/aacenc.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> index 9a7d3a8..2e0db7d 100644
> --- a/libavcodec/aacenc.c
> +++ b/libavcodec/aacenc.c
> @@ -29,6 +29,7 @@
>   * add sane pulse detection
>   ***/
>
> +#include "libavutil/libm.h"
>  #include "libavutil/thread.h"
>  #include "libavutil/float_dsp.h"
>  #include "libavutil/opt.h"
> @@ -606,14 +607,14 @@ static int aac_encode_frame(AVCodecContext *avctx, 
> AVPacket *avpkt,
>  s->mdct1024.mdct_calc(>mdct1024, sce->lcoeffs, 
> sce->ret_buf);
>  }
>
> -if (isnan(cpe->ch->coeffs[0]) || isinf(cpe->ch->coeffs[
> 0]) ||
> -isnan(cpe->ch->coeffs[  128]) || isinf(cpe->ch->coeffs[  
> 128]) ||
> -isnan(cpe->ch->coeffs[2*128]) || 
> isinf(cpe->ch->coeffs[2*128]) ||
> -isnan(cpe->ch->coeffs[3*128]) || 
> isinf(cpe->ch->coeffs[3*128]) ||
> -isnan(cpe->ch->coeffs[4*128]) || 
> isinf(cpe->ch->coeffs[4*128]) ||
> -isnan(cpe->ch->coeffs[5*128]) || 
> isinf(cpe->ch->coeffs[5*128]) ||
> -isnan(cpe->ch->coeffs[6*128]) || 
> isinf(cpe->ch->coeffs[6*128]) ||
> -isnan(cpe->ch->coeffs[7*128]) || 
> isinf(cpe->ch->coeffs[7*128])
> +if (!(isfinite(cpe->ch->coeffs[0]) &&
> +  isfinite(cpe->ch->coeffs[  128]) &&
> +  isfinite(cpe->ch->coeffs[2*128]) &&
> +  isfinite(cpe->ch->coeffs[3*128]) &&
> +  isfinite(cpe->ch->coeffs[4*128]) &&
> +  isfinite(cpe->ch->coeffs[5*128]) &&
> +  isfinite(cpe->ch->coeffs[6*128]) &&
> +  isfinite(cpe->ch->coeffs[7*128]))
>  ) {
>  av_log(avctx, AV_LOG_ERROR, "Input contains NaN/+-Inf\n");
>  return AVERROR(EINVAL);


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

Re: [FFmpeg-devel] [PATCH] avcodec/aacenc_is: replace pow(x, 0.75) by x/sqrtf(sqrtf(x))

[Claudio Freire]

On Mon, Jan 11, 2016 at 7:23 PM, Ganesh Ajjanagadde
 wrote:
> This is quite an accurate approximation; testing shows ~ 2ulp error in
> the floating point result. Tested with FATE.
>
> Alternatively, if one wants "full accuracy", one can use powf, or sqrt
> instead of sqrtf. With powf, one gets 1 ulp error (theoretically should be 0, 
> as
> 0.75 is exactly representable) on GNU libm, with sqrt, 0 ulp error.
>
> Signed-off-by: Ganesh Ajjanagadde 


Pushed.

Took the liberty of abstracting out the pow implementation we
discussed on IRC into a utility function and push that. It's more
readable, and next time we'll know clearly that's the way to go.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: fix assertion error with prediction

[Claudio Freire]

On Tue, Dec 29, 2015 at 1:12 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Tue, Dec 29, 2015 at 6:18 AM, Rostislav Pehlivanov
> <atomnu...@gmail.com> wrote:
>> Wouldn't it be simpler to just check if the maximum codebook was 0 after
>> calculating cost1 and skipping the rest of the code in the loop?
>>
>> On 29 December 2015 at 08:23, Claudio Freire <klaussfre...@gmail.com> wrote:
>>
>>> Fixes an assertion error reported in #2686 that happens when
>>> using prediction (either explicitly or implicitly by setting
>>> the AAC main profile), since prediction code would allow
>>> creating new zeroes or removing existing ones, without
>>> properly checking for SF delta violations.
>>>
>>> This patch forbids creating/removing zeroes, perhaps an
>>> overly conservative approach, but a safe one. More permissive
>>> and sophisticated approaches may be attempted in the future.
>
>
> It is possible, and in fact does happen and it helps considerably to
> hide some artifacts, to use prediction in zeroed bands.
>
> It basically uses the predicted coefficient with no correction.
>
> May well be better than using a zero.

If there are no further objections I may push it soon.

I've encoded and listened to lots of test samples without issue
(though I'm testing other patches too).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: fix assertion error with prediction

[Claudio Freire]

On Tue, Dec 29, 2015 at 6:18 AM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> Wouldn't it be simpler to just check if the maximum codebook was 0 after
> calculating cost1 and skipping the rest of the code in the loop?
>
> On 29 December 2015 at 08:23, Claudio Freire <klaussfre...@gmail.com> wrote:
>
>> Fixes an assertion error reported in #2686 that happens when
>> using prediction (either explicitly or implicitly by setting
>> the AAC main profile), since prediction code would allow
>> creating new zeroes or removing existing ones, without
>> properly checking for SF delta violations.
>>
>> This patch forbids creating/removing zeroes, perhaps an
>> overly conservative approach, but a safe one. More permissive
>> and sophisticated approaches may be attempted in the future.


It is possible, and in fact does happen and it helps considerably to
hide some artifacts, to use prediction in zeroed bands.

It basically uses the predicted coefficient with no correction.

May well be better than using a zero.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] AAC encoder: fix assertion error with prediction

[Claudio Freire]

Fixes an assertion error reported in #2686 that happens when
using prediction (either explicitly or implicitly by setting
the AAC main profile), since prediction code would allow
creating new zeroes or removing existing ones, without
properly checking for SF delta violations.

This patch forbids creating/removing zeroes, perhaps an
overly conservative approach, but a safe one. More permissive
and sophisticated approaches may be attempted in the future.

Attached
From dbda62b2a59054a76e317850a4a0a7037ca3cc02 Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Tue, 29 Dec 2015 05:18:40 -0300
Subject: [PATCH] AAC encoder: fix assertion error with prediction

Fixes an assertion error reported in #2686 that happens when
using prediction (either explicitly or implicitly by setting
the AAC main profile), since prediction code would allow
creating new zeroes or removing existing ones, without
properly checking for SF delta violations.

This patch forbids creating/removing zeroes, perhaps an
overly conservative approach, but a safe one. More permissive
and sophisticated approaches may be attempted in the future.
---
 libavcodec/aacenc_pred.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavcodec/aacenc_pred.c b/libavcodec/aacenc_pred.c
index d76a575..e77a3de 100644
--- a/libavcodec/aacenc_pred.c
+++ b/libavcodec/aacenc_pred.c
@@ -257,7 +257,9 @@ void ff_aac_search_for_pred(AACEncContext *s, SingleChannelElement *sce)
 for (sfb = PRED_SFB_START; sfb < pmax; sfb++) {
 int cost1, cost2, cb_p;
 float dist1, dist2, dist_spec_err = 0.0f;
-const int cb_n = sce->band_type[sfb];
+const int cb_n = sce->zeroes[sfb] ? 0 : sce->band_type[sfb];
+const int cb_min = sce->zeroes[sfb] ? 0 : 1;
+const int cb_max = sce->zeroes[sfb] ? 0 : RESERVED_BT;
 const int start_coef = sce->ics.swb_offset[sfb];
 const int num_coeffs = sce->ics.swb_offset[sfb + 1] - start_coef;
 const FFPsyBand *band = >psy.ch[s->cur_channel].psy_bands[sfb];
@@ -279,7 +281,7 @@ void ff_aac_search_for_pred(AACEncContext *s, SingleChannelElement *sce)
 SENT[i] = sce->coeffs[start_coef + i] - sce->prcoeffs[start_coef + i];
 abs_pow34_v(S34, SENT, num_coeffs);
 if (cb_n < RESERVED_BT)
-cb_p = find_min_book(find_max_val(1, num_coeffs, S34), sce->sf_idx[sfb]);
+cb_p = av_clip(find_min_book(find_max_val(1, num_coeffs, S34), sce->sf_idx[sfb]), cb_min, cb_max);
 else
 cb_p = cb_n;
 quantize_and_encode_band_cost(s, NULL, SENT, QERR, S34, num_coeffs,
@@ -291,7 +293,7 @@ void ff_aac_search_for_pred(AACEncContext *s, SingleChannelElement *sce)
 sce->prcoeffs[start_coef + i] += QERR[i] != 0.0f ? (sce->prcoeffs[start_coef + i] - QERR[i]) : 0.0f;
 abs_pow34_v(P34, >prcoeffs[start_coef], num_coeffs);
 if (cb_n < RESERVED_BT)
-cb_p = find_min_book(find_max_val(1, num_coeffs, P34), sce->sf_idx[sfb]);
+cb_p = av_clip(find_min_book(find_max_val(1, num_coeffs, P34), sce->sf_idx[sfb]), cb_min, cb_max);
 else
 cb_p = cb_n;
 dist2 = quantize_and_encode_band_cost(s, NULL, >prcoeffs[start_coef], NULL,
-- 
1.8.4.5

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

Re: [FFmpeg-devel] [PATCH] aacenc: remove depricated avctx->frame_bits use

[Claudio Freire]

On Fri, Dec 18, 2015 at 10:59 AM, Rostislav Pehlivanov
 wrote:
> The type of last_frame_pb_count was chosen to be an int since overflow
> is impossible (the spec says the maximum bits per frame is 6144 per
> channel and the encoder checks for that).


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

Re: [FFmpeg-devel] [PATCH]lavc/libvo-aac: Mark the encoder as experimental

[Claudio Freire]

On Wed, Dec 9, 2015 at 11:15 AM, Carl Eugen Hoyos  wrote:
> On Sunday 06 December 2015 01:31:17 am Carl Eugen Hoyos wrote:
>> Hi!
>>
>> I prefer attached patch over removing the encoder immediately.
>
> No opinions?
>
> Carl Eugen


LGTM, but I don't use the libvo-aac, so... :-p
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder

[Claudio Freire]

On Sun, Dec 6, 2015 at 6:36 PM, Andreas Cadhalpun
 wrote:
> The other is a regression since 01ecb71, so I hope you know how to fix that.
> In search_for_pns in libavcodec/aaccoder.c:
> for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
> [...]
> for (g = 0;  g < sce->ics.num_swb; g++) {
> [...]
> for (w2 = 0; w2 < sce->ics.group_len[w]; w2++) {
> [...]
> }
> if (g && sce->sf_idx[(w+w2)*16+g-1] == NOISE_BT) {
>
> At this point w+w2 can be sce->ics.num_windows, which causes an
> out-of-bounds read.

I don't see how that can happen.

Do you have the input that triggers this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder

[Claudio Freire]

On Wed, Dec 9, 2015 at 4:42 PM, Andreas Cadhalpun
 wrote:
>>> [...]
>>> for (w2 = 0; w2 < sce->ics.group_len[w]; w2++) {
>>> [...]
>>> }
>
> Now we are after the w2-loop and thus:
> w2 = sce->ics.group_len[w] = 2
>


Ah, I see, it's outside the loop!

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

Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder

[Claudio Freire]

On Wed, Dec 9, 2015 at 5:29 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Wed, Dec 9, 2015 at 4:42 PM, Andreas Cadhalpun
> <andreas.cadhal...@googlemail.com> wrote:
>>>> [...]
>>>> for (w2 = 0; w2 < sce->ics.group_len[w]; w2++) {
>>>> [...]
>>>> }
>>
>> Now we are after the w2-loop and thus:
>> w2 = sce->ics.group_len[w] = 2
>>
>
>
> Ah, I see, it's outside the loop!
>
> Pushing a fix.


Seems I cannot push from this computer (I don't have the key at hand).

So, attached a patch that ought to fix this crash
From 73c14b6684d7ccbd91b80c3dc4a615e40a321494 Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Wed, 9 Dec 2015 17:36:32 -0300
Subject: [PATCH] AAC encoder: fix OOB access in search_for_pns

Fix OOB access in search_for_pns which was using
w2 outside the window group loop, and fix a typo
in which it was checking sf_idx instead of band_type
---
 libavcodec/aaccoder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 14862ec..7e55494 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -711,7 +711,7 @@ static void search_for_pns(AACEncContext *s, AVCodecContext *avctx, SingleChanne
 /* Estimate rd on average as 5 bits for SF, 4 for the CB, plus spread energy * lambda/thr */
 dist2 += band->energy/(band->spread*band->spread)*lambda*dist_thresh/band->threshold;
 }
-if (g && sce->sf_idx[(w+w2)*16+g-1] == NOISE_BT) {
+if (g && sce->band_type[w*16+g-1] == NOISE_BT) {
 dist2 += 5;
 } else {
 dist2 += 9;
-- 
1.8.4.5

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

Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder

[Claudio Freire]

On Fri, Dec 4, 2015 at 2:23 PM, Andreas Cadhalpun
 wrote:
> If minq is negative, the range of sf_idx can be larger than
> SCALE_MAX_DIFF allows, causing assertion failures later in
> encode_scale_factors.
>
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/aaccoder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> index 2a0cb1f..e8a61ce 100644
> --- a/libavcodec/aaccoder.c
> +++ b/libavcodec/aaccoder.c
> @@ -370,7 +370,7 @@ static void search_for_quantizers_anmr(AVCodecContext 
> *avctx, AACEncContext *s,
>  }
>  while (idx) {
>  sce->sf_idx[bandaddr[idx]] = minq + q0;
> -minq = paths[idx][minq].prev;
> +minq = FFMAX(paths[idx][minq].prev, 0);
>  idx--;
>  }
>  //set the same quantizers inside window groups

Actually, a negative .prev signals an impossible path.

So perhaps the attached patch (well, git diff) would go better? (if
you confirm it passes fuzzing I'll push it)
diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 2a0cb1f..b3c8eea 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -363,7 +363,7 @@ static void search_for_quantizers_anmr(AVCodecContext *avctx, AACEncContext *s,
 mincost = paths[idx][0].cost;
 minq= 0;
 for (i = 1; i < TRELLIS_STATES; i++) {
-if (paths[idx][i].cost < mincost) {
+if (paths[idx][i].cost < mincost && paths[idx][i].prev >= 0) {
 mincost = paths[idx][i].cost;
 minq = i;
 }
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder

[Claudio Freire]

On Fri, Dec 4, 2015 at 9:21 PM, Andreas Cadhalpun
<andreas.cadhal...@googlemail.com> wrote:
> On 04.12.2015 23:49, Claudio Freire wrote:
>> On Fri, Dec 4, 2015 at 2:23 PM, Andreas Cadhalpun
>> <andreas.cadhal...@googlemail.com> wrote:
>>> If minq is negative, the range of sf_idx can be larger than
>>> SCALE_MAX_DIFF allows, causing assertion failures later in
>>> encode_scale_factors.
>>>
>>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>>> ---
>>>  libavcodec/aaccoder.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
>>> index 2a0cb1f..e8a61ce 100644
>>> --- a/libavcodec/aaccoder.c
>>> +++ b/libavcodec/aaccoder.c
>>> @@ -370,7 +370,7 @@ static void search_for_quantizers_anmr(AVCodecContext 
>>> *avctx, AACEncContext *s,
>>>  }
>>>  while (idx) {
>>>  sce->sf_idx[bandaddr[idx]] = minq + q0;
>>> -minq = paths[idx][minq].prev;
>>> +minq = FFMAX(paths[idx][minq].prev, 0);
>>>  idx--;
>>>  }
>>>  //set the same quantizers inside window groups
>>
>> Actually, a negative .prev signals an impossible path.
>>
>> So perhaps the attached patch (well, git diff) would go better? (if
>> you confirm it passes fuzzing I'll push it)
>
> That diff doesn't change anything, because in the problematic case
> paths[idx][i].cost is always inf, anyway.

Ok, lets push your patch then.

Do you have the problematic input at hand? If so, send it privately.
If I find a better solution I may try to push that instead, or at
least add the file to fate tests.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder

[Claudio Freire]

On Fri, Dec 4, 2015 at 9:52 PM, Andreas Cadhalpun
 wrote:
> Pushed.
>
>> Do you have the problematic input at hand? If so, send it privately.
>
> Sure, I'll send you a sample.
>
>> If I find a better solution I may try to push that instead, or at
>> least add the file to fate tests.
>
> Thanks for your efforts.

Alright, I see what's going on. minscaler-maxscaler and/or q0-q1 are
empty ranges, so that results in no possible solutions.

I pushed a fix for that, leaving your "safety net" intact.

You may wish to re-fuzz ;)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aacenc: mark coders other than twoloop as experimental

[Claudio Freire]

On Thu, Dec 3, 2015 at 3:01 PM, Rostislav Pehlivanov
 wrote:
> Any opposition?


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

[FFmpeg-devel] AAC experimental flag: the sequel

[Claudio Freire]

So, here comes the discussion again.

This time, the AAC encoder is in good shape. It's not perfect. I have
a list of known bugs to address that still has some issues, but I'm
not really certain whether they should block the flag's removal.

The bugs will be addressed in time, but maybe the encoder is in good
enough shape as it is?

It needs more testing. My tests have all been centered on one
configuration: AAC-LC + PNS + M/S + I/S, ABR, twoloop coder. I haven't
yet thoroughly tested any other configuration, or any other coder, and
it's possible the other coders are kaputt.

I have plans for the other coders but they haven't been fleshed out yet.

There are two possibilities here:

1. Wait until the other coders at least don't crash before removing the flag

2. Remove the flag now, and document the other coders' instability

Which one to go for depends a lot on what removing the flag means to
other people.

Does it mean "it is stable" or does it mean "it's stable for most
uses"? (a subtle but important distinction). IE: what's the contract
between us developers and users when we remove the flag? I'm rather
new at this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] AAC experimental flag: the sequel

[Claudio Freire]

On Wed, Dec 2, 2015 at 12:50 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> On Wed, Dec 2, 2015 at 4:42 PM, Clément Bœsch <u...@pkh.me> wrote:
>> On Wed, Dec 02, 2015 at 12:37:00PM -0300, Claudio Freire wrote:
>>> So, here comes the discussion again.
>>>
>>> This time, the AAC encoder is in good shape. It's not perfect. I have
>>> a list of known bugs to address that still has some issues, but I'm
>>> not really certain whether they should block the flag's removal.
>>>
>>
>> What kind of bugs? If it's crashes, I think they should be fixed before
>> removing the flag (for security reasons).
>
>
> Maybe the other coders could just be disabled unless the experimental
> flag is set, and only allow twoloop otherwise.

That's not a bad idea in fact.

But yeah, the issue is that they might crash. I don't know, because
they haven't been tested enough.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aacenc: remove experimental flag

[Claudio Freire]

On Wed, Dec 2, 2015 at 4:14 PM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> This commit removes the experimental flag from the native AAC Encoder
> and thus makes it the default.
>
> After a lot of work, done by myself and Claudio Freire, the quality of
> this encoder rivals and surpasses libfdk_aac in some situations. The
> encoder had instability issues earlier which prevented it from having
> its experimental flag removed, however the last commit done by Claudio
> removed the last known source of instability and solved a lot of
> problems which were previously observed. The issues were caused by the
> various coding tools interfering with the scalefactor indices. Thus,
> with these problems solved, it should now be possible to declare this
> encoder as the default and recommend that the users should use it
> instead of others provided by external libraries, as it is both faster
> and has a subjectively higher quality with selected tracks.
> The encoder has still yet to be fine tuned for every possible audio file
> type like music or voice, so it is hoped that with the experimental flag
> removed the users should be able to provide feedback and make the
> encoder better than the alternatives for every type of audio.
>
> The documentation will be edited and commited with a later commit.


Did you check the discussion about this?

http://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184393.html

There Hendrik Leppkes had a good idea to shield us from possible
instability in the nondefault coders (anmr, fast, etc).

It would be desirable to implement it with this commit (unless you did
the testing/fuzzing of those coders of course).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aacenc: mark coders other than twoloop as experimental

[Claudio Freire]

On Wed, Dec 2, 2015 at 4:47 PM, Rostislav Pehlivanov
 wrote:
> This commit marks any coders beside twoloop as experimental and gives
> out a warning that some of they might be silently removed in the future.
>
> Users are highly encouraged to use the twoloop coder, which is the
> default.
> ---
>  libavcodec/aacenc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> index 971f8ab..7a34806 100644
> --- a/libavcodec/aacenc.c
> +++ b/libavcodec/aacenc.c
> @@ -967,6 +967,8 @@ static av_cold int aac_encode_init(AVCodecContext *avctx)
>  s->coder = _aac_coders[s->options.coder];
>
>  if (s->options.coder != AAC_CODER_TWOLOOP) {
> +ERROR_IF(avctx->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL,
> + "Coders other than twoloop require -strict -2 and some may 
> be removed in the future\n");
>  s->options.intensity_stereo = 0;
>  s->options.pns = 0;
>  }


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

Re: [FFmpeg-devel] AAC experimental flag: the sequel

[Claudio Freire]

On Wed, Dec 2, 2015 at 12:51 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Wed, Dec 2, 2015 at 10:37 AM, Claudio Freire <klaussfre...@gmail.com> 
> wrote:
>> So, here comes the discussion again.
>>
>> This time, the AAC encoder is in good shape. It's not perfect. I have
>> a list of known bugs to address that still has some issues, but I'm
>> not really certain whether they should block the flag's removal.
>>
>> The bugs will be addressed in time, but maybe the encoder is in good
>> enough shape as it is?
>
> IIUC, JBK responded at VDD 2015 to the question of for which types of
> files they don't use avcodec. He mentioned AAC due to some user's
> complaints about quality on certain files. This was likely referring
> to the decoder and not encoder. Nevertheless, I suggest a conversation
> with them (or other users) to obtain problematic files to test.

I'm always on the lookout for problematic files to add to my "test
samples" collection, so if anyone has one they should contact me in
private.

In fact fate needs a few more samples since most of the samples there
fail to trigger any of the bugs we've been working on of late.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

On Tue, Dec 1, 2015 at 11:12 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 10:47 PM, Michael Niedermayer <michae...@gmx.at> wrote:
>>>  libavcodec/aaccoder.c |   60 --
>>>  libavcodec/aaccoder_twoloop.h |  136 
>>> --
>>>  libavcodec/aacenc.c   |2
>>>  libavcodec/aacenc_is.c|   11 ++-
>>>  libavcodec/aacenc_utils.h |   63 +++
>>>  libavcodec/aacpsy.c   |   20 --
>>>  libavcodec/psymodel.c |1
>>>  libavcodec/psymodel.h |1
>>>  tests/fate/aac.mak|   18 ++---
>>>  9 files changed, 231 insertions(+), 81 deletions(-)
>>> 404ff616bb267619be431a7c45afa4db82070b65  
>>> 0001-AAC-encoder-improve-SF-range-utilization.patch
>>
>> some stuff in mips will need to be updated by someone or disabled
>> after this is pushed:
>> CC  libavcodec/aacpsy.o
>> /ffmpeg/libavcodec/aacpsy.c: In function ‘psy_3gpp_analyze_channel’:
>> /ffmpeg/libavcodec/aacpsy.c:666: error: too many arguments to function 
>> ‘calc_thr_3gpp_mips’
>> make: *** [libavcodec/aacpsy.o] Error 1
>> make: Target `all' not remade because of errors.
>
>
> Oh, didn't realize mips had an alternative calc_thr_3gpp.
>
> I'll see about updating it as I push.

Also refreshed search_for_ms_mips and pushed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

On Tue, Dec 1, 2015 at 10:47 PM, Michael Niedermayer  wrote:
>>  libavcodec/aaccoder.c |   60 --
>>  libavcodec/aaccoder_twoloop.h |  136 
>> --
>>  libavcodec/aacenc.c   |2
>>  libavcodec/aacenc_is.c|   11 ++-
>>  libavcodec/aacenc_utils.h |   63 +++
>>  libavcodec/aacpsy.c   |   20 --
>>  libavcodec/psymodel.c |1
>>  libavcodec/psymodel.h |1
>>  tests/fate/aac.mak|   18 ++---
>>  9 files changed, 231 insertions(+), 81 deletions(-)
>> 404ff616bb267619be431a7c45afa4db82070b65  
>> 0001-AAC-encoder-improve-SF-range-utilization.patch
>
> some stuff in mips will need to be updated by someone or disabled
> after this is pushed:
> CC  libavcodec/aacpsy.o
> /ffmpeg/libavcodec/aacpsy.c: In function ‘psy_3gpp_analyze_channel’:
> /ffmpeg/libavcodec/aacpsy.c:666: error: too many arguments to function 
> ‘calc_thr_3gpp_mips’
> make: *** [libavcodec/aacpsy.o] Error 1
> make: Target `all' not remade because of errors.


Oh, didn't realize mips had an alternative calc_thr_3gpp.

I'll see about updating it as I push.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

On Mon, Nov 30, 2015 at 2:20 PM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> On Mon, 2015-11-30 at 12:50 -0300, Claudio Freire wrote:
>> Also I don't see how a static var would help or even be correct here.
>> Perhaps you meant something else?
> static uint8_t cond1 = param1 && param2;
> static uint8_t cond2 = param3 && !param4;
> ...etc
> return cond1 && cond2;

I don't think the static there does what you think it does.

Just an:

int cond1 = ...;

Would be enough. Not even uint8_t is a win (the conversion to uint8_t
tends to be slower than leaving it at the native int register size).
Same with naming variables, that can tie the compiler's hands
regarding optimizations.

So, as before, I'll see what can be done. If it doesn't break any
optimizations and really makes things more readable I'll do that.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

On Mon, Nov 30, 2015 at 12:27 PM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> On Sun, 2015-11-29 at 16:54 -0300, Claudio Freire wrote:
>> Before pushing this, I'd like some feedback,
>> especially about
>> the implementation of point 3. I'm not sure the AAC encoder
>> setting the cutoff in the encoder context like this is legal or
>> desirable.
> I think setting the cutoff is necessary. You can't avoid holes and yet
> keep a full bandwidth as you decrease bits unless you like the sound of 
> massive quantization errors.

My point was more about whether a codec should write into the context
struct like that or not. Basically an API technicality that's unclear
to me.

>> +unsigned char nextband[128];
> Code wise, could you rename unsigned chars to uint8_t for consistency
> with the rest of the code?

Sure

>> + if (!ff_sfdelta_can_remove_band(sce, nextband, prev, w*16+g)) {
>> + sce-->band_type[w*16+g] = 1;
>> + } else {
>> + sce->zeroes[w*16+g] = 1;
>> + sce->band_type[w*16+g] = 0;
>> + }
> I'd recommend putting sce->zeroes[w*16+g] = 0 in the first part for
> consistency and also band_type[] = ZERO_BT (for 0)

Yeah, not sure why I didn't do that from the start.

> and some random _BT
> for non-zero bands. The band_type[] gets overriden from what I can see.

That I cannot. The value there is taken as the minimum allowed band
type by the codebook_trellis_rate function, which picks the optimum
codebook selection. So it cannot be set to a random number, it really
needs to be set to 1 (first nonzero codebook), and it has no constant
or name other than "1".

>> +static inline int ff_sfdelta_can_replace(const SingleChannelElement
>> *sce, const unsigned char *nextband, int prev_sf, int new_sf, int
>> band)
>> +{
>> +return new_sf = (prev_sf - SCALE_MAX_DIFF)  new_sf
>> = (prev_sf + SCALE_MAX_DIFF)
>> + sce-sf_idx[nextband[band]] = (new_sf -
>> SCALE_MAX_DIFF)  sce-sf_idx[nextband[band]] =
>> (new_sf + SCALE_MAX_DIFF);
>> +}
> Consider splitting up the function definition and using some static
> uint8_t to split up the ternary operator because it's really long.

I'll see how to make it smaller and easier to read, but this really
needs to be inlineable (it's called in a tight loop and is very small,
perhaps amounting to at most a dozen assembler instructions).

Also I don't see how a static var would help or even be correct here.
Perhaps you meant something else?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

On Mon, Nov 30, 2015 at 1:04 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> On Mon, Nov 30, 2015 at 4:50 PM, Claudio Freire <klaussfre...@gmail.com> 
> wrote:
>> On Mon, Nov 30, 2015 at 12:27 PM, Rostislav Pehlivanov
>> <atomnu...@gmail.com> wrote:
>>> On Sun, 2015-11-29 at 16:54 -0300, Claudio Freire wrote:
>>>> Before pushing this, I'd like some feedback,
>>>> especially about
>>>> the implementation of point 3. I'm not sure the AAC encoder
>>>> setting the cutoff in the encoder context like this is legal or
>>>> desirable.
>>> I think setting the cutoff is necessary. You can't avoid holes and yet
>>> keep a full bandwidth as you decrease bits unless you like the sound of 
>>> massive quantization errors.
>>
>> My point was more about whether a codec should write into the context
>> struct like that or not. Basically an API technicality that's unclear
>> to me.
>>
>
> It seems slightly odd to write into that variable, since thats the
> cutoff the user requests.
> Maybe you can use/write into a private variable instead? ac3enc seems
> to use a private variable for similar purposes.

I tried, but it gets very invasive and dirty, because the one that
needs to write is the coder in libavcodec/aaccoder, but the one that
needs to read it is libavcodec/aacpsy, which are abstracted from each
other.

That is, aaccoder has no access to aacpsy's internal state and
viceversa. Doing the above would imply making one of the two able to
access the other's internal state, which seems dirty.

Alternatively, I could add the cutoff to the FFPsyContext struct and
write into that instead. That seems like a cleaner approach.

I'll look into that possibility.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

On Mon, Nov 30, 2015 at 1:04 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> On Mon, Nov 30, 2015 at 4:50 PM, Claudio Freire <klaussfre...@gmail.com> 
> wrote:
>> On Mon, Nov 30, 2015 at 12:27 PM, Rostislav Pehlivanov
>> <atomnu...@gmail.com> wrote:
>>> On Sun, 2015-11-29 at 16:54 -0300, Claudio Freire wrote:
>>>> Before pushing this, I'd like some feedback,
>>>> especially about
>>>> the implementation of point 3. I'm not sure the AAC encoder
>>>> setting the cutoff in the encoder context like this is legal or
>>>> desirable.
>>> I think setting the cutoff is necessary. You can't avoid holes and yet
>>> keep a full bandwidth as you decrease bits unless you like the sound of 
>>> massive quantization errors.
>>
>> My point was more about whether a codec should write into the context
>> struct like that or not. Basically an API technicality that's unclear
>> to me.
>>
>
> It seems slightly odd to write into that variable, since thats the
> cutoff the user requests.
> Maybe you can use/write into a private variable instead? ac3enc seems
> to use a private variable for similar purposes.

Attached patch does that.

I'm far more comfortable with this implementation, so if all agree, I'll push.

On Mon, Nov 30, 2015 at 2:20 PM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> On Mon, 2015-11-30 at 12:50 -0300, Claudio Freire wrote:
>> Also I don't see how a static var would help or even be correct here.
>> Perhaps you meant something else?
> static uint8_t cond1 = param1 && param2;
> static uint8_t cond2 = param3 && !param4;
> ...etc
> return cond1 && cond2;

I didn't do that since it would not short circuit some terms that
ought to be short-circuited.

But I did split lines and it does read better. In fact, maybe that was
all that was needed?
From 1ccc8f8e165ee8111e31253939c3152ff6071f9b Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Tue, 1 Dec 2015 03:28:36 -0300
Subject: [PATCH] AAC encoder: improve SF range utilization

This patch does 4 things, all of which interact and thus it
woudln't be possible to commit them separately without causing
either quality regressions or assertion failures.

Fate comparison targets don't all reflect improvements in
quality, yet listening tests show substantially improved quality
and stability.

1. Increase SF range utilization.

The spec requires SF delta values to be constrained within the
range -60..60. The previous code was applying that range to
the whole SF array and not only the deltas of consecutive values,
because doing so requires smarter code: zeroing or otherwise
skipping a band may invalidate lots of SF choices.

This patch implements that logic to allow the coders to utilize
the full dynamic range of scalefactors, increasing quality quite
considerably, and fixing delta-SF-related assertion failures,
since now the limitation is enforced rather than asserted.

2. PNS tweaks

The previous modification makes big improvements in twoloop's
efficiency, and every time that happens PNS logic needs to be
tweaked accordingly to avoid it from stepping all over twoloop's
decisions. This patch includes modifications of the sort.

3. Account for lowpass cutoff during PSY analysis

The closer PSY's allocation is to final allocation the better
the quality is, and given these modifications, twoloop is now
very efficient at avoiding holes. Thus, to compute accurate
thresholds, PSY needs to account for the lowpass applied
implicitly during twoloop (by zeroing high bands).

This patch makes twoloop set the cutoff in psymodel's context
the first time it runs, and makes PSY account for it during
threshold computation, making PE and threshold computations
closer to the final allocation and thus achieving better
subjective quality.

4. Tweaks to RC lambda tracking loop in relation to PNS

Without this tweak some corner cases cause quality regressions.
Basically, lambda needs to react faster to overall bitrate
efficiency changes since now PNS can be quite successful in
enforcing maximum bitrates, when PSY allocates too many bits
to the lower bands, suppressing the signals RC logic uses to
lower lambda in those cases and causing aggressive PNS.

This tweak makes PNS much less aggressive, though it can still
use some further tweaks.
---
 libavcodec/aaccoder.c |  60 +--
 libavcodec/aaccoder_twoloop.h | 136 +-
 libavcodec/aacenc.c   |   2 +-
 libavcodec/aacenc_is.c|  11 +++-
 libavcodec/aacenc_utils.h |  63 +++
 libavcodec/aacpsy.c   |  20 ---
 libavcodec/psymodel.c |   1 +
 libavcodec/psymodel.h |   1 +
 tests/fate/aac.mak|  18 +++---
 9 files changed, 231 insertions(+), 81 deleti

[FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

[Claudio Freire]

Attached

Before pushing this, I'd like some feedback, especially about
the implementation of point 3. I'm not sure the AAC encoder
setting the cutoff in the encoder context like this is legal or desirable.
It does work quite well, and all attempts to do it otherwise were either
very invasive or repeated lots of complex code, so I opted for this
approach. But perhaps someone more familiar with the interfaces
and the contracts behind them could comment on this.

This patch does 4 things, all of which interact and thus it
woudln't be possible to commit them separately without causing
either quality regressions or assertion failures.

Fate comparison targets don't all reflect improvements in
quality, yet listening tests show substantially improved quality
and stability.

1. Increase SF range utilization.

The spec requires SF delta values to be constrained within the
range -60..60. The previous code was applying that range to
the whole SF array and not only the deltas of consecutive values,
because doing so requires smarter code: zeroing or otherwise
skipping a band may invalidate lots of SF choices.

This patch implements that logic to allow the coders to utilize
the full dynamic range of scalefactors, increasing quality quite
considerably, and fixing delta-SF-related assertion failures,
since now the limitation is enforced rather than asserted.

2. PNS tweaks

The previous modification makes big improvements in twoloop's
efficiency, and every time that happens PNS logic needs to be
tweaked accordingly to avoid it from stepping all over twoloop's
decisions. This patch includes modifications of the sort.

3. Account for lowpass cutoff during PSY analysis

The closer PSY's allocation is to final allocation the better
the quality is, and given these modifications, twoloop is now
very efficient at avoiding holes. Thus, to compute accurate
thresholds, PSY needs to account for the lowpass applied
implicitly during twoloop (by zeroing high bands).

This patch makes twoloop set the cutoff in the encoder context
the first time it runs, and makes PSY account for it during
threshold computation, making PE and threshold computations
closer to the final allocation and thus achieving better
subjective quality.

4. Tweaks to RC lambda tracking loop in relation to PNS

Without this tweak some corner cases cause quality regressions.
Basically, lambda needs to react faster to overall bitrate
efficiency changes since now PNS can be quite successful in
enforcing maximum bitrates, when PSY allocates too many bits
to the lower bands, suppressing the signals RC logic uses to
lower lambda in those cases and causing aggressive PNS.

This tweak makes PNS much less aggressive, though it can still
use some further tweaks.
From e236ab0021eb28012ed05f46e7e520b5cbec413e Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Sun, 29 Nov 2015 16:33:31 -0300
Subject: [PATCH] AAC encoder: improve SF range utilization

This patch does 4 things, all of which interact and thus it
woudln't be possible to commit them separately without causing
either quality regressions or assertion failures.

Fate comparison targets don't all reflect improvements in
quality, yet listening tests show substantially improved quality
and stability.

1. Increase SF range utilization.

The spec requires SF delta values to be constrained within the
range -60..60. The previous code was applying that range to
the whole SF array and not only the deltas of consecutive values,
because doing so requires smarter code: zeroing or otherwise
skipping a band may invalidate lots of SF choices.

This patch implements that logic to allow the coders to utilize
the full dynamic range of scalefactors, increasing quality quite
considerably, and fixing delta-SF-related assertion failures,
since now the limitation is enforced rather than asserted.

2. PNS tweaks

The previous modification makes big improvements in twoloop's
efficiency, and every time that happens PNS logic needs to be
tweaked accordingly to avoid it from stepping all over twoloop's
decisions. This patch includes modifications of the sort.

3. Account for lowpass cutoff during PSY analysis

The closer PSY's allocation is to final allocation the better
the quality is, and given these modifications, twoloop is now
very efficient at avoiding holes. Thus, to compute accurate
thresholds, PSY needs to account for the lowpass applied
implicitly during twoloop (by zeroing high bands).

This patch makes twoloop set the cutoff in the encoder context
the first time it runs, and makes PSY account for it during
threshold computation, making PE and threshold computations
closer to the final allocation and thus achieving better
subjective quality.

4. Tweaks to RC lambda tracking loop in relation to PNS

Without this tweak some corner cases cause quality regressions.
Basically, lambda needs to react faster to overall bitrate
efficiency changes since now PNS can be quite successful in
enforcing maximum bitrates

Re: [FFmpeg-devel] [PATCH 1/2] aaccoder_twoloop: Mark sfdiff as av_unused

[Claudio Freire]

On Tue, Nov 10, 2015 at 1:41 AM, Timothy Gu <timothyg...@gmail.com> wrote:
> On Sun, Nov 8, 2015 at 9:28 AM Claudio Freire <klaussfre...@gmail.com>
> wrote:
>>
>> This particular piece of code is going to disappear soon, so not sure
>> it's worth applying the patch.
>
>
> Oh, okay then.


Funny thing, in the end it's going to survive, so I'm going to apply
this soon :)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [FFmpeg-cvslog] AAC encoder: tweak PNS usage to be more aggressive

[Claudio Freire]

On Fri, Sep 25, 2015 at 10:39 PM, James Almer <jamr...@gmail.com> wrote:
>> ffmpeg | branch: master | Claudio Freire  | Fri 
>> Sep 25 03:56:32 2015 -0300| [9458a62decfcaa1313b1ba69276466de536d0768] | 
>> committer: Claudio Freire
>>
>> AAC encoder: tweak PNS usage to be more aggressive
>>
>> This patch tweaks search_for_pns to be both more
>> aggressive and more careful when applying PNS. On
>> the one side, it will again try to use PNS on zero
>> (or effectively zero) bands. For this, both zeroes
>> and band_type have to be checked (some ZERO bands
>> aren't marked in zeroes). On the other side, a more
>> accurate rate-distortion measure avoids using PNS
>> where it would cause audible distortion.
>>
>> Also fixed a small bug in the computation of freq
>> that caused PNS usage on low-frequency bands during
>> 8-short windows. This allows re-enabling PNS during
>> 8-short.
>
> Clang and gcc's address sanitizer complain about this change
>
> http://fate.ffmpeg.org/report.cgi?time=20150925234050=x86_64-archlinux-gcc-asan
> http://fate.ffmpeg.org/report.cgi?time=20150925220641=x86_64-debian-asan-144800


It was a pre-existing issue that got exposed by that commit, not the
commit itself.

But fixed in any case.

There may be other instances of the bug, so I'll be on the lookout for
this specific case.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: refactor to resynchronize MIPS port

[Claudio Freire]

On Tue, Sep 15, 2015 at 8:11 AM, Michael Niedermayer <michae...@gmx.at> wrote:
> On Tue, Sep 15, 2015 at 04:24:02AM -0300, Claudio Freire wrote:
>> This patch refactors the AAC coders to reuse code
>> between the MIPS port and the regular, portable C code.
>> There were two main functions that had to use
>> hand-optimized versions of quantization code:
>>  - search_for_quantizers_twoloop
>>  - codebook_trellis_rate
>>
>> Those two were split into their own template header
>> files so they can be inlined inside both the MIPS port
>> and the generic code. In each context, they'll link
>> to their specialized implementations, and thus be
>> optimized by the compiler.
>>
>> This approach I believe is better than maintaining
>> several copies of each function. As past experience has
>> proven, having to keep those in sync was error prone.
>> In this way, they will remain in sync by default.
>>
>> Also, an implementation of the reconstructed output
>> argument for the optimized quantize_and_encode
>> functions is included in the patch. While the current
>> implementation of search_for_pred still isn't using
>> it, future iterations of main prediction probably will.
>> It should not imply any measurable performance hit while
>> not being used.
>>
>>
>> Patch attached.
>>
>> I thought it was worth a review.
>>
>> It does include lots of copypaste.
>>
>> FTR, I tested MIPS 74Kf and x86_64 with make fate-aac
>
> full fate passes on qemu mips here as well!

If there's no objections then, I will be pushing it later today,
before it stops applying cleanly.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC encoder: refactor to resynchronize MIPS port

[Claudio Freire]

On Wed, Sep 16, 2015 at 12:30 PM, Nedeljko Babic
 wrote:


 Patch attached.

 I thought it was worth a review.

 It does include lots of copypaste.

 FTR, I tested MIPS 74Kf and x86_64 with make fate-aac
>>>
>>> full fate passes on qemu mips here as well!
>>
>>If there's no objections then, I will be pushing it later today,
>>before it stops applying cleanly.
>
> LGTM regarding mips part. I have no objection.
>
> This is very helpful. Thanks.
>
> Regarding problem with ffserver, I don't think that the cause is the same 
> with the one in:
> http://fate.ffmpeg.org/log.cgi?time=20150914220602=compile=mips-linux-gcc-4.3.2
>
> The problem on the link is caused by environment on which tests are being 
> compiled and run.
>
> You can send me your configuration line and how did you tested it (I am 
> afraid that I didn't work with ffserver), so I can check what is going on.

This is the configure line that works:

./configure --target-os=linux --arch=mips --enable-cross-compile
--cross-prefix=mips-linux- --target-exec='qemu-mips -cpu 74Kf'
--prefix=/opt/cross
--samples=/home/claudiofreire/tmp/audiosamples/ffsamples
--extra-ldflags=-static --ranlib=mips-linux-ranlib --disable-ffserver

And this one blows with the ICE:

./configure --target-os=linux --arch=mips --enable-cross-compile
--cross-prefix=mips-linux- --target-exec='qemu-mips -cpu 74Kf'
--prefix=/opt/cross
--samples=/home/claudiofreire/tmp/audiosamples/ffsamples
--extra-ldflags=-static --ranlib=mips-linux-ranlib

>
> Also, are you building it on some board, or are you cross compiling it?

I'm cross-compiling as you may notice from the above configure lines.

$> /opt/cross/bin/mips-linux-gcc --version
mips-linux-gcc (GCC) 4.5.3
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] AAC encoder: refactor to resynchronize MIPS port

[Claudio Freire]

This patch refactors the AAC coders to reuse code
between the MIPS port and the regular, portable C code.
There were two main functions that had to use
hand-optimized versions of quantization code:
 - search_for_quantizers_twoloop
 - codebook_trellis_rate

Those two were split into their own template header
files so they can be inlined inside both the MIPS port
and the generic code. In each context, they'll link
to their specialized implementations, and thus be
optimized by the compiler.

This approach I believe is better than maintaining
several copies of each function. As past experience has
proven, having to keep those in sync was error prone.
In this way, they will remain in sync by default.

Also, an implementation of the reconstructed output
argument for the optimized quantize_and_encode
functions is included in the patch. While the current
implementation of search_for_pred still isn't using
it, future iterations of main prediction probably will.
It should not imply any measurable performance hit while
not being used.


Patch attached.

I thought it was worth a review.

It does include lots of copypaste.

FTR, I tested MIPS 74Kf and x86_64 with make fate-aac

PS: yes, Nedeljko beat me by a small margin in syncing up the
implementations, but still I believe sharing code is better than
maintaining copies. Especially having in mind the big rewrite that is
coming from the work in #2686. So I submit this approach for
consideration.

PS2: ffserver is causing some ICEs on cross gcc 4.5.3, a slight
variation on this fate failure:
http://fate.ffmpeg.org/log.cgi?time=20150914220602=compile=mips-linux-gcc-4.3.2
- let me know if I can provide some more info on this to get it fixed
somehow.
From 3a530153a0c4ef18f5a0f936fd356e9493b5a00e Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Tue, 15 Sep 2015 03:59:45 -0300
Subject: [PATCH] AAC encoder: refactor to resynchronize MIPS port

This patch refactors the AAC coders to reuse code
between the MIPS port and the regular, portable C code.
There were two main functions that had to use
hand-optimized versions of quantization code:
 - search_for_quantizers_twoloop
 - codebook_trellis_rate

Those two were split into their own template header
files so they can be inlined inside both the MIPS port
and the generic code. In each context, they'll link
to their specialized implementations, and thus be
optimized by the compiler.

This approach I believe is better than maintaining
several copies of each function. As past experience has
proven, having to keep those in sync was error prone.
In this way, they will remain in sync by default.

Also, an implementation of the dequantized output
argument for the optimized quantize_and_encode
functions is included in the patch. While the current
implementation of search_for_pred still isn't using
it, future iterations of main prediction probably will.
It should not imply any measurable performance hit while
not being used.
---
 libavcodec/aaccoder.c| 284 +---
 libavcodec/aaccoder_trellis.h| 194 +
 libavcodec/aaccoder_twoloop.h| 203 ++
 libavcodec/aacenc_quantization.h |  14 ++
 libavcodec/mips/aaccoder_mips.c  | 452 ++-
 tests/fate/aac.mak   |  15 +-
 6 files changed, 538 insertions(+), 624 deletions(-)
 create mode 100644 libavcodec/aaccoder_trellis.h
 create mode 100644 libavcodec/aaccoder_twoloop.h

diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 524987d..8d5ea77 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -48,6 +48,8 @@
 #include "aacenc_tns.h"
 #include "aacenc_pred.h"
 
+#include "libavcodec/aaccoder_twoloop.h"
+
 /** Frequency in Hz for lower limit of noise substitution **/
 #define NOISE_LOW_LIMIT 4000
 
@@ -59,6 +61,8 @@
  * replace low energy non zero bands */
 #define NOISE_LAMBDA_REPLACE 1.948f
 
+#include "libavcodec/aaccoder_trellis.h"
+
 /**
  * structure used in optimal codebook search
  */
@@ -181,137 +185,6 @@ static void encode_window_bands_info(AACEncContext *s, SingleChannelElement *sce
 }
 }
 
-static void codebook_trellis_rate(AACEncContext *s, SingleChannelElement *sce,
-  int win, int group_len, const float lambda)
-{
-BandCodingPath path[120][CB_TOT_ALL];
-int w, swb, cb, start, size;
-int i, j;
-const int max_sfb  = sce->ics.max_sfb;
-const int run_bits = sce->ics.num_windows == 1 ? 5 : 3;
-const int run_esc  = (1 << run_bits) - 1;
-int idx, ppos, count;
-int stackrun[120], stackcb[120], stack_len;
-float next_minbits = INFINITY;
-int next_mincb = 0;
-
-abs_pow34_v(s->scoefs, sce->coeffs, 1024);
-start = win*128;
-for (cb = 0; cb < CB_TOT_ALL; cb++) {
-path[0][cb].cost = run_bits+4;
-path[0][cb].prev_idx = -1;
-path[0][cb].run  = 0;
- 

Re: [FFmpeg-devel] MIPS and PNS

[Claudio Freire]

I'm also looking into it.

It's simply that it tends to get out of sync with the rest really.

I will remove some duplicated code (namely the twoloop and trellis
functions which are verbatim copies of their counterparts in
aaccoder), while somehow keeping the fact that they used the optimized
quantize_band_cost functions.

That should make it simpler to maintain in the future.


On Thu, Sep 3, 2015 at 5:26 AM, Nedeljko Babic
 wrote:
> Hi,
>
> Thanks Michael.
>
> I'll look in to it.
>
> -Nedeljko
> 
> Od: Michael Niedermayer [michae...@gmx.at]
> Poslato: 2. septembar 2015 13:11
> Za: FFmpeg development discussions and patches
> Cc: Nedeljko Babic
> Tema: MIPS and PNS
>
> Hi
>
> Enabling pns by default (b7eb7cb3a18093aea22d1aee2cbe293f6a214a5b)
> broke aac-pns-encode
> (http://fate.ffmpeg.org/report.cgi?time=20150901213100=mips-ubuntu-qemu-gcc-4.4)
>
> ive disabled the optimizations which cause this breakage
> (933309a6ca0f18bf1d40e917fff455221f57fb4b),
> someone should check, update and re-enable them
>
> Thanks
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Opposition brings concord. Out of discord comes the fairest harmony.
> -- Heraclitus
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC]Remove experimental flag from aac encoder

[Claudio Freire]

On Tue, Sep 1, 2015 at 5:54 AM, Robert Krüger  wrote:
>> It's been the plan all along to negotiate the removal of the
>> experimental flag after pushing those changes discussed and heavily
>> tested in ticket #2686.
>>
>> If anything, this thread expresses support for that plan ;)
>>
>> So, I'd stick to that plan.
>>
>>
> I know this is probably an annoying question but do you have a gut feeling
> very roughly when you will feel comfortable with pushing those changes for
> ticket #2686?

Well, a part of them is already in.

The remaining part I'm planning to push during the next few weeks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC]Remove experimental flag from aac encoder

[Claudio Freire]

On Mon, Aug 31, 2015 at 6:59 AM, Robert Krüger  wrote:
> On Mon, Aug 31, 2015 at 10:44 AM, Carl Eugen Hoyos  wrote:
>
>> Hi!
>>
>> I didn't test myself but iiuc, the aac encoder produces better
>> quality than all other libavcodec audio encoders and better
>> quality than the libvo-aac encoder that is not marked as
>> experimental, so the flag should be removed.
>>
>
> I haven't done scientifically thorough testing but it has improved a lot
> over the past few months and is definitely in a very usable state
> quality-wise (in 256kbit I couldn't hear differences to the Apple encoder,
> which regularly gets very high test scores, tested with HQ source material
> and equipment) and probably much better maintained than a lot of other
> pieces of non-experimental code. If there are no known, significant
> stability issues +1.

It has some relatively annoying issues with transients. We've got
patches in the queue already to fix those.

On Mon, Aug 31, 2015 at 8:49 AM, Rostislav Pehlivanov
 wrote:
> Claudio also has some changes which he still hasn't merged yet, not sure if
> he has the time right now. I'll write to him to see what he's up to.
>
> PNS, Intensity Stereo and maybe M/S need to get enabled by default since
> they definitely improve the quality a ton. Might do that for PNS and IS
> when I push the last TNS changes tonight.
>
> Bottom line is, there are still some things which need to be pushed and I
> think after that, in 4-5 days depending on what's happening with Claudio's
> changes, I will personally be comfortable with removing the experimental
> flag and declaring the encoder stable.

This.

It's been the plan all along to negotiate the removal of the
experimental flag after pushing those changes discussed and heavily
tested in ticket #2686.

If anything, this thread expresses support for that plan ;)

So, I'd stick to that plan.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 02/10] aacenc: Add missing ff_ prefixes

[Claudio Freire]

On Sat, Aug 22, 2015 at 4:51 AM, Nicolas George geo...@nsup.org wrote:

 Le quintidi 5 fructidor, an CCXXIII, Claudio Freire a écrit :
  They were included in the symbol table but only as local, the
 libavcodec.v
  file makes sure to make everything not explicitly mentioned for export
  local.
 
  Though it's possible that it depends on the compiler version?

 It depends on the kind of library: libavcodec.v affects shared objects, but
 not static libraries. Static libraries are just an indexed archive of
 object
 files, the exported symbols are exactly the non-static ones.



Ah, that clears things up.

Thanks for the explanation :)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 02/10] aacenc: Add missing ff_ prefixes

[Claudio Freire]

On Sat, Aug 22, 2015 at 1:01 AM, Timothy Gu timothyg...@gmail.com wrote:

 On Fri, Aug 21, 2015 at 10:49:01PM -0400, Ganesh Ajjanagadde wrote:
  There are too many entries here for me to verify which ones are exposed,
 etc.
  I trust you identified them correctly.

 To the best of my knowledge, yes. To anyone reading this message, I used
 the following command to get a list of all symbols without the prefix:

 nm libavcodec/*.o | grep ' [TD] ' | cut -d ' ' -f 3- | grep -v
 '^\(av\|ff_\)'

 After this patchset, the command outputs nothing except for the
 deprecated `audio_resample` functions.



When reviewing the patches with Rostislav, I actually tested to see whether
those symbols were exported, and they weren't.

They were included in the symbol table but only as local, the libavcodec.v
file makes sure to make everything not explicitly mentioned for export
local.

Though it's possible that it depends on the compiler version?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/6] aacenc: Move small misc. functions to a separate file

[Claudio Freire]

On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 As well as tables littered everywhere, functions were spread
 out all across the encoder's files. This moves them to a single
 place where they can be used by either the encoder's main files
 or additional encoder files. Additionally, it changes the type
 of some to 'inline' to enable us to simply put them in a header
 file and possibly gain some speed due to compiler optimizations.


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

Re: [FFmpeg-devel] [PATCH 6/6] aacenc: Move Intensity Stereo search functions to a new file

[Claudio Freire]

On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 With the new method to determine the phase, the functions
 got sufficiently large to have their own file.
 There are absolutely no changes from the IS patch from
 this patchset, this commit simply moves it out of aaccoder.c
 Encoding is still being done within the aacenc.c file due
 to the way IS and M/S interact and affect the spectral coefficients.
 This commit also makes use of the separated quantization functions
 from the previous commit.


I guess this needs a rebase re. the improvements on patch 3. Right?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/6] aacenc: Move local encoder specific tables to a separate file

[Claudio Freire]

On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit moves any tables specific to the encoder from aacenc
 and aaccoder to a separate file called 'aacenctab.c/.h'.
 This was done as a clean up attempt as the encoder was filled with
 tables pasted in between functions which made it confusing to follow
 and track where each table and definition had been used.
 This commit solves this by simply exporting the smaller tables out to
 the aacenctab.h while the larger ones are compiled using aacenctab.c
 and are referenced from the header file.
 ---


Applied.

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

Re: [FFmpeg-devel] [PATCH 2/6] aacenc: Improve Intensity Stereo phase detection

[Claudio Freire]

On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 +if (cpe-ms_mode)
 +phase = 1 - 2 * cpe-ms_mask[w*16+g];


Shouldn't it be ?

phase *= 1 - ... ;

phase is an argument, the original code would step on it, with a value
that doesn't depend on phase, so it would fail to evaluate both
phases. Using phase *= would make sure to test both phases.

Well, that's the general idea, except it breaks the phase assigned to
the struct. Something like the following does work though:

ephase = phase;
if (cpe-ms_mode)
ephase *= 1 - 2 * cpe-ms_mask[w*16+g];

and then change all uses of phase into ephase, except the last that remains:

is_error.phase = phase; // original phase
is_error.pass  = dist2 = dist1;


On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
  for (w = 0; w  128; w++)
  if (sce1-band_type[w] = INTENSITY_BT2)
  sce1-band_type[w] = 0;

 -if (!cpe-common_window)
 -return;
 -for (w = 0; w  sce0-ics.num_windows; w += sce0-ics.group_len[w]) {
 -start = 0;
 -for (g = 0;  g  sce0-ics.num_swb; g++) {
 -if (start*freq_mult  INT_STEREO_LOW_LIMIT*(lambda/170.0f) 
 -cpe-ch[0].band_type[w*16+g] != NOISE_BT  
 !cpe-ch[0].zeroes[w*16+g] 
 -cpe-ch[1].band_type[w*16+g] != NOISE_BT  
 !cpe-ch[1].zeroes[w*16+g]) {
 -int phase = 0;
 -float ener0 = 0.0f, ener1 = 0.0f, ener01 = 0.0f;
 -float dist1 = 0.0f, dist2 = 0.0f;
 +if (!cpe-common_window)
 +return;
 +for (w = 0; w  sce0-ics.num_windows; w += sce0-ics.group_len[w]) {
 +start = 0;
 +for (g = 0;  g  sce0-ics.num_swb; g++) {
 +if (start*freq_mult  
 INT_STEREO_LOW_LIMIT*(s-lambda/170.0f) 

This looks strange. As it is that patch, it ends up with code like:

for (w = 0; w  128; w++)
if (sce1-band_type[w] = INTENSITY_BT2)
sce1-band_type[w] = 0;

if (!cpe-common_window)
return;
for (w = 0; w  sce0-ics.num_windows; w += sce0-ics.group_len[w]) {
start = 0;
for (g = 0;  g  sce0-ics.num_swb; g++) {

Which looks wrong. Bad indentation right?

I think you meant:

for (w = 0; w  128; w++)
if (sce1-band_type[w] = INTENSITY_BT2)
sce1-band_type[w] = 0;

if (!cpe-common_window)
return;
for (w = 0; w  sce0-ics.num_windows; w += sce0-ics.group_len[w]) {
start = 0;
for (g = 0;  g  sce0-ics.num_swb; g++) {

A big part of the diff in that hunk is reindent, so I believe if you
fix that indentation snafu the patch will shrink.

On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
  for (w2 = 0; w2  sce0-ics.group_len[w]; w2++) {
 +abs_pow34_v(L34, sce0-coeffs+start+(w+w2)*128, 
 sce0-ics.swb_sizes[g]);
 +abs_pow34_v(R34, sce1-coeffs+start+(w+w2)*128, 
 sce0-ics.swb_sizes[g]);
  for (i = 0; i  sce0-ics.swb_sizes[g]; i++) {
  float coef0 = sce0-pcoeffs[start+(w+w2)*128+i];
  float coef1 = sce1-pcoeffs[start+(w+w2)*128+i];
 -phase += coef0*coef1 = 0.0f ? 1 : -1;
  ener0 += coef0*coef0;
  ener1 += coef1*coef1;
  ener01 += (coef0 + coef1)*(coef0 + coef1);
  }
  }

Careful, you're stepping on L34 and R34 on eight_short_window, and
passing the last results only to calc_encoding_err_is.

In fact, I'm thinking I may have induced you to make that mistake when
I suggested not to compute R34 / L34 twice (once for each phase),
since L34 and R34 only have room for one window, and
calc_encoding_err_is needs to process a whole window group.

I think you'll have to move it back to inside calc_encoing_err_is and
just compute it twice. Redundant, but at least it's correct.

Also, you should use pcoeffs (coeffs will have M/S applied to it when ms_mask).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] aacenc: remove redundant argument from coder functions

[Claudio Freire]

On Fri, Jul 31, 2015 at 9:56 PM, Michael Niedermayer
mich...@niedermayer.cc wrote:

 Rostislav, Claudio
 please both of you send me your public SSH keys, i think you both
 should have git write access

Sent mine on a private email. Let me know if you didn't get it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] aacenc: remove redundant argument from coder functions

[Claudio Freire]

On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit removes a redundant argument from the functions in aaccoder.
 The argument lambda was redundant as it was just a copy of s-lambda,
 to which all functions have access to anyway. This cleans up the function
 pointers a bit which is helpful as there are a lot of other search_for_*
 functions under development and with them populated it gets messy.

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

Re: [FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Claudio Freire]

On Wed, Jul 29, 2015 at 4:15 PM, Michael Niedermayer
mich...@niedermayer.cc wrote:
 On Mon, Jul 27, 2015 at 07:28:35PM +0200, Michael Niedermayer wrote:
 On Mon, Jul 20, 2015 at 11:40:54PM -0300, Claudio Freire wrote:
  On Mon, Jul 20, 2015 at 11:39 PM, Claudio Freire klaussfre...@gmail.com 
  wrote:
   On Fri, Jul 17, 2015 at 8:42 PM, Michael Niedermayer
   mich...@niedermayer.cc wrote:
   If you mean a transition in time, I don't think it makes any
   difference. 0.95 is a ~0.5db change in intensity, which ought to be
   inaudible, and windowing will already take care to make the transition
   smooth. And the logic wouldn't be completely free either to ramp
   gradually, as it would have to ramp fully to 0.95 by the time it
   reaches the first window marked as clipping hazard, and it could very
   well be the frist window.
  
   what i meant was that whatever condition is used to hard switch
   between 1.0 and 0.95 could be rather a soft transition that is
   for example
   insteda of
   if (x  0.0) f = 0.95
  
   something like
   if (x  0.0  x1.0) f = 1.0 + (0.95 - 1.0)*x
  
   so theres no discontinuity in the transition
  
   Attached is a patch with something like that.
  
   1. It measures per-window minimum clip avoidance factor
   2. Computes a whole-frame factor (minimum of all)
   3. Applies the clip avoidance factor to the whole frame (to make it
   smooth and almost linear, and avoid adding harmonic distortion).
 
 
  Sorry, that one had trailing whitespace again.
 
  Attached.

   libavcodec/aac.h  |4 +
   libavcodec/aaccoder.c |  108 
  --
   libavcodec/aacenc.c   |   38 +
   libavcodec/aacenc.h   |2
   libavcodec/aacpsy.c   |   30 +
   libavcodec/psymodel.h |1
   tests/fate/aac.mak|4 -
   7 files changed, 145 insertions(+), 42 deletions(-)
  fb3816accbd479ea10b2be6c104e28eee81ce743  
  0001-AAC-Encoder-clipping-avoidance.patch
  From 57522de7c5fcdbef222c2425a4add6fa4528f0e7 Mon Sep 17 00:00:00 2001
  From: Claudio Freire klaussfre...@gmail.com
  Date: Mon, 20 Jul 2015 22:53:24 -0300
  Subject: [PATCH] AAC Encoder: clipping avoidance

 applied

 it seems this breaks mips-qemu


It does, I know what it is. I'll get a patch to fix it ASAP
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] aacdec: move the LTP and TNS tables out of aacdectab.h

[Claudio Freire]

On Wed, Jul 22, 2015 at 6:18 AM, Nedeljko Babic
nedeljko.ba...@imgtec.com wrote:
On Mon, Jul 20, 2015 at 8:50 AM, Nedeljko Babic
nedeljko.ba...@imgtec.com wrote:
This commit moves the tables required for encoding and decoding
LTP and TNS AAC files out of the decoder's standalone tables file
and into the shared aactab.h, where they can be used by both the
encoder and the decoder.

This commit does not break the already-broken aac_fixed decoder.

 Not sure I would say that it is broken, since it is not implemented fully 
 yet...

I think even with the other patches submitted to the list, TNS (which
uses lpc) will break. It will be sending an int* to a function that
takes float* (check lpc.h, vs aacdec_template.c:3223 + aac.h:244)


 I am failing to see this.

 If we are talking about functions defined in lpc.h (compute_ref_coefs and 
 compute_lpc_coefs), they are taking LPC_TYPE and it depends on definition of 
 USE_FIXED.

 In case of lpc.h, USE_FIXED is by default set to zero in aac_defines.h 
 included in lpc.h

 apply_tns is sending int* to compute_lpc_coefs() only in case of aacdec_fixed 
 (and in that case compute_lpc_coefs() expects int*, not float*).

Sorry, when I checked, USE_FIXED was either double or float, but never
int. Now that I check again, it's as you say.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] trying ffprobe on AAC file with LATM fails

[Claudio Freire]

On Mon, Jul 20, 2015 at 5:08 AM, Venelin Efremov veffremov...@gmail.com wrote:
 The error message I get from the latest git head is:
 [aac_latm @ 0x3a226e0] Non-byte-aligned audio-specific config is not
 implemented. Update your FFmpeg version to the newest one from Git. If the
 problem still occurs, it means that your file has a feature which has not
 been implemented.
 [aac_latm @ 0x3a226e0] If you want to help, upload a sample of this file to
 ftp://upload.ffmpeg.org/incoming/ and contact the ffmpeg-devel mailing
 list. (ffmpeg-devel@ffmpeg.org)

 I uploaded the file as aac_latm_non_byte_aligned.bin.

 The file is aac-lc 2 channel 44100 sampling rate.

If someone can create a ticket with that attachment and cc me in the
ticket, I'll be happy to take a stab at it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] trying ffprobe on AAC file with LATM fails

[Claudio Freire]

On Tue, Jul 21, 2015 at 2:50 PM, Claudio Freire klaussfre...@gmail.com wrote:
 On Mon, Jul 20, 2015 at 5:08 AM, Venelin Efremov veffremov...@gmail.com 
 wrote:
 The error message I get from the latest git head is:
 [aac_latm @ 0x3a226e0] Non-byte-aligned audio-specific config is not
 implemented. Update your FFmpeg version to the newest one from Git. If the
 problem still occurs, it means that your file has a feature which has not
 been implemented.
 [aac_latm @ 0x3a226e0] If you want to help, upload a sample of this file to
 ftp://upload.ffmpeg.org/incoming/ and contact the ffmpeg-devel mailing
 list. (ffmpeg-devel@ffmpeg.org)

 I uploaded the file as aac_latm_non_byte_aligned.bin.

 The file is aac-lc 2 channel 44100 sampling rate.

 If someone can create a ticket with that attachment and cc me in the
 ticket, I'll be happy to take a stab at it.


Created:

https://trac.ffmpeg.org/ticket/4730
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] aacdec: move the LTP and TNS tables out of aacdectab.h

[Claudio Freire]

On Mon, Jul 20, 2015 at 4:15 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 Yep, with this patch compute_lpc_coefs() (referenced in aacdec_template.c
 when applying TNS) in ltp.c prints a warning when compiling the fixed
 decoder, which is fine since it changed a type from INTFLOAT to float.

And it will probably be accepted first anyhow :)
 I'm fine either way too, it's not hard to substitute a few words :)
 The encoder has no problem with INTFLOATs (as long as its 'float' when
 compiling the encoder) so feel free to change any shared tables later when
 you fully implement the fixed point decoder.

 Looking at it I was wrong, compute_lpc_coefs() is used in ra288.c as well,
 so disregard what I said about moving that in the decoder, it's fine as it
 is.

You're probably referring to lpc.c (not ltp).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] aacdec: move the LTP and TNS tables out of aacdectab.h

[Claudio Freire]

On Mon, Jul 20, 2015 at 8:50 AM, Nedeljko Babic
nedeljko.ba...@imgtec.com wrote:
This commit moves the tables required for encoding and decoding
LTP and TNS AAC files out of the decoder's standalone tables file
and into the shared aactab.h, where they can be used by both the
encoder and the decoder.

This commit does not break the already-broken aac_fixed decoder.

 Not sure I would say that it is broken, since it is not implemented fully 
 yet...

I think even with the other patches submitted to the list, TNS (which
uses lpc) will break. It will be sending an int* to a function that
takes float* (check lpc.h, vs aacdec_template.c:3223 + aac.h:244)

It's a bit OT in this thread but I didn't see any thread for TNS in
the fixed point decoder ones.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

[Claudio Freire]

On Fri, Jul 17, 2015 at 11:19 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
But even if not used for avoding I/S, it can be used to pick whether
to invert the phases, where it was clearly more stable.
 In case the phase is very clearly wrong then there will be an increase in
 the distortion which should cause the dist2 = dist1 check to fail and thus
 not use IS. So the phase isn't even very important. The whole point of the
 phase check is to pick out the obviously wrong phases and save time by not
 having to calculate the error of the spectral coefficients. And this works
 better when you individually pick out a majority rather than just summing
 them up and then doing the decisions.

All the more reason to use energy then.

If you use this sign-difference count, low-energy noise may change the
count while being inaudible, yielding an incorrect phase value. After
that, distortion will be huge and the band won't be I/S encoded, but
it might if the phase value was otherwise.

In general, this algorithm seems fragile (ie: not robust in the
presence of noise).

Why not forego the optimization and optimize efficiency instead? (it's
the priority now, running time optimization should be done after
exploiting the technique to increase quality as much as posibble).

An alternative technique that may be better in that regard, then,
would be to measure distortion with both phases, and pick the phase
that yields the lowest distortion?

Give it a try.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] aacenc: move the generation of ff_aac_pow34sf_tab[]

[Claudio Freire]

This will need rebasing, the fixed tablegen got in recently

On Fri, Jul 17, 2015 at 6:20 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit moves the generation of ff_aac_pow34sf_tab[] out of the
 encoder and into the table generator. The original commit log for
 this table in 2011 actually mentions that it should be moved outside
 but this never happened.

 This is the first commit which cleans up the encoder a little.
 ---
  libavcodec/aac_tablegen.c  | 2 ++
  libavcodec/aac_tablegen.h  | 5 -
  libavcodec/aac_tablegen_decl.h | 2 ++
  libavcodec/aaccoder.c  | 1 +
  libavcodec/aacenc.c| 4 
  libavcodec/aacenc.h| 2 --
  6 files changed, 9 insertions(+), 7 deletions(-)

 diff --git a/libavcodec/aac_tablegen.c b/libavcodec/aac_tablegen.c
 index 33a179f..2d13211 100644
 --- a/libavcodec/aac_tablegen.c
 +++ b/libavcodec/aac_tablegen.c
 @@ -33,5 +33,7 @@ int main(void)

  WRITE_ARRAY(const, float, ff_aac_pow2sf_tab);

 +WRITE_ARRAY(const, float, ff_aac_pow34sf_tab);
 +
  return 0;
  }
 diff --git a/libavcodec/aac_tablegen.h b/libavcodec/aac_tablegen.h
 index bf71e59..8b223f9 100644
 --- a/libavcodec/aac_tablegen.h
 +++ b/libavcodec/aac_tablegen.h
 @@ -30,12 +30,15 @@
  #else
  #include libavutil/mathematics.h
  float ff_aac_pow2sf_tab[428];
 +float ff_aac_pow34sf_tab[428];

  av_cold void ff_aac_tableinit(void)
  {
  int i;
 -for (i = 0; i  428; i++)
 +for (i = 0; i  428; i++) {
  ff_aac_pow2sf_tab[i] = pow(2, (i - POW_SF2_ZERO) / 4.0);
 +ff_aac_pow34sf_tab[i] = pow(ff_aac_pow2sf_tab[i], 3.0/4.0);
 +}
  }
  #endif /* CONFIG_HARDCODED_TABLES */

 diff --git a/libavcodec/aac_tablegen_decl.h b/libavcodec/aac_tablegen_decl.h
 index 5105dae..ef86f85 100644
 --- a/libavcodec/aac_tablegen_decl.h
 +++ b/libavcodec/aac_tablegen_decl.h
 @@ -28,9 +28,11 @@
  #if CONFIG_HARDCODED_TABLES
  #define ff_aac_tableinit()
  extern const float ff_aac_pow2sf_tab[428];
 +extern const float ff_aac_pow34sf_tab[428];
  #else
  void ff_aac_tableinit(void);
  extern   float ff_aac_pow2sf_tab[428];
 +extern   float ff_aac_pow34sf_tab[428];
  #endif /* CONFIG_HARDCODED_TABLES */

  #endif /* AVCODEC_AAC_TABLEGEN_DECL_H */
 diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
 index 5bdba46..17b14d6 100644
 --- a/libavcodec/aaccoder.c
 +++ b/libavcodec/aaccoder.c
 @@ -39,6 +39,7 @@
  #include aac.h
  #include aacenc.h
  #include aactab.h
 +#include aac_tablegen_decl.h

  /** Frequency in Hz for lower limit of noise substitution **/
  #define NOISE_LOW_LIMIT 4500
 diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
 index f05f51b..a3c31de 100644
 --- a/libavcodec/aacenc.c
 +++ b/libavcodec/aacenc.c
 @@ -58,7 +58,6 @@
  av_log(avctx, AV_LOG_WARNING, __VA_ARGS__); \
  }

 -float ff_aac_pow34sf_tab[428];

  static const uint8_t swb_size_1024_96[] = {
  4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 8, 8, 8, 8,
 @@ -855,9 +854,6 @@ static av_cold int aac_encode_init(AVCodecContext *avctx)

  ff_aac_tableinit();

 -for (i = 0; i  428; i++)
 -ff_aac_pow34sf_tab[i] = sqrt(ff_aac_pow2sf_tab[i] * 
 sqrt(ff_aac_pow2sf_tab[i]));
 -
  avctx-initial_padding = 1024;
  ff_af_queue_init(avctx, s-afq);

 diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
 index 966c708..4210455 100644
 --- a/libavcodec/aacenc.h
 +++ b/libavcodec/aacenc.h
 @@ -95,8 +95,6 @@ typedef struct AACEncContext {
  } buffer;
  } AACEncContext;

 -extern float ff_aac_pow34sf_tab[428];
 -
  void ff_aac_coder_init_mips(AACEncContext *c);

  #endif /* AVCODEC_AACENC_H */
 --
 2.4.3.573.g4eafbef

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

Re: [FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Claudio Freire]

On Fri, Jul 17, 2015 at 8:42 PM, Michael Niedermayer
mich...@niedermayer.cc wrote:
 If you mean a transition in time, I don't think it makes any
 difference. 0.95 is a ~0.5db change in intensity, which ought to be
 inaudible, and windowing will already take care to make the transition
 smooth. And the logic wouldn't be completely free either to ramp
 gradually, as it would have to ramp fully to 0.95 by the time it
 reaches the first window marked as clipping hazard, and it could very
 well be the frist window.

 what i meant was that whatever condition is used to hard switch
 between 1.0 and 0.95 could be rather a soft transition that is
 for example
 insteda of
 if (x  0.0) f = 0.95

 something like
 if (x  0.0  x1.0) f = 1.0 + (0.95 - 1.0)*x

 so theres no discontinuity in the transition

Attached is a patch with something like that.

1. It measures per-window minimum clip avoidance factor
2. Computes a whole-frame factor (minimum of all)
3. Applies the clip avoidance factor to the whole frame (to make it
smooth and almost linear, and avoid adding harmonic distortion).
From 57522de7c5fcdbef222c2425a4add6fa4528f0e7 Mon Sep 17 00:00:00 2001
From: Claudio Freire klaussfre...@gmail.com
Date: Mon, 20 Jul 2015 22:53:24 -0300
Subject: [PATCH] AAC Encoder: clipping avoidance

Avoid clipping due to quantization noise to produce audible
artifacts, by detecting near-clipping signals and both attenuating
them a little and encoding escape-encoded bands (usually the
loudest) rounding towards zero instead of nearest, which tends to
decrease overall energy and thus clipping.

Currently fate tests measure numerical error so this change makes
tests using asynth (which are near clipping) report higher error
not less, because of window attenuation. Yet, they sound better,
not worse (albeit subtle, other samples aren't subtle at all).
Only measuring psychoacoustically weighted error would make for
a representative test, so that will be left for a future patch.
---
 libavcodec/aac.h  |   4 ++
 libavcodec/aaccoder.c | 108 --
 libavcodec/aacenc.c   |  38 +-
 libavcodec/aacenc.h   |   2 +-
 libavcodec/aacpsy.c   |  30 ++
 libavcodec/psymodel.h |   1 +
 tests/fate/aac.mak|   4 +-
 7 files changed, 145 insertions(+), 42 deletions(-)

diff --git a/libavcodec/aac.h b/libavcodec/aac.h
index d62455d..3e3e479 100644
--- a/libavcodec/aac.h
+++ b/libavcodec/aac.h
@@ -50,6 +50,8 @@
 #define TNS_MAX_ORDER 20
 #define MAX_LTP_LONG_SFB 40
 
+#define CLIP_AVOIDANCE_FACTOR 0.95f
+
 enum RawDataBlockType {
 TYPE_SCE,
 TYPE_CPE,
@@ -180,6 +182,8 @@ typedef struct IndividualChannelStream {
 int predictor_initialized;
 int predictor_reset_group;
 uint8_t prediction_used[41];
+uint8_t window_clipping[8]; /// set if a certain window is near clipping
+float clip_avoidance_factor; /// set if any window is near clipping to the necessary atennuation factor to avoid it
 } IndividualChannelStream;
 
 /**
diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 5bdba46..d606119 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -78,6 +78,9 @@ static const uint8_t * const run_value_bits[2] = {
 run_value_bits_long, run_value_bits_short
 };
 
+#define ROUND_STANDARD 0.4054f
+#define ROUND_TO_ZERO 0.1054f
+
 /** Map to convert values from BandCodingPath index to a codebook index **/
 static const uint8_t aac_cb_out_map[CB_TOT_ALL]  = {0,1,2,3,4,5,6,7,8,9,10,11,13,14,15};
 /** Inverse map to convert from codebooks to BandCodingPath indices **/
@@ -88,20 +91,20 @@ static const uint8_t aac_cb_in_map[CB_TOT_ALL+1] = {0,1,2,3,4,5,6,7,8,9,10,11,0,
  * @return absolute value of the quantized coefficient
  * @see 3GPP TS26.403 5.6.2 Scalefactor determination
  */
-static av_always_inline int quant(float coef, const float Q)
+static av_always_inline int quant(float coef, const float Q, const float rounding)
 {
 float a = coef * Q;
-return sqrtf(a * sqrtf(a)) + 0.4054;
+return sqrtf(a * sqrtf(a)) + rounding;
 }
 
 static void quantize_bands(int *out, const float *in, const float *scaled,
-   int size, float Q34, int is_signed, int maxval)
+   int size, float Q34, int is_signed, int maxval, const float rounding)
 {
 int i;
 double qc;
 for (i = 0; i  size; i++) {
 qc = scaled[i] * Q34;
-out[i] = (int)FFMIN(qc + 0.4054, (double)maxval);
+out[i] = (int)FFMIN(qc + rounding, (double)maxval);
 if (is_signed  in[i]  0.0f) {
 out[i] = -out[i];
 }
@@ -133,7 +136,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
 const float *scaled, int size, int scale_idx,
 int cb, const float lambda, const float uplim,
 int *bits, int BT_ZERO, int BT_UNSIGNED,
-int BT_PAIR, int

Re: [FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Claudio Freire]

On Mon, Jul 20, 2015 at 11:39 PM, Claudio Freire klaussfre...@gmail.com wrote:
 On Fri, Jul 17, 2015 at 8:42 PM, Michael Niedermayer
 mich...@niedermayer.cc wrote:
 If you mean a transition in time, I don't think it makes any
 difference. 0.95 is a ~0.5db change in intensity, which ought to be
 inaudible, and windowing will already take care to make the transition
 smooth. And the logic wouldn't be completely free either to ramp
 gradually, as it would have to ramp fully to 0.95 by the time it
 reaches the first window marked as clipping hazard, and it could very
 well be the frist window.

 what i meant was that whatever condition is used to hard switch
 between 1.0 and 0.95 could be rather a soft transition that is
 for example
 insteda of
 if (x  0.0) f = 0.95

 something like
 if (x  0.0  x1.0) f = 1.0 + (0.95 - 1.0)*x

 so theres no discontinuity in the transition

 Attached is a patch with something like that.

 1. It measures per-window minimum clip avoidance factor
 2. Computes a whole-frame factor (minimum of all)
 3. Applies the clip avoidance factor to the whole frame (to make it
 smooth and almost linear, and avoid adding harmonic distortion).


Sorry, that one had trailing whitespace again.

Attached.
From 57522de7c5fcdbef222c2425a4add6fa4528f0e7 Mon Sep 17 00:00:00 2001
From: Claudio Freire klaussfre...@gmail.com
Date: Mon, 20 Jul 2015 22:53:24 -0300
Subject: [PATCH] AAC Encoder: clipping avoidance

Avoid clipping due to quantization noise to produce audible
artifacts, by detecting near-clipping signals and both attenuating
them a little and encoding escape-encoded bands (usually the
loudest) rounding towards zero instead of nearest, which tends to
decrease overall energy and thus clipping.

Currently fate tests measure numerical error so this change makes
tests using asynth (which are near clipping) report higher error
not less, because of window attenuation. Yet, they sound better,
not worse (albeit subtle, other samples aren't subtle at all).
Only measuring psychoacoustically weighted error would make for
a representative test, so that will be left for a future patch.
---
 libavcodec/aac.h  |   4 ++
 libavcodec/aaccoder.c | 108 --
 libavcodec/aacenc.c   |  38 +-
 libavcodec/aacenc.h   |   2 +-
 libavcodec/aacpsy.c   |  30 ++
 libavcodec/psymodel.h |   1 +
 tests/fate/aac.mak|   4 +-
 7 files changed, 145 insertions(+), 42 deletions(-)

diff --git a/libavcodec/aac.h b/libavcodec/aac.h
index d62455d..3e3e479 100644
--- a/libavcodec/aac.h
+++ b/libavcodec/aac.h
@@ -50,6 +50,8 @@
 #define TNS_MAX_ORDER 20
 #define MAX_LTP_LONG_SFB 40
 
+#define CLIP_AVOIDANCE_FACTOR 0.95f
+
 enum RawDataBlockType {
 TYPE_SCE,
 TYPE_CPE,
@@ -180,6 +182,8 @@ typedef struct IndividualChannelStream {
 int predictor_initialized;
 int predictor_reset_group;
 uint8_t prediction_used[41];
+uint8_t window_clipping[8]; /// set if a certain window is near clipping
+float clip_avoidance_factor; /// set if any window is near clipping to the necessary atennuation factor to avoid it
 } IndividualChannelStream;
 
 /**
diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 5bdba46..d606119 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -78,6 +78,9 @@ static const uint8_t * const run_value_bits[2] = {
 run_value_bits_long, run_value_bits_short
 };
 
+#define ROUND_STANDARD 0.4054f
+#define ROUND_TO_ZERO 0.1054f
+
 /** Map to convert values from BandCodingPath index to a codebook index **/
 static const uint8_t aac_cb_out_map[CB_TOT_ALL]  = {0,1,2,3,4,5,6,7,8,9,10,11,13,14,15};
 /** Inverse map to convert from codebooks to BandCodingPath indices **/
@@ -88,20 +91,20 @@ static const uint8_t aac_cb_in_map[CB_TOT_ALL+1] = {0,1,2,3,4,5,6,7,8,9,10,11,0,
  * @return absolute value of the quantized coefficient
  * @see 3GPP TS26.403 5.6.2 Scalefactor determination
  */
-static av_always_inline int quant(float coef, const float Q)
+static av_always_inline int quant(float coef, const float Q, const float rounding)
 {
 float a = coef * Q;
-return sqrtf(a * sqrtf(a)) + 0.4054;
+return sqrtf(a * sqrtf(a)) + rounding;
 }
 
 static void quantize_bands(int *out, const float *in, const float *scaled,
-   int size, float Q34, int is_signed, int maxval)
+   int size, float Q34, int is_signed, int maxval, const float rounding)
 {
 int i;
 double qc;
 for (i = 0; i  size; i++) {
 qc = scaled[i] * Q34;
-out[i] = (int)FFMIN(qc + 0.4054, (double)maxval);
+out[i] = (int)FFMIN(qc + rounding, (double)maxval);
 if (is_signed  in[i]  0.0f) {
 out[i] = -out[i];
 }
@@ -133,7 +136,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
 const float *scaled, int size, int scale_idx,
 int cb, const float lambda

Re: [FFmpeg-devel] [PATCH 2/3] aacenc: move the generation of ff_aac_pow34sf_tab[]

[Claudio Freire]

On Mon, Jul 20, 2015 at 10:05 PM, Claudio Freire klaussfre...@gmail.com wrote:
 This will need rebasing, the fixed tablegen got in recently


 On Fri, Jul 17, 2015 at 6:20 PM, Rostislav Pehlivanov
 atomnu...@gmail.com wrote:
 This commit moves the generation of ff_aac_pow34sf_tab[] out of the
 encoder and into the table generator. The original commit log for
 this table in 2011 actually mentions that it should be moved outside
 but this never happened.

 This is the first commit which cleans up the encoder a little.
 ---
  libavcodec/aac_tablegen.c  | 2 ++
  libavcodec/aac_tablegen.h  | 5 -
  libavcodec/aac_tablegen_decl.h | 2 ++
  libavcodec/aaccoder.c  | 1 +
  libavcodec/aacenc.c| 4 
  libavcodec/aacenc.h| 2 --
  6 files changed, 9 insertions(+), 7 deletions(-)


Forget I said anything, the tablegen changes that got in don't conflict.

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

[FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Claudio Freire]

Avoid clipping due to quantization noise to produce audible
artifacts, by detecting near-clipping signals and both attenuating
them a little and encoding escape-encoded bands (usually the
loudest) rounding towards zero instead of nearest, which tends to
decrease overall energy and thus clipping.

Currently fate tests measure numerical error so this change makes
tests using asynth (which are near clipping) report higher error
not less, because of window attenuation. Yet, they sound better,
not worse (albeit subtle, other samples aren't subtle at all).
Only measuring psychoacoustically weighted error would make for
a representative test, so that will be left for a future patch.
From 9da94f02574b34025a56c225c11269802f49949b Mon Sep 17 00:00:00 2001
From: Claudio Freire klaussfre...@gmail.com
Date: Fri, 17 Jul 2015 05:47:25 -0300
Subject: [PATCH] AAC Encoder: clipping avoidance

Avoid clipping due to quantization noise to produce audible
artifacts, by detecting near-clipping signals and both attenuating
them a little and encoding escape-encoded bands (usually the
loudest) rounding towards zero instead of nearest, which tends to
decrease overall energy and thus clipping.

Currently fate tests measure numerical error so this change makes
tests using asynth (which are near clipping) report higher error
not less, because of window attenuation. Yet, they sound better,
not worse (albeit subtle, other samples aren't subtle at all).
Only measuring psychoacoustically weighted error would make for
a representative test, so that will be left for a future patch.
---
 libavcodec/aac.h  |   1 +
 libavcodec/aaccoder.c | 108 --
 libavcodec/aacenc.c   |  28 -
 libavcodec/aacenc.h   |   2 +-
 libavcodec/aacpsy.c   |  31 +++
 libavcodec/psymodel.h |   1 +
 tests/fate/aac.mak|   2 +-
 7 files changed, 132 insertions(+), 41 deletions(-)

diff --git a/libavcodec/aac.h b/libavcodec/aac.h
index f6fd446..7548897 100644
--- a/libavcodec/aac.h
+++ b/libavcodec/aac.h
@@ -230,6 +230,7 @@ typedef struct IndividualChannelStream {
 int predictor_initialized;
 int predictor_reset_group;
 uint8_t prediction_used[41];
+uint8_t window_clipping[8]; /// set if a certain window is near clipping
 } IndividualChannelStream;
 
 /**
diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 5bdba46..d606119 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -78,6 +78,9 @@ static const uint8_t * const run_value_bits[2] = {
 run_value_bits_long, run_value_bits_short
 };
 
+#define ROUND_STANDARD 0.4054f
+#define ROUND_TO_ZERO 0.1054f
+
 /** Map to convert values from BandCodingPath index to a codebook index **/
 static const uint8_t aac_cb_out_map[CB_TOT_ALL]  = {0,1,2,3,4,5,6,7,8,9,10,11,13,14,15};
 /** Inverse map to convert from codebooks to BandCodingPath indices **/
@@ -88,20 +91,20 @@ static const uint8_t aac_cb_in_map[CB_TOT_ALL+1] = {0,1,2,3,4,5,6,7,8,9,10,11,0,
  * @return absolute value of the quantized coefficient
  * @see 3GPP TS26.403 5.6.2 Scalefactor determination
  */
-static av_always_inline int quant(float coef, const float Q)
+static av_always_inline int quant(float coef, const float Q, const float rounding)
 {
 float a = coef * Q;
-return sqrtf(a * sqrtf(a)) + 0.4054;
+return sqrtf(a * sqrtf(a)) + rounding;
 }
 
 static void quantize_bands(int *out, const float *in, const float *scaled,
-   int size, float Q34, int is_signed, int maxval)
+   int size, float Q34, int is_signed, int maxval, const float rounding)
 {
 int i;
 double qc;
 for (i = 0; i  size; i++) {
 qc = scaled[i] * Q34;
-out[i] = (int)FFMIN(qc + 0.4054, (double)maxval);
+out[i] = (int)FFMIN(qc + rounding, (double)maxval);
 if (is_signed  in[i]  0.0f) {
 out[i] = -out[i];
 }
@@ -133,7 +136,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
 const float *scaled, int size, int scale_idx,
 int cb, const float lambda, const float uplim,
 int *bits, int BT_ZERO, int BT_UNSIGNED,
-int BT_PAIR, int BT_ESC, int BT_NOISE, int BT_STEREO)
+int BT_PAIR, int BT_ESC, int BT_NOISE, int BT_STEREO,
+const float ROUNDING)
 {
 const int q_idx = POW_SF2_ZERO - scale_idx + SCALE_ONE_POS - SCALE_DIV_512;
 const float Q   = ff_aac_pow2sf_tab [q_idx];
@@ -157,7 +161,7 @@ static av_always_inline float quantize_and_encode_band_cost_template(
 abs_pow34_v(s-scoefs, in, size);
 scaled = s-scoefs;
 }
-quantize_bands(s-qcoefs, in, scaled, size, Q34, !BT_UNSIGNED, aac_cb_maxval[cb]);
+quantize_bands(s-qcoefs, in, scaled, size, Q34, !BT_UNSIGNED, aac_cb_maxval[cb], ROUNDING);
 if (BT_UNSIGNED) {
 off

Re: [FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Claudio Freire]

On Fri, Jul 17, 2015 at 7:09 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
attachment.bin:357: trailing whitespace.
attachment.bin:436: trailing whitespace.
attachment.bin:454: trailing whitespace.

Oops

 You should probably configure your editor to strip that off automatically.

I don't like it to be done automatically since it could (and in
general would) modify parts of the code I didn't touch should they
contain whitespace.

I prefer doing it manually. I guess those slipped past me. I thought I
had taken care of patchcheck issues already.


sce-sf_idx[win*16+swb],
 Why the change from (win+w)*2? Shouldn't you remove the entire loop around
 w because now nothing depends on it?

AFAIR, it's not possible to select different scalefactors for the
different windows in the window group, and a lot of code assumes that
only the first window has the proper scalefactor set.

I'm not sure, but I'd rather err on the safe side. Indexing on sf_idx
should always index the first group to avoid inconsistencies.

The loop however isn't rendered pointless by that change, there's
still the scoefs pointer being advanced by w (and that's the whole
point of the loop in fact).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

[Claudio Freire]

On Fri, Jul 17, 2015 at 10:32 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 That previous idea discriminated way too many bands for it to be actually
 useful. And it would require special cases for coefficients which 'blow up'
 and have an insane value.
...
 What happened to the idea of comparing the energies of the addition
 and diferrence and deciding on that?

 It looked better at rejecting these cases than this one when we talked
 about it.

From my earlier testing, it's a bit too conservative, and if you make
it even more conservative, it may end up reducing the effectiveness of
I/S.

But even if not used for avoding I/S, it can be used to pick whether
to invert the phases, where it was clearly more stable.

 This method is naive but it handles spikes better since a single spike in
 one channel will only cause a single phase to switch among a reasonably
 sized number of coefficients summed over.

The spike will be very audible though, and it should be ok if it
dominates. I don't remember any case where it was mistaken when I was
testing.

Do you have a sample that exhibits this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

[Claudio Freire]

On Fri, Jul 17, 2015 at 6:20 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit adds a slightly more robust way of determining whether the phases
 match or are too different for IS to be used.
 ---
  libavcodec/aaccoder.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

 diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
 index 17b14d6..bd232f6 100644
 --- a/libavcodec/aaccoder.c
 +++ b/libavcodec/aaccoder.c
 @@ -56,6 +56,9 @@
  /** Frequency in Hz for lower limit of intensity stereo   **/
  #define INT_STEREO_LOW_LIMIT 6100

 +/** If less than this fraction of coeff phases agree disable IS for that 
 band **/
 +#define IS_PHASE_DIFF_LIM 0.11
 +
  /** Total number of usable codebooks **/
  #define CB_TOT 12

 @@ -1218,9 +1221,9 @@ static void search_for_is(AACEncContext *s, 
 AVCodecContext *avctx, ChannelElemen
  ener01 += (coef0 + coef1)*(coef0 + coef1);
  }
  }
 -if (!phase) { /* Too much phase difference between channels 
 */
 +if 
 (fabs(phase)/(sce0-ics.group_len[w]*sce0-ics.swb_sizes[g])  
 IS_PHASE_DIFF_LIM) {
  start += sce0-ics.swb_sizes[g];
 -continue;
 +continue; /* Too much phase difference */
  }
  phase = av_clip(phase, -1, 1);
  for (w2 = 0; w2  sce0-ics.group_len[w]; w2++) {

What happened to the idea of comparing the energies of the addition
and diferrence and deciding on that?

It looked better at rejecting these cases than this one when we talked about it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] AAC Encoder: clipping avoidance

[Claudio Freire]

On Fri, Jul 17, 2015 at 8:42 PM, Michael Niedermayer
mich...@niedermayer.cc wrote:
 If you mean a transition in time, I don't think it makes any
 difference. 0.95 is a ~0.5db change in intensity, which ought to be
 inaudible, and windowing will already take care to make the transition
 smooth. And the logic wouldn't be completely free either to ramp
 gradually, as it would have to ramp fully to 0.95 by the time it
 reaches the first window marked as clipping hazard, and it could very
 well be the frist window.

 what i meant was that whatever condition is used to hard switch
 between 1.0 and 0.95 could be rather a soft transition that is
 for example
 insteda of
 if (x  0.0) f = 0.95

 something like
 if (x  0.0  x1.0) f = 1.0 + (0.95 - 1.0)*x

 so theres no discontinuity in the transition

Could be done, but that would require a different structure in aacpsy.
Will take a look.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 2/8] aaccoder: remove previous PNS implementation from twoloop

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit undoes commit c5d4f87e8427c0952278ec247fa8ab1e6e52 and 
 removes PNS band marking from the twoloop coder, which has been reimplemented 
 in a better way in this series of patches.


LGTM as long as the rest of the patch set is committed as well
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 5/8] aacenc: use the new function for setting special band scalefactor indices

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit enables the function added with commit 7c10b87 and uses that new 
 function for setting any special scalefactor indices. This commit does not 
 change the behaviour of the encoder since no bands are being marked as either 
 NOISE_BT(due to the previous PNS implementation removed in the previous 
 commit) or INTENSITY_BT2/INTENSITY_BT.


LGTM in conjunction with other patches in the set
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 1/8] aacpsy: use a different metric for the spread of a band

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit modifies 02dbed6 to use band-active_lines to better gauge how 
 much information is contained within a single band and thus allow the 
 perceptual noise subsitution to more accurately determine which bands to code 
 as noise. The spread[w+g] used before this patch behaved more like a low-pass 
 filter for PNS band_types, which could mistakingly mark some low frequency 
 bands as noise.

 Reviewed-by: Claudio Freire klaussfre...@gmail.com
 ---
  libavcodec/aacpsy.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavcodec/aacpsy.c b/libavcodec/aacpsy.c
 index 78232d4..b16f6b9 100644
 --- a/libavcodec/aacpsy.c
 +++ b/libavcodec/aacpsy.c
 @@ -787,7 +787,7 @@ static void psy_3gpp_analyze_channel(FFPsyContext *ctx, 
 int channel,

  psy_band-threshold = band-thr;
  psy_band-energy= band-energy;
 -psy_band-spread= spread_en[w+g];
 +psy_band-spread= band-active_lines * 2.0f / band_sizes[g];
  }
  }


Can't say much since I proposed it myself, so... LGTM ;-)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 6/8] aaccoder: add a new perceptual noise substitution implementation

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit finalizes the PNS implementation previously added to the encoder 
 by moving it to a seperate function search_for_pns() and thus making it 
 coder-generic. This new implementation makes use of the spread field of the 
 psy bands and the lambda quality feedback paremeter. The spread of the 
 spectrum in a band prevents PNS from being used excessively and thus preserve 
 more phase information in high frequencies.  The lambda parameter allows the 
 number of PNS-marked bands to vary based on the lambda parameter and the 
 amount of bits available, making better choices on which bands are to be 
 marked as noise. Comparisons with the previous PNS implementation can be 
 found here: https://trac.ffmpeg.org/attachment/wiki/Encode/AAC/

 This is V2 of the patch, the changes from the previous version being that 
 this version uses the new band-spread metric from aacpsy and normalizes the 
 energy using the group size. These changes were suggested by Claudio Freire 
 on the mailing list. Another change is the use of lambda to alter the 
 frequency threshold. This change makes the actual threshold frequencies vary 
 between +-2Khz of what's specified, depending on frame encoding performance.


LGTM. I probably should mention I already thoroughly tested a WIP
version of the patch (which looks identical).

This depends on #5, which is also good if it's pushed at the same time
as other patches in the set (I already tested them as a whole). To
push #5 in isolation I would have to test it a bit, to see it doesn't
break anything. In any case it's not likely to break anything, but
better safe than sorry.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 7/8] aacenc: add support for coding of IS spectral coefficients

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit adds support for the coding of intensity stereo spectral 
 coefficients. It also fixes the Mid/Side coding of band_types higher than 
 RESERVED_BT (M/S must not be applied to their spectral coefficients, but 
 marking M/S as present in encode_ms_info() is okay). Much of the changes here 
 were taken from the decoder and inverted. This commit does not change the 
 functionality of the decoder as the previous patch in this series zeroes 
 ms_mask and is_mask.

 Reviewed-by: Claudio Freire klaussfre...@gmail.com


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

Re: [FFmpeg-devel] [FFmpeg-cvslog] aacenc: add support for coding of intensity stereo scalefactor indices

[Claudio Freire]

On Wed, Jul 1, 2015 at 2:02 PM, James Almer jamr...@gmail.com wrote:
 ffmpeg | branch: master | Rostislav Pehlivanov atomnuker at gmail.com | 
 Fri Jun 26 21:16:34 2015 +0100| [7c10b87b5744179f16411f5981e96738021ec7ca] | 
 committer: Michael Niedermayer

 aacenc: add support for coding of intensity stereo scalefactor indices

 This commit adds support for the coding of intensity stereo scalefactor 
 indices.
 It does not do any marking of such bands and as such does no functional 
 changes
 to the encoder. It removes any old twoloop specific code for PNS and moves it
 into a seperate function which handles setting of scalefactor indices for
 PNS and IS bands.

 Reviewed-by: Claudio Freire klaussfreire at gmail.com
 Signed-off-by: Michael Niedermayer michaelni at gmx.at

  http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=7c10b87b5744179f16411f5981e96738021ec7ca
 ---

  libavcodec/aaccoder.c |   37 +
  libavcodec/aacenc.c   |6 +-
  2 files changed, 42 insertions(+), 1 deletion(-)

 diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
 index 2f99924..cd996b2 100644
 --- a/libavcodec/aaccoder.c
 +++ b/libavcodec/aaccoder.c
 @@ -595,6 +595,43 @@ typedef struct TrellisPath {
  #define TRELLIS_STAGES 121
  #define TRELLIS_STATES (SCALE_MAX_DIFF+1)

 +static void set_special_band_scalefactors(AACEncContext *s, 
 SingleChannelElement *sce)

 This function is unused and being optimized out with a warning during 
 compilation.

A further patch makes use of it, but this is moot since a new set of
patches was pushed by Rostislav today, so lets take a look at those
instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 3/8] aaccoder: fix M/S coding

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 There were some mistakes in the code for M/S stereo, this commit fixes them. 
 The start variable was not being reset for every window and every access to 
 the coefficients was incorrect as well. This fixes that by properly 
 addressing the coefficients using both windows and setting the start on every 
 window to zero.
 ---
  libavcodec/aaccoder.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

 diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
 index 3fcc8b4..33cbe7b 100644
 --- a/libavcodec/aaccoder.c
 +++ b/libavcodec/aaccoder.c
 @@ -1143,6 +1143,7 @@ static void search_for_ms(AACEncContext *s, 
 ChannelElement *cpe,
  if (!cpe-common_window)
  return;
  for (w = 0; w  sce0-ics.num_windows; w += sce0-ics.group_len[w]) {
 +start = 0;
  for (g = 0;  g  sce0-ics.num_swb; g++) {
  if (!cpe-ch[0].zeroes[w*16+g]  !cpe-ch[1].zeroes[w*16+g]) {
  float dist1 = 0.0f, dist2 = 0.0f;
 @@ -1152,22 +1153,22 @@ static void search_for_ms(AACEncContext *s, 
 ChannelElement *cpe,
  float minthr = FFMIN(band0-threshold, band1-threshold);
  float maxthr = FFMAX(band0-threshold, band1-threshold);
  for (i = 0; i  sce0-ics.swb_sizes[g]; i++) {
 -M[i] = (sce0-pcoeffs[start+w2*128+i]
 -  + sce1-pcoeffs[start+w2*128+i]) * 0.5;
 +M[i] = (sce0-pcoeffs[start+(w+w2)*128+i]
 +  + sce1-pcoeffs[start+(w+w2)*128+i]) * 0.5;
  S[i] =  M[i]
 -  - sce1-pcoeffs[start+w2*128+i];
 +  - sce1-pcoeffs[start+(w+w2)*128+i];
  }
 -abs_pow34_v(L34, sce0-coeffs+start+w2*128, 
 sce0-ics.swb_sizes[g]);
 -abs_pow34_v(R34, sce1-coeffs+start+w2*128, 
 sce0-ics.swb_sizes[g]);
 +abs_pow34_v(L34, sce0-coeffs+start+(w+w2)*128, 
 sce0-ics.swb_sizes[g]);
 +abs_pow34_v(R34, sce1-coeffs+start+(w+w2)*128, 
 sce0-ics.swb_sizes[g]);
  abs_pow34_v(M34, M, 
 sce0-ics.swb_sizes[g]);
  abs_pow34_v(S34, S, 
 sce0-ics.swb_sizes[g]);
 -dist1 += quantize_band_cost(s, sce0-coeffs + start + 
 w2*128,
 +dist1 += quantize_band_cost(s, sce0-coeffs + start + 
 (w+w2)*128,
  L34,
  sce0-ics.swb_sizes[g],
  sce0-sf_idx[(w+w2)*16+g],
  sce0-band_type[(w+w2)*16+g],
  lambda / band0-threshold, 
 INFINITY, NULL);
 -dist1 += quantize_band_cost(s, sce1-coeffs + start + 
 w2*128,
 +dist1 += quantize_band_cost(s, sce1-coeffs + start + 
 (w+w2)*128,
  R34,
  sce1-ics.swb_sizes[g],
  sce1-sf_idx[(w+w2)*16+g],


LGTM.

Though I should probably mention that this patch shouldn't make any
difference in the current state of search_for_ms.

Unless there's an error somewhere (which is common in my experience
with the aac encoder, so the proposed code is more robust in that
regard and should be pushed), the current code will result in the same
behavior exactly.

However, that's not always the case in other cases with the same
coding pattern is used, usually in search_for_X functions, where start
is used to compute the frequency that corresponds to a particular
coefficient. In those uses, if start isn't based at 0 for any given
window, the frequency will be wrongly computed.

So I'd push this patch if only to keep things consistent, and to avoid
similar errors in the future (I've corrected tons of instances of this
in #2686).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 8/8] aacenc: implement Intensity Stereo encoding support

[Claudio Freire]

On Thu, Jul 2, 2015 at 3:13 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit implements intensity stereo coding support to the native aac 
 encoder. This is a way to increase the efficiency of the encoder by zeroing 
 the right channel's spectral coefficients (in a channel pair) and rederiving 
 them in the decoder using information from the scalefactor indices of special 
 band types. This commit confomrs to the official ISO 13818-7 specifications, 
 although due to their ambiguity certain deviations have been taken to ensure 
 maximum sound quality. This commit has been extensively tested and has shown 
 to not result in audiable audio artifacts unless in extreme cases. This 
 commit also adds an option, aac_is, which has the value of 0 by default. 
 Intensity Stereo is part of the scalable aac profile and is thus non-default.

 The way IS coding works is that it rederives the right channel's spectral 
 coefficients from the left channel via the scalefactor index values left in 
 the right channel. Since an entire band's spectral coefficients do not need 
 to be coded, the encoder's efficiency jumps up and it unzeroes some high 
 frequency values which it previously did not have enough bits to encode. That 
 way less information is lost than the information lost by rederiving the 
 spectral coefficients with some error. This is why the filesize of files 
 encoded with IS do not decrease significantly. Users wishing that IS coding 
 should reduce filesize are expected to reduce their encoding bitrates 
 appropriately.

 This is V2 of the commit. The old version did not mark ms_mask as 0 since M/S 
 and IS coding are incompactible, which resulted in distortions with M/S 
 coding enabled. This version also improves phase detection by measuring it 
 for every spectral coefficient in the band and using a simple majority rule 
 to determine whether the coefficients are in or out of phase. Also, the 
 energy values per spectral coefficient were changed as to reflect the 
 official specifications.


This one also looks identical to a WIP I thoroughly tested, so I
believe this means the whole set is good to be pushed, if a committer
agrees.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 07/11] aacenc: add support for coding of IS spectral coefficients

[Claudio Freire]

LGTM, but it cannot be committed without the others, or a memset to
zero of is_mask somewhere at the beginning of each frame (or encoder
init).

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit adds support for the coding of intensity stereo spectral 
 coefficients. It also fixes the Mid/Side coding of band_types higher than 
 RESERVED_BT (M/S must not be applied to their spectral coefficients, but 
 marking M/S as present in encode_ms_info() is okay). Much of the changes here 
 were taken from the decoder and inverted. This commit does not change the 
 functionality of the decoder.
 ---
  libavcodec/aacenc.c | 39 +--
  1 file changed, 29 insertions(+), 10 deletions(-)

 diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
 index 3a512ff..a2ff04f 100644
 --- a/libavcodec/aacenc.c
 +++ b/libavcodec/aacenc.c
 @@ -312,19 +312,28 @@ static void encode_ms_info(PutBitContext *pb, 
 ChannelElement *cpe)
  static void adjust_frame_information(ChannelElement *cpe, int chans)
  {
  int i, w, w2, g, ch;
 -int start, maxsfb, cmaxsfb;
 +int maxsfb, cmaxsfb;
 +IndividualChannelStream *ics;

 -for (ch = 0; ch  chans; ch++) {
 -IndividualChannelStream *ics = cpe-ch[ch].ics;
 -start = 0;
 -maxsfb = 0;
 -cpe-ch[ch].pulse.num_pulse = 0;
 +if (cpe-common_window) {
 +ics = cpe-ch[0].ics;
  for (w = 0; w  ics-num_windows; w += ics-group_len[w]) {
 -for (w2 = 0; w2  ics-group_len[w]; w2++) {
 -start = (w+w2) * 128;
 +for (w2 =  0; w2  ics-group_len[w]; w2++) {
 +int start = (w+w2) * 128;
  for (g = 0; g  ics-num_swb; g++) {
 -//apply M/S
 -if (cpe-common_window  !ch  cpe-ms_mask[w*16 + g]) 
 {
 +//apply Intensity stereo coeffs transformation
 +if (cpe-is_mask[w*16 + g]) {
 +int p = -1 + 2 * (cpe-ch[1].band_type[w*16+g] - 14);
 +float scale = 
 sqrtf(cpe-ch[0].is_ener[w*16+g]*cpe-ch[1].is_ener[w*16+g]);
 +if (cpe-ms_mask[w*16 + g])
 +p *= 1 - 2 * cpe-ms_mask[w*16 + g];
 +for (i = 0; i  ics-swb_sizes[g]; i++) {
 +cpe-ch[0].coeffs[start+i] = 
 (cpe-ch[0].pcoeffs[start+i] + p*cpe-ch[1].pcoeffs[start+i]) * scale;
 +cpe-ch[1].coeffs[start+i] = 0.0f;
 +}
 +} else if (cpe-ms_mask[w*16 + g] 
 +   cpe-ch[0].band_type[w*16 + g]  NOISE_BT 
 +   cpe-ch[1].band_type[w*16 + g]  NOISE_BT) {
  for (i = 0; i  ics-swb_sizes[g]; i++) {
  cpe-ch[0].coeffs[start+i] = 
 (cpe-ch[0].pcoeffs[start+i] + cpe-ch[1].pcoeffs[start+i]) * 0.5f;
  cpe-ch[1].coeffs[start+i] = 
 cpe-ch[0].coeffs[start+i] - cpe-ch[1].pcoeffs[start+i];
 @@ -332,6 +341,16 @@ static void adjust_frame_information(ChannelElement 
 *cpe, int chans)
  }
  start += ics-swb_sizes[g];
  }
 +}
 +}
 +}
 +
 +for (ch = 0; ch  chans; ch++) {
 +IndividualChannelStream *ics = cpe-ch[ch].ics;
 +maxsfb = 0;
 +cpe-ch[ch].pulse.num_pulse = 0;
 +for (w = 0; w  ics-num_windows; w += ics-group_len[w]) {
 +for (w2 =  0; w2  ics-group_len[w]; w2++) {
  for (cmaxsfb = ics-num_swb; cmaxsfb  0  
 cpe-ch[ch].zeroes[w*16+cmaxsfb-1]; cmaxsfb--)
  ;
  maxsfb = FFMAX(maxsfb, cmaxsfb);
 --
 2.1.4

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

Re: [FFmpeg-devel] [PATCH 09/11] aaccoder: add a new perceptual noise substitution implementation

[Claudio Freire]

On Mon, Jun 29, 2015 at 10:58 PM, Claudio Freire klaussfre...@gmail.com wrote:
 On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
 atomnu...@gmail.com wrote:
 +if (spread  NOISE_SPREAD_THRESHOLD 
 +((sce-zeroes[w*16+g]  energy = threshold) ||
 +energy  threshold*(NOISE_LAMBDA_NUMERATOR/lambda))) {
 +sce-band_type[w*16+g] = NOISE_BT;
 +sce-pns_ener[w*16+g] = energy;
 +sce-zeroes[w*16+g] = 0;
 +}


 This should be:
 sce-pns_ener[w*16+g] = energy / sce-ics.group_len[w];

 Also, spread  NOISE_SPREAD_THRESHOLD works beetter as:

 spread  (NOISE_SPREAD_THRESHOLD * 120.f / lambda)

 And  energy  threshold*(NOISE_LAMBDA_NUMERATOR/lambda) as:

 energy  threshold*(NOISE_LAMBDA_NUMERATOR*sce-ics.group_len[w]/lambda))

 (to account for the transient situation in which the encoder is
 switching to and form short windows)


With the new spread measure in patch #06, if committed, in this one
NOISE_SPREAD_THRESHOLD should be redefined to 0.5 (the active_lines
measure is normalized).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 10/11] aaccoder: implement intensity stereo

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 +if (dist2 = dist1) {
 +cpe-is_mask[w*16+g] = 1;
 +cpe-ch[0].is_ener[w*16+g] = ener1/ener01;
 +cpe-ch[1].is_ener[w*16+g] = ener0/ener1;
 +if (s_coef0*s_coef1 = 0.0f)
 +cpe-ch[1].band_type[w*16+g] = INTENSITY_BT;
 +else
 +cpe-ch[1].band_type[w*16+g] = INTENSITY_BT2;
 +count++;
 +}


If you don't add an:

cpe-ms_mask[w*16+g] = 0;

In there, you get horrible artifacts. Tested it quite a bit already.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 06/11] aacpsy: Add energy spread for each band

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 +++ b/libavcodec/aacpsy.c
 @@ -781,6 +781,7 @@ static void psy_3gpp_analyze_channel(FFPsyContext *ctx, 
 int channel,

  psy_band-threshold = band-thr;
  psy_band-energy= band-energy;
 +psy_band-spread= spread_en[w+g];
  }
  }



Sorry, but I just noticed while reviewing the other patches.

spread_en isn't as good a proxy for band tonality as we'd hoped, but
band-active_lines works (adjusting the rest of the patch set).

So it should read like:

 +++ b/libavcodec/aacpsy.c
 @@ -781,6 +781,7 @@ static void psy_3gpp_analyze_channel(FFPsyContext *ctx, 
 int channel,

  psy_band-threshold = band-thr;
  psy_band-energy= band-energy;
 +psy_band-spread= band-active_lines;
  }
  }


The following patch over head (well, git diff) does that:

diff --git a/libavcodec/aacpsy.c b/libavcodec/aacpsy.c
index 78232d4..ca21664 100644
--- a/libavcodec/aacpsy.c
+++ b/libavcodec/aacpsy.c
@@ -787,7 +787,7 @@ static void psy_3gpp_analyze_channel(FFPsyContext
*ctx, int channel,

 psy_band-threshold = band-thr;
 psy_band-energy= band-energy;
-psy_band-spread= spread_en[w+g];
+psy_band-spread= band-active_lines * 2.0f / band_sizes[g];
 }
 }

The rest of the patch set will need to adapt to this other spread
measure, I'll comment on the relevant patches.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 09/11] aaccoder: add a new perceptual noise substitution implementation

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 +if (spread  NOISE_SPREAD_THRESHOLD 
 +((sce-zeroes[w*16+g]  energy = threshold) ||
 +energy  threshold*(NOISE_LAMBDA_NUMERATOR/lambda))) {
 +sce-band_type[w*16+g] = NOISE_BT;
 +sce-pns_ener[w*16+g] = energy;
 +sce-zeroes[w*16+g] = 0;
 +}


This should be:
sce-pns_ener[w*16+g] = energy / sce-ics.group_len[w];

Also, spread  NOISE_SPREAD_THRESHOLD works beetter as:

spread  (NOISE_SPREAD_THRESHOLD * 120.f / lambda)

And  energy  threshold*(NOISE_LAMBDA_NUMERATOR/lambda) as:

energy  threshold*(NOISE_LAMBDA_NUMERATOR*sce-ics.group_len[w]/lambda))

(to account for the transient situation in which the encoder is
switching to and form short windows)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 04/11] aaccoder: add intensity stereo support to encode_window_bands_info quantizer

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit adds support for both PNS and IS (intensity stereo) codebooks to 
 the encode_window_bands_info() quantizer, used by the faast, faac and anmr 
 non-default, native coders. This does not mean that both extensions now work 
 with those coders, some are simply unsuited and will trigger an assertion in 
 the encoder while others simply ignore the changed scalefactor indices and 
 band types. This commit simply adds support for encoding said band types with 
 the alternative coders. Future commits to the coders will be required to make 
 them suitable.

Did some testing, and the assertion failures happen in HEAD as well
(there's already a ticket for that but I just wanted to confirm that
it was not asserting more often at least).

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

Re: [FFmpeg-devel] [PATCH 11/11] aaccoder: zero ms_mask bands on execution

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 +memset(cpe-ms_mask, 0, sizeof(uint8_t)*128);

sizeof(cpe-ms_mask) would be safer
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 05/11] aacenc: add support for coding of intensity stereo scalefactor indices

[Claudio Freire]

Seems straightforward enough. LGTM.

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit adds support for the coding of intensity stereo scalefactor 
 indices. It does not do any marking of such bands and as such does no 
 functional changes to the encoder. It removes any old twoloop specific code 
 for PNS and moves it into a seperate function which handles setting of 
 scalefactor indices for PNS and IS bands.
 ---
  libavcodec/aaccoder.c | 37 +
  libavcodec/aacenc.c   |  6 +-
  2 files changed, 42 insertions(+), 1 deletion(-)

 diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
 index 3c90d27..02ffe17 100644
 --- a/libavcodec/aaccoder.c
 +++ b/libavcodec/aaccoder.c
 @@ -607,6 +607,43 @@ typedef struct TrellisPath {
  #define TRELLIS_STAGES 121
  #define TRELLIS_STATES (SCALE_MAX_DIFF+1)

 +static void set_special_band_scalefactors(AACEncContext *s, 
 SingleChannelElement *sce)
 +{
 +int w, g, start = 0;
 +int minscaler_n = sce-sf_idx[0], minscaler_i = sce-sf_idx[0];
 +int bands = 0;
 +
 +for (w = 0; w  sce-ics.num_windows; w += sce-ics.group_len[w]) {
 +start = 0;
 +for (g = 0;  g  sce-ics.num_swb; g++) {
 +if (sce-band_type[w*16+g] == INTENSITY_BT || 
 sce-band_type[w*16+g] == INTENSITY_BT2) {
 +sce-sf_idx[w*16+g] = 
 av_clip(ceilf(log2f(sce-is_ener[w*16+g])*2), -155, 100);
 +minscaler_i = FFMIN(minscaler_i, sce-sf_idx[w*16+g]);
 +bands++;
 +} else if (sce-band_type[w*16+g] == NOISE_BT) {
 +sce-sf_idx[w*16+g] = 
 av_clip(4+log2f(sce-pns_ener[w*16+g])*2, -100, 155);
 +minscaler_n = FFMIN(minscaler_n, sce-sf_idx[w*16+g]);
 +bands++;
 +}
 +start += sce-ics.swb_sizes[g];
 +}
 +}
 +
 +if (!bands)
 +return;
 +
 +/* Clip the scalefactor indices */
 +for (w = 0; w  sce-ics.num_windows; w += sce-ics.group_len[w]) {
 +for (g = 0;  g  sce-ics.num_swb; g++) {
 +if (sce-band_type[w*16+g] == INTENSITY_BT || 
 sce-band_type[w*16+g] == INTENSITY_BT2) {
 +sce-sf_idx[w*16+g] = av_clip(sce-sf_idx[w*16+g], 
 minscaler_i, minscaler_i + SCALE_MAX_DIFF);
 +} else if (sce-band_type[w*16+g] == NOISE_BT) {
 +sce-sf_idx[w*16+g] = av_clip(sce-sf_idx[w*16+g], 
 minscaler_n, minscaler_n + SCALE_MAX_DIFF);
 +}
 +}
 +}
 +}
 +
  static void search_for_quantizers_anmr(AVCodecContext *avctx, AACEncContext 
 *s,
 SingleChannelElement *sce,
 const float lambda)
 diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
 index 897c3a1..3a512ff 100644
 --- a/libavcodec/aacenc.c
 +++ b/libavcodec/aacenc.c
 @@ -389,7 +389,7 @@ static void encode_scale_factors(AVCodecContext *avctx, 
 AACEncContext *s,
   SingleChannelElement *sce)
  {
  int diff, off_sf = sce-sf_idx[0], off_pns = sce-sf_idx[0] - 
 NOISE_OFFSET;
 -int noise_flag = 1;
 +int off_is = 0, noise_flag = 1;
  int i, w;

  for (w = 0; w  sce-ics.num_windows; w += sce-ics.group_len[w]) {
 @@ -402,6 +402,10 @@ static void encode_scale_factors(AVCodecContext *avctx, 
 AACEncContext *s,
  put_bits(s-pb, NOISE_PRE_BITS, diff + NOISE_PRE);
  continue;
  }
 +} else if (sce-band_type[w*16 + i] == INTENSITY_BT  ||
 +   sce-band_type[w*16 + i] == INTENSITY_BT2) {
 +diff = sce-sf_idx[w*16 + i] - off_is;
 +off_is = sce-sf_idx[w*16 + i];
  } else {
  diff = sce-sf_idx[w*16 + i] - off_sf;
  off_sf = sce-sf_idx[w*16 + i];
 --
 2.1.4

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

Re: [FFmpeg-devel] [PATCH 02/11] aaccoder: remove previous PNS implementation from twoloop

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 This commit essentially undoes commit 
 c5d4f87e8427c0952278ec247fa8ab1e6e52 and removes PNS band marking from 
 the twoloop coder.


LGTM, but I wouldn't apply it before #09
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 03/11] aaccoder: add intensity stereo coding support for the trellis quantizer

[Claudio Freire]

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 +/* Energy spread threshold value below which no PNS is used, this 
 corresponds to
 + * typically around 17Khz, after which PNS usage decays ending at 19Khz */
 +#define NOISE_SPREAD_THRESHOLD 152234544.0f
 +
 +/* Above ~1.26*threshold all normally-zeroed values are PNS'd. Lambda divides
 + * the defined value below as to try to get a ~1.26 multiplier so that there 
 is
 + * a balance between noise and zero bands leaving more bits for actual 
 signal */
 +#define NOISE_LAMBDA_NUMERATOR 252.1f

This should go to #09 shouldn't it?

 +
 +/** Frequency in Hz for lower limit of intensity stereo   **/
 +#define INT_STEREO_LOW_LIMIT 6000

And that to #10.

Or wherever it's used first, I'd say.

Other than that, it looks good.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 06/11] aacpsy: Add energy spread for each band

[Claudio Freire]

LGTM

On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
atomnu...@gmail.com wrote:
 -float distortion;
 -float perceptual_weight;

Those are in fact in disuse. Thought I'd clarify.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [libav-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

[Claudio Freire]

On Mon, Apr 20, 2015 at 8:32 PM, Andreas Cadhalpun
andreas.cadhal...@googlemail.com wrote:
 The long version:

 ath should approximate the shape of the absolute hearing threshold, so
 yes, it's best if it really uses the minimum, since that will prevent
 clipping of the ath curve and result in a more accurate threshold
 computation.

 So you agree with my patch fixing minath?
 Or would you prefer a version with:
 minath = ath(3410 - 0.733 * ATH_ADD, ATH_ADD)

 Well, that's not really closer to the minimum (a few tests with gnuplot say).

 Are you sure your plots were done correctly?
 Because I'm quite sure this is the correct first order approximation
 of the minimum.

 For ATH_ADD = 4 this gives 3407.068, which is quite close to Michael's value
 (3407.080774800152).

I checked the formula several times, but still, I could have made a mistake.


 But yeah, something that makes it closer, sure.

 I'd try to symbollicaly deriving the minimum if you really mean to
 improve that (should be doable, it's all differentiable stuff).

 I actually derived the dependency of the minimum on ATH_ADD in first
 order around ATH_ADD = 0 (using sagemath).

ATH_ADD is defined as 4, that may make a significant difference
(changes a high-order term).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel