[x264-devel] [PATCH] OpenBSD support... again.

[Dominik 'Rathann' Mierzejewski]

On Saturday, 20 September 2008 at 18:09, Brad wrote:
> Here are a handful of patches to fix x264 on OpenBSD... again.

I'm not directly involved with x264, but here are some comments.

[...]
> Switch from using the signals method of AltiVec detection to using
> the machdep.altivec sysctl and add support for detecting the number
> of CPUs in the system using the hw.ncpu sysctl.
> 
> $OpenBSD: patch-common_cpu_c,v 1.3 2008/08/28 15:23:31 brad Exp $
> --- common/cpu.c.orig	Sun Jul 13 16:45:06 2008
> +++ common/cpu.c	Sat Sep  6 15:22:29 2008
> @@ -32,6 +32,10 @@
>  #include <sys/types.h>
>  #include <sys/sysctl.h>
>  #endif
> +#ifdef SYS_OPENBSD
> +#include <sys/param.h>
> +#include <sys/sysctl.h>
> +#endif
>  
>  #include "common.h"
>  #include "cpu.h"
> @@ -182,13 +186,20 @@ uint32_t x264_cpu_detect( void )
>  
>  #elif defined( ARCH_PPC )
>  
> -#ifdef SYS_MACOSX
> +#if defined(SYS_MACOSX) || defined(SYS_OPENBSD)
>  #include <sys/sysctl.h>
> +#ifdef SYS_OPENBSD
> +#include <machine/cpu.h>
> +#endif
>  uint32_t x264_cpu_detect( void )
>  {
>      /* Thank you VLC */
>      uint32_t cpu = 0;
> +#ifdef SYS_OPENBSD
> +    int      selectors[2] = { CTL_MACHDEP, CPU_ALTIVEC };
> +#else
>      int      selectors[2] = { CTL_HW, HW_VECTORUNIT };
> +#endif
>      int      has_altivec = 0;
>      size_t   length = sizeof( has_altivec );
>      int      error = sysctl( selectors, 2, &has_altivec, &length, NULL, 0 );
> @@ -286,6 +297,17 @@ int x264_cpu_num_processors( void )
>      int numberOfCPUs;
>      size_t length = sizeof( numberOfCPUs );
>      if( sysctlbyname("hw.ncpu", &numberOfCPUs, &length, NULL, 0) )
> +    {
> +        numberOfCPUs = 1;
> +    }
> +    return numberOfCPUs;
> +
> +#elif defined(SYS_OPENBSD)
> +    int numberOfCPUs;
> +    int mib[2] = { CTL_HW, HW_NCPU };
> +    size_t length = sizeof( numberOfCPUs );
> +
> +    if( sysctl(mib, 2, &numberOfCPUs, &length, NULL, 0) )
>      {
>          numberOfCPUs = 1;
>      }

I see code duplication.

> OpenBSD 4.4 now has the C99 macro isfinite().
> 
> $OpenBSD: patch-common_osdep_h,v 1.1 2008/08/20 01:44:44 jakemsr Exp $
> --- common/osdep.h.orig	Mon Sep  1 16:45:08 2008
> +++ common/osdep.h	Mon Sep  1 21:08:50 2008
> @@ -51,7 +51,7 @@
>  #define X264_VERSION "" // no configure script for msvc
>  #endif
>  
> -#if defined(SYS_OPENBSD) || defined(SYS_SunOS)
> +#if defined(SYS_OPENBSD) && !defined(isfinite) || defined(SYS_SunOS)
>  #define isfinite finite
>  #endif
>  #if defined(_MSC_VER) || defined(SYS_SunOS) || defined(SYS_MACOSX)

This is ugly. You should just test for isfinite presence and use
#ifndef HAVE_ISFINITE
#define isfinite finite
#endif
instead.

> Fix compilation on OpenBSD PowerPC architectures when using
> AltiVec.
> 
> FYI, I really think a test should be added to the configure script
> to look for altivec.h and define a symbol name like HAVE_ALTIVEC_H
> instead of maintaining a list of OS's.

Exactly. I'm sure a patch that does that would be welcome.

[...]
> This fixed an issue found on amd64 where there were unresolved
> symbols.
> 
> $OpenBSD: patch-common_x86_mc-a_asm,v 1.1 2008/07/22 20:24:04 brad Exp $
> --- common/x86/mc-a.asm.orig	Mon Sep  1 16:45:08 2008
> +++ common/x86/mc-a.asm	Mon Sep  1 21:07:49 2008
> @@ -365,12 +365,12 @@ AVG_CACHELINE_CHECK  8, 32, mmxext
>  AVG_CACHELINE_CHECK 12, 32, mmxext
>  AVG_CACHELINE_CHECK 16, 32, mmxext
>  AVG_CACHELINE_CHECK 20, 32, mmxext
> -AVG_CACHELINE_CHECK 16, 64, mmxext
> -AVG_CACHELINE_CHECK 20, 64, mmxext
>  %endif
>  
>  AVG_CACHELINE_CHECK  8, 64, mmxext
>  AVG_CACHELINE_CHECK 12, 64, mmxext
> +AVG_CACHELINE_CHECK 16, 64, mmxext
> +AVG_CACHELINE_CHECK 20, 64, mmxext
>  AVG_CACHELINE_CHECK 16, 64, sse2
>  AVG_CACHELINE_CHECK 20, 64, sse2

What was the *exact* error message?

Regards,
R.

-- 
Fedora http://fedoraproject.org/wiki/User:Rathann
Livna http://rpm.livna.org | MPlayer http://mplayerhq.hu
"Faith manages."
        -- Delenn to Lennier in Babylon 5:"Confessions and Lamentations"