[FFmpeg-devel] [PATCH] Move Kaiser-Bessel Derived window to mdct.c

[Robert Swain]

Hello,

On 11/01/2008, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Jan 11, 2008 at 12:53:10PM +0000, Robert Swain wrote:
> > From 2c1ff501a911f61251eb4ccd4e8f90386c178337 Mon Sep 17 00:00:00 2001
> > From: Robert Swain <rob at opendot.cl>
> > Date: Fri, 11 Jan 2008 09:06:52 +0000
> > Subject: [PATCH] Move Kaiser-Bessel Derived window generation to mdct.c
> >
> > ---
> >  libavcodec/ac3dec.c  |   26 +-------------------------
> >  libavcodec/dsputil.h |    1 +
> >  libavcodec/mdct.c    |   24 ++++++++++++++++++++++++
> >  3 files changed, 26 insertions(+), 25 deletions(-)
> [...]
> > diff --git a/libavcodec/dsputil.h b/libavcodec/dsputil.h
> > index d541fe0..8dc5ea1 100644
> > --- a/libavcodec/dsputil.h
> > +++ b/libavcodec/dsputil.h
> > @@ -645,6 +645,7 @@ typedef struct MDCTContext {
> >      FFTContext fft;
> >  } MDCTContext;
> >
> > +void ff_kbd_window_init(float *window);
> >  int ff_mdct_init(MDCTContext *s, int nbits, int inverse);
> >  void ff_imdct_calc(MDCTContext *s, FFTSample *output,
> >                  const FFTSample *input, FFTSample *tmp);
> > diff --git a/libavcodec/mdct.c b/libavcodec/mdct.c
> > index de32752..668c514 100644
> > --- a/libavcodec/mdct.c
> > +++ b/libavcodec/mdct.c
> > @@ -26,6 +26,30 @@
> >   */
> >
> >  /**
> > + * Generate a Kaiser-Bessel Derived Window.
> > + */
>
> Doxygen comments should be in the header ideally (they are easier to find
> in the header) especially with installed headers though that doesnt apply
> here

It seems going back and editing the incremental commits isn't
particularly easy so I'm going to give up on git's wonderfulness
(unless anyone has suggestions for managing this) and approach each
step one at a time. Back to svn for the moment.

I've moved the Doxygen comment to dsputil.h but I think it's not
necessarily immediately obvious from the function name what the
function is, so I added a regular comment with the same text. I also
added a Doxygen parameter definition in the comment when moving to
dsputil.h, I hope this is OK.

> the remainder of patch #1 looks ok

See attached. When I know this one is OK, I will resubmit the next.

> [...]
>
> > + * @param window    pointer to window coefficient array
> > + * @param alpha     determines window shape
> > + * @param n         half of window length
>
> hmm, this sounds like n = half of the length of the window array

What would you prefer? "half of the window size"? "half of the number
of window elements"? I don't really know what to suggest if what I
wrote is easily misinterpreted. It seemed the clearest definition to
me.

> > + * @param iter      iterations for Bessel I0 approximation
> >   */
> > -void ff_kbd_window_init(float *window)
> > +void ff_kbd_window_init(float *window, float alpha, int n, int iter)
> >  {
>
> iam for hardcoding the iterations

I suppose I should test how many iterations are required to achieve
the maximum precision of float, right?

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080111-kaiser_move.diff
Type: application/octet-stream
Size: 2683 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080111/35bb5d11/attachment.obj>