[FFmpeg-devel] [RFC] Lowpass filter
Michael Niedermayer
michaelni
Tue Aug 26 19:33:28 CEST 2008
On Tue, Aug 26, 2008 at 07:48:07PM +0300, Kostya wrote:
> On Tue, Aug 26, 2008 at 02:18:18PM +0200, Michael Niedermayer wrote:
> > On Tue, Aug 26, 2008 at 08:48:54AM +0300, Kostya wrote:
> [...]
> > > /*
> > > * Lowpass IIR filter
> >
> > Is there anything specific to lowpassing in the code?
> > If not than IIR filter would be a better name.
>
> Here's your generic version (but with implementation for only one particular case).
>
> > [...]
> > > /**
> > > * Initialize filter coefficients.
> > > *
> > > * @param order filter order
> > > * @param cutoff_ratio cutoff to input frequency ratio
> > > *
> > > * @return pointer to filter coefficients structure or NULL if filter cannot be created
> > > */
> > > struct FFLPFilterCoeffs* ff_lowpass_filter_init_coeffs(int order, float cutoff_ratio);
> >
> > This should also have a enum for filter type.
> >
> >
> > [...]
> > > //maximum supported filter order
> > > #define MAXORDER 30
> >
> > does it work with order 30 or are roundoff errors causing problems already,
> > i mean at some order they will ...
>
> They do, but with this filter order I've managed to get filtered signal
> resembling original one.
>
> [...]
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > I have never wished to cater to the crowd; for what I know they do not
> > approve, and what they approve I do not know. -- Epicurus
> /*
> * IIR filter
> * Copyright (c) 2008 Konstantin Shishkov
> *
> * 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
> */
>
> /**
> * @file filter.h
> * IIR filter interface
> (maybe FIR will be here too)
I dont think so, it doesnt fit that well, it might fit better in the
resample code ...
> */
>
> #ifndef FFMPEG_FILTER_H
> #define FFMPEG_FILTER_H
>
> #include "avcodec.h"
>
> struct FFFilterCoeffs;
> struct FFFilterState;
>
> enum FilterType{
> FF_FILTER_TYPE_BESSEL,
> FF_FILTER_TYPE_BUTTERWORTH,
> FF_FILTER_TYPE_CHEBYSHEV,
> FF_FILTER_TYPE_ELLIPTIC,
> };
>
> enum FilterMode{
> FF_FILTER_MODE_LOWPASS,
> FF_FILTER_MODE_HIGHPASS,
> FF_FILTER_MODE_BANDPASS,
> FF_FILTER_MODE_BANDSTOP,
> };
I think all the stuff should have IIR in their names
[...]
> /**
> * IIR filter state
> */
> typedef struct FFFilterState{
> float *x;
> }FFFilterState;
no, we do not want to do the extra pointer dereference nor do we want an
extra pointer to be stored, the correct way is:
typedef struct FFFilterState{
float x[];
}
>
> //maximum supported filter order
> #define MAXORDER 30
doxygen
>
> struct FFFilterCoeffs* ff_filter_init_coeffs(enum FilterType filt_type,
> enum FilterMode filt_mode,
> int order, float cutoff_ratio,
> float stopband, float ripple)
> {
> int i, j, size;
> FFFilterCoeffs *c;
> double wa;
> complex p[MAXORDER + 1];
>
> if(filt_type != FF_FILTER_TYPE_BESSEL || filt_mode != FF_FILTER_MODE_LOWPASS)
> return NULL;
> if(order <= 1 || (order & 1) || order > MAXORDER || cutoff_ratio >= 1.0)
> return NULL;
>
> c = av_malloc(sizeof(FFFilterCoeffs));
> c->cx = av_mallocz(sizeof(c->cx[0]) * (order / 2 + 1));
> c->cy = av_malloc (sizeof(c->cy[0]) * order);
> c->order = order;
>
> wa = 2 * tan(M_PI * 0.5 * cutoff_ratio);
>
> c->cx[0] = 1;
> for(i = 1; i < order / 2 + 1; i++)
> c->cx[i] = c->cx[i - 1] * (order - i + 1LL) / i;
>
> p[0] = 1.0;
> for(i = 1; i <= order; i++)
> p[i] = 0.0;
> for(i = 0; i < order; i++){
> complex zp;
> double th = (i + order/2 + 0.5) * M_PI / order;
> zp = cexp(I*th) * wa;
> zp = (zp + 2.0) / (zp - 2.0);
>
> for(j = order; j >= 1; j--)
> p[j] = zp*p[j] + p[j - 1];
> p[0] *= zp;
> }
> c->gain = creal(p[order]);
> for(i = 0; i < order; i++){
> c->gain += creal(p[i]);
> c->cy[i] = creal(-p[i] / p[order]);
> }
> c->gain /= 1 << order;
>
> return c;
trailing whitespace
[...]
> void ff_filter(const struct FFFilterCoeffs *c, struct FFFilterState *s, int size, int16_t *src, int sstep, int16_t *dst, int dstep)
src should be const
> {
> int i;
>
> if(c->order == 4){
> for(i = 0; i < size; i += 4){
> float in, res;
>
> FILTER(0, 1, 2, 3);
> FILTER(1, 2, 3, 0);
> FILTER(2, 3, 0, 1);
> FILTER(3, 0, 1, 2);
> }
> }else{
> for(i = 0; i < size; i++){
> int j;
> float in, res;
> in = *src * c->gain;
> for(j = 0; j < c->order; j++)
> in += c->cy[j] * s->x[j];
> res = s->x[0] + in + s->x[c->order/2] * c->cx[c->order/2];
> for(j = 1; j < (c->order / 2); j++)
should be >>1 as /2 is tricky for the compiler when it doesnt realize that
the value is positive
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080826/9334f80e/attachment.pgp>
More information about the ffmpeg-devel
mailing list