[FFmpeg-devel] [PATCH] libavutil: add an FFT & MDCT implementation

Michael Niedermayer michael at niedermayer.cc
Tue May 7 00:26:23 EEST 2019


On Mon, May 06, 2019 at 02:23:26AM +0200, Lynne wrote:
> May 5, 2019, 1:52 PM by dev at lynne.ee:
> 
> > May 4, 2019, 10:00 PM by > dev at lynne.ee <mailto:dev at lynne.ee>> :
> >
> >> May 4, 2019, 8:10 PM by > >> michael at niedermayer.cc <mailto:michael at niedermayer.cc>>>  <mailto:>> michael at niedermayer.cc <mailto:michael at niedermayer.cc>>> >> :
> >>
> >>> On Fri, May 03, 2019 at 09:08:57PM +0200, Lynne wrote:
> >>>
> >>>> This commit adds a new API to libavutil to allow for arbitrary transformations
> >>>> on various types of data.
> >>>>
> >>> breaks build on mips
> >>>
> >>> CC	libavutil/fft.o
> >>> src/libavutil/fft.c:47: error: redefinition of typedef ‘AVFFTContext’
> >>> src/libavutil/fft.h:25: note: previous declaration of ‘AVFFTContext’ was here
> >>> make: *** [libavutil/fft.o] Error 1
> >>>
> >>> [...]
> >>>
> >>
> >> Fixed, v2 attached. Changes:
> >> -Stride really is in bytes now.
> >> -Corrected some comments (stride supported by all (i)mdcts, not just compound
> >>  ones, some clarifications regarding the scale).
> >>
> >> Also that 28-point FFT comparison was a typo, its 128.
> >>
> >
> > Managed to further optimize the 15-point transform by rewriting it as an exptab-less
> > compound 3x5 transform and embedding its input map into the parent transform's map.
> > Updated comparisons to libfftw3f:
> > 120:
> >   22353 decicycles in     fftwf_execute,     1024 runs,      0 skips
> >   21836 decicycles in compound_fft_15x8,     1024 runs,      0 skips
> >
> > 480:
> >   103998 decicycles in       fftwf_execute,    1024 runs,      0 skips
> >   102747 decicycles in compound_fft_15x32,    1024 runs,      0 skips
> > 960:
> >   186210 decicycles in      fftwf_execute,    1024 runs,      0 skips
> >   215256 decicycles in compound_fft_15x64,    1024 runs,      0 skips
> >
> 
> Attached a v4 of the patch which adjusts transform direction by reordering the
> coefficients like the power of two transforms do. This allowed for the exptabs
> to be computed just once on startup and stored in a global array.
> Didn't even consider it was possible to do so for odd-sized transforms and
> especially for compound 5x3 transforms but after some experimentation I found
> the key was to perform the permutation before the second permutation to
> embed the 5x3's input map in.
> 
> I don't think there are any more feasible ways to improve the code, short of
> having 15 different versions for all power of two transforms by hardcoding
> the output reindexing, so I'd like to get some feedback on the API.
> The old SIMD from lavc is unusable, especially the power of two part,
> so it would be nice to get started on rewriting that soon.

>  Makefile |    2 
>  fft.c    |  795 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fft.h    |   80 ++++++
>  3 files changed, 877 insertions(+)
> 68f2931ec951fc53d53e6820afee1d490abbfebf  0001-libavutil-add-an-FFT-MDCT-implementation.patch
> From 77cd00aa76f97e9116585a4b409b3efb0096094a Mon Sep 17 00:00:00 2001
> From: Lynne <dev at lynne.ee>
> Date: Thu, 2 May 2019 15:07:12 +0100
> Subject: [PATCH] libavutil: add an FFT & MDCT implementation
> 
> v2 changes:
> Stride really is in bytes now.
> Corrected some comments (stride supported by all (i)mdcts, not just compound
> ones, some clarifications regarding the scale).
> 
> v3 changes:
> Optimized the 15-point transform by making it compound as well.
> 
> v4 changes:
> Reorder coefficients instead of relying on exptab phase for the nptwo
> transforms, and compute the exptabs just once on startup.
> 
> This commit adds a new API to libavutil to allow for arbitrary transformations
> on various types of data.
> This is a partly new implementation, with the power of two transforms taken
> from libavcodec/fft_template, the 5 and 15-point FFT taken from mdct15, while
> the 3-point FFT was written from scratch.
> The (i)mdct folding code is taken from mdct15 as well, as the mdct_template
> code was somewhat old, messy and not easy to separate.
> 
> A notable feature of this implementation is that it allows for 3xM and 5xM
> based transforms, where M is a power of two, e.g. 384, 640, 768, 1280, etc.
> AC-4 uses 3xM transforms while Siren uses 5xM transforms, so the code will
> allow for decoding of such streams.
> A non-exaustive list of supported sizes:
> 4, 8, 12, 16, 20, 24, 32, 40, 48, 60, 64, 80, 96, 120, 128, 160, 192, 240,
> 256, 320, 384, 480, 512, 640, 768, 960, 1024, 1280, 1536, 1920, 2048, 2560...
> 
> The API was designed such that it allows for not only 1D transforms but also
> 2D transforms of certain block sizes. This was partly on accident as the stride
> argument is required for Opus MDCTs, but can be used in the context of a 2D
> transform as well.
> Also, various data types would be implemented eventually as well, such as
> "double" and "int32_t".
> 
> The avfft_transform() function is awkward but avoids some other more
> awkward ideas to isolate the private parts of the structure and not
> make them part of the API, as well as reducing call overhead from
> an additional function call.
> 
> Some performance comparisons with libfftw3f (SIMD disabled for both):
> 120:
>   22353 decicycles in     fftwf_execute,     1024 runs,      0 skips
>   21836 decicycles in compound_fft_15x8,     1024 runs,      0 skips
> 
> 128:
>   22003 decicycles in       fftwf_execute,   1024 runs,      0 skips
>   23132 decicycles in monolithic_fft_ptwo,   1024 runs,      0 skips
> 
> 384:
>   75939 decicycles in      fftwf_execute,    1024 runs,      0 skips
>   73973 decicycles in compound_fft_3x128,    1024 runs,      0 skips
> 
> 640:
>  104354 decicycles in       fftwf_execute,   1024 runs,      0 skips
>  149518 decicycles in compound_fft_5x128,    1024 runs,      0 skips
> 
> 768:
>  109323 decicycles in      fftwf_execute,    1024 runs,      0 skips
>  164096 decicycles in compound_fft_3x256,    1024 runs,      0 skips
> 
> 960:
>  186210 decicycles in      fftwf_execute,    1024 runs,      0 skips
>  215256 decicycles in compound_fft_15x64,    1024 runs,      0 skips
> 
> 1024:
>  163464 decicycles in       fftwf_execute,   1024 runs,      0 skips
>  199686 decicycles in monolithic_fft_ptwo,   1024 runs,      0 skips
> 
> With SIMD we should be faster than fftw for 15xM transforms as our fft15 SIMD
> is around 2x faster than theirs, even if our ptwo SIMD is slightly slower.
> 
> The goal is to remove the libavcodec/mdct15 code and deprecate the
> libavcodec/avfft interface once aarch64 and x86 SIMD code has been ported.
> It is unlikely that libavcodec/fft will be removed soon as there's much SIMD
> written for exotic or old platforms there, but nevertheless new code
> should use this new API throughout the project.
> 
> The implementation passes fate when used in Opus and AAC, and the output
> is identical in ATRAC9 as well.
> ---
>  libavutil/Makefile |   2 +
>  libavutil/fft.c    | 795 +++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/fft.h    |  80 +++++
>  3 files changed, 877 insertions(+)
>  create mode 100644 libavutil/fft.c
>  create mode 100644 libavutil/fft.h
[...]
> +static av_cold void ff_init_53_tabs(void)
> +{
> +    const double t = 2 * M_PI;
> +
> +    /* 3-point FFT exptab */
> +    ff_53_tabs[0] = (FFTComplex){ -1.0/2.0, -1.0/2.0 };
> +    ff_53_tabs[1] = (FFTComplex){ sin(t / 3), sin(t / 3) };
> +
> +    /* 5-point FFT exptab */
> +    ff_53_tabs[2] = (FFTComplex){ cos(t /  5), sin(t /  5) };
> +    ff_53_tabs[3] = (FFTComplex){ cos(t / 10), sin(t / 10) };
> +}
> +
> +#define sqrthalf (float)M_SQRT1_2
> +

> +#define BF(x, y, a, b) do {                                                    \
> +        x = a - b;                                                             \
> +        y = a + b;                                                             \
> +    } while (0)

the a / b should be protected by  ()


[...]
> diff --git a/libavutil/fft.h b/libavutil/fft.h
> new file mode 100644
> index 0000000000..b2137d19fb
> --- /dev/null
> +++ b/libavutil/fft.h
> @@ -0,0 +1,80 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVUTIL_FFT_H
> +#define AVUTIL_FFT_H
> +
> +#include <stdint.h>
> +#include "attributes.h"
> +
> +typedef struct AVFFTContext AVFFTContext;
> +
> +typedef struct FFTComplexFloat {
> +    float re, im;
> +} FFTComplexFloat;
> +

> +enum AVFFTTransformType {
> +    /* Standard complex to complex FFT with sample data type FFTComplexFloat
> +     * and a scale type of float */
> +    AVFFT_TX_FLOAT_FFT = 0,
> +    /* Standard MDCT with sample data type of float and a scale type of
> +     * float */
> +    AVFFT_TX_FLOAT_MDCT = 1,
> +
> +    /* Not part of the API */
> +    AVFFT_TX_NB,
> +};

i think doxygen will not pick these comments up, would need /** or some other variant


> +
> +/**
> + * Do the transform
> + *
> + * @param s the transform context for this functionfor
> + * @param out the output array
> + * @param in the input array
> + * @param stride the input or output stride (depending on transform direction)
> + *               in bytes, currently implemented for all MDCT transforms
> + */
> +static av_always_inline void avfft_transform(AVFFTContext *s, void *out,
> +                                             void *in, ptrdiff_t stride)
> +{
> +    struct {
> +        void (*fn)(AVFFTContext *, void *, void *, ptrdiff_t);
> +    } *p = (void *)s;

this looks like a strict aliassing violation


> +    p->fn(s, out, in, stride);
> +}
> +
> +/* Initialize a transform context with the given configuration
> + * Currently power of two lengths from 4 to 131072 are supported, along with
> + * any length decomposable to a power of two and either 3, 5 or 15.
> + *
> + * @param ctx the context to allocate, will be NULL on error
> + * @param type type the type of transform
> + * @param inv whether to do an inverse or a forward transform
> + * @param len the size of the transform in samples
> + * @param scale pointer to the value to scale the output by
> + * @param flags currently unused
> + *
> + * @return 0 on success, negative error code on failure
> + */

this comment also needs a /**

also i agree with paul, the API looks good
implementation not really reviewed

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190506/4e617354/attachment.sig>


More information about the ffmpeg-devel mailing list