[FFmpeg-devel] [FFmpeg-cvslog] avcodec/aacenc: Check for +-Inf too

[Ganesh Ajjanagadde]

On Wed, Jan 13, 2016 at 6:28 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> On Thu, Jan 14, 2016 at 12:24 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>> On Wed, Jan 13, 2016 at 6:16 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>>> On Wed, Jan 13, 2016 at 06:03:32PM -0500, Ganesh Ajjanagadde wrote:
>>>> On Wed, Jan 13, 2016 at 5:53 PM, Michael Niedermayer <git at videolan.org> wrote:
>>>> > ffmpeg | branch: master | Michael Niedermayer <michael at niedermayer.cc> | Wed Jan 13 22:33:59 2016 +0100| [92465a2347d959cbd9864b017a39b2a4ab9313ff] | committer: Michael Niedermayer
>>>> >
>>>> > avcodec/aacenc: Check for +-Inf too
>>>> >
>>>> > Fixes out of array read
>>>> > Fixes: 04442da73d935b776d2236282588d4f9/signal_sigsegv_2625a69_8790_ae85ffc889070663319b3417ede777b0.mov
>>>> >
>>>> > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>>>> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>>> >
>>>> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=92465a2347d959cbd9864b017a39b2a4ab9313ff
>>>> > ---
>>>> >
>>>> >  libavcodec/aacenc.c |   18 +++++++++---------
>>>> >  1 file changed, 9 insertions(+), 9 deletions(-)
>>>> >
>>>> > diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
>>>> > index 2a3fc6e..9a7d3a8 100644
>>>> > --- a/libavcodec/aacenc.c
>>>> > +++ b/libavcodec/aacenc.c
>>>> > @@ -606,16 +606,16 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>>>> >                  s->mdct1024.mdct_calc(&s->mdct1024, sce->lcoeffs, sce->ret_buf);
>>>> >              }
>>>> >
>>>> > -            if (isnan(cpe->ch->coeffs[0]) ||
>>>> > -                isnan(cpe->ch->coeffs[  128]) ||
>>>> > -                isnan(cpe->ch->coeffs[2*128]) ||
>>>> > -                isnan(cpe->ch->coeffs[3*128]) ||
>>>> > -                isnan(cpe->ch->coeffs[4*128]) ||
>>>> > -                isnan(cpe->ch->coeffs[5*128]) ||
>>>> > -                isnan(cpe->ch->coeffs[6*128]) ||
>>>> > -                isnan(cpe->ch->coeffs[7*128])
>>>> > +            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])
>>>>
>>>> A cleaner solution would be to do a !isfinite. As usual, msvc stops
>>>> the whole show, so someone can add a trivial fallback, e.g isfinite =
>>>> !(isnan || isinf).
>>>
>>> yes, i didnt want to risk breakig build so went for the known to be
>>> working function. Also i dont want to complicate security fixes
>>> as they might always need backporting
>>
>> good point
>>
>>>
>>> Feel free to change this to isfinite in master if you prefer, its no
>>> problem for master to have a more complex solution ...
>>>
>>
>> I don't want to risk a direct commit either; it would be nice if
>> Hendrik (or someone with missing isfinite) could test.
>
> isfinite is only available in recent MSVC versions, older ones which
> we still support do not have it, so a configure check and compat
> wrapper in libm.h is needed.

Thanks for clarifying the status. What I meant was that although I can
create such a patch, I would prefer at least a build test by you
before committing to master ideally.

Submitted patch just now to bring in isfinite support.

>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel