On Mon, Nov 04, 2013 at 06:55:34PM -0800, Alex Converse wrote:
> On Mon, Nov 4, 2013 at 10:14 AM, Diego Biurrun <[email protected]> wrote:
> >
> > ---
> >
> > While clearly not a thing of supreme beauty, this silences all warnings
> > on a previously very noisy file, both with hardcoded and generated tables.
>
> One alternate approach (that I'm not necessarily advocating), is to
> make the actual arrays in aacps_table prefixed, do the casting in that
> file and have all the pointers used from aacps.c full const, hiding
> all the ugly in the tablegen header but ultimately giving both
> implementations a consistent interface.
What exactly do you mean by "prefixed"? Making them all TABLE_CONST?
> > --- a/libavcodec/aacps.c
> > +++ b/libavcodec/aacps.c
> > @@ -606,7 +609,8 @@ static void map_val_20_to_34(float
> > par[PS_MAX_NR_IIDICC])
> > par[ 0] = par[ 0];
> > }
> >
> > -static void decorrelation(PSContext *ps, float (*out)[32][2], const float
> > (*s)[32][2], int is34)
> > +static void decorrelation(PSContext *ps, float (*out)[32][2],
> > + TABLE_CONST float (*s)[32][2], int is34)
> > {
> > LOCAL_ALIGNED_16(float, power, [34], [PS_QMF_TIME_SLOTS]);
> > LOCAL_ALIGNED_16(float, transient_gain, [34], [PS_QMF_TIME_SLOTS]);
> > @@ -634,7 +638,7 @@ static void decorrelation(PSContext *ps, float
> > (*out)[32][2], const float (*s)[3
> >
> > for (k = 0; k < NR_BANDS[is34]; k++) {
> > int i = k_to_i[k];
> > - ps->dsp.add_squares(power[i], s[k], nL - n0);
> > + ps->dsp.add_squares(power[i], (const float (*)[2]) s[k], nL - n0);
> > }
> >
> > //Transient detection
> > @@ -747,7 +751,7 @@ static void stereo_processing(PSContext *ps, float
> > (*l)[32][2], float (*r)[32][2
> > int8_t (*ipd_mapped)[PS_MAX_NR_IIDICC] = ipd_mapped_buf;
> > int8_t (*opd_mapped)[PS_MAX_NR_IIDICC] = opd_mapped_buf;
> > const int8_t *k_to_i = is34 ? k_to_i_34 : k_to_i_20;
> > - const float (*H_LUT)[8][4] = (PS_BASELINE || ps->icc_mode < 3) ? HA :
> > HB;
> > + TABLE_CONST float (*H_LUT)[8][4] = (PS_BASELINE || ps->icc_mode < 3) ?
> > HA : HB;
>
> I'd rather see a cast on the RHS here.
Like this:
const float (*H_LUT)[8][4] = (const float(*)[8][4]) (PS_BASELINE ||
ps->icc_mode < 3) ? HA : HB;
That results in the following warning:
libavcodec/aacps.c:755:34: warning: cast to pointer from integer of different
size [-Wint-to-pointer-cast]
> > @@ -897,7 +901,7 @@ int ff_ps_apply(AVCodecContext *avctx, PSContext *ps,
> > float L[2][38][64], float
> > memset(ps->ap_delay + top, 0, (NR_ALLPASS_BANDS[is34] -
> > top)*sizeof(ps->ap_delay[0]));
> >
> > hybrid_analysis(&ps->dsp, Lbuf, ps->in_buf, L, is34, len);
> > - decorrelation(ps, Rbuf, Lbuf, is34);
> > + decorrelation(ps, Rbuf, (const float (*)[32][2]) Lbuf, is34);
>
> The changes wrt this Lbuf (s inside the function) seem inconsistent.
> You are unconditionally adding the const in the call, then using
> TABLE_CONST in the function declaration, then recasting it again when
> you pass it to add_squares().
>
> Meanwhile calls inside this function that take transient_gain still warn.
>
> > --- a/libavcodec/aacps_tablegen.h
> > +++ b/libavcodec/aacps_tablegen.h
> > @@ -28,6 +28,7 @@
> >
> > #if CONFIG_HARDCODED_TABLES
> > #define ps_tableinit()
> > +#define TABLE_CONST const
> > #include "libavcodec/aacps_tables.h"
> > #else
> > @@ -37,6 +38,7 @@
> > #define NR_ALLPASS_BANDS34 50
> > #define PS_AP_LINKS 3
> > +#define TABLE_CONST
> > static float pd_re_smooth[8*8*8];
> > static float pd_im_smooth[8*8*8];
> > --- a/libavcodec/aacpsdsp.h
> > +++ b/libavcodec/aacpsdsp.h
> > @@ -38,7 +38,7 @@ typedef struct PSDSPContext {
> > void (*decorrelate)(float (*out)[2], float (*delay)[2],
> > float
> > (*ap_delay)[PS_QMF_TIME_SLOTS+PS_MAX_AP_DELAY][2],
> > - const float phi_fract[2], float (*Q_fract)[2],
> > + const float phi_fract[2], const float
> > (*Q_fract)[2],
> > const float *transient_gain,
> > float g_decay_slope,
> > int len);
>
> Shouldn't the implementations be updated to match?
My patch was still buggy. I'll send another (intermediate) version that
has these issues addressed.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
