[vlc-devel] [PATCH] add ARM/NEON conversions for audio_filter/channel_mixer/simple
remi at remlab.net
Wed Apr 4 16:21:59 CEST 2012
Le mercredi 4 avril 2012 16:03:45 David Geldreich, vous avez écrit :
> Le 4 avr. 2012 à 14:36, Rémi Denis-Courmont a écrit :
> > We have a separate directory for NEON acceleration plugins, aptly named
> > arm_neon. Please stick the code there in a dedicated plugin.
> As j-b mentioned, this is a tradeoff between duplicating the code or
> "polluting" the original file. Also, I am not expert with the VLC code
> architecture so all advices are welcome.
What do you think will happen when someone adds another series of
optimizations? The plugin code will become unreadable. We've been there and
done that with the deinterlacer already. No thanks.
Also if someone adds another unaccelerated case, you cannot expect him/her to
know NEON and be able to write the corresponding assembler.
> >> inline assembly is in separate functions for clarity and will be inlined
> >> by the compiler
> > Yes but inlined assembler is harder to read and it cannot selected at
> > run-time. The overhead of a function is neglible here. Inlining assembler
> > makes sense if you want to mix C code, especially for branching. But you
> > have already implemented branching in assembler anyway. So you might as
> > well use a dedicated assembler source file, then.
> There is some branching in the DoWork function to switch between all the
> input/output combinations
In other C functions, so totally irrelevant and orthogonal to my point.
Besides, all those if's really should be done once during plugin instantiation
rather than at every filtering iteration. I do not expect you to clean up the
existing code, but please don't reproduce the same mistakes again.
More information about the vlc-devel