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

[Andre Weber]

Hi Jean-Paul Saman,

ok I will to tidy up the code - so that meets your requirements and rules - no problem.
And provide you with the required patches.

> 4) don't use that many void *
most void * could be replaced by correct type you are right.
At one case I'am not sure if it will work - because there is some cross definition
one needs a pointer to function - which gets this structure as pointer... ;-)

> 5) write function pointers in structs as (it is much more readible and
> directly clear it is meant to be a function):
mmh - ok - if you prefer that in (Borland Delphi we use "T"...;-) ) that
happens if you jump between a lot of languages...

> 6) Coding style:
> - we use i_width iso width
will replace this by another name including i_height to keep the naming consistent

> - tabs are 4 spaces
ok.

> - indentation is 4 spaces
ok.

> - in "C" files declare variables at the beginning of a code block (just
> proper software engineering)
ok - you are right. we are using C not C++ -- is it possible to use C++
also inside a module? because some of my code which I may use with
a linux implemenation is written object oriented in C++?

> 7) don't prefix functions with nonwin32_ when using #if !defined(WIN32),
> functions can have the same name in the #else of this clause.
ok.

> 8) it looks like it only works on windows and not under Linux. Why? Is
> it impossible to compile the atmo library for linux?
the libary won't compile for linux - because it make use of COM objects -
for linux I will try to implement it directly into the plugin.
Under win32 it works that way:
vlc loads atmo-plugin --- atmo-plugins loads my AtmoCtrlLib.dll  ---
this library searches in the Running Object Table (ROT) for an registered COM
object from AtmoWinA.exe (my userspace driver) which does all the very time
consuming calculation to provide the atmo light controller the values)

I think for a Linux development I will need a linux environment, because
there is no crosscompile below windows for linux? I'am right? I am not
a professional linux user / or developer - which distribution would you
advice for setting up a build VLC linux environment?



> 9) Remove debug/dead code
ok - did I forget some? mmh I thought that I have allready removed it.. ;-)
 is it enought to define the debug code out? (with a local define inside my source?)
 so that I could keep him? if there is later something to debug?

> 10) when using vlc_object_create() there should
> also be vlc_object_attach()/vlc_object_detach() and vlc_object_ready()
> in the thread functions.
what is the meaning of this functions? most code I used currently is just copy
and paste from other filter -- I'am far away from understanding everything,
what I'am doing there.


> 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!
I will try...


André



----- Original Message ----- 
From: "Jean-Paul Saman" <jean-paul.saman at planet.nl>
To: "Mailing list for VLC media player developers" <vlc-devel at videolan.org>; <WeberAndre at gmx.de>
Sent: Sunday, November 25, 2007 11:05 AM
Subject: Re: [vlc-devel] [Request for Comment] new filter module for AtmoLight


> 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.
>
>
>