On Tuesday, November 5, 2013, Alex Converse wrote:
> On Mon, Nov 4, 2013 at 10:14 AM, Diego Biurrun
> <[email protected]<javascript:;>>
> 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.
> >
>
> The only warning my compiler generates with this file is:
> "warning: unknown warning option '-Wno-maybe-uninitialized'; did you
> mean '-Wno-uninitialized'?"
> but the gcc developers seem pretty emphatic that their warnings are
> correct. So I guess I'm ok with this in general. See specific comments
> below.
You should have they warning only when using clang.
Can you just confirm you are using clang? If gcc what version is it and
which operating system?
Thank you,
Vittorio
>
> 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.
>
> > diff --git a/libavcodec/aacps.c b/libavcodec/aacps.c
> > index 1abafe2..2b9ca4f 100644
> > --- 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.
>
> >
> > //Remapping
> > if (ps->num_env_old) {
> > @@ -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.
>
> > diff --git a/libavcodec/aacps_tablegen.h b/libavcodec/aacps_tablegen.h
> > index 701812b..87383bd 100644
> > --- 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
> > #include "libavutil/common.h"
> > @@ -37,6 +38,7 @@
> > #define NR_ALLPASS_BANDS20 30
> > #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];
> > static float HA[46][8][4];
> > diff --git a/libavcodec/aacpsdsp.h b/libavcodec/aacpsdsp.h
> > index 93737d2..dc380b1 100644
> > --- a/libavcodec/aacpsdsp.h
> > +++ b/libavcodec/aacpsdsp.h
> > @@ -38,7 +38,7 @@ typedef struct PSDSPContext {
> > int i, int len);
> > 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?
> _______________________________________________
> libav-devel mailing list
> [email protected] <javascript:;>
> https://lists.libav.org/mailman/listinfo/libav-devel
>
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
