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

Babic, Nedeljko nbabic at mips.com
Mon May 21 17:12:58 CEST 2012


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).

> 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.

> 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?


Thanks,
Nedeljko
________________________________________
From: ffmpeg-devel-bounces at ffmpeg.org [ffmpeg-devel-bounces at ffmpeg.org] on behalf of Michael Niedermayer [michaelni at gmx.at]
Sent: Friday, May 18, 2012 17:42
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [PATCH] Optimization of AMR NB and WB decoders      for MIPS

On Fri, May 18, 2012 at 03:47:30PM +0200, Nedeljko Babic wrote:
> AMR NB and WB decoders are optimized for MIPS architecture.
> Appropriate Makefiles are changed accordingly.
>
> Configure script is changed in order to support optimizations.
>  Optimizations are enabled by default when compiling is done for
>   mips architecture.
>  Appropriate cflags are automatically set.
>  Support for several mips CPUs is added in configure script.
>
> New ffmpeg options are added for disabling optimizations.
>
> The FFMPEG option --disable-mipsfpu disables MIPS floating point
>  optimizations.
> The FFMPEG option --disable-mips32r2 disables MIPS32R2
>  optimizations.
> The FFMPEG option --disable-mipsdspr1 disables MIPS DSP ASE R1
>  optimizations.
> The FFMPEG option --disable-mipsdspr2 disables MIPS DSP ASE R2
>  optimizations.
>
> Change-Id: I566311805c05c6dae544c19f9d5194c157910014
> Signed-off-by: Nedeljko Babic <nbabic at mips.com>
> ---
>  configure                            |   47 ++++++
>  libavcodec/acelp_filters.c           |    4 +
>  libavcodec/acelp_vectors.c           |    2 +
>  libavcodec/acelp_vectors.h           |    6 +
>  libavcodec/amrwbdec.c                |    5 +
>  libavcodec/celp_filters.c            |    2 +
>  libavcodec/celp_math.c               |    3 +-
>  libavcodec/celp_math.h               |    4 +
>  libavcodec/lsp.c                     |    6 +
>  libavcodec/lsp.h                     |    6 +
>  libavcodec/mips/Makefile             |    6 +-
>  libavcodec/mips/acelp_filters_mips.c |  211 +++++++++++++++++++++++++
>  libavcodec/mips/acelp_vectors_mips.h |   93 +++++++++++
>  libavcodec/mips/amrwb_lsp2lpc.h      |   97 ++++++++++++
>  libavcodec/mips/amrwbdec_mips.h      |  186 ++++++++++++++++++++++
>  libavcodec/mips/celp_filters_mips.c  |  289 ++++++++++++++++++++++++++++++++++
>  libavcodec/mips/celp_math_mips.h     |   82 ++++++++++
>  libavcodec/mips/lsp_mips.h           |  109 +++++++++++++
>  libavutil/libm.h                     |    4 +
>  libavutil/mips/libm_mips.h           |   74 +++++++++
>  20 files changed, 1234 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/mips/acelp_filters_mips.c
>  create mode 100644 libavcodec/mips/acelp_vectors_mips.h
>  create mode 100644 libavcodec/mips/amrwb_lsp2lpc.h
>  create mode 100644 libavcodec/mips/amrwbdec_mips.h
>  create mode 100644 libavcodec/mips/celp_filters_mips.c
>  create mode 100644 libavcodec/mips/celp_math_mips.h
>  create mode 100644 libavcodec/mips/lsp_mips.h
>  create mode 100644 libavutil/mips/libm_mips.h
>
> diff --git a/configure b/configure
> index e070c0b..b8fe719 100644
> --- a/configure
> +++ b/configure
> @@ -268,6 +268,10 @@ Optimization options (experts only):
>    --disable-neon           disable NEON optimizations
>    --disable-vis            disable VIS optimizations
>    --disable-yasm           disable use of yasm assembler
> +  --disable-mips32r2       disable MIPS32R2 optimizations
> +  --disable-mipsdspr1      disable MIPS DSP ASE R1 optimizations
> +  --disable-mipsdspr2      disable MIPS DSP ASE R2 optimizations
> +  --disable-mipsfpu        disable floating point MIPS optimizations
>    --postproc-version=V     build libpostproc version V.
>                             Where V can be '$ALT_PP_VER_MAJOR.$ALT_PP_VER_MINOR.$ALT_PP_VER_MICRO' or 'current'. [$postproc_version_default]
>
> @@ -1141,6 +1145,10 @@ ARCH_EXT_LIST='
>      ssse3
>      vfpv3
>      vis
> +    mipsfpu
> +    mips32r2
> +    mipsdspr1
> +    mipsdspr2
>  '
>
>  HAVE_LIST_PUB='
> @@ -1359,6 +1367,10 @@ armvfp_deps="arm"
>  neon_deps="arm"
>  vfpv3_deps="armvfp"
>
> +mipsfpu_deps="mips"
> +mips32r2_deps="mips"
> +mipsdspr1_deps="mips"
> +mipsdspr2_deps="mips"
>  mmi_deps="mips"
>
>  altivec_deps="ppc"
> @@ -2577,6 +2589,28 @@ elif enabled mips; then
>
>      cpuflags="-march=$cpu"
>
> +    case $cpu in
> +        24kc)
> +            disable mipsfpu
> +            disable mipsdspr1
> +            disable mipsdspr2
> +        ;;
> +        24kf*)
> +            disable mipsdspr1
> +            disable mipsdspr2
> +        ;;
> +        24kec|34kc|1004kc)
> +            disable mipsfpu
> +            disable mipsdspr2
> +        ;;
> +        24kef*|34kf*|1004kf*)
> +            disable mipsdspr2
> +        ;;
> +        74kc)
> +            disable mipsfpu
> +        ;;
> +    esac
> +
>  elif enabled avr32; then
>
>      case $cpu in
> @@ -2948,6 +2982,15 @@ elif enabled mips; then
>
>      check_asm loongson '"dmult.g $1, $2, $3"'
>      enabled mmi     && check_asm mmi     '"lq $2, 0($2)"'
> +    enabled mips32r2  && add_cflags "-mips32r2" &&
> +     check_asm mips32r2  '"rotr $t0, $t1, 1"'
> +    enabled mipsdspr1 && add_cflags "-mdsp" &&
> +     check_asm mipsdspr1 '"addu.qb $t0, $t1, $t2"'
> +    enabled mipsdspr2 && add_cflags "-mdspr2" &&
> +     check_asm mipsdspr2 '"absq_s.qb $t0, $t1"'
> +    enabled mipsfpu   && add_cflags "-mhard-float" &&
> +     check_asm mipsfpu   '"madd.d $f0, $f2, $f4, $f6"'
> +
>
>  elif enabled ppc; then
>
> @@ -3547,6 +3590,10 @@ if enabled arm; then
>  fi
>  if enabled mips; then
>      echo "MMI enabled               ${mmi-no}"
> +    echo "MIPS FPU enabled          ${mipsfpu-no}"
> +    echo "MIPS32R2 enabled          ${mips32r2-no}"
> +    echo "MIPS DSP R1 enabled       ${mipsdspr1-no}"
> +    echo "MIPS DSP R2 enabled       ${mipsdspr2-no}"
>  fi
>  if enabled ppc; then
>      echo "AltiVec enabled           ${altivec-no}"
> diff --git a/libavcodec/acelp_filters.c b/libavcodec/acelp_filters.c
> index 1ce5eed..f623212 100644


> --- a/libavcodec/acelp_filters.c
> +++ b/libavcodec/acelp_filters.c
> @@ -73,6 +73,7 @@ void ff_acelp_interpolate(int16_t* out, const int16_t* in,
>      }
>  }
>
> +#if !HAVE_MIPSFPU || !HAVE_INLINE_ASM
>  void ff_acelp_interpolatef(float *out, const float *in,
>                             const float *filter_coeffs, int precision,
>                             int frac_pos, int filter_length, int length)
> @@ -92,6 +93,7 @@ void ff_acelp_interpolatef(float *out, const float *in,
>          out[n] = v;
>      }
>  }
> +#endif /* !HAVE_MIPSFPU || !HAVE_INLINE_ASM */
>
>
>  void ff_acelp_high_pass_filter(int16_t* out, int hpf_f[2],

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 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



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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


More information about the ffmpeg-devel mailing list