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

[Andre Weber]

Hello Dionoea,

now the second try to submit my filter

because the attachment is too big I put it on my temp webserver for immediate download*g*
http://eldo.gotdns.com/vlc_atmo.zip

it contains the diff of my new folder /video_filter/atmo/ including the readme.txt

configure.diff - with check for windows or linux build, and option to disable atmo
durring configure. (--disable-atmo) if you don't like it.

> 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.
because I need it only internaly I moved the changes from there to my own
file "AtmoDefs.h" ... but I still think it could also be part of  "vlc_codecs.diff:"
just to have a complete definition of a bitmap file there.

> vout_intf.c:
> Looks fine :)
the same again - no changes just to not forget it

> atmo_video_filter.diff:
>>> You have windows line endings in some of the files.
removed ... through dos2unix

>>> You have trailing spaces.
remove by a replace run with regular expression ;-) the lazy way

>>> It would be better if your source files were in a subdirectory.
done

>>>    Files don't mention a license (well they mention a README.txt file
>>>    which isn't present in the diff)
readme and license there (GPL2)

>>>  * atmo.cpp:
>>>    Lines should be 80 characters max.
ok ... the - pervert part

>>>    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.
ok I tried my best -- currently linking to german wiki pages of the project
I hope somebody will translate it ...*g*

>>>    The switch line 900 can be simplified.
done... I hope the way you wanted it?

>>>    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.
stays there - because I need no interpolation just a fast copy... and "downscale"
- directly to my memory internal buffer....

>>>    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).
remove polling - and replaced by callback -- which isn't currently called,
because of the issue? with the settings dialog - which isn't modifiying the
object variables... the code is still there so that I can improve this later.
(if it is required)

so far thats all I think ... now hopeing again....

--
André Weber
Atmo




----- Original Message ----- 
From: "Antoine Cellerier" <dionoea at videolan.org>
To: "Mailing list for VLC media player developers" <vlc-devel at videolan.org>
Sent: Sunday, January 13, 2008 4:42 PM
Subject: Re: [vlc-devel] [Submit] video filter module - for the homebrewAmbiLight (AtmoLight)


> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel