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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160330/0641f2bd/attachment.sig>