On Thu, May 25, 2017 at 03:29:59PM +0100, Rostislav Pehlivanov wrote: > On 25 May 2017 at 15:10, Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408 > > > > Found-by: continuous fuzzing process https://github.com/google/oss- > > fuzz/tree/master/projects/ffmpeg > > Signed-off-by > > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg%0ASigned-off-by>: > > Michael Niedermayer <mich...@niedermayer.cc> > > --- > > libavcodec/fft_template.c | 50 +++++++++++++++++++++++------- > > ----------------- > > 1 file changed, 25 insertions(+), 25 deletions(-) > > > > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c > > index 480557f49f..e3a37e5d69 100644 > > --- a/libavcodec/fft_template.c > > +++ b/libavcodec/fft_template.c > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) { > > > > int nbits, i, n, num_transforms, offset, step; > > int n4, n2, n34; > > - FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8; > > + SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8; > > FFTComplex *tmpz; > > const int fft_size = (1 << s->nbits); > > int64_t accu; > > @@ -260,14 +260,14 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > offset = ff_fft_offsets_lut[n] << 2; > > tmpz = z + offset; > > > > - tmp1 = tmpz[0].re + tmpz[1].re; > > - tmp5 = tmpz[2].re + tmpz[3].re; > > - tmp2 = tmpz[0].im + tmpz[1].im; > > - tmp6 = tmpz[2].im + tmpz[3].im; > > - tmp3 = tmpz[0].re - tmpz[1].re; > > - tmp8 = tmpz[2].im - tmpz[3].im; > > - tmp4 = tmpz[0].im - tmpz[1].im; > > - tmp7 = tmpz[2].re - tmpz[3].re; > > + tmp1 = tmpz[0].re + (SUINT)tmpz[1].re; > > + tmp5 = tmpz[2].re + (SUINT)tmpz[3].re; > > + tmp2 = tmpz[0].im + (SUINT)tmpz[1].im; > > + tmp6 = tmpz[2].im + (SUINT)tmpz[3].im; > > + tmp3 = tmpz[0].re - (SUINT)tmpz[1].re; > > + tmp8 = tmpz[2].im - (SUINT)tmpz[3].im; > > + tmp4 = tmpz[0].im - (SUINT)tmpz[1].im; > > + tmp7 = tmpz[2].re - (SUINT)tmpz[3].re; > > > > tmpz[0].re = tmp1 + tmp5; > > tmpz[2].re = tmp1 - tmp5; > > @@ -288,19 +288,19 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > offset = ff_fft_offsets_lut[n] << 3; > > tmpz = z + offset; > > > > - tmp1 = tmpz[4].re + tmpz[5].re; > > - tmp3 = tmpz[6].re + tmpz[7].re; > > - tmp2 = tmpz[4].im + tmpz[5].im; > > - tmp4 = tmpz[6].im + tmpz[7].im; > > + tmp1 = tmpz[4].re + (SUINT)tmpz[5].re; > > + tmp3 = tmpz[6].re + (SUINT)tmpz[7].re; > > + tmp2 = tmpz[4].im + (SUINT)tmpz[5].im; > > + tmp4 = tmpz[6].im + (SUINT)tmpz[7].im; > > tmp5 = tmp1 + tmp3; > > tmp7 = tmp1 - tmp3; > > tmp6 = tmp2 + tmp4; > > tmp8 = tmp2 - tmp4; > > > > - tmp1 = tmpz[4].re - tmpz[5].re; > > - tmp2 = tmpz[4].im - tmpz[5].im; > > - tmp3 = tmpz[6].re - tmpz[7].re; > > - tmp4 = tmpz[6].im - tmpz[7].im; > > + tmp1 = tmpz[4].re - (SUINT)tmpz[5].re; > > + tmp2 = tmpz[4].im - (SUINT)tmpz[5].im; > > + tmp3 = tmpz[6].re - (SUINT)tmpz[7].re; > > + tmp4 = tmpz[6].im - (SUINT)tmpz[7].im; > > > > tmpz[4].re = tmpz[0].re - tmp5; > > tmpz[0].re = tmpz[0].re + tmp5; > > @@ -311,13 +311,13 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > tmpz[6].im = tmpz[2].im + tmp7; > > tmpz[2].im = tmpz[2].im - tmp7; > > > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp1 + tmp2); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp1 + tmp2); > > tmp5 = (int32_t)((accu + 0x40000000) >> 31); > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 - tmp4); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 - tmp4); > > tmp7 = (int32_t)((accu + 0x40000000) >> 31); > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp2 - tmp1); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp2 - tmp1); > > tmp6 = (int32_t)((accu + 0x40000000) >> 31); > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 + tmp4); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 + tmp4); > > tmp8 = (int32_t)((accu + 0x40000000) >> 31); > > tmp1 = tmp5 + tmp7; > > tmp3 = tmp5 - tmp7; > > @@ -348,10 +348,10 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > offset = ff_fft_offsets_lut[n] << nbits; > > tmpz = z + offset; > > > > - tmp5 = tmpz[ n2].re + tmpz[n34].re; > > - tmp1 = tmpz[ n2].re - tmpz[n34].re; > > - tmp6 = tmpz[ n2].im + tmpz[n34].im; > > - tmp2 = tmpz[ n2].im - tmpz[n34].im; > > + tmp5 = tmpz[ n2].re + (SUINT)tmpz[n34].re; > > + tmp1 = tmpz[ n2].re - (SUINT)tmpz[n34].re; > > + tmp6 = tmpz[ n2].im + (SUINT)tmpz[n34].im; > > + tmp2 = tmpz[ n2].im - (SUINT)tmpz[n34].im; > > > > tmpz[ n2].re = tmpz[ 0].re - tmp5; > > tmpz[ 0].re = tmpz[ 0].re + tmp5; > > -- > > 2.13.0 > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > libavcodec/avfft.h:typedef float FFTSample; > libavcodec/fft.h:typedef int32_t FFTSample; > libavcodec/fft.h:typedef int16_t FFTSample; > > You're forcing something to be integer in a templated file, and also I'm > unhappy with how you change typedef'd types to this SUINT thing. > Please never change typedef'd types even if they're always integer and > instead change the typedef to go from "typedef int <something>" to "typedef > SUINT <something>". Things are typedeffed for a reason and clarity (e.g. to > not mislead devs into thinking they're just integers and there's nothing > special going on).
The changed code is under #if FFT_FIXED_32 so the changed type is always int32 and FFTSample is actually misleading as it suggests it can be something else, especially in a template. Not that i really have an oppinion on this, just pointing it out. Iam not sure i understand what exactly you suggest but I can introduce a SUFFTSample typdef if thats preferred? or do you prefer / meant something else ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
