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

Rémi Denis-Courmont remi at remlab.net
Thu Sep 26 22:12:41 CEST 2013


Le lundi 23 septembre 2013 12:28:57 Andreas Fuchs a écrit :
> @@ -4068,6 +4068,24 @@ AS_IF([test "${enable_growl}" != "no"], [
>    ]
>  )
> 
> +dnl
> +dnl  NetCDF library for SOFAlizer plugin
> +dnl
> +AC_ARG_ENABLE(sofalizer,
> +  [  --enable-sofalizer           netcdf-binarual support (default
> disabled)],, +  [enable_sofalizer=no])
> +if test "${enable_sofalizer}" != "no"
> +then
> +   PKG_CHECK_MODULES(NETCDF,netcdf >= 4.1.3,
> +   [
> +      AC_CHECK_HEADERS(netcdf.h, [], [AC_MSG_ERROR([Missing header file
> netcdf.h.])] )

Why do you need to check the header when you know the package is present?

> +      VLC_ADD_PLUGIN([sofalizer])
> +      VLC_ADD_LDFLAGS([sofalizer],[$NETCDF_LIBS])

You may be confusing LDFLAGS with LIBS.

> +      VLC_ADD_CFLAGS([sofalizer], [$NETCDF_CFLAGS]) ],
> +   [
> +      AC_MSG_ERROR([${NETCDF_PKG_ERRORS}.]) ])
> +fi
> +
>  dnl
>  dnl Libnotify notification plugin
>  dnl
> diff --git a/modules/audio_filter/Modules.am
> b/modules/audio_filter/Modules.am index 89f8783..7345907 100644
> --- a/modules/audio_filter/Modules.am
> +++ b/modules/audio_filter/Modules.am
> @@ -8,6 +8,7 @@ SOURCES_param_eq = param_eq.c
>  SOURCES_scaletempo = scaletempo.c
>  SOURCES_chorus_flanger = chorus_flanger.c
>  SOURCES_stereo_widen = stereo_widen.c
> +SOURCES_sofalizer = sofalizer.c
>  SOURCES_spatializer = \
>  	spatializer/allpass.cpp spatializer/allpass.hpp \
>  	spatializer/comb.cpp spatializer/comb.hpp \
> @@ -27,7 +28,8 @@ audio_filter_LTLIBRARIES += \
>  	libparam_eq_plugin.la \
>  	libscaletempo_plugin.la \
>  	libspatializer_plugin.la \
> -	libstereo_widen_plugin.la
> +	libstereo_widen_plugin.la \
> +	libsofalizer_plugin.la

Either the plugin is built always or it is triggered conditionally (in this 
case by VLC_ADD_PLUGIN), but it cannot be both.

> 
>  # Channel mixers
>  SOURCES_trivial_channel_mixer = channel_mixer/trivial.c
> diff --git a/modules/audio_filter/sofalizer.c
> b/modules/audio_filter/sofalizer.c new file mode 100644
> index 0000000..fa263a9
> --- /dev/null
> +++ b/modules/audio_filter/sofalizer.c
> @@ -0,0 +1,1266 @@
> +/*************************************************************************
> ***** + * sofalizer.c : SOFAlizer plugin to use SOFA files in vlc
> +
> ***************************************************************************
> ** + * Copyright (C) 2013 Andreas Fuchs, ARI
> + *
> + * Authors: Andreas Fuchs <andi.fuchs.mail at gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or + *
> (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> +**************************************************************************
> ***/ +
> +/**************************************************************************
> *** + * Preamble
> +
> ***************************************************************************
> **/ +
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_aout.h>
> +#include <vlc_filter.h>
> +#include <vlc_block.h>
> +#include <vlc_modules.h>
> +
> +#include <math.h>
> +#include <vlc_interface.h>

Why do you need that header?

