[MPlayer-dev-eng] [PATCH] Remove duplicated CPU detection in libmpeg2

[Guillaume Poirier]

Hello Jim,

jserv at linux2.cc.ntu.edu.tw wrote:
> Hello list,
> 
>   While implementing SSE2-optimized idct and motion compensation
> routines for libmpeg2 in MPlayer, I found that there is a duplicated cpu
> detection routine in function arch_accel of libmpeg2/cpu_accel.c when
> arch = x86 or x86_64. The attachment contains a small patch to use
> MPlayer's cpu capability property instead of hand-coded assembly. Also,
> the patch added MPEG2_ACCEL_X86_SSE2 definition for identification of
> Intel SSE2 instructions.

Looks good to me, expect the point mention below.

[..]

> Index: libmpeg2/cpu_accel.c
> ===================================================================
> --- libmpeg2/cpu_accel.c	(revision 18717)
> +++ libmpeg2/cpu_accel.c	(working copy)
> @@ -26,6 +26,7 @@
>   */
>  
>  #include "config.h"
> +#include "cpudetect.h"
>  
>  #include <inttypes.h>
>  
> @@ -37,6 +38,7 @@
>  #if defined(ARCH_X86) || defined(ARCH_X86_64)
>  static inline uint32_t arch_accel (void)
>  {
> +#if 0 /* MPlayer has its own CPU detection mechanisms */

I think it would be better (because it would make the code more
explicit instead of relying only on the comment) to have smth like that:

#define USE_MPLAYER_CPUDETECT

[..]

#ifndef USE_MPLAYER_CPUDETECT

/* libmpeg cpudetect */

#else

/* Use MPlayer's cpu capability property */

#endif


another (probably cleaner) way to do the same thing would be:


#ifndef CPUDETECT_H

/* libmpeg cpudetect */

#else

/* Use MPlayer's cpu capability property */

#endif



What do you guys think?

I'll commit the 2nd version in a few days if no one objects...


Guillaume