On Mon, Nov 4, 2013 at 10:14 AM, Diego Biurrun <di...@biurrun.de> 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.
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
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