> +
> +#include <netcdf.h>
> +
> +#define N_SOFA 5
> +/**************************************************************************
> *** + * Local prototypes
> +
> ***************************************************************************
> **/ +struct nc_sofa_t
> +{
> +   int i_ncid;
> +   int i_n_samples;
> +   int i_m_dim;
> +   int *p_data_delay;
> +   float *p_sp_a;
> +   float *p_sp_e;
> +   float *p_sp_r;
> +   float *p_data_ir;
> +};
> +
> +struct filter_sys_t
> +{
> +    filter_t *p_parent;

Such back pointer should not be needed. Pass the filter_t instead.

> +    vlc_mutex_t lock;
> +    bool b_Enable;
> +    float *p_speaker_pos;
> +
> +    /* Control variables */
> +    float f_gain;
> +    float f_rotation;
> +    float f_elevation;
> +    float f_radius;
> +    int i_azimuth_array[4];
> +    int i_elevation_array[4];
> +    int i_switch;
> +    bool b_mute;
> +
> +    /* N of Channels to convolute */
> +    int i_n_conv;
> +    bool b_lfe;
> +
> +    /* Buffer variables */
> +    float *p_ringbuffer_l;
> +    float *p_ringbuffer_r;
> +    int i_write;
> +    int i_buffer_length;
> +
> +    /* NetCDF variables */
> +    struct nc_sofa_t sofa[N_SOFA];
> +    int i_i_sofa;  /* Selected Sofa File */
> +    int *p_delay_l;
> +    int *p_delay_r;
> +    float *p_ir_l;
> +    float *p_ir_r;
> +
> +};
> +
> +struct t_thread_data
> +{
> +    filter_sys_t *p_sys;
> +    block_t *p_in_buf;
> +    int *p_input_nb;
> +    int *p_delay;
> +    int i_write;
> +    int *p_n_clippings;
> +    float *p_ringbuffer;
> +    float *p_dest;
> +    float *p_ir;
> +    float f_gain_lfe;
> +};
> +
> +struct data_findM_t
> +{
> +    filter_sys_t *p_sys;
> +    int i_azim;
> +    int i_elev;
> +    int *p_m;
> +    float f_radius;
> +};
> +



> +#define POS1_AZIMUTH_VALUE_TEXT N_( "   Azimuth Position 1 ")
> +#define POS1_ELEVATION_VALUE_TEXT N_( "   Elevation Position 1 ")
> +#define POS2_AZIMUTH_VALUE_TEXT N_( "   Azimuth Position 2 ")
> +#define POS2_ELEVATION_VALUE_TEXT N_( "   Elevation Position 2 ")
> +#define POS3_AZIMUTH_VALUE_TEXT N_( "   Azimuth Position 3 ")
> +#define POS3_ELEVATION_VALUE_TEXT N_( "   Elevation Position 3 ")
> +#define POS4_AZIMUTH_VALUE_TEXT N_( "   Azimuth Position 4 ")
> +#define POS4_ELEVATION_VALUE_TEXT N_( "   Elevation Position 4 ")

I don't think leading white space will do much good.


> +static int Open( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t *)p_this;
> +    filter_sys_t *p_sys = p_filter->p_sys = malloc( sizeof( *p_sys ) );
> +    if( unlikely( p_sys == NULL ) )
> +        return VLC_ENOMEM;
> +    p_sys->p_parent = p_filter;
> +    vlc_object_t *p_aout = p_filter->p_parent;

There is no warranty that the parent object is an audio output. It might be a 
transcode plugin instance, say.

> +    intf_thread_t *intf = (intf_thread_t *)p_this; /* Output Interface */

Invalid cast.

> +
> +    p_sys->b_Enable = false;

> +    char *c_filename[N_SOFA];
> +    c_filename[0] = var_CreateGetStringCommand( p_filter,
> "sofalizer-filename1" );
> +    c_filename[1] = var_CreateGetStringCommand(
> p_filter, "sofalizer-filename2" );
> +    c_filename[2] =
> var_CreateGetStringCommand( p_filter, "sofalizer-filename3" );
> +   
> c_filename[3] = var_CreateGetStringCommand( p_filter, "sofalizer-filename4"
> );
> +    c_filename[4] = var_CreateGetStringCommand( p_filter,
> "sofalizer-filename5" );

Could use a for loop. Also below.

(...)

> +    /* Callbacks can call function LoadIR */
> +    var_AddCallback( p_aout, "sofalizer-gain", GainCallback, p_sys );
> +    var_AddCallback( p_aout, "sofalizer-rotation", RotationCallback,
> p_sys);
> +    var_AddCallback( p_aout, "sofalizer-elevation",
> ElevationCallback, p_sys );
> +    var_AddCallback( p_aout,
> "sofalizer-switch", SwitchCallback, p_sys );
> +    var_AddCallback( p_aout,
> "sofalizer-select", SelectCallback, p_sys );
> +    var_AddCallback( p_aout,
> "sofalizer-radius", RadiusCallback, p_sys );
> +    p_sys->b_Enable = true;

You cannot write a value while it is potentially read (or written) by other 
tasks.

> +
> +    return VLC_SUCCESS;
> +}
> +
> +/* Prepares the data structures for the threads and starts them */
> +static block_t *DoWork( filter_t *p_filter, block_t *p_in_buf )
> +{
> +    struct filter_sys_t *p_sys = p_filter->p_sys;
> +    int i_n_clippings_l = 0;
> +    int i_n_clippings_r = 0;
> +
> +    int i_input_nb = aout_FormatNbChannels( &p_filter->fmt_in.audio );
> +    int i_output_nb = aout_FormatNbChannels( &p_filter->fmt_out.audio );
> +
> +    size_t i_out_size = p_in_buf->i_buffer * i_output_nb / i_input_nb;
> +    block_t *p_out_buf = block_Alloc( i_out_size );
> +    if ( unlikely( !p_out_buf ) )
> +    {
> +        msg_Warn( p_filter, "can't get output buffer" );
> +        block_Release( p_in_buf );
> +        goto out;
> +    }
> +    p_out_buf->i_nb_samples = p_in_buf->i_nb_samples;
> +    p_out_buf->i_dts        = p_in_buf->i_dts;
> +    p_out_buf->i_pts        = p_in_buf->i_pts;
> +    p_out_buf->i_length     = p_in_buf->i_length;
> +
> +    vlc_thread_t left_thread, right_thread;
> +    struct t_thread_data t_data_l;
> +    struct t_thread_data t_data_r;
> +
> +    float f_gain_lfe = exp( (p_sys->f_gain - 3 * i_input_nb - 6) / 20 *
> log(10)); // -6 dB to get LFE on a similar level +
> +    t_data_l.p_sys = t_data_r.p_sys = p_sys;
> +    t_data_l.p_in_buf = t_data_r.p_in_buf = p_in_buf;
> +    t_data_l.p_input_nb = t_data_r.p_input_nb = &i_input_nb;
> +    t_data_l.f_gain_lfe = t_data_r.f_gain_lfe = f_gain_lfe;
> +    t_data_l.p_input_nb = t_data_r.p_input_nb = &i_input_nb;
> +    t_data_l.f_gain_lfe = t_data_r.f_gain_lfe = f_gain_lfe;
> +    t_data_l.i_write = t_data_r.i_write = p_sys->i_write;
> +    t_data_l.p_ringbuffer = p_sys->p_ringbuffer_l;
> +    t_data_r.p_ringbuffer = p_sys->p_ringbuffer_r;
> +    t_data_l.p_ir = p_sys->p_ir_l;
> +    t_data_r.p_ir = p_sys->p_ir_r;
> +    t_data_l.p_n_clippings = &i_n_clippings_l;
> +    t_data_r.p_n_clippings = &i_n_clippings_r;
> +    t_data_l.p_dest = (float *)p_out_buf->p_buffer;
> +    t_data_r.p_dest = (float *)p_out_buf->p_buffer + 1;
> +    t_data_l.p_delay = p_sys->p_delay_l;
> +    t_data_r.p_delay = p_sys->p_delay_r;
> +
> +    if ( p_sys->b_mute )
> +    {
> +        memset( (float *)p_out_buf->p_buffer , 0 , sizeof( float ) *
> i_input_nb * 2 ); +    }
> +    else
> +    {
> +        if( vlc_clone( &left_thread, (void *)&sofalizer_Convolute, (void
> *)&t_data_l, VLC_THREAD_PRIORITY_HIGHEST ) ) goto out;
> +        if(
> vlc_clone( &right_thread, (void *)&sofalizer_Convolute, (void *)&t_data_r,
> VLC_THREAD_PRIORITY_HIGHEST ) ) goto out;
> +        vlc_join ( left_thread, NULL );
> +        vlc_join ( right_thread, NULL );

Threads are relatively cheap, but creating and destroying two per audio block 
may be pushing it a bit...

> +        p_sys->i_write = t_data_l.i_write;
> +    }
> +
> +    if ( ( i_n_clippings_l + i_n_clippings_r ) > 0 )
> +    {
> +        msg_Err(p_filter, "%d of %d Samples in the Outputbuffer clipped.
> Please reduce gain.", i_n_clippings_l + i_n_clippings_r,
> p_out_buf->i_nb_samples * 2 );
> +    }
> +out: block_Release( p_in_buf );
> +    return p_out_buf;
> +}
> +

(...)
> +    char *psz_conventions;
> +    psz_conventions = (char *)malloc( sizeof( char ) * ( i_att_len + 1 ) );

Don't cast malloc in C code anywhere.

sizeof(char) is intrinsically redundant.

> +    nc_get_att_text( i_ncid , NC_GLOBAL, "Conventions", psz_conventions);
> +    *( psz_conventions + i_att_len ) = 0;

Unchecked memory allocation. Use xmalloc() if you cannot be bothered.

-- 
Rémi Denis-Courmont
http://www.remlab.net/




More information about the vlc-devel mailing list