[FFmpeg-devel] [PATCH] MPEG-4 Parametric Stereo decoder

[Ronald S. Bultje]

Hi Alex,

On Thu, May 20, 2010 at 1:51 PM, Alex Converse <alex.converse at gmail.com> wrote:
> Yes there are some places where it can be further optimized but I'm
> losing motivation on this quickly so perhaps some review will respark
> my interest.

That's unfortunate to hear, but I hope you'll continue. Simple stuff:

+#define MAP_GENERIC_20_TO_34(out, in, full) \
+    if (full) {                            \
+    out[33] =  in[19];                     \
+    out[32] =  in[19];                     \
+    out[31] =  in[18];                     \
+    out[30] =  in[18];                     \
+    out[29] =  in[18];                     \
+    out[28] =  in[18];                     \
+    out[27] =  in[17];                     \
+    out[26] =  in[17];                     \
+    out[25] =  in[16];                     \
+    out[24] =  in[16];                     \
+    out[23] =  in[15];                     \
+    out[22] =  in[15];                     \
+    out[21] =  in[14];                     \
+    out[20] =  in[14];                     \
+    out[19] =  in[13];                     \
+    out[18] =  in[12];                     \
+    out[17] =  in[11];                     \
+    }                                      \
+    out[16] =  in[10];                     \
+    out[15] =  in[ 9];                     \
+    out[14] =  in[ 9];                     \
+    out[13] =  in[ 8];                     \
+    out[12] =  in[ 8];                     \
+    out[11] =  in[ 7];                     \
+    out[10] =  in[ 6];                     \
+    out[ 9] =  in[ 5];                     \
+    out[ 8] =  in[ 5];                     \
+    out[ 7] =  in[ 4];                     \
+    out[ 6] =  in[ 4];                     \
+    out[ 5] =  in[ 3];                     \
+    out[ 4] = (in[ 2] + in[ 3]) / 2;       \
+    out[ 3] =  in[ 2];                     \
+    out[ 2] =  in[ 1];                     \
+    out[ 1] = (in[ 0] + in[ 1]) / 2;       \
+    out[ 0] =  in[ 0];

Can /2 be >>1? Also indenting is off.

+                if (!PS_BASELINE && ps->enable_ipdopd) {
+                h11i += h11i_step;
+                h12i += h12i_step;
+                h21i += h21i_step;
+                h22i += h22i_step;
+#if 0
+            h11r = 0;
+            h12r = 1;
+            h21r = 1;
+            h22r = 0;
+            h11i = h12i = h21i = h22i = 0;
+#endif
+                l[k][n][0] = h11r*l_re + h21r*r_re - h11i*l_im - h21i*r_im;
+                l[k][n][1] = h11r*l_im + h21r*r_im + h11i*l_re + h21i*r_re;
+                r[k][n][0] = h12r*l_re + h22r*r_re - h12i*l_im - h22i*r_im;
+                r[k][n][1] = h12r*l_im + h22r*r_im + h12i*l_re + h22i*r_re;
+                } else {
+                l[k][n][0] = h11r*l_re + h21r*r_re;
+                l[k][n][1] = h11r*l_im + h21r*r_im;
+                r[k][n][0] = h12r*l_re + h22r*r_re;
+                r[k][n][1] = h12r*l_im + h22r*r_im;
+                }

Your indent is off here (also above this piece).

+        for (m = 0; m < PS_AP_LINKS; m++) {
+            Q_fract_allpass[1][k][m][0] = cos(-M_PI *
fractional_delay_links[m] * f_center);
+            Q_fract_allpass[1][k][m][1] = sin(-M_PI *
fractional_delay_links[m] * f_center);
+        }

Calculate -M_PI * ... before the cosf()/sinf() call so you calulate it
once instead of twice.

+                HA[iid][icc][0] = c2 * cosf(beta + alpha);
+                HA[iid][icc][1] = c1 * cosf(beta - alpha);
+                HA[iid][icc][2] = c2 * sinf(beta + alpha);
+                HA[iid][icc][3] = c1 * sinf(beta - alpha);

Here, values for cosf(x-y) and cosf(x+y) can be calculated once
instead of twice. Should save 2 calls to math funcs.

+                HB[iid][icc][0] =  M_SQRT2 * cosf(alpha) * cosf(gamma);
+                HB[iid][icc][1] =  M_SQRT2 * sinf(alpha) * cosf(gamma);
+                HB[iid][icc][2] = -M_SQRT2 * sinf(alpha) * sinf(gamma);
+                HB[iid][icc][3] =  M_SQRT2 * cosf(alpha) * sinf(gamma);

Same here for cosf/sinf(alpha/gamma). Saves 4 calls to expensive float
funcs. I know this last few is just init, but it's still something.

Generally, many of these variables have short 1-2letter names and no
or very little doxy, maybe you can document a few of them, at least
those declared in .h files? Same for functions, doxyment what they do?

Ronald