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

Jean-Baptiste Kempf jb at videolan.org
Wed Sep 25 15:50:34 CEST 2013


On 23 Sep, Andreas Fuchs wrote :
> +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.])] ) 
> +      VLC_ADD_PLUGIN([sofalizer]) 
> +      VLC_ADD_LDFLAGS([sofalizer],[$NETCDF_LIBS]) 
> +      VLC_ADD_CFLAGS([sofalizer], [$NETCDF_CFLAGS]) ],
> +   [
> +      AC_MSG_ERROR([${NETCDF_PKG_ERRORS}.]) ])
> +fi 

No.
PKG_ENABLE_MODULES_VLC([SOFALIZER], [], [netcdf >= 4.1.3],
    [netcdf-binarual support], [auto])


> +struct filter_sys_t
> +{
> +    filter_t *p_parent;
> +    vlc_mutex_t lock;
> +    bool b_Enable;

Avoid that. You are destroying packing.

> +    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;

idem

> +    /* N of Channels to convolute */
> +    int i_n_conv;
> +    bool b_lfe;

idem.

> +    /* 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];

And probably the same.

> +    int i_i_sofa;  /* Selected Sofa File */
> +    int *p_delay_l;
> +    int *p_delay_r;
> +    float *p_ir_l;
> +    float *p_ir_r;
> +};

Why so many pointers to float and int?

> +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;
> +};

Trailing spaces...

> +static int LoadSofa ( filter_t *p_filter, char *filename, int i_i_sofa,  int *p_samplingrate);
> +static void CloseSofa( filter_t *p_filter);
> +static int LoadIR ( filter_t *p_filter, int i_azim, int i_elev, float f_radius);
> +static int GetSpeakerPos ( filter_t *p_filter, float *p_speaker_pos );
> +static int MaxDelay ( struct nc_sofa_t *sofa );
> +void sofalizer_Convolute ( void *data );
> +void sofalizer_FindM ( void *data );
> +static int CompensateVolume( filter_t *p_filter);

You could reorder those functions to avoid declaring so many.

> +#define HELP_TEXT N_( "SOFAlizer creates a virtual auditory display, i.e., virtual loudspeakers around the user for listening via headphones. The position of the virtual loudspeakers depends on the audio format of the input file (up to 8.1 supported). SOFAlizer filters audio channels with head-related transfer functions (HRTFs). SOFAlizer uses HRTFs stored in SOFA files (www.sofaconventions.org) following the SimpleFreeFieldHRIR Convention. A database of SOFA files can be found at www.sofacoustics.org.\nWith listener-specific HRTFs, the virtual loudspeakers can be rotated and elevated. With near-field HRTFs, the distance between the loudspeakers and the listener can be varied. SOFAlizer allows to load 5 different SOFA files and easily switch between them. The audio channels can also be presented from one of four pre-defined virtual positions.\nSOFAlizer is developed at the Acoustics Research Institute (ARI) of the Austrian Academy of Sciences." )

Come on... This is way too long...

> +#define GAIN_VALUE_TEXT N_( "Gain" )
> +#define GAIN_VALUE_LONGTEXT N_( "Sets the volume of the module." )

Volume or gain?

> +#define FILE1_NAME_TEXT N_( "SOFA file 1" )
> +#define FILE2_NAME_TEXT N_( "SOFA file 2" )
> +#define FILE3_NAME_TEXT N_( "SOFA file 3" )
> +#define FILE4_NAME_TEXT N_( "SOFA file 4" )
> +#define FILE5_NAME_TEXT N_( "SOFA file 5" )

Useless strings addition.

> +#define SELECT_VALUE_TEXT N_( "Select SOFA file" )
> +#define SELECT_VALUE_LONGTEXT N_( "Select the SOFA file to use." )

> +#define ROTATION_VALUE_TEXT N_( "Rotation" )
> +#define ROTATION_VALUE_LONGTEXT N_( "Rotation angle. >0 -> Right, <0 -> Left. [-180;180]" )

Please reuse existing strings.

> +vlc_module_begin ()
> +    set_description( N_("SOFAlizer") )
> +    set_shortname( N_("SOFAlizer") )
> +    set_capability( "audio filter", 0)
> +    set_help( HELP_TEXT )

> +    add_loadfile( "sofalizer-filename1", "", FILE1_NAME_TEXT, FILE_NAME_LONGTEXT, false)
> +    add_loadfile( "sofalizer-filename2", "", FILE2_NAME_TEXT, FILE_NAME_LONGTEXT, false)
> +    add_loadfile( "sofalizer-filename3", "", FILE3_NAME_TEXT, FILE_NAME_LONGTEXT, false)
> +    add_loadfile( "sofalizer-filename4", "", FILE4_NAME_TEXT, FILE_NAME_LONGTEXT, false)
> +    add_loadfile( "sofalizer-filename5", "", FILE5_NAME_TEXT, FILE_NAME_LONGTEXT, false)

I doubt one actually need 5.

> +    add_float( "sofalizer-select", 1 ,  SELECT_VALUE_TEXT, SELECT_VALUE_LONGTEXT, false)
> +    add_float( "sofalizer-gain", 0.0, GAIN_VALUE_TEXT, GAIN_VALUE_LONGTEXT, false )
> +    add_float( "sofalizer-rotation", 0, ROTATION_VALUE_TEXT, ROTATION_VALUE_LONGTEXT, false )
> +    add_float( "sofalizer-elevation", 0, ELEVATION_VALUE_TEXT, ELEVATION_VALUE_LONGTEXT, false )
> +    add_float( "sofalizer-radius", 1, RADIUS_VALUE_TEXT, RADIUS_VALUE_LONGTEXT, false )
> +    add_float( "sofalizer-switch" , 0 , SWITCH_VALUE_TEXT, SWITCH_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos1-azi", 90 , POS1_AZIMUTH_VALUE_TEXT, POS_AZIMUTH_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos1-ele", 0, POS1_ELEVATION_VALUE_TEXT, POS_ELEVATION_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos2-azi", 180, POS2_AZIMUTH_VALUE_TEXT, POS_AZIMUTH_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos2-ele", 0, POS2_ELEVATION_VALUE_TEXT, POS_ELEVATION_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos3-azi", -90, POS3_AZIMUTH_VALUE_TEXT, POS_AZIMUTH_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos3-ele", 0, POS3_ELEVATION_VALUE_TEXT, POS_ELEVATION_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos4-azi", 0, POS4_AZIMUTH_VALUE_TEXT, POS_AZIMUTH_VALUE_LONGTEXT, false )
> +    add_integer( "sofalizer-pos4-ele", 90, POS4_ELEVATION_VALUE_TEXT, POS_ELEVATION_VALUE_LONGTEXT, false )

Many of them should be limited in range.

> +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;

Add a newline here.

> +    p_sys->p_parent = p_filter;
> +    vlc_object_t *p_aout = p_filter->p_parent;

> +    intf_thread_t *intf = (intf_thread_t *)p_this; /* Output Interface */
Why is this an intf?

> +    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" );
> +    p_sys->i_i_sofa     = ( (int) (var_CreateGetFloat ( p_aout, "sofalizer-select" ) )  + 100 - 1 ) % 5  ;
> +    p_sys->f_gain       = var_CreateGetFloat( p_aout, "sofalizer-gain" );
> +    p_sys->f_rotation   = abs ( ( - (int) var_CreateGetFloat( p_aout, "sofalizer-rotation" ) + 720 ) % 360 );
> +    p_sys->f_elevation  = var_CreateGetFloat( p_aout, "sofalizer-elevation" );
> +    p_sys->i_switch     = (int)( var_CreateGetFloat ( p_aout, "sofalizer-switch" ) ) ;
> +    p_sys->f_radius     = var_CreateGetFloat( p_aout, "sofalizer-radius");
> +
> +    p_sys->i_azimuth_array[0] = ( var_CreateGetInteger ( p_aout, "sofalizer-pos1-azi" ) + 720 ) % 360 ;
> +    p_sys->i_azimuth_array[1] = ( var_CreateGetInteger ( p_aout, "sofalizer-pos2-azi" ) + 720 ) % 360 ;
> +    p_sys->i_azimuth_array[2] = ( var_CreateGetInteger ( p_aout, "sofalizer-pos3-azi" ) + 720 ) % 360 ;
> +    p_sys->i_azimuth_array[3] = ( var_CreateGetInteger ( p_aout, "sofalizer-pos4-azi" ) + 720 ) % 360 ;
> +
> +    p_sys->i_elevation_array[0] = var_CreateGetInteger ( p_aout, "sofalizer-pos1-ele" );
> +    p_sys->i_elevation_array[1] = var_CreateGetInteger ( p_aout, "sofalizer-pos2-ele" );
> +    p_sys->i_elevation_array[2] = var_CreateGetInteger ( p_aout, "sofalizer-pos3-ele" );
> +    p_sys->i_elevation_array[3] = var_CreateGetInteger ( p_aout, "sofalizer-pos4-ele" );

Use var_Inherit*

> +    vlc_mutex_init( &p_sys->lock );

Why do you need a lock here?

> +/* 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;
> +    }
> +*/

Remove this.

> +go_on:   p_filter->fmt_in.audio.i_rate = i_samplingrate_old;

You should avoid those kinds of label.

> +    p_filter->fmt_in.audio.i_format = VLC_CODEC_FL32 ;
> +    p_filter->fmt_out.audio = p_filter->fmt_in.audio;
> +
> +    p_filter->fmt_out.audio.i_physical_channels = AOUT_CHANS_STEREO; // required for filter output set to stereo
> +    p_filter->fmt_out.audio.i_original_channels = AOUT_CHANS_STEREO;
> +
> +    int i_input_nb = aout_FormatNbChannels( &p_filter->fmt_in.audio );
> +    if ( p_filter->fmt_in.audio.i_physical_channels & AOUT_CHAN_LFE )
> +    {
> +        p_sys->b_lfe = true;
> +        p_sys->i_n_conv = i_input_nb - 1 ;
> +    }
> +    else
> +    {
> +        p_sys->b_lfe = false;
> +        p_sys->i_n_conv = i_input_nb ;
> +    }
> +    

Trailing spaces, again.

> +    p_sys->p_speaker_pos = malloc( sizeof( float) * p_sys->i_n_conv ) ;
> +    if ( unlikely ( !p_sys->p_speaker_pos ) )
> +    {
> +        CloseSofa( p_filter );        
> +        free ( p_sys );        
> +        return VLC_ENOMEM;
> +    }
> +    if ( GetSpeakerPos ( p_filter, p_sys->p_speaker_pos ) != VLC_SUCCESS )
> +    {
> +        msg_Err (p_filter, "Couldn't get Speaker Positions. Input Channel Configuration not supported. ");
> +        free( p_sys->p_speaker_pos );
> +        free( p_sys );
> +        CloseSofa( p_filter );
> +        return VLC_EGENERIC;
> +    }
> +    
> +    /* Find the minimum size (length of impulse response plus maximal Delay) of the Ringbuffer as power of 2.  */
> +    int i_n_max = 0;
> +    int i_n_current;
> +    int i_n_max_ir = 0;
> +    for ( i = 0 ; i < N_SOFA ; i++ )
> +    {
> +        if ( p_sys->sofa[i].i_ncid != 0 )
> +        {
> +            i_n_current = p_sys->sofa[i].i_n_samples + MaxDelay ( &p_sys->sofa[i] );
> +            if ( i_n_current > i_n_max )
> +            {
> +                i_n_max = i_n_current;
> +                i_n_max_ir = p_sys->sofa[i].i_n_samples;
> +            }
> +        }
> +    }
> +    i_n_max = pow(2, ceil(log( i_n_max )/ log(2) ) );
> +    
> +    /* Allocate Memory for the impulse responses, delays and the ringbuffers */
> +    float *p_ir_l=malloc( sizeof(float) * i_n_max_ir * p_sys->i_n_conv  ); /* minus LFE .*/
> +    if( unlikely( !p_ir_l ) )
> +    {
> +        free( p_sys->p_speaker_pos);
> +        CloseSofa( p_filter );
> +        free( p_sys );
> +        return VLC_ENOMEM;
> +    }
> +    float *p_ir_r=malloc( sizeof(float) * i_n_max_ir * p_sys->i_n_conv ); /* minus LFE */
> +    if( unlikely( !p_ir_r ) )
> +    {
> +        free( p_sys->p_speaker_pos);
> +        free( p_ir_l );
> +        CloseSofa( p_filter );
> +        free( p_sys );
> +        return VLC_ENOMEM;
> +    }
> +
> +    int *p_delay_l = malloc ( sizeof( int ) * p_sys->i_n_conv );
> +    if( unlikely( !p_delay_l ) )
> +    {
> +        CloseSofa( p_filter );
> +        free( p_sys->p_speaker_pos);
> +        free( p_sys );
> +        free( p_ir_l );
> +        free( p_ir_r );
> +        return VLC_ENOMEM;
> +    }
> +    int *p_delay_r = malloc ( sizeof( int ) * p_sys->i_n_conv );
> +    if ( unlikely ( !p_delay_r ) )
> +    {
> +        CloseSofa( p_filter );
> +        free (p_sys->p_speaker_pos );
> +        free ( p_ir_l );
> +        free ( p_ir_r );
> +        free ( p_delay_l );
> +        free( p_sys );
> +        return VLC_ENOMEM;
> +    }
> +
> +    p_sys->p_ir_l = p_ir_l;
> +    p_sys->p_ir_r = p_ir_r;
> +    p_sys->p_delay_l = p_delay_l;
> +    p_sys->p_delay_r = p_delay_r;
> +
> +    p_sys->i_buffer_length = i_n_max; 
> +    p_sys->p_ringbuffer_l = calloc( p_sys->i_buffer_length * i_input_nb, sizeof( float ) );
> +    if( unlikely( !p_sys->p_ringbuffer_l ) )
> +    {
> +        CloseSofa( p_filter );
> +        free( p_sys->p_speaker_pos);
> +        free( p_ir_l );
> +        free( p_ir_r );
> +        free( p_delay_l );
> +        free( p_delay_r );
> +        free( p_sys );
> +        return VLC_ENOMEM;
> +    }
> +    p_sys->p_ringbuffer_r = calloc( p_sys->i_buffer_length * i_input_nb, sizeof( float ) );
> +    if( unlikely( !p_sys->p_ringbuffer_r ) )
> +    {
> +        CloseSofa( p_filter );
> +        free( p_sys->p_speaker_pos);
> +        free( p_ir_l );
> +        free( p_ir_r );
> +        free( p_delay_l );
> +        free( p_delay_r );
> +        free( p_sys );
> +        free( p_sys->p_ringbuffer_l );
> +        return VLC_ENOMEM;
> +    }
> +    p_sys->i_write = 0;
> +
> +    CompensateVolume ( p_filter );
> +    
> +    /* Load the Impulse Responses into p_ir_l and p_ir_r for the required directions */
> +    if ( LoadIR ( p_filter, p_sys->f_rotation, p_sys->f_elevation, p_sys->f_radius) != VLC_SUCCESS )
> +    {
> +        CloseSofa( p_filter );
> +        free( p_sys->p_speaker_pos);
> +        free( p_ir_l );
> +        free( p_ir_r );
> +        free( p_delay_l );
> +        free( p_delay_r );
> +        free( p_sys );
> +        free( p_sys->p_ringbuffer_l );
> +        free( p_sys->p_ringbuffer_r );
> +        return VLC_ENOMEM;
> +    }

You should factor error cases and use less pointers just for an int or a
float.

> +    
> +    /* Loads of Infos */
> +    const char *c_netcdf_v=nc_inq_libvers ();
> +    msg_Info(intf, "Samplerate: %d\nKanäle: %d Ph: 0x%x Org: 0x%x\nVersion Netcdf: %s", p_filter->fmt_in.audio.i_rate, i_input_nb, p_filter->fmt_in.audio.i_physical_channels, p_filter->fmt_in.audio.i_original_channels,  c_netcdf_v);
> +
> +    msg_Info(intf, "Bytes/Frame: %d, Framelänge: %d, Bits/Sample: %d, Blockalign: %d",  p_filter->fmt_in.audio.i_bytes_per_frame,  p_filter->fmt_in.audio.i_frame_length,  p_filter->fmt_in.audio.i_bitspersample,  p_filter->fmt_in.audio.i_blockalign);
> +    msg_Info(intf, "Outkanäle: Phys: 0x%x, Kanäle zum Falten: %d, Ringbufferlänge: %d x %d" , p_filter->fmt_out.audio.i_physical_channels, p_sys->i_n_conv, i_input_nb, (int )p_sys->i_buffer_length );

No. TOo many msg_Info.

> +    vlc_thread_t left_thread, right_thread;
> +    struct t_thread_data t_data_l;
> +    struct t_thread_data t_data_r;

Why so many threads?

> +/* Load the Sofa files, check for the most important SOFAconventions and load the whole IR Data, Source-Positions and Delays */
> +
> +static int LoadSofa ( filter_t *p_filter, char *c_filename, int i_i_sofa , int *p_samplingrate)
> +{
> +    struct filter_sys_t *p_sys = p_filter->p_sys;
> +    int i_ncid, i_n_dims, i_n_vars, i_n_gatts, i_n_unlim_dim_id, i_status;
> +    unsigned int i_samplingrate;
> +    int i_n_samples = 0;
> +    int i_m_dim = 0;
> +    p_sys->sofa[i_i_sofa].i_ncid = 0;
> +    i_status = nc_open( c_filename , NC_NOWRITE, &i_ncid);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Can't find SOFA-file '%s'", c_filename);
> +        return VLC_EGENERIC;
> +    }
> +    nc_inq(i_ncid, &i_n_dims, &i_n_vars, &i_n_gatts, &i_n_unlim_dim_id); /* Get Number of Dimensions, Vars, Globa Attributes and Id of unlimited Dimensions */
> +
> +    char c_dim_names[i_n_dims][NC_MAX_NAME];   /* Get Dimensions */
> +    uint32_t i_dim_length[i_n_dims];
> +    int i_m_dim_id = 0;
> +    int i_n_dim_id = 0; 
> +    for( int ii = 0; ii<i_n_dims; ii++ )
> +    {
> +        nc_inq_dim( i_ncid, ii, c_dim_names[ii], &i_dim_length[ii] );
> +        if ( !strcmp("M", c_dim_names[ii] ) )
> +            i_m_dim_id = ii;
> +        if ( !strcmp("N", c_dim_names[ii] ) )
> +            i_n_dim_id = ii;
> +        else { }
> +    }
> +    i_n_samples = i_dim_length[i_n_dim_id];
> +    i_m_dim =  i_dim_length[i_m_dim_id];
> +
> +    uint32_t i_att_len;
> +    i_status = nc_inq_attlen(i_ncid, NC_GLOBAL, "Conventions", &i_att_len);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Can't get Length of Attribute Conventions.");
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    char *psz_conventions;
> +    psz_conventions = (char *)malloc( sizeof( char ) * ( i_att_len + 1 ) );
> +    nc_get_att_text( i_ncid , NC_GLOBAL, "Conventions", psz_conventions);
> +    *( psz_conventions + i_att_len ) = 0;
> +    if ( strcmp( "SOFA" , psz_conventions ) )
> +    {
> +        msg_Err(p_filter, "Not a SOFA file!");
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    nc_inq_attlen (i_ncid, NC_GLOBAL, "SOFAConventions", &i_att_len );
> +    char *psz_sofa_conventions;
> +    psz_sofa_conventions = (char *)malloc( sizeof ( char ) * ( i_att_len + 1 ) );
> +    nc_get_att_text(i_ncid, NC_GLOBAL, "SOFAConventions", psz_sofa_conventions);
> +    *( psz_sofa_conventions + i_att_len ) = 0;
> +    if ( strcmp( "SimpleFreeFieldHRIR" , psz_sofa_conventions ) )
> +    {
> +       msg_Err(p_filter, "No SimpleFreeFieldHRIR file!");
> +       nc_close(i_ncid);
> +       return VLC_EGENERIC;
> +   }
> +
> +    int i_samplingrate_id;
> +    i_status = nc_inq_varid( i_ncid, "Data.SamplingRate", &i_samplingrate_id);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read variable Data.SamplingRate ID");
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    i_status = nc_get_var_uint( i_ncid, i_samplingrate_id, &i_samplingrate);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read value of Data.SamplingRate.");
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    *p_samplingrate = i_samplingrate;
> +
> +    free(psz_sofa_conventions);
> +    free(psz_conventions);
> +
> +    int i_data_ir_id;
> +    i_status = nc_inq_varid( i_ncid, "Data.IR", &i_data_ir_id);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read Id of Data.IR." );
> +        return VLC_EGENERIC;
> +    }
> +
> +    float *p_data_ir = malloc ( sizeof( float ) * 2 * i_m_dim * i_n_samples );
> +    if ( unlikely( !p_data_ir ) )
> +    {
> +        nc_close(i_ncid);
> +        return VLC_ENOMEM;
> +    }
> +    i_status = nc_get_var_float( i_ncid, i_data_ir_id, p_data_ir );
> +    if ( i_status != NC_NOERR )
> +    {
> +        msg_Err( p_filter, "Couldn't read Data.IR!" );
> +        free( p_data_ir );
> +        nc_close( i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +
> +    int i_sp_id;
> +    i_status = nc_inq_varid(i_ncid, "SourcePosition", &i_sp_id);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read ID of SourcePosition");
> +        free( p_data_ir );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +
> +    float *p_sp_a=(float *)malloc( sizeof(float) * i_m_dim);
> +    if( unlikely( !p_sp_a ) )
> +    {
> +        free( p_data_ir );
> +        nc_close(i_ncid);
> +        return VLC_ENOMEM;
> +    }
> +    size_t start_a[]= { 0 , 0 };
> +    size_t count_a[] = { i_m_dim , 1 };
> +    i_status = nc_get_vara_float (i_ncid, i_sp_id, start_a, count_a, p_sp_a );
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read SourcePosition.");
> +        free( p_data_ir );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +
> +    float *p_sp_e=(float *)malloc( sizeof(float) * i_m_dim);
> +    if( unlikely( !p_sp_e ) )
> +    {
> +        free( p_data_ir );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_ENOMEM;
> +    }
> +    size_t start_e[]= { 0 , 1 };
> +    size_t count_e[] = { i_m_dim , 1 };
> +    i_status = nc_get_vara_float (i_ncid, i_sp_id, start_e, count_e, p_sp_e );
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read SourcePosition.");
> +        free( p_data_ir );
> +        free( p_sp_e );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +
> +    float *p_sp_r=(float *)malloc( sizeof(float) * i_m_dim);
> +    if( unlikely( !p_sp_r ) )
> +    {
> +        free( p_data_ir );
> +        free( p_sp_a );
> +        free( p_sp_e );
> +        nc_close(i_ncid);
> +        return VLC_ENOMEM;
> +    }
> +
> +    size_t start_r[]= { 0 , 2 };
> +    size_t count_r[] = { i_m_dim , 1 };
> +    i_status = nc_get_vara_float (i_ncid, i_sp_id, start_r, count_r, p_sp_r );
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read SourcePosition.");
> +        free( p_data_ir );
> +        free( p_sp_e );
> +        free( p_sp_a );
> +        free( p_sp_r );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +
> +    int i_data_delay_id;
> +    int i_data_delay_dim_id[2];
> +    char i_data_delay_dim_name[NC_MAX_NAME];
> +
> +    int *p_data_delay = calloc ( sizeof( int ) , i_m_dim * 2 );
> +    if ( unlikely( !p_data_delay ) )
> +    {
> +        free( p_data_ir );
> +        free( p_sp_e );
> +        free( p_sp_r );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_ENOMEM;
> +    }
> +    i_status = nc_inq_varid(i_ncid, "Data.Delay", &i_data_delay_id);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read Id of Data.Delay." );
> +        free( p_data_delay );
> +        free( p_data_ir );
> +        free( p_sp_r );
> +        free( p_sp_e );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    i_status = nc_inq_vardimid ( i_ncid, i_data_delay_id, &i_data_delay_dim_id[0]);
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read Dimension Ids of Data.Delay." );
> +        free( p_data_delay );
> +        free( p_sp_r );
> +        free( p_data_ir );
> +        free( p_sp_e );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    i_status = nc_inq_dimname ( i_ncid, i_data_delay_dim_id[0], i_data_delay_dim_name );
> +    if (i_status != NC_NOERR)
> +    {
> +        msg_Err(p_filter, "Couldn't read Dimension Name of Data.Delay." );
> +        free( p_data_delay );
> +        free( p_data_ir );
> +        free( p_sp_r );
> +        free( p_sp_e );
> +        free( p_sp_a );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    if ( !strncmp ( i_data_delay_dim_name, "I", 1 ) )
> +    {
> +        msg_Dbg ( p_filter, "DataDelay in Dimension IR");
> +        int i_Delay[2];
> +        i_status = nc_get_var_int( i_ncid, i_data_delay_id, &i_Delay[0] );
> +        if ( i_status != NC_NOERR )
> +        {
> +            free( p_sp_r );
> +            free( p_sp_e );
> +            free( p_sp_a );
> +            free( p_data_ir );
> +            free( p_data_delay );
> +            nc_close(i_ncid);
> +            return VLC_EGENERIC;
> +        }
> +        int *p_data_delay_R = p_data_delay + i_m_dim;
> +        for ( int i = 0 ; i < i_m_dim ; i++ )
> +        {
> +            *( p_data_delay + i ) = i_Delay[0];
> +            *( p_data_delay_R + i ) = i_Delay[1];
> +        }
> +    }
> +    else if ( strncmp ( i_data_delay_dim_name, "M", 1 ) )
> +    {
> +        msg_Err ( p_filter, "DataDelay not in the required Dimensions IR or MR.");
> +        free( p_data_delay );
> +        free( p_data_ir );
> +        free( p_sp_e );
> +        free( p_sp_a );
> +        free( p_sp_r );
> +        nc_close(i_ncid);
> +        return VLC_EGENERIC;
> +    }
> +    else if ( !strncmp ( i_data_delay_dim_name, "M", 1 ) )
> +    {
> +        msg_Dbg( p_filter, "DataDelay in Dimension MR");
> +        i_status = nc_get_var_int( i_ncid, i_data_delay_id, p_data_delay );
> +        if (i_status != NC_NOERR)
> +        {
> +            msg_Err(p_filter, "Couldn't read Data.Delay");
> +            free( p_sp_e );
> +            free( p_sp_a );
> +            free( p_data_ir );
> +            free( p_data_delay );
> +            free( p_sp_r );
> +            nc_close(i_ncid);
> +            return VLC_EGENERIC;

Seriously, factor error paths.

> +static int GetSpeakerPos ( filter_t *p_filter, float *p_speaker_pos )
> +{
> +    uint16_t i_physical_channels = p_filter->fmt_in.audio.i_physical_channels;
> +    if ( i_physical_channels == AOUT_CHAN_CENTER )
> +    {
> +        memcpy( p_speaker_pos , (float[1]){ 0 } , 1 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_STEREO || i_physical_channels == AOUT_CHANS_2_1 )
> +    {
> +        memcpy( p_speaker_pos , (float[2]){ 30 , 330 }, 2 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_3_0 || i_physical_channels == AOUT_CHANS_3_1 )
> +    {
> +        memcpy( p_speaker_pos , (float[3]){ 30 , 330 , 0 }, 3 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_4_0 || i_physical_channels == AOUT_CHANS_4_1 )
> +    {
> +        memcpy( p_speaker_pos , (float[4]){ 30 , 330 , 120 , 240 }, 4 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_5_0 || i_physical_channels == AOUT_CHANS_5_1 )
> +    {
> +        memcpy( p_speaker_pos , (float[5]){ 30 , 330 , 120 , 240 , 0 } , 5 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_5_0_MIDDLE || i_physical_channels == ( AOUT_CHANS_5_0_MIDDLE | AOUT_CHAN_LFE ) )
> +    {
> +        memcpy( p_speaker_pos , (float[5]){ 30 , 330 , 120 , 240 , 0 } , 5 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_6_0 )
> +    {
> +        memcpy( p_speaker_pos , (float[6]){ 30 , 330 , 90 , 270 , 150 , 210 }, 6 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_7_0 || i_physical_channels == AOUT_CHANS_7_1 )
> +    {
> +        memcpy( p_speaker_pos , (float[7]){ 30 , 330 , 90 , 270 , 150 , 210 , 0 }, 7 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    else if ( i_physical_channels == AOUT_CHANS_8_1 )
> +    {
> +        memcpy( p_speaker_pos , (float[8]){ 30 , 330 , 90 , 270 , 150 , 210 , 180 , 0 }, 8 * sizeof (float ) ) ;
> +        return VLC_SUCCESS;
> +    }
> +    return VLC_EGENERIC;
> +}

Use switch and factor this code.

> +    struct filter_sys_t *p_sys = p_filter->p_sys;
> +    if ( p_sys && p_sys->b_Enable )

I doubt p_sys can be null here.

And why do you need p_sys1 and p_sys...

Best regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device



More information about the vlc-devel mailing list