[FFmpeg-devel] [PATCH] lzo build system checks

[Reimar Döffinger]

On Thu, Apr 09, 2009 at 03:58:29PM +0200, Diego Biurrun wrote:
> Index: configure
> ===================================================================
> --- configure	(revision 18382)
> +++ configure	(working copy)
> @@ -895,6 +895,8 @@
>      llrint
>      lrint
>      lrintf
> +    lzo_lzo1x_h
> +    lzo1x_999_compress
>      machine_ioctl_bt848_h
>      machine_ioctl_meteor_h
>      malloc_h
> @@ -1970,11 +1972,13 @@
>  check_func  mkstemp
>  check_func  posix_memalign
>  check_func_headers io.h setmode
> +check_func_headers lzo/lzo1x.h lzo1x_999_compress
>  check_func_headers windows.h GetProcessTimes
>  check_func_headers windows.h VirtualAlloc
>  
>  check_header conio.h
>  check_header dlfcn.h
> +check_header lzo/lzo1x.h
>  check_header malloc.h
>  check_header poll.h
>  check_header sys/mman.h
> Index: libavutil/Makefile
> ===================================================================
> --- libavutil/Makefile	(revision 18382)
> +++ libavutil/Makefile	(working copy)
> @@ -42,6 +42,7 @@
>         utils.o                                                          \
>  
>  TESTPROGS = adler32 aes base64 crc des lls md5 pca sha1 softfloat tree
> +TESTPROGS-$(HAVE_LZO_LZO1X_H) += lzo
>  
>  DIRS = arm bfin sh4 x86
>  

I have to admit I do not know configure that well, does that compile
lzotest only when lzo/lzo1x.h is available and defines
lzo1x_999_compress (though I admit just checking for lzo/lzo1x.h should
be enough)? Also, doesn't it need -llzo as well to link??

> +//#define HAVE_LZO1X_DECOMPRESS_SAFE
> +//#define HAVE_LZO1X_DECOMPRESS
> +
>  int main(int argc, char *argv[]) {
>      FILE *in = fopen(argv[1], "rb");
>      uint8_t *orig = av_malloc(MAXSZ + 16);
> @@ -254,9 +258,9 @@
>      for (i = 0; i < 300; i++) {
>  START_TIMER
>          inlen = clen; outlen = MAXSZ;
> -#ifdef LIBLZO
> +#if HAVE_LZO1X_DECOMPRESS_SAFE
>          if (lzo1x_decompress_safe(comp, inlen, decomp, &outlen, NULL))
> -#elif defined(LIBLZO_UNSAFE)
> +#elif HAVE_LZO1X_DECOMPRESS
>          if (lzo1x_decompress(comp, inlen, decomp, &outlen, NULL))

If you use #if you should probably define it.
Also HAVE_* is just a bad name, it implies it enables some extra
feature. But it selects which function to test/benchmark.
So if anything you could name it BENCHMARK_LIBZO and BENCHMARK_LIBLZO_SAFE
though that still would be mostly unrelated to this patch.
I'd be more in favour of just leaving this as is and doing
TESTPROGS-$(HAVE_LZO_LZO1X_H) += lzo lzo-liblzo lzo-liblzo-unsafe
adding -DLIBLZO/-DLIBLZO_UNSAFE
And I do not like adding the prefix to the _SAFE variant instead of the
_UNSAFE, it gives people a wrong impression by treating the unsafe
variant as normal although in FFmpeg it certainly can not be used,
making liblzo look better in benchmarks than realistic.