[vlc-devel] [PATCH] Vovoid VSXu audio visualization integration

[Jean-Baptiste Kempf]

On Mon, Mar 12, 2012 at 10:45:16PM +0100, Jonatan Wallmander wrote :
> Subject: [PATCH 1/2] Added support for audio visualization: Vovoid VSXu

Please squash your 2 patches...

>  dnl
> +dnl Vovoid VSXu visualization plugin
> +dnl
> +AC_ARG_ENABLE(vsxu,
> +  [  --enable-vsxu           Vovoid VSXu visualization plugin (default enabled)])
> +AS_IF([test "${enable_vsxu}" != "no"],
> +  [
> +    PKG_CHECK_MODULES(VSXU, libvsxu,
> +    [
> +      VLC_ADD_PLUGIN([vsxu])
> +      VLC_ADD_CXXFLAGS([vsxu],[$VSXU_CFLAGS])
> +      VLC_ADD_LIBS([vsxu],[$VSXU_LIBS $PROJECTM_LIBS])
> +    ],[
> +      AC_MSG_WARN([${VSXU_PKG_ERRORS}.])
> +    ])
> +  ])

Please use PKG_ENABLE_MODULES_VLC

> +SOURCES_vsxu = vsxu.cpp

and not cyclic_buffer.h ?

> + * Copyright ?? 2009-2011 the VideoLAN team

Doubtful dates

> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_aout.h>
> +#include <vlc_vout.h>
> +#include <vlc_vout_wrapper.h>
> +#include <vlc_opengl.h>
> +#include <vlc_filter.h>
> +#include <vlc_rand.h>

All of those are needed ?

> +    add_directory( "vsxu-asset-path", "/usr/share/vsxu",
> +                  CONFIG_TEXT, CONFIG_LONGTEXT, true )

And on Win32?

> +    add_integer( "vsxu-width", 1280, WIDTH_TEXT, WIDTH_LONGTEXT,
> +                 false )
> +    add_integer( "vsxu-height", 720, HEIGHT_TEXT, HEIGHT_LONGTEXT,
> +                 false )

Why this aspect ratio ?

> +struct filter_sys_t
> +{
> +    /* */
> +    vlc_thread_t thread;
> +    vlc_sem_t    ready;
> +    bool         b_error;
> +
> +    /* Opengl */
> +    vout_thread_t  *p_vout;
> +    vout_display_t *p_vd;
> +
> +    /* Window size */
> +    int i_width;
> +    int i_height;
> +
> +    /* audio info */
> +    int i_channels;
> +
> +    /* */
> +    vlc_mutex_t lock;
> +    bool  b_quit;
> +    float *p_buffer;
> +    unsigned i_buffer_size;
> +    unsigned i_nb_samples;
> +};
I think you can pack it better.


> +    p_sys = p_filter->p_sys = (filter_sys_t*)malloc( sizeof( *p_sys ) );
> +    if( !p_sys )
> +        return VLC_ENOMEM;

Use unlikely.

> +// our abstract holder
> +vsx_manager_abs* manager;
> +
> +// keep track of first frame
> +bool first = true;
> +
> +// keep track of iterations
> +int i_iterations = 0;
> +
> +// mutex around the cyclic block
> +vlc_mutex_t cyclic_block_mutex;
> +
> +// cyclic buffer to cache sound frames in
> +cyclic_block_queue vsxu_cyclic_buffer;
> +
> +// small logo intro
> +vsx_logo_intro* intro;

Why these variables here?

Best regards,

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