[FFmpeg-cvslog] r21219 - in trunk/libavcodec: aac.c arm/aac.h

[Michael Niedermayer]

On Sat, Jan 16, 2010 at 12:27:45PM +0000, M?ns Rullg?rd wrote:
> Alex Converse <alex.converse at gmail.com> writes:
> 
> > 2010/1/15 M?ns Rullg?rd <mans at mansr.com>:
> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >>
> >>> On Fri, Jan 15, 2010 at 09:17:24PM +0000, M?ns Rullg?rd wrote:
> >>>> Michael Niedermayer <michaelni at gmx.at> writes:
> >>>>
> >>>> > maybe a header that describes in english what the functions do and contains
> >>>> > ISO C implementations named like VMUL2_C.
> >>>> > And a seperate header for each arch that contains a full set of these
> >>>> > implementations using "#define VMUL2 VMUL2_C" where no better is available.
> >>>> > Also there would then be a ISO C only header that contained a
> >>>> > #define VMUL2 VMUL2_C
> >>>> > for every function.
> >>>> > That way there would be no ifdefs except for including the right header
> >>>>
> >>>> That would require creating and maintaining dummy headers for every
> >>>> arch. ?I consider that for more of a burden than looking at 4 simple
> >>>> ifdefs from time to time.
> >>>
> >>> I dont think it would need dummy headers
> >>>
> >>> #if ARCH_X86
> >>> #include "x86/foo.h"
> >>> #else if ARCH_ARM
> >>> #include "arm/foo.h"
> >>> #else
> >>> #include "foo.h"
> >>> #endif
> >>> // not yet implemented for alpha & sparc
> >>
> >> What if only a subset is implemented for some arch?
> >>
> >>> also, having a complete list of optimized functions in each arch file is a
> >>> great reminder of what is not optimized yet and could be optimized.
> >>
> >> It also means that any changes have to be done to *all* arch headers.
> >> I know from experience that this will not work very well.
> >>
> >
> > It doesn't ahve to be taht bad. Consider...
> >
> > +++ aacopt.h
> > static av_always_inline float FUNC_C(...) {
> >     //ISO C90 stuff
> > }
> > #define FUNC FUNC_C
> >
> > +++ arm/aac.h
> > #include "aacopt.h"
> > static av_always_inline float FUNC_NEON(...) {
> >    //Your super awesome neon stuff
> > }
> > #undef FUNC
> > #define FUNC FUNC_NEON
> >
> > Adding new functions doesn't break any other archs.
> >
> > The unused static functions should be eliminated at compile time.
> 
> If you're lucky.
> 
> > Everybody wins!
> 
> I consider your suggestion uglier than mine.
> 
> >>> anyway iam not aac maintainer so its none of my business but for files
> >>> i maintain i dont want such ifdefery
> >>
> >> Files you maintain? ?Bah! ?As far as I can tell, I'm the only one
> >> doing anything resembling $arch maintenance, and I've *reduced* the
> >> amount of ugly ifdeffery substantially compared to what it used to be,
> >> so please don't complain to me. ?There is still a lot of cleanup work
> >> to be done, but I hope to get round to it sooner or later.
> >
> > I was unaware cleaning up after others gives you license to make new messes.
> 
> I disagree about me having creating anything remotely messy.  We've
> have the same scheme in use in numerous places (mathops.h, bswap.h,
> intreadwrite.h, ...) for years without anyone complaining.  Why the
> sudden outrage?

i definitly dont and didnt like how intreadwrite.h was done
And i would much prefer it to be done without the completely opaque
ifdefery
If for example i want to debug AV_WL24() due to a bug i have to trace
my way through quite a bit of code to find out what function in what file
actually implements it. (unless i happen to know it alraedy)

Ive had the same problem with some of your scripts. No doubt they are
very compact but its hard to work with them, normaly one just greps for
what one needs  .... well heres an example
where is the code that generates the line of 
198667 ./tests/data/a-mpeg2.mpg
?
grep a-mpeg2.mpg fails
what one ends up to have to do is reverse engeneer the obfuscation to
then find that the mpeg2 is in a list in the Makefile of the parent directory
because some wise person considered it a good idea to remoev a- and .mpg
and use lots layers over layers of complexity to put it back.
The original script from fabrices times was something that anyone could
open in an editor and change t his needs, what we have now is code that
is only understood by 2-3 people. This isnt speed critical it doesnt
benefit from this entangled compaction IMO


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20100116/2a90a4fb/attachment-0001.pgp>