[FFmpeg-devel] MPEG-2 Acceleration Refactor

Michael Niedermayer michaelni
Fri Jun 22 13:10:06 CEST 2007


Hi

On Fri, Jun 22, 2007 at 01:44:38AM -0700, Greg Hulands wrote:
> Hi,
> Michael Niedermayer wrote:
> >Hi
> >
> >On Fri, Jun 22, 2007 at 09:08:23AM +0100, M?ns Rullg?rd wrote:
> >[...]
> >  
> >>The inline keyword is nothing but a hint to the compiler that the
> >>function might benefit from being inlined.  The compiler is free to
> >>ignore it or to inline functions without it.  The final choice is
> >>(ideally) that which makes the calling function faster.  Now if some
> >>other function is forcibly inlined, the effect of inlining other
> >>functions can quite reasonably be expected to change, just like had
> >>more code been added to function body itself.  Expecting the decisions
> >>whether to inline other function calls to be unaffected by this
> >>addition of code doesn't seem quite rational to me.
> >>    
> >
> >it does seem quite rational to me
> >i want a attribute which behaves as if i copy and pasted the code or
> >replaced it by a macro but without the source code duplication of the
> >first and uglyness of \ at the end of each line of the second case
> >
> >  
> I have attached the symbols from running the finline-limit=32768 as well 
> as the original ones for comparison. If it turns out that the only 
> solution is to go with a macro, would you have objections to that? 

yes i do have objections to a macro


> It 
> might be ugly, but if it is the only way to get the performance, then 
> there isn't much of a choice. Or would the patch remain pending until 
> gcc put in an attribute that does what is required? I hope the later is 
> not the case.
> 
> I have said before that I'm out of my depth here, so I just want to try 
> and get some clarification on this.

i am not saying how to solve this, the original intent was some hardware
acceleration support for mpeg1/2 with specific hardware _IIRC_, iam just

saying how not to do it, which is
* no slowdown for the normal case
* no messy code which also means not replacing large functions by large
  macros

if the function merge can be done within these limits, that is maybe by
finetuning some command line arguments or other tricks that might be fine
if no other unexpected sideeffects are caused by the solution

if not then this doesnt preclude the specific hw acceleration support it
just has to be done in a way which doesnt need the function merge

also if no clean solution can be found then there will be no solution
in ffmpeg svn, code clarity and maintainability outweights new features


> 
> Does the current code that just have inline in the functions not 
> inlining properly?

the current code has the functions duplicated and they are IIRC called
just from one place so they get always inlined even with gcc

your patch merges the function and here the problems start, gccs very broken
inliner, does not realize that the pairs of calls to each function are
orthogonal and at no time will both be used, always just one depending on
the user parameters
and thus its always better to inline them no matter how large they are or
how large the code cache is


> 
> Is the av_always_inline behaving the same as inline or different?

inline is pretty much just ignored by gcc and av_always_inline has
random sideeffects as you can see
so in the end if you want something inlined which gcc doesnt inline
by default you have to duplicate the code or spend a week finetuning
50 parameters and redoing this for every gcc version

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070622/99fbb32a/attachment.pgp>



More information about the ffmpeg-devel mailing list