[vlc-devel] [Submit] video filter module - for the homebrew AmbiLight (AtmoLight)

[Antoine Cellerier]

I'll comment the patches one by one:

configure.diff:
Does the module compile on all the platforms supported by VLC? If not,
you should check that you're on a supported platform before adding the
plugin. It might also be a good idea to add an option --disable-atmo
configure flag in case people want to disable it (if it takes too long
to compile).

vlc_codecs.diff:
Why do you need to define this structure? Can't you use the VLC API to
save a picture to a file? (check the image vout module, or the snapshot
feature in the vout core)
If you only need it for internal communication between the VLC filter
and the atmo driver, you should include the define in your private
headers.

vout_intf.c:
Looks fine :)

atmo_video_filter.diff:
You have windows line endings in some of the files.
You have trailing spaces.
It would be better if your source files were in a subdirectory.
 * Atmo*.{cpp,h}:
   Comments should be in English, not German
   Files don't mention a license (well they mention a README.txt file
   which isn't present in the diff)
 * atmo.cpp:
   Lines should be 80 characters max.
   You might want to include a long description of your module using
   set_help() in the module definition (see example in
   modules/audio_filter/channel_mixer/headphone.c). Links to a webpage
   describing the setup would be more than welcome.
   The switch line 900 can be simplified.
   ExtractMiniImage_YUV: is this just a chroma convert + scale function?
   If it is you could use the ImageConvert VLC function which would call
   ffmpeg to do the chroma + scale transform.
   SaveBitmap: Same comment as vlc_codecs.diff
   Filter: calls to var_GetInteger/Bool/... are slow (especially since
   your filter has loads of variables). You should update the values
   only if they change (like in a variable callback).

That'll be it for today.

-- 
Antoine Cellerier
dionoea