On Wed, Jul 26, 2017 at 12:51:31AM +0100, Rostislav Pehlivanov wrote: > On 17 July 2017 at 16:17, Tyler Jones <tdjones...@gmail.com> wrote: > > > + float last_var; > > + const float eps = 1e-4; > > > > Use normal notation for floats and add an f at the end to inform the > compiler the constant is a float.
Fixed.
> > +{
> > + if (vpctx) {
> > + if (vpctx->filter_delay)
> > + av_freep(&vpctx->filter_delay);
> > +
> > + if (vpctx->variance)
> > + av_freep(&vpctx->variance);
> > +
> >
>
> You can free NULL pointers, n o need to check.
Fixed.
> > +#ifndef AVCODEC_VORBISPSY_H
> > +#define AVCODEC_VORBISPSY_H
> > +
> > +#include "libavutil/attributes.h"
> > +
> > +/**
> > + * Second order IIR Filter
> > + */
> > +typedef struct IIRFilter {
> > + float b[3]; ///< Normalized cofficients for numerator of transfer
> > function
> > + float a[3]; ///< Normalized coefficiets for denominator of transfer
> > function
> > +} IIRFilter;
> >
>
> We already have an IIR filter (libavcodec/iirfilter.h), could you check it
> out if it can be reused perhaps?
This is where I had initially looked, and my issue was that it was not possible
to generate a high-pass butterworth or biquads that behaved like butterworths
when cascaded (controlling the quality factor). Manually initializing the
structs
and only using the filter function resulted in more complex code and more
boilerplate
than implementing it separately here.
If necessary, I can switch to using iirfilter.h now, but I'd rather use this
implementation
temporarily and add the functionality needed in iirfilter.h in a separate patch
if that
is acceptable. I'd argue that this would result in cleaner code. From what I
can see,
psymodel.c is the only component that uses iirfilter.h, and the biquad filter
is unused,
so it should be a less difficult change to make.
> Apart from those patch looks good and should be ready to merge once those
> nits get fixed.
> I can hear a noticeable positive difference at low rates, good job.
Thanks for catching these mistakes and taking a look.
Thanks again,
Tyler Jones
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
