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

[Jonatan Wallmander]

Ok, here we go again, I hope all is fixed. :)

On 03/12/2012 11:22 PM, Jean-Baptiste Kempf wrote:
> 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...
Done.

>
>>   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
As per our IRC conversation, I'm ignoring this one.
>
>> +SOURCES_vsxu = vsxu.cpp
> and not cyclic_buffer.h ?
Ah, of course.
>
>> + * Copyright ?? 2009-2011 the VideoLAN team
> Doubtful dates
Not sure what is best, I increased 2011 to 2012 and added Vovoid etc..

>
>> +#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 ?
Nope. Removed a few.

>
>> +    add_directory( "vsxu-asset-path", "/usr/share/vsxu",
>> +                  CONFIG_TEXT, CONFIG_LONGTEXT, true )
> And on Win32?
VSXu has a default behaviour which works well, so I removed this option 
entirely.

>
>> +    add_integer( "vsxu-width", 1280, WIDTH_TEXT, WIDTH_LONGTEXT,
>> +                 false )
>> +    add_integer( "vsxu-height", 720, HEIGHT_TEXT, HEIGHT_LONGTEXT,
>> +                 false )
> Why this aspect ratio ?
Now using 1280x800 (16:10). If that is too large a window, change it at 
your leisure.

>
>> +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.
Done (I hope).

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

>
>> +// 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?
I put them in the filter_sys_t struct instead. One variable was moved 
into the thread as static.


Take care!

/j

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-support-for-audio-visualization-Vovoid-VSXu.patch
Type: text/x-diff
Size: 21480 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120314/e40dc5e4/attachment.patch>