[FFmpeg-devel] [PATCH] Optimization of AMR NB and WB decoders for MIPS

[Babic, Nedeljko]

Hello Michael,

> that is a shitty design, i would suggest that this is fixed either
> in kernel or hardware so that theres a portable way to get this
> information from user space at least in the medium term future.

We are already aver that this is a serious problem and there is already
work in progress for solving this problem. However it will not be solved soon.

> for functions that could be optimized in SIMD SSE* easily, structures
> with function pointers should be added. Because SSE is runtime
> detectable and once SSE optimizations for them are written there will
> be function pointers in structures for them. It would be quite
> inconvenient if we had a different system for these cases for MIPS in
> place at that point.

This is good suggestion although some optimizaitons would be less efficient this way.
For example small functions that are inlined would not be inlined anymore and we would
loose on performance. The other example where this solution would be impractical to 
implement is with static functions that we optimized.


Regards,
Nedeljko
________________________________________
From: ffmpeg-devel-bounces at ffmpeg.org [ffmpeg-devel-bounces at ffmpeg.org] on behalf of Michael Niedermayer [michaelni at gmx.at]
Sent: Tuesday, May 22, 2012 17:31
To: FFmpeg development discussions and patches
Cc: Lukac, Zeljko
Subject: Re: [FFmpeg-devel] [PATCH] Optimization of AMR NB and WB       decoders        for MIPS

Hi

On Mon, May 21, 2012 at 03:12:58PM +0000, Babic, Nedeljko wrote:
> Hi,
>
> CPU features can be detected at runtime in MIPS architectures, however this is quite cumbersome.
> For example, information stored in status registers, can't be read from user space.
> On the other hand we could obtain some information from /proc/cpuinfo. This was done in MIPS optimizations for pixman. However, this solution also has problems. For example, some vendors (like Broadcom) have their own version of the /proc/cpuinfo description, where they don't mention at all on which MIPS core these platforms are based on. So, this way of runtime detection would prevent MIPS optimizations although they are available for use.
> You can see discussion regarding this problem on pixman mailing list (http://www.mail-archive.com/pixman@lists.freedesktop.org/msg01577.html).

that is a shitty design, i would suggest that this is fixed either
in kernel or hardware so that theres a portable way to get this
information from user space at least in the medium term future.


>
> > disabling the C functions like this is quite unpractical, consider
> > how this would look with 7 cpu architectures (not to mention the
> > maintaince burden)
> > I think there either should be function pointers in a structure
> > like reimar suggested (like LPCContext) or if this causes a
> > "meassureable" overhead on MIPS and runtime cpu feature detection
> > isnt possible or doesnt make sense for MIPS then
>
> There are a lot of functions that we optimized that are not optimized for other architectures. For most of these functions structures with appropriate function pointers don’t exit. In order to use this approach I would have to make probably a lot of changes in architecture independent parts of code and I am not sure if this is justifiably to do just for our optimizations.

for functions that could be optimized in SIMD SSE* easily, structures
with function pointers should be added. Because SSE is runtime
detectable and once SSE optimizations for them are written there will
be function pointers in structures for them. It would be quite
inconvenient if we had a different system for these cases for MIPS in
place at that point.


>
> > there simply could be a:
> >
> > #ifndef ff_acelp_interpolatef
> > void ff_acelp_interpolatef(float *out, const float *in,
> >                               const float *filter_coeffs, int precision,
> >                               int frac_pos, int filter_length, int length){
> >  ...C code here ...
> > }
> > #endif
> > 
> > and a
> > 
> > void ff_acelp_interpolatef_mips(float *out, const float *in,
> >                               const float *filter_coeffs, int precision,
> >                               int frac_pos, int filter_length, int length){
> >  ... MIPS code here ...
> > }
> > 
> > and in some internal header a
> > #define ff_acelp_interpolatef ff_acelp_interpolatef_mips
>
> This is maybe bather approach for us, but we have a lot of code to submit. This will also create maintenance burden.
>
> What do you suggest how should we proceed regarding this?

what kind of maintaince burden do you see with this case here ?


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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein