[vlc-devel] [PATCH] audio_filter: SOFAlizer new module

[Andi Fuchs]

Thank you both for your comments and suggestions! 

Am 23.09.2013 um 19:51 schrieb Ilkka Ollakka:

>> 
>> + *
>> + * Authors: Andreas Fuchs <andi.fuchs.mail at gmail.com>
>> + *
>> + * Licensed under the EUPL, Version 1.1 or – as soon they will be approved by
>> + * the European Commission - subsequent versions of the EUPL (the "Licence");
>> + * You may not use this work except in compliance with the Licence. You may
>> + * obtain a copy of the Licence at: http://ec.europa.eu/idabc/eupl
> 
> EUPL seems to be close to GPL?

It's more or less the GPL but for EU, but if you are more happy with the LGPL, it's also fine by me. I thought VLC is GPL. 

> 
>> +#define N_SOFA 5
> 
> Why 5?

If you don't have the SOFA file with your own HRTFs you have to compare different files to find the best suited for you. So it is useful to load more than one SOFA file and switch between them on the fly without stopping playback. 

> 
>> +/* if ( p_filter->fmt_in.audio.i_rate != i_samplingrate ) //Check Sampling Rate
>> +    {
>> +        msg_Err( p_filter, "Sampling rate of Input und Sofa file doesn't match. Can't load filter" );
>> +        return  VLC_EGENERIC;
>> +    }
>> +*/
> 
> Any reason it's included but commented out?

Yes, as described here https://forum.videolan.org/viewtopic.php?f=32&t=113578 if you have wav files with 16bit with a different samplingrate than the sofa file, vlc crashes. This code could prevent this by only allowing the same samplerate. So it's kind of a fallback if the other problem can't be solved. 

> 
>> +/* Writes the samples of the input buffer in the ringbuffer and convolutes with the impulse response */
>> +void sofalizer_Convolute ( void *p_ptr )
>> +{
>> +    for ( int i = t_data->p_in_buf->i_nb_samples ; i-- ; )
>> +    {
>> +        *( p_dest ) = 0;
>> +        for ( int l = 0 ; l < i_input_nb ; l++ )
>> +        {
>> +            *( p_ringbuffer[l] + i_write ) = *( p_src++);  
> 
> memcpy ?

I don't think memcpy will work because p_rinbuffer[n+1] is i_buffer_length samples away from p_ringbuffer[n]

Am 23.09.2013 um 19:33 schrieb Jean-Baptiste Kempf:
> 
>> +dnl 
>> +dnl  NetCDF library for SOFAlizer Plugin 
>> +dnl 
>> +AS_IF([test "${SYS}" = "linux"], [ 
>> +AC_ARG_ENABLE(sofalizer, 
>> +  [  --enable-sofalizer            netcdf-binarual support (default enabled)]) 
>> +if test "${enable_sofalizer}" != "no" 
>> +then 
>> +   PKG_CHECK_MODULES(NETCDF,netcdf, 
>> +   [ 
>> +      AC_CHECK_HEADERS(netcdf.h, [], [AC_MSG_ERROR([Missing header file netcdf.h.])] ) 
>> +      VLC_ADD_PLUGIN([sofalizer]) 
>> +      VLC_ADD_LDFLAGS([sofalizer],[$NETCDF_LIBS]) 
>> +      VLC_ADD_CFLAGS([sofalizer], [$NETCDF_CFLAGS]) 
>> +])
>> +fi 
>> +])
>> +
>> +AS_IF([test "${SYS}" = "mingw32"], [ 
>> +AC_ARG_ENABLE(sofalizer,
>> +  [  --enable-sofalizer            netcdf-sofalizer support (default enabled)])
>> +if test "${enable_sofalizer}" != "no"
>> +then
>> +VLC_ADD_PLUGIN([sofalizer])
>> +VLC_ADD_LIBS( [sofalizer], [ -lnetcdf])
>> +AC_CHECK_HEADERS(netcdf.h, [], [AC_MSG_ERROR([Missing header file netcdf.h.])] )
>> +AC_ARG_WITH([netcdf-include-path],
>> +     [AS_HELP_STRING([--with-netcdf-include-path],
>> +        [location of the NetCDF headers, default to /usr/include/])],
>> +     [NETCDF_CFLAGS='-I${CONTRIB_DIR}/include'])
>> +AC_SUBST([NETCDF_CFLAGS])
>> +
>> +AC_ARG_WITH([netcdf-lib-path], 
>> +    [AS_HELP_STRING([--with-netcdf-lib-path], [location of the netCDF libraries])],
>> +    [NETCDF_LIBS='-lnetcdf'])
>> +AC_SUBST([NETCDF_LIBS])
>> +
>> +VLC_ADD_LDFLAGS([sofalizer],[$NETCDF_LIBS])
>> +VLC_ADD_CFLAGS([sofalizer], [$NETCDF_CFLAGS])
> 
> No. Please use just the PKG check, in one line, for all the
> architectures.
> 
>> 
>> +	
>> +if HAVE_WIN32
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/bin/netcdf.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/bin/msvcp100.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/bin/msvcr100.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/deps/shared/bin/hdf5.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/deps/shared/bin/hdf5_hl.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/deps/shared/bin/libcurl.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/deps/shared/bin/szip.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/deps/shared/bin/zlib.dll $(win32_destdir)
>> +	cp $(srcdir)/contrib/i686-w64-mingw32/deps/shared/bin/zlib1.dll $(win32_destdir)
>> +endif	
> 
> This is very very wrong, so no.
> 

Sorry, i forgot to mention that i this is for now a workaround. It's rather complicated to cross compile netcdf, so i took for windows cross compiling the precompiled libraries without a pkfconfig file. What is the ideal way to deal with external libraries for different architectures? Or is an AC_ARG_ENABLE and PKG_CHECK_MODULES (as for linux now) enough and the person who compiles for the architecture needs to get the correct libraries to suit the code in configure.ac?


best regards, 
Andi