[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 <klaussfreire at gmail.com> wrote:
> On Wed, Mar 30, 2016 at 8:33 AM, Michael Niedermayer
> <michael at 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 <klaussfreire at gmail.com> wrote:
>>> > On Tue, Mar 29, 2016 at 10:51 PM, Michael Niedermayer
>>> > <michael at 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 <michael at 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.