[vlc-devel] [Request for Comment] new filter module for AtmoLight

[Jean-Paul Saman]

Andre Weber wrote:
> Hello,
> 
> some days ago I started to work on the module - so far I am ready -
> the users at VDR-Portal.de say it works stable - it doesn't crash my private
> VLC build - so I  think its now on some persons here to have a look
> at it - for a second opinion if it is coded clean enough - and if it could
> be integrated into one of the next releases of VLC I read here that
> VLC 0.8.6d should be released shortly - I think my module will
> compile with this version without bigger problems - because my
> development was done with 0.8.6c ... I will spent some time
> this weekend to check if my module will compile and work with
> the VLC trunk version - is this the right way? to get it into the sourcetree?
> 
> attached the source of my filter module - the other software parts of the libary and AtmoWin
> controlsoftware - which is related to this module could be found here:
> http://eldo.gotdns.com/atmo-src/ --> the complete Atmolight source tree
> http://eldo.gotdns.com/atmo-src/AtmoCtrlLib/  --> the com wrapper library
> http://eldo.gotdns.com/vlc-src/vlc-086c my current vlc development folder
> beaware this files are hosted on my workstation so the sites is no allways
> available...

Thanks for providing this work for inclusion in vlc mainline development 
tree. Some remarks though:

1) Please resubmit the work as patch against current development tree, 
using svn diff > atmo.patch
2) include changes to configure.ac and modules/video_filter/Modules.am
3) provide patch against extras/contrib/* for the atmo library
4) don't use that many void *
5) write function pointers in structs as (it is much more readible and 
directly clear it is meant to be a function):

struct mystruct
{
	int (*pf_atmo_initialize) (void);
	void (*pf_atmo_finalize) (int);
	int (*pf_atmo_switch_effect) (int);
	int (*pf_atmo_set_live_source) (int);
	void (*pf_atmo_create_transfer_buffers) (int, int, int, int);
	uint8_t* (*pf_atmo_lock_transfer_buffer) (void);
	void (*pf_atmo_send_pixel_data) (void);
}

6) Coding style:
- we use i_width iso width
- tabs are 4 spaces
- indentation is 4 spaces
- in "C" files declare variables at the beginning of a code block (just 
proper software engineering)

7) don't prefix functions with nonwin32_ when using #if !defined(WIN32), 
functions can have the same name in the #else of this clause.
8) it looks like it only works on windows and not under Linux. Why? Is 
it impossible to compile the atmo library for linux?
9) Remove debug/dead code 
                        10) when using vlc_object_create() there should 
also be vlc_object_attach()/vlc_object_detach() and vlc_object_ready() 
in the thread functions.


I think it still needs a bit of cleanup, but you are on the right way. 
Hope to see a further developed video_filter for atmo very soon. Keep up 
the good work!

Gtz,
Jean-Paul Saman.