On Sat, Mar 12, 2016 at 10:21:10AM -0500, Ganesh Ajjanagadde wrote:
> Ok. Let me put it this way: I have a super simple patch that simply
> moves stuff to cbrt_data.c and works perfectly well under default
> configure, with no changes to the Makefile apart from adding
> cbrt_data.o to the list of objects for AAC.
>
> As soon as I test under --enable-hardcoded-tables, I get linker errors
> during the host table generation phase due to undefined ff_cbrt_tab,
> etc. I also know why they occur: I declare ff_cbrt_tab as extern in
> cbrt_tablegen.h, so basically during host generation phase cbrt_data.c
> needs to also get compiled in, where the implementation is done. But
> the host gen simply #includes cbrt_tablegen.h, sees the extern
> declaration (since during host gen it #defs hardcoded to 0, that is
> how it gets the "magic" done), does not find the implementation, and
> naturally complains.
>
> So one could of course duplicate the code in tablegen_template etc and
> not #include cbrt_tablegen, it can actually be very short since we
> don't care about efficiency there and the fancy current algorithm does
> not need to be used.
>
> But whatever it is, be it Makefile chicanery or some ad-hoc methods
> like above, something needs to be done; there is no "natural"
> solution.
>
> TL;DR: I am not going to waste time on this. Anyone wants to do it, it
> is conceptually trivial.
I really don't see what the problem is??
Attached is a (approx.) 4 line change that avoids the Makefile
mess and does not duplicate code.
It needs one hack due to config.h usage in cbrt_data.c.
It and the ugly cbrt_data.c include in the table generator
could be avoided by simply leaving ff_cbrt_tab and the table
generation code in cbrt_tablegen.h and using a cbrt_data.h
in aacenc.c etc. which is more logical anyway than having
a cbrt_tablegen.h header that contains declarations for
stuff that is actually in cbrt_data.c.
It seems to me the only problem here was caused by
trying to physically move stuff into cbrt_data.c instead
of just including whatever needs to be exported for
use by the other files, and in addition (mis?) using
the cbrt_tablegen.h header for the stuff in cbrt_data.c.
I can of course try to clean up these last things,
but I'm not really familiar with the code and
some of the changes seem not completely trivial
(e.g. the aacenc_quantization.h one) so I'd rather not.
(I admit that there must be something about that
tablegen code that is genuinely aligned with my
thinking but not most else's, because to me it
looks clean and obvious while a lot of people think
it is a confusing horror... I have been unable to
figure out a way to fix this)
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 1cd9572..fca1da0 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -137,17 +137,17 @@ OBJS-$(CONFIG_A64MULTI_ENCODER) += a64multienc.o elbg.o
OBJS-$(CONFIG_A64MULTI5_ENCODER) += a64multienc.o elbg.o
OBJS-$(CONFIG_AAC_DECODER) += aacdec.o aactab.o aacsbr.o aacps_float.o \
aacadtsdec.o mpeg4audio.o kbdwin.o \
- sbrdsp.o aacpsdsp_float.o
+ sbrdsp.o aacpsdsp_float.o cbrt_data.o
OBJS-$(CONFIG_AAC_FIXED_DECODER) += aacdec_fixed.o aactab.o aacsbr_fixed.o aacps_fixed.o \
aacadtsdec.o mpeg4audio.o kbdwin.o \
- sbrdsp_fixed.o aacpsdsp_fixed.o
+ sbrdsp_fixed.o aacpsdsp_fixed.o cbrt_data.o
OBJS-$(CONFIG_AAC_ENCODER) += aacenc.o aaccoder.o aacenctab.o \
aacpsy.o aactab.o \
aacenc_is.o \
aacenc_tns.o \
aacenc_ltp.o \
aacenc_pred.o \
- psymodel.o mpeg4audio.o kbdwin.o
+ psymodel.o mpeg4audio.o kbdwin.o cbrt_data.o
OBJS-$(CONFIG_AASC_DECODER) += aasc.o msrledec.o
OBJS-$(CONFIG_AC3_DECODER) += ac3dec_float.o ac3dec_data.o ac3.o kbdwin.o
OBJS-$(CONFIG_AC3_FIXED_DECODER) += ac3dec_fixed.o ac3dec_data.o ac3.o kbdwin.o
@@ -1017,8 +1017,7 @@ $(GEN_HEADERS): $(SUBDIR)%_tables.h: $(SUBDIR)%_tablegen$(HOSTEXESUF)
$(M)./$< > $@
ifdef CONFIG_HARDCODED_TABLES
-$(SUBDIR)aacdec.o: $(SUBDIR)cbrt_tables.h
-$(SUBDIR)aacdec_fixed.o: $(SUBDIR)cbrt_fixed_tables.h
+$(SUBDIR)cbrt_data.o: $(SUBDIR)cbrt_tables.h
$(SUBDIR)aacps_float.o: $(SUBDIR)aacps_tables.h
$(SUBDIR)aacps_fixed.o: $(SUBDIR)aacps_fixed_tables.h
$(SUBDIR)aactab_fixed.o: $(SUBDIR)aac_fixed_tables.h
diff --git a/libavcodec/aacdec_fixed.c b/libavcodec/aacdec_fixed.c
index 396a874..04ebe99 100644
--- a/libavcodec/aacdec_fixed.c
+++ b/libavcodec/aacdec_fixed.c
@@ -155,9 +155,9 @@ static void vector_pow43(int *coefs, int len)
for (i=0; i<len; i++) {
coef = coefs[i];
if (coef < 0)
- coef = -(int)cbrt_tab[-coef];
+ coef = -(int)ff_cbrt_tab_fixed[-coef];
else
- coef = (int)cbrt_tab[coef];
+ coef = (int)ff_cbrt_tab_fixed[coef];
coefs[i] = coef;
}
}
diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index 6bc94c8..883ed52 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -1104,7 +1104,7 @@ static av_cold void aac_static_table_init(void)
AAC_RENAME(ff_init_ff_sine_windows)( 9);
AAC_RENAME(ff_init_ff_sine_windows)( 7);
- AAC_RENAME(cbrt_tableinit)();
+ AAC_RENAME(ff_cbrt_tableinit)();
}
static AVOnce aac_table_init = AV_ONCE_INIT;
@@ -1795,7 +1795,7 @@ static int decode_spectrum_and_dequant(AACContext *ac, INTFLOAT coef[1024],
v = -v;
*icf++ = v;
#else
- *icf++ = cbrt_tab[n] | (bits & 1U<<31);
+ *icf++ = ff_cbrt_tab[n] | (bits & 1U<<31);
#endif /* USE_FIXED */
bits <<= 1;
} else {
diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
index 023260a..863df65 100644
--- a/libavcodec/aacenc.c
+++ b/libavcodec/aacenc.c
@@ -45,6 +45,7 @@
#include "aacenc.h"
#include "aacenctab.h"
#include "aacenc_utils.h"
+#include "cbrt_tablegen.h"
#include "psymodel.h"
@@ -897,6 +898,7 @@ alloc_fail:
static av_cold void aac_encode_init_tables(void)
{
ff_aac_tableinit();
+ AAC_RENAME(ff_cbrt_tableinit)();
}
static av_cold int aac_encode_init(AVCodecContext *avctx)
diff --git a/libavcodec/aacenc_quantization.h b/libavcodec/aacenc_quantization.h
index 4250407..b20669c 100644
--- a/libavcodec/aacenc_quantization.h
+++ b/libavcodec/aacenc_quantization.h
@@ -32,6 +32,7 @@
#include "aacenc.h"
#include "aacenctab.h"
#include "aacenc_utils.h"
+#include "cbrt_tablegen.h"
/**
* Calculate rate distortion cost for quantizing with given codebook
@@ -105,7 +106,7 @@ static av_always_inline float quantize_and_encode_band_cost_template(
curbits += 21;
} else {
int c = av_clip_uintp2(quant(t, Q, ROUNDING), 13);
- quantized = c*cbrtf(c)*IQ;
+ quantized = av_int2float(ff_cbrt_tab[c])*IQ;
curbits += av_log2(c)*2 - 4 + 1;
}
} else {
diff --git a/libavcodec/cbrt_tablegen.c b/libavcodec/cbrt_tablegen.c
index 8c2235e..60ce8ad 100644
--- a/libavcodec/cbrt_tablegen.c
+++ b/libavcodec/cbrt_tablegen.c
@@ -20,5 +20,22 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#define USE_FIXED 0
-#include "cbrt_tablegen_template.c"
+#include <stdlib.h>
+#define CONFIG_HARDCODED_TABLES 0
+#include "libavutil/tablegen.h"
+#include "cbrt_tablegen.h"
+#include "tableprint.h"
+#include "cbrt_data.c"
+
+int main(void)
+{
+ ff_cbrt_tableinit();
+ ff_cbrt_tableinit_fixed();
+
+ write_fileheader();
+
+ WRITE_ARRAY("const", uint32_t, ff_cbrt_tab);
+ WRITE_ARRAY("const", uint32_t, ff_cbrt_tab_fixed);
+
+ return 0;
+}
diff --git a/libavcodec/cbrt_tablegen.h b/libavcodec/cbrt_tablegen.h
index 21e4b9a..446f9c1 100644
--- a/libavcodec/cbrt_tablegen.h
+++ b/libavcodec/cbrt_tablegen.h
@@ -29,55 +29,18 @@
#include "libavutil/intfloat.h"
#include "libavcodec/aac_defines.h"
-#if USE_FIXED
-#define CBRT(x) lrint((x) * 8192)
-#else
-#define CBRT(x) av_float2int((float)(x))
-#endif
-
#if CONFIG_HARDCODED_TABLES
-#if USE_FIXED
-#define cbrt_tableinit_fixed()
-#include "libavcodec/cbrt_fixed_tables.h"
-#else
-#define cbrt_tableinit()
-#include "libavcodec/cbrt_tables.h"
-#endif
+#define ff_cbrt_tableinit_fixed()
+#define ff_cbrt_tableinit()
+extern const uint32_t ff_cbrt_tab[1 << 13];
+extern const uint32_t ff_cbrt_tab_fixed[1 << 13];
#else
-static uint32_t cbrt_tab[1 << 13];
-
-static av_cold void AAC_RENAME(cbrt_tableinit)(void)
-{
- static double cbrt_tab_dbl[1 << 13];
- if (!cbrt_tab[(1<<13) - 1]) {
- int i, j, k;
- double cbrt_val;
-
- for (i = 1; i < 1<<13; i++)
- cbrt_tab_dbl[i] = 1;
-
- /* have to take care of non-squarefree numbers */
- for (i = 2; i < 90; i++) {
- if (cbrt_tab_dbl[i] == 1) {
- cbrt_val = i * cbrt(i);
- for (k = i; k < 1<<13; k *= i)
- for (j = k; j < 1<<13; j += k)
- cbrt_tab_dbl[j] *= cbrt_val;
- }
- }
+extern uint32_t ff_cbrt_tab[1 << 13];
+extern uint32_t ff_cbrt_tab_fixed[1 << 13];
- for (i = 91; i <= 8191; i+= 2) {
- if (cbrt_tab_dbl[i] == 1) {
- cbrt_val = i * cbrt(i);
- for (j = i; j < 1<<13; j += i)
- cbrt_tab_dbl[j] *= cbrt_val;
- }
- }
+av_cold void ff_cbrt_tableinit_fixed(void);
+av_cold void ff_cbrt_tableinit(void);
- for (i = 0; i < 1<<13; i++)
- cbrt_tab[i] = CBRT(cbrt_tab_dbl[i]);
- }
-}
#endif /* CONFIG_HARDCODED_TABLES */
#endif /* AVCODEC_CBRT_TABLEGEN_H */
diff --git a/libavcodec/cbrt_tablegen_template.c b/libavcodec/cbrt_tablegen_template.c
deleted file mode 100644
index 7dcab91..0000000
--- a/libavcodec/cbrt_tablegen_template.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Generate a header file for hardcoded AAC cube-root table
- *
- * Copyright (c) 2010 Reimar Döffinger <reimar.doeffin...@gmx.de>
- *
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include <stdlib.h>
-#define CONFIG_HARDCODED_TABLES 0
-#include "libavutil/tablegen.h"
-#include "cbrt_tablegen.h"
-#include "tableprint.h"
-
-int main(void)
-{
- AAC_RENAME(cbrt_tableinit)();
-
- write_fileheader();
-
- WRITE_ARRAY("static const", uint32_t, cbrt_tab);
-
- return 0;
-}
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
