[FFmpeg-devel] [PATCH] Optimization of AC3 floating point decoder for MIPS

[Babic, Nedeljko]

I will fix indention and resubmit patch.

-Nedeljko 
________________________________________
From: Michael Niedermayer [michaelni at gmx.at]
Sent: Saturday, August 18, 2012 0:32
To: FFmpeg development discussions and patches
Cc: Lukac, Zeljko; Babic, Nedeljko
Subject: Re: [FFmpeg-devel] [PATCH] Optimization of AC3 floating point decoder for MIPS

On Fri, Aug 17, 2012 at 12:12:40PM +0000, Babic, Nedeljko wrote:
> Hello,
>
> Is this patch OK now, or should I make some more changes?

it looks fine, but the {} placement is totally inconsistent.
i normally dont care but i dont think you intended to place them
like that

also i just took a quick look at the asm, i assume you know MIPS
asm much better than i do so deeply reviewing that would be wasted
time.


[...]> +void ff_fft_lut_init(uint16_t *table, int off, int size, int *index)
> +{
> +  if (size < 16)
> +  {
> +    table[*index] = off >> 2;
> +    (*index)++;
> +  }
> +  else
> +  {
> +    ff_fft_lut_init(table, off, size>>1, index);
> +    ff_fft_lut_init(table, off+(size>>1), size>>2, index);
> +    ff_fft_lut_init(table, off+3*(size>>2), size>>2, index);
> +  }
> +}

indention depth in ffmpeg should be 4 space, K&R style


[...]
> +    for (n=0; n<num_transforms; n++)
> +    {
> +        offset = 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;
> +
> +        tmpz[0].re = tmp1 + tmp5;
> +        tmpz[2].re = tmp1 - tmp5;
> +        tmpz[0].im = tmp2 + tmp6;
> +        tmpz[2].im = tmp2 - tmp6;
> +        tmpz[1].re = tmp3 + tmp8;
> +        tmpz[3].re = tmp3 - tmp8;
> +        tmpz[1].im = tmp4 - tmp7;
> +        tmpz[3].im = tmp4 + tmp7;
> +
> +}

this "}" looks misplaced

[...]

> +        for (n=0; n<num_transforms; n++)
> +        {
[...]
> +    for(k = 0; k < n4; k += 2) {


[...]
> +av_cold void ff_fft_init_mips(FFTContext *s)
> +{
> +  int n=0;
> +
> +  ff_fft_lut_init(fft_offsets_lut, 0, 1 << 16, &n);
> +
> +#if HAVE_INLINE_ASM
> +    s->fft_calc     = ff_fft_calc_mips;
> +#endif
> +#if CONFIG_MDCT
> +    s->imdct_calc   = ff_imdct_calc_mips;
> +    s->imdct_half   = ff_imdct_half_mips;
> +#endif

2 and 4 space indention is mixed here

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data