[FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation
From: Michael NiedermayerThis also simplifies the code the resulting values are binary identical to what pow(10, i/10.0) produces --- libavcodec/on2avc.c | 37 - 1 file changed, 4 insertions(+), 33 deletions(-) diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c index 3309d99..e6ccd88 100644 --- a/libavcodec/on2avc.c +++ b/libavcodec/on2avc.c @@ -912,23 +912,7 @@ static av_cold void on2avc_free_vlcs(On2AVCContext *c) static av_cold int on2avc_decode_init(AVCodecContext *avctx) { On2AVCContext *c = avctx->priv_data; -int i, ph; -/* 10^(i*0.1) for 0 <= i < 10 */ -/* TODO: possibly statically allocate scale_tab; this may help with FATE - * and reproducibility if the binary size is not impacted much */ -static const double exp10_lut[] = { -1, -1.2589254117941673, -1.5848931924611136, -1.9952623149688795, -2.5118864315095806, -3.1622776601683795, -3.9810717055349727, -5.0118723362727229, -6.3095734448019334, -7.9432823472428158, -}; -int64_t exp10_base = 10; +int i; if (avctx->channels > 2U) { avpriv_request_sample(avctx, "Decoding more than 2 channels"); @@ -950,23 +934,10 @@ static av_cold int on2avc_decode_init(AVCodecContext *avctx) av_log(avctx, AV_LOG_WARNING, "Stereo mode support is not good, patch is welcome\n"); - -/* Fast and more accurate way of doing for (i = 0; i < 20; i++) -c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16) / 32; +for (i = 0; i < 20; i++) +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32; for (; i < 128; i++) -c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5); */ -for (i = 0; i < 10; i++) { -c->scale_tab[i] = ceil(exp10_lut[i] * 16) / 32; -c->scale_tab[i+10] = ceil(exp10_lut[i] * 160) / 32; -} - -for (i = 20, ph = 0; i < 128; i++, ph++) { -if (i % 10 == 0) { -exp10_base *= 10; -ph = 0; -} -c->scale_tab[i] = ceil(exp10_base * exp10_lut[ph] * 0.5); -} +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01); if (avctx->sample_rate < 32000 || avctx->channels == 1) memcpy(c->long_win, ff_on2avc_window_long_24000, -- 1.7.9.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation
On Sun, Dec 27, 2015 at 07:40:53AM -0500, Ronald S. Bultje wrote: > Hi, > > On Sun, Dec 27, 2015 at 6:02 AM, Michael Niedermayer> wrote: > > > +for (i = 0; i < 20; i++) > > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32; > > for (; i < 128; i++) > > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01); > > > > To innocent bystanders, this is hard to understand. Let's keep it a habit > to document things (what and why), where the "what" portion is probably > "this code emulates pow(10, x) with ff_pow10(x - 0.01)". As for why (most > specifically, why the 0.01?), I'm going to assume that here, you're trying > to get unit integers to not go to unit.000[..]001, so you subtract 0.01 > before the ceil so it works fine again to get the exact unit integer output > number. If that's correct, please add a comment saying that, and then lgtm. yes changed applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation
On Sun, Dec 27, 2015 at 7:44 AM, Michael Niedermayerwrote: > On Sun, Dec 27, 2015 at 07:40:53AM -0500, Ronald S. Bultje wrote: >> Hi, >> >> On Sun, Dec 27, 2015 at 6:02 AM, Michael Niedermayer >> wrote: >> >> > +for (i = 0; i < 20; i++) >> > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32; >> > for (; i < 128; i++) >> > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01); >> > >> >> To innocent bystanders, this is hard to understand. Let's keep it a habit >> to document things (what and why), where the "what" portion is probably >> "this code emulates pow(10, x) with ff_pow10(x - 0.01)". As for why (most >> specifically, why the 0.01?), I'm going to assume that here, you're trying >> to get unit integers to not go to unit.000[..]001, so you subtract 0.01 >> before the ceil so it works fine again to get the exact unit integer output >> number. If that's correct, please add a comment saying that, and then lgtm. In my view this change is not good for the exact reasons above. It is a "magic constant" that is a pure hack. The commit I made made accuracy more consistent across platforms (instead of relying on the whims of libm's pow/exp2), and was commented so that intent was clear. Nack from me. > > yes > changed > applied > thx > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Concerning the gods, I have no means of knowing whether they exist or not > or of what sort they may be, because of the obscurity of the subject, and > the brevity of human life -- Protagoras > > ___ > 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