| Message ID | 20201120073327.820745-62-andreas.rheinhardt@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | VLC, esp. init_vlc patches | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
On Fri, Nov 20, 2020 at 08:33:26AM +0100, Andreas Rheinhardt wrote: > The csa_tables (which always consist of 32 entries of four byte each, > but the type depends upon whether the decoder is fixed or > floating-point) are currently initialized once during decoder > initialization; yet it turns out that this is actually no benefit: The > code used to initialize these tables takes up 153 (fixed point) and 122 > (floating point) bytes when compiled with GCC 9.3 with -O3 on x64, so it > is better to just hardcode these tables. > > Essentially the same applies to the is_tables: They have a size or 128 > each and the code to initialize them occupies 149 (fixed point) resp. > 140 (floating point) bytes. So hardcode them, too. Is it intended not to use CONFIG_HARDCODED_TABLES ? also if the code generating the tables is removed then it should be made easy to re-build the tables. That would be in the spirit of the *GPL (to be able to change and rebuild a binary\H table for potentially a different use) thx [...]
Michael Niedermayer: > On Fri, Nov 20, 2020 at 08:33:26AM +0100, Andreas Rheinhardt wrote: >> The csa_tables (which always consist of 32 entries of four byte each, >> but the type depends upon whether the decoder is fixed or >> floating-point) are currently initialized once during decoder >> initialization; yet it turns out that this is actually no benefit: The >> code used to initialize these tables takes up 153 (fixed point) and 122 >> (floating point) bytes when compiled with GCC 9.3 with -O3 on x64, so it >> is better to just hardcode these tables. >> >> Essentially the same applies to the is_tables: They have a size or 128 >> each and the code to initialize them occupies 149 (fixed point) resp. >> 140 (floating point) bytes. So hardcode them, too. > > Is it intended not to use CONFIG_HARDCODED_TABLES ? > Yes. It makes no sense to ever generate the tables at runtime if the code for this takes more space than the tables itself. > also if the code generating the tables is removed then it should be made > easy to re-build the tables. That would be in the spirit of the *GPL > (to be able to change and rebuild a binary\H table > for potentially a different use) > How about I refer to the git history for that? (I don't even have the mp3-spec, so I don't know on what part of it these tables are based.) - Andreas
On Fri, Nov 27, 2020 at 08:56:46PM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Fri, Nov 20, 2020 at 08:33:26AM +0100, Andreas Rheinhardt wrote: > >> The csa_tables (which always consist of 32 entries of four byte each, > >> but the type depends upon whether the decoder is fixed or > >> floating-point) are currently initialized once during decoder > >> initialization; yet it turns out that this is actually no benefit: The > >> code used to initialize these tables takes up 153 (fixed point) and 122 > >> (floating point) bytes when compiled with GCC 9.3 with -O3 on x64, so it > >> is better to just hardcode these tables. > >> > >> Essentially the same applies to the is_tables: They have a size or 128 > >> each and the code to initialize them occupies 149 (fixed point) resp. > >> 140 (floating point) bytes. So hardcode them, too. > > > > Is it intended not to use CONFIG_HARDCODED_TABLES ? > > > Yes. It makes no sense to ever generate the tables at runtime if the > code for this takes more space than the tables itself. > > > also if the code generating the tables is removed then it should be made > > easy to re-build the tables. That would be in the spirit of the *GPL > > (to be able to change and rebuild a binary\H table > > for potentially a different use) > > > How about I refer to the git history for that? (I don't even have the > mp3-spec, so I don't know on what part of it these tables are based.) thats probably ok, maybe make the reference so it would work even with a different VCS. (i assume the commit title is uniqe then it together with the git hash would achieve this) thx [...]
diff --git a/libavcodec/mips/compute_antialias_fixed.h b/libavcodec/mips/compute_antialias_fixed.h index a967f67de7..1f395d2302 100644 --- a/libavcodec/mips/compute_antialias_fixed.h +++ b/libavcodec/mips/compute_antialias_fixed.h @@ -59,7 +59,8 @@ static void compute_antialias_mips_fixed(MPADecodeContext *s, GranuleDef *g) { - int32_t *ptr, *csa; + const int32_t *csa; + int32_t *ptr; int n, i; int MAX_lo = 0xffffffff; diff --git a/libavcodec/mips/compute_antialias_float.h b/libavcodec/mips/compute_antialias_float.h index e2b4f29f4a..633eb9589d 100644 --- a/libavcodec/mips/compute_antialias_float.h +++ b/libavcodec/mips/compute_antialias_float.h @@ -63,7 +63,7 @@ static void compute_antialias_mips_float(MPADecodeContext *s, GranuleDef *g) { float *ptr, *ptr_end; - float *csa = &csa_table[0][0]; + const float *csa = &csa_table[0][0]; /* temporary variables */ float in1, in2, in3, in4, in5, in6, in7, in8; float out1, out2, out3, out4; diff --git a/libavcodec/mpegaudiodata.h b/libavcodec/mpegaudiodata.h index ec969353f3..01b1f88cd0 100644 --- a/libavcodec/mpegaudiodata.h +++ b/libavcodec/mpegaudiodata.h @@ -65,9 +65,6 @@ extern uint16_t ff_scale_factor_modshift[64]; extern const uint8_t ff_mpa_pretab[2][22]; -/* table for alias reduction (XXX: store it as integer !) */ -extern const float ff_ci_table[8]; - /* Initialize tables shared between the fixed and * floating point MPEG audio decoders. */ void ff_mpegaudiodec_common_init_static(void); diff --git a/libavcodec/mpegaudiodec_common.c b/libavcodec/mpegaudiodec_common.c index 2ac9bb95bc..4333746e9a 100644 --- a/libavcodec/mpegaudiodec_common.c +++ b/libavcodec/mpegaudiodec_common.c @@ -396,10 +396,6 @@ const uint8_t ff_mpa_pretab[2][22] = { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 3, 3, 3, 2, 0 }, }; -const float ff_ci_table[8] = { - -0.6, -0.535, -0.33, -0.185, -0.095, -0.041, -0.0142, -0.0037, -}; - static av_cold void mpegaudiodec_common_init_static(void) { const uint8_t *huff_sym = mpa_huffsymbols, *huff_lens = mpa_hufflens; diff --git a/libavcodec/mpegaudiodec_fixed.c b/libavcodec/mpegaudiodec_fixed.c index 853c55528d..26aaffe8a6 100644 --- a/libavcodec/mpegaudiodec_fixed.c +++ b/libavcodec/mpegaudiodec_fixed.c @@ -36,6 +36,22 @@ #define OUT_FMT AV_SAMPLE_FMT_S16 #define OUT_FMT_P AV_SAMPLE_FMT_S16P +static const int32_t is_table[2][16] = { + { 0x000000, 0x1B0CB1, 0x2ED9EC, 0x400000, 0x512614, 0x64F34F, 0x800000 }, + { 0x800000, 0x64F34F, 0x512614, 0x400000, 0x2ED9EC, 0x1B0CB1, 0x000000 } +}; + +static const int32_t csa_table[8][4] = { + { 0x36E129F8, 0xDF128056, 0x15F3AA4E, 0xA831565E }, + { 0x386E75F2, 0xE1CF24A5, 0x1A3D9A97, 0xA960AEB3 }, + { 0x3CC6B73A, 0xEBF19FA6, 0x28B856E0, 0xAF2AE86C }, + { 0x3EEEA054, 0xF45B88BC, 0x334A2910, 0xB56CE868 }, + { 0x3FB6905C, 0xF9F27F18, 0x39A90F74, 0xBA3BEEBC }, + { 0x3FF23F20, 0xFD60D1E4, 0x3D531104, 0xBD6E92C4 }, + { 0x3FFE5932, 0xFF175EE4, 0x3F15B816, 0xBF1905B2 }, + { 0x3FFFE34A, 0xFFC3612F, 0x3FC34479, 0xBFC37DE5 } +}; + #include "mpegaudiodec_template.c" #if CONFIG_MP1_DECODER diff --git a/libavcodec/mpegaudiodec_float.c b/libavcodec/mpegaudiodec_float.c index 88438ea21a..612ef55188 100644 --- a/libavcodec/mpegaudiodec_float.c +++ b/libavcodec/mpegaudiodec_float.c @@ -36,6 +36,34 @@ #define OUT_FMT AV_SAMPLE_FMT_FLT #define OUT_FMT_P AV_SAMPLE_FMT_FLTP +static const float is_table[2][16] = { + { 0.000000000000000000e+00, 2.113248705863952637e-01, 3.660253882408142090e-01, + 5.000000000000000000e-01, 6.339746117591857910e-01, 7.886751294136047363e-01, + 1.000000000000000000e+00 }, + { 1.000000000000000000e+00, 7.886751294136047363e-01, 6.339746117591857910e-01, + 5.000000000000000000e-01, 3.660253882408142090e-01, 2.113248705863952637e-01, + 0.000000000000000000e+00 } +}; + +static const float csa_table[8][4] = { + { 8.574929237365722656e-01, -5.144957900047302246e-01, + 3.429971337318420410e-01, -1.371988654136657715e+00 }, + { 8.817420005798339844e-01, -4.717319905757904053e-01, + 4.100100100040435791e-01, -1.353474020957946777e+00 }, + { 9.496286511421203613e-01, -3.133774697780609131e-01, + 6.362511515617370605e-01, -1.263006091117858887e+00 }, + { 9.833145737648010254e-01, -1.819131970405578613e-01, + 8.014013767242431641e-01, -1.165227770805358887e+00 }, + { 9.955177903175354004e-01, -9.457419067621231079e-02, + 9.009436368942260742e-01, -1.090092062950134277e+00 }, + { 9.991605877876281738e-01, -4.096558317542076111e-02, + 9.581949710845947266e-01, -1.040126085281372070e+00 }, + { 9.998992085456848145e-01, -1.419856864959001541e-02, + 9.857006072998046875e-01, -1.014097809791564941e+00 }, + { 9.999931454658508301e-01, -3.699974622577428818e-03, + 9.962931871414184570e-01, -1.003693103790283203e+00 } +}; + #include "mpegaudiodec_template.c" #if CONFIG_MP1FLOAT_DECODER diff --git a/libavcodec/mpegaudiodec_template.c b/libavcodec/mpegaudiodec_template.c index fa75445036..1e8fd6064f 100644 --- a/libavcodec/mpegaudiodec_template.c +++ b/libavcodec/mpegaudiodec_template.c @@ -99,9 +99,7 @@ typedef struct MPADecodeContext { #include "mpegaudio_tablegen.h" /* intensity stereo coef table */ -static INTFLOAT is_table[2][16]; static INTFLOAT is_table_lsf[2][2][16]; -static INTFLOAT csa_table[8][4]; /* [i][j]: 2^(-j/3) * FRAC_ONE * 2^(i+2) / (2^(i+2) - 1) */ static int32_t scale_factor_mult[15][3]; @@ -258,22 +256,6 @@ static av_cold void decode_init_static(void) mpegaudio_tableinit(); - for (i = 0; i < 7; i++) { - float f; - INTFLOAT v; - if (i != 6) { - f = tan((double)i * M_PI / 12.0); - v = FIXR(f / (1.0 + f)); - } else { - v = FIXR(1.0); - } - is_table[0][ i] = v; - is_table[1][6 - i] = v; - } - /* invalid values */ - for (i = 7; i < 16; i++) - is_table[0][i] = is_table[1][i] = 0.0; - for (i = 0; i < 16; i++) { double f; int e, k; @@ -289,24 +271,6 @@ static av_cold void decode_init_static(void) (float) is_table_lsf[j][1][i]); } } - - for (i = 0; i < 8; i++) { - double ci, cs, ca; - ci = ff_ci_table[i]; - cs = 1.0 / sqrt(1.0 + ci * ci); - ca = cs * ci; -#if !USE_FLOATS - csa_table[i][0] = FIXHR(cs/4); - csa_table[i][1] = FIXHR(ca/4); - csa_table[i][2] = FIXHR(ca/4) + FIXHR(cs/4); - csa_table[i][3] = FIXHR(ca/4) - FIXHR(cs/4); -#else - csa_table[i][0] = cs; - csa_table[i][1] = ca; - csa_table[i][2] = ca + cs; - csa_table[i][3] = ca - cs; -#endif - } RENAME(ff_mpa_synth_init)(); ff_mpegaudiodec_common_init_static(); } @@ -970,7 +934,8 @@ static void compute_stereo(MPADecodeContext *s, GranuleDef *g0, GranuleDef *g1) { int i, j, k, l; int sf_max, sf, len, non_zero_found; - INTFLOAT (*is_tab)[16], *tab0, *tab1, v1, v2; + INTFLOAT *tab0, *tab1, v1, v2; + const INTFLOAT (*is_tab)[16]; SUINTFLOAT tmp0, tmp1; int non_zero_found_short[3];
The csa_tables (which always consist of 32 entries of four byte each, but the type depends upon whether the decoder is fixed or floating-point) are currently initialized once during decoder initialization; yet it turns out that this is actually no benefit: The code used to initialize these tables takes up 153 (fixed point) and 122 (floating point) bytes when compiled with GCC 9.3 with -O3 on x64, so it is better to just hardcode these tables. Essentially the same applies to the is_tables: They have a size or 128 each and the code to initialize them occupies 149 (fixed point) resp. 140 (floating point) bytes. So hardcode them, too. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- The MIPS changes are untested; they are intended to avoid a discard-qualifier warning. libavcodec/mips/compute_antialias_fixed.h | 3 +- libavcodec/mips/compute_antialias_float.h | 2 +- libavcodec/mpegaudiodata.h | 3 -- libavcodec/mpegaudiodec_common.c | 4 --- libavcodec/mpegaudiodec_fixed.c | 16 ++++++++++ libavcodec/mpegaudiodec_float.c | 28 ++++++++++++++++ libavcodec/mpegaudiodec_template.c | 39 ++--------------------- 7 files changed, 49 insertions(+), 46 deletions(-)