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

[Diego Biurrun]

On Thu, Apr 09, 2009 at 04:12:09PM +0200, Reimar D?ffinger wrote:
> On Thu, Apr 09, 2009 at 03:58:29PM +0200, Diego Biurrun wrote:
> > --- 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
> > --- libavutil/Makefile	(revision 18382)
> > +++ libavutil/Makefile	(working copy)
> > @@ -42,6 +42,7 @@
> >  
> >  TESTPROGS = adler32 aes base64 crc des lls md5 pca sha1 softfloat tree
> > +TESTPROGS-$(HAVE_LZO_LZO1X_H) += lzo
> 
> 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??

It compiles it when just the header is available.  On second thought, I
can skip the header check and just check for the function.

The required linker flags are already being added by the last line in
libavutil/Makefile:

  $(SUBDIR)lzo-test$(EXESUF): ELIBS = -llzo2

> > +//#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.

Of course, I'm writing this on a machine without lzo libs and no (easy)
way to install them, hence I cannot do sufficient testing and am writing
this as proof of concept without sufficient testing...

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

Well, it is your file, whatever you prefer...

I'm trying to get this hooked up properly in the build system, surely
there are ways in which this test program could be improved further.

Here's my next attempt..

Diego
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lzo3.diff
Type: text/x-diff
Size: 1823 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090409/8885573d/attachment.diff>