[FFmpeg-devel] [PATCH] hardcoded ff_cos tables

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:

> On Wed, Oct 14, 2009 at 07:15:58PM +0200, Reimar D?ffinger wrote:
>> > As for the need, everything with external linkage should be
>> > declared in some header IMHO.  Furthermore, someone might want to use
>> > those tables from C code too, not just asm.
>> 
>> Which is a completely unrelated, already existing mess.
>> And no, I can't see a reason to use those tables from C code, that's

Same reason as using it from asm...

>> what ff_cos_tab is there for (and it _might_ even make sense to use it
>> from asm code).

That would be slower.

> This patch should demonstrate it is an independent issue, it needs no
> extra header file and eliminates a good bit of code duplication.

I fail to see how this patch demonstrates anything related to that
issue.  There are still external symbols with no declaration in a
header file.  On the contrary, you now have symbols referenced in
multiple places with no common declaration.  This is *always* a bad
thing.

> Index: libavcodec/Makefile
> ===================================================================
> --- libavcodec/Makefile	(revision 20231)
> +++ libavcodec/Makefile	(working copy)
> @@ -27,7 +27,8 @@
>  # parts needed for many different codecs
>  OBJS-$(CONFIG_AANDCT)                  += aandcttab.o
>  OBJS-$(CONFIG_ENCODERS)                += faandct.o jfdctfst.o jfdctint.o
> -OBJS-$(CONFIG_FFT)                     += fft.o
> +FFT-OBJS-$(CONFIG_HARDCODED_TABLES)    += cos_tables.o
> +OBJS-$(CONFIG_FFT)                     += fft.o $(FFT-OBJS-yes)
>  OBJS-$(CONFIG_GOLOMB)                  += golomb.o
>  OBJS-$(CONFIG_MDCT)                    += mdct.o
>  OBJS-$(CONFIG_RDFT)                    += rdft.o
> @@ -571,6 +572,14 @@
>  
>  DIRS = alpha arm bfin mlib ppc ps2 sh4 sparc x86
>  
> +CLEANFILES = cos_tables.c costablegen$(HOSTEXESUF)
> +
>  include $(SUBDIR)../subdir.mak
>  
>  $(SUBDIR)dct-test$(EXESUF): $(SUBDIR)dctref.o
> +
> +$(SUBDIR)costablegen$(HOSTEXESUF): $(SUBDIR)costablegen.c
> +	$(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $< $(HOSTLIBS)
> +
> +$(SUBDIR)cos_tables.c: $(SUBDIR)costablegen$(HOSTEXESUF)
> +	$(BUILD_ROOT)/libavcodec/costablegen$(HOSTEXESUF) > $@

Please keep the ./$< you had before.  It's more readable, and
obviously works when I'm thinking.

-- 
M?ns Rullg?rd
mans at mansr.com