[FFmpeg-soc] [soc]: r2552 - mlp/mlpdec.c

[Diego Biurrun]

On Sun, Jun 22, 2008 at 07:15:10PM +0100, Ramiro Polla wrote:
> ramiro wrote:
> > 
> > Log:
> > Copy IIR precision to FIR if only IIR is used, since the filtering code
> > always uses the FIR precision.
> > 
> > --- mlp/mlpdec.c	(original)
> > +++ mlp/mlpdec.c	Sun Jun 22 20:02:56 2008
> > @@ -685,6 +685,11 @@ static int read_decoding_params(MLPDecod
> >              }
> > +            /* Both filters must have the same precision, so the filtering
> > +             * code always use the FIR precision. If only IIR is used, we copy
>                                ^^^
> 
> Oops, typo. Since I'll have to fix this anyways, does anyone have a 
> better idea for the comment? I don't like the text the way it is now. 
> It's supposed to mean something like "To simplify the filtering code, 
> since both precisions are the same, only the FIR precision is used. If 
> only IIR filtering is used, copy its values to the FIR filter so it can 
> be used by the filtering code."

First off: If you want to have a comment on the text, don't cut it off.
I had to dig out the source file for this review.  That's cumbersome and
I cannot promise you that I will bother the next time around.

I think the main problem in the comment is the lack of proper subjects.
You need too much context to figure out what you are talking about:

Compare

  /* Both filters must have the same precision, so the filtering
   * code always uses the FIR precision. If only IIR is used,
   * we copy its precision to FIR. */

with

  /* The FIR and IIR filters must have the same precision, so the
   * filtering code always uses the precision of the FIR filter.
   * If only the IIR filter is used, we copy its precision value
   * to the FIR filter. */

which should already be much better.  The following is more verbose but
hopefully easier to understand.

  /* The FIR and IIR filters must have the same precision.
   * To simplify the filtering code, only the precision of the
   * FIR filter is considered. If only the IIR filter is employed,
   * the FIR filter precision is set to that of the IIR filter, so
   * that the filtering code can use it. */

BTW, all of this begs the question why they have two different precision
values to begin with ...

Diego