[FFmpeg-devel] [PATCH] Extra build options for ALS (and others)

[Thilo Borgmann]

M?ns Rullg?rd schrieb:
> Thilo Borgmann <thilo.borgmann at googlemail.com> writes:
> 
>> M?ns Rullg?rd schrieb:
>>> Thilo Borgmann <thilo.borgmann at googlemail.com> writes:
>>>
>>>> Hi,
>>>>
>>>> recently the need for an extra build option for the ALS decoder arose.
>>> Is it impossible to achieve the desired outcome with some combination
>>> of always_inline, noinline, and flatten attributes?
>> No. See [PATCH] Split reading and decoding of blocks in ALS.
>>
>> Although I've managed to have the functions from the alsdec.c inlined
>> manually according to the grep'ed output of the assembler code, it seems
>> like it is not enough to manually inline functions from within that .c
>> file only using these technique.
> 
> I'm confused.  Can it be done in the C code only or not?  This kind of
> issue should really not be solved in the makefile.

The issue is the big slowdown. The patch that causes this splits a big
function into two, which are then called successively.

To overcome the slowdown issue, I inspected the functions being inlined
with and without the -finline-limit option. I can use av_always_inline
for many functions within alsdec.c to have the same functions inlined
like -finline-limit does.

Unfortunately, using -finline-limit removes the slowdown introduced by
the patch while using av_always_inline does not.


> 
>>>> It seems like we are lacking a possibility for this or I have not found
>>>> it... hints, discussion or better approaches welcome.
>>>>
>>>> The applied patch extends the implicit build rule for the object file,
>>>> allowing to define extra compiler/linker/etc. options per file.
>>> We've never had a need for this before, so there is no clean way of
>>> doing it.  I'll have to think about how to do it properly, but first
>>> answer the question above.
>> In some way there is something like that already. In libavcodec/Makefile
>> there is a block which is also extending the implicit rule that way:
>>
>> ifdef CONFIG_MPEGAUDIO_HP
>> $(SUBDIR)mpegaudio_tablegen$(HOSTEXESUF): HOSTCFLAGS += -DFRAC_BITS=23
>> $(SUBDIR)mpegaudio_tablegen.ho: CPPFLAGS += -DFRAC_BITS=23
>> else
>> $(SUBDIR)mpegaudio_tablegen$(HOSTEXESUF): HOSTCFLAGS += -DFRAC_BITS=15
>> $(SUBDIR)mpegaudio_tablegen.ho: CPPFLAGS += -DFRAC_BITS=15
>> endif
> 
> I'd like to get rid of that somehow.
> 
>> That way is also suggested by the GNU make documentation:
>> "... You would want to write a rule for foo.o with no command lines if
>> you need to specify additional prerequisites, such as header files, that
>> the implicit rule cannot supply."
>> from:
>> http://www.gnu.org/software/make/manual/html_node/Using-Implicit.html#Using-Implicit
> 
> I know it's legal make syntax.  That's not the issue here.
> 
>>>> Regards,
>>>> Thilo
>>>> Index: libavcodec/Makefile
>>>> ===================================================================
>>>> --- libavcodec/Makefile	(Revision 20627)
>>>> +++ libavcodec/Makefile	(Arbeitskopie)
>>>> @@ -%ld,%ld +%ld,%ld @@
>>>>  SKIPHEADERS-$(CONFIG_VDPAU)            += vdpau.h
>>>>  SKIPHEADERS-$(CONFIG_XVMC)             += xvmc.h
>>>>
>>>> +# extra build options
>>>> +$(SUBDIR)alsdec.o: CFLAGS += -finline-limit=4096
>>> This will break with many non-gcc compilers.
>> I think it might be feasible to adopt this to the compiler in use, not?
>> How many compilers do we care for in such a sense?
>> We might even check for "-finline-limit" in the configure script to use
>> it at least with gcc or accept 10% slowdown on "unsupported" compilers...
> 
> I think using -finline-limit is the wrong solution here.  It's a
> brute-force approach that happens to work for now.  If the code
> changes, it may no longer have the right effect.

Of course, any other approach that significantly reduces the slowdown
would be preferable - but I don't see any...


-Thilo