[FFmpeg-devel] [PATCH] lavc/aacenc_quantization: use cbrt table

[Reimar Döffinger]

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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cbrt_table2.diff
Type: text/x-diff
Size: 9118 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160312/402d22d2/attachment.diff>