[vlc-devel] [Submit] video filter module - for the homebrew AmbiLight (AtmoLight)
dionoea at videolan.org
Sun Jan 13 16:42:07 CET 2008
I'll comment the patches one by one:
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
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
Looks fine :)
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.
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)
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.
More information about the vlc-devel