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

[Andre Weber]

> 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).
no it only compiles on Linux and Win32 so far I know - other OS may need some Ifdefs
(so I will need a little bit more understanding of the script configure.ac to do the required
 changes... would end in copy and paste some other code...;-)

> 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.
the structure was used only for storing a .bmp file to disk - as debug option
not for real production use.

>Can't you use the VLC API to save a picture to a file?
there is an API for it ... never looked for ... will try to use the API if there is allready one.
(and not reinventing the wheel?)

> You have windows line endings in some of the files.
may be - i'am editing the source with Visual Studio  - Ok will process the files next time with dos2unix to avoid

> You have trailing spaces.
mmh - is there a tool to remove them automatic? I don't want to check every single file*g*

> It would be better if your source files were in a subdirectory.
Ok what I have to do?
- create folder?
-- add a modules.am file to it?
-- and whats more? I have no very good knowledge about modifying the build system.
(I see I will have to look into configure.ac? bootstrap? where else?)

>  * atmo.cpp:
>    Lines should be 80 characters max.
mmh ... I have no problem with long lines *g* @1600x1200 resolution. Ok will try
to wrap lines so far as possible.

>    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.
mmh... long description Ok...should be possible to describe it a little bit in detail,
a webpage - must look who feels responsible in german community offering some
webspace for this .. and writing a english description for setup.

>    Comments should be in English, not German
mmh? where I have left german comments?

>    Files don't mention a license (well they mention a README.txt file
>    which isn't present in the diff)
Ok .. forgotten to change the header. The source is under GPL because the source
from VDR project I use are also under GPL (two I  think).

>    ExtractMiniImage_YUV: is this just a chroma convert + scale function?
yes ... and puts the resulting RGB image directly in my transfer buffers so
I have no need to copy them again for the processing. -> which needs an in
memory RGB (BGR) image.

>    If it is you could use the ImageConvert VLC function which would call
>    ffmpeg to do the chroma + scale transform.
will this be faster than my simple code? I need no interpolation between the pixels
just dropping not required pixels is good enough for my further use of the image.

>    SaveBitmap: Same comment as vlc_codecs.diff
I didn't know that there are allready functions for saveing images... which could be used by other modules.
so I will look into this.

>    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).
Ok ... I thought this allready ... but didn't know how fast these functions are... so I will
add callbacks to these variables for update my config object. (is cleaner you are right)

> That'll be it for today.
and enough to do for the next week...

Do you have birthday today? I read something like this on IRC? If this is true

Happy Birthday... and the best wishes from me...  to you....


André Weber
--
atmo