| Message ID | 20210528201551.16703-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | a7a7f32c8ad0179a1a85d0a8cff35924e6d90be8 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/aacenc: Avoid 0 lambda | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Fri, May 28, 2021 at 10:15:51PM +0200, Michael Niedermayer wrote: > Fixes: Ticket8003 > Fixes: CVE-2020-20453 > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/aacenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) will apply [...]
Quoting Michael Niedermayer (2021-05-28 22:15:51) > Fixes: Ticket8003 > Fixes: CVE-2020-20453 > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/aacenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c > index aa223cf25f..e80591ba86 100644 > --- a/libavcodec/aacenc.c > +++ b/libavcodec/aacenc.c > @@ -28,6 +28,7 @@ > * TODOs: > * add sane pulse detection > ***********************************/ > +#include <float.h> > > #include "libavutil/libm.h" > #include "libavutil/float_dsp.h" > @@ -852,7 +853,7 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > /* Not so fast though */ > ratio = sqrtf(ratio); > } > - s->lambda = FFMIN(s->lambda * ratio, 65536.f); > + s->lambda = av_clipf(s->lambda * ratio, FLT_MIN, 65536.f); Would FLT_EPSILON not be more appropriate? IIUC FLT_MIN is still effectively zero.
On Mon, May 31, 2021 at 09:22:09AM +0200, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-05-28 22:15:51) > > Fixes: Ticket8003 > > Fixes: CVE-2020-20453 > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/aacenc.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c > > index aa223cf25f..e80591ba86 100644 > > --- a/libavcodec/aacenc.c > > +++ b/libavcodec/aacenc.c > > @@ -28,6 +28,7 @@ > > * TODOs: > > * add sane pulse detection > > ***********************************/ > > +#include <float.h> > > > > #include "libavutil/libm.h" > > #include "libavutil/float_dsp.h" > > @@ -852,7 +853,7 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > > /* Not so fast though */ > > ratio = sqrtf(ratio); > > } > > - s->lambda = FFMIN(s->lambda * ratio, 65536.f); > > + s->lambda = av_clipf(s->lambda * ratio, FLT_MIN, 65536.f); > > Would FLT_EPSILON not be more appropriate? IIUC FLT_MIN is still > effectively zero. yes, i was just trying to eliminate the x/0. In theory, lambda = 0 or lambda = infinity is not semantically wrong, one would mean smallest file disregarding distortion the other lowest distortion disregarding size. I do not know what the intend of the author of the original code was But the places where divisions by lambda seem to happen semm all followed by cliping so i guess the minimum doesnt matter and all reasonable options will give the same result more or less. ill change it to FLT_EPSILON thx [...]
diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c index aa223cf25f..e80591ba86 100644 --- a/libavcodec/aacenc.c +++ b/libavcodec/aacenc.c @@ -28,6 +28,7 @@ * TODOs: * add sane pulse detection ***********************************/ +#include <float.h> #include "libavutil/libm.h" #include "libavutil/float_dsp.h" @@ -852,7 +853,7 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, /* Not so fast though */ ratio = sqrtf(ratio); } - s->lambda = FFMIN(s->lambda * ratio, 65536.f); + s->lambda = av_clipf(s->lambda * ratio, FLT_MIN, 65536.f); /* Keep iterating if we must reduce and lambda is in the sky */ if (ratio > 0.9f && ratio < 1.1f) {
Fixes: Ticket8003 Fixes: CVE-2020-20453 Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/aacenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)