[FFmpeg-devel] [PATCH] Lowpass functionality for lavc

[Kostya]

On Sat, Aug 16, 2008 at 03:15:55PM +0200, Michael Niedermayer wrote:
> On Sat, Aug 16, 2008 at 01:12:36PM +0300, Kostya wrote:
> > On Fri, Aug 15, 2008 at 09:37:07PM +0200, Michael Niedermayer wrote:
> > [...] 
> > > > 
> > > > /**
> > > >  * Free filter coefficients.
> > > >  *
> > > >  * @param coeffs pointer allocated with ff_lowpass_filter_init_coeffs()
> > > >  */
> > > > void ff_lowpass_filter_free_coeffs(struct FFLPFilterCoeffs *coeffs);
> > > > 
> > > > /**
> > > >  * Free filter state.
> > > >  *
> > > >  * @param state pointer allocated with ff_lowpass_filter_init_state()
> > > >  */
> > > > void ff_lowpass_filter_free_state(struct FFLPFilterState *state);
> > > 
> > > we do not need wrapers around av_free()
> > > this is not public API where it would be hard to replace later
> > 
> > So I made FFLPFilterState const for now and #if 0'd free funcs.
> > Better implementation will allocate arrays for states depending on order,
> > so more complicated free function will be needed.
> > 
> > [...]
> > > > /*
> > > >  * Lowpass IIR filter
> > > 
> > > actually, this could do more generic IIR filtering one day ...
> > 
> > *sigh* indeed. After I deal with AAC and RV I will read filter theory
> > and do that. Too bad I didn't have time to do it earlier. 
> >  
> > [...]
> > > > static const float lp_filter_data[][LOWPASS_FILTER_ORDER+2] = {
> > > >     { 0.5000000000, 9.398085e-01, -0.0176648009,  0.0000000000, -0.4860288221,  0.0000000000 },
> > > >     { 0.4535147392, 6.816645e-01, -0.4646665999, -2.2127207402, -3.9912017501, -3.2380429984 },
> > > >     { 0.4166666667, 4.998150e-01, -0.2498216698, -1.3392807613, -2.7693097862, -2.6386277439 },
> > > >     { 0.3628117914, 3.103469e-01, -0.0965076902, -0.5977763360, -1.4972580903, -1.7740085241 },
> > > >     { 0.3333333333, 2.346995e-01, -0.0557639007, -0.3623690447, -1.0304538354, -1.3066051440 },
> > > >     { 0.2916666667, 1.528432e-01, -0.0261686639, -0.1473794606, -0.6204721225, -0.6514716536 },
> > > >     { 0.2267573696, 6.917529e-02, -0.0202414073,  0.0780167640, -0.5277442247,  0.3631641670 },
> > > >     { 0.2187500000, 6.178391e-02, -0.0223681543,  0.1069446609, -0.5615167033,  0.4883976841 },
> > > >     { 0.2083333333, 5.298685e-02, -0.0261686639,  0.1473794606, -0.6204721225,  0.6514716536 },
> > > >     { 0.1587301587, 2.229030e-02, -0.0647354087,  0.4172275190, -1.1412129810,  1.4320761385 },
> > > >     { 0.1458333333, 1.693903e-02, -0.0823177861,  0.5192354923, -1.3444768251,  1.6365345642 },
> > > >     { 0.1133786848, 7.374053e-03, -0.1481421788,  0.8650973862, -1.9894244796,  2.1544844308 },
> > > >     { 0.1041666667, 5.541768e-03, -0.1742301048,  0.9921936565, -2.2090801108,  2.3024482658 },
> > > > };
> > > 
> > > an array of FFLPFilterCoeffs makes more sense i think
> > 
> > Indeed, and init_coeffs() return a pointer in it now (hence the const qualifier).
> > 
> > [...]
> 
> > > you are mixing float and int, iam not sure if this is ideal, at least
> > > the float->int convertion should use lrintf()
> > 
> > I'm not sure that fixed-point filter is a good idea and only output is
> > converted from float (with lrintf() now).
> 
> iam not sure either ...
> so lets leave the floats
> 
> [...]
> 
> > #define FILTER(i0, i1, i2, i3)                  \
> >     in = *src * c->gain;                        \
> 
> >     res =  (s->x[i0] + in      )*1              \
> >          + (s->x[i1] + s->x[i3])*4              \
> >          +  s->x[i2]            *6              \
> >          + c->c[0]*s->y[i0] + c->c[1]*s->y[i1]  \
> >          + c->c[2]*s->y[i2] + c->c[3]*s->y[i3]; \
> >     *dst = av_clip_int16(lrintf(res));          \
> >     s->x[i0] = in;                              \
> >     s->y[i0] = res;                             \
> 
> i think the followng is equivalent and needs a few memory accesses less
> 
> in +=  c->c[0]*s->y[i0] + c->c[1]*s->y[i1]
>      + c->c[2]*s->y[i2] + c->c[3]*s->y[i3];
> res =   s->y[i0] + in
>      + (s->y[i1] + s->y[i3])*4
>      +  s->y[i2]            *6
> s->y[i0]= in;
 
err, I am not sure that will work at all
how those s->x[] and s->y[] are merged into one array?
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> If you really think that XML is the answer, then you definitly missunderstood
> the question -- Attila Kinali