search

dyne / frei0r

A large collection of free and portable video plugins
https://frei0r.dyne.org/
GNU General Public License v2.0
427 stars 89 forks source link

Added normaliz0r plugin. #15

Closed Chungzuwalla closed 6 years ago

Chungzuwalla commented 6 years ago

I've implemented a filter to normalize video, which seemed to be a missing feature (there's an equalizer, but I couldn't get the results I needed with it).

This filter linearly stretches the contrast of each input frame to the full dynamic range [0,255]. To avoid rapid changes of brightness (flickering), the input min and max are smoothed using a rolling average over a user-specified number of previous frames. It's good for tweaking low-contrast video and removing unwanted color casts. The comment in the source file gives a pretty thorough description of its capabilities.

Build has only been tested on Eclipse Neon CDT with MinGW toolchain, as that's all I've got.

Anyway hope this is useful.

Chungzuwalla commented 6 years ago

And... it looks like Travis didn't even try to build it. So I probably missed a build step...

jaromil commented 6 years ago

Nice filter! the code looks very fine to me. The Travis CI buids using the autoconf framework, which is also optional in frei0r. You just need to add your filter into src/Makefile.am, let me know if you like to patch your PR for this, I can also do it after merging.

Chungzuwalla commented 6 years ago

Hmm. Are these trailing semicolons in frei0r_math.h a bug? https://github.com/dyne/frei0r/blob/master/include/frei0r_math.h#L67

I'm still not sure why I don't get this error locally.

jaromil commented 6 years ago

At first glance they look like a bug since other macros don't have trailing semicolons and that should be considered sane of course. I believe you don't get the error since on your system MIN/MAX are provided by libm and then skipped in frei0r by ifdefs. We should bring this up on the mailinglist before any change.

Chungzuwalla commented 6 years ago

Yeh, I figured out that they're already defined in my environment. I've worked around the trailing semicolons, in a way that will still work if they are removed. Lets see if I can be lucky this time.

jaromil commented 6 years ago

you made it correct this way, but I have a request after discussing on the dev mailinglist everyone with an opinion agrees the safe way is to remove the semicolons in the header: https://lists.dyne.org/lurker/message/20170822.153104.76f9159d.en.html would this fix it?

Also we do not want to have all this in the same PR. Another one needs to be open with any fixes to the header.

Chungzuwalla commented 6 years ago

Yes, I think it is better to remove the semicolons if everyone agrees to it. Thanks for asking the community!

I will open a new issue and PR for frei0r_math.h. Once that's resolved I will reset the head of this PR to 722bf87, it should compile then.

jaromil commented 6 years ago

Great job with #16 - now just rebase this to include only the new filter and we'll merge it too. And you are very welcome to stick around frei0r, great contributions! ;^)

Chungzuwalla commented 6 years ago

Thanks jaromil! I hope it will be a useful filter! ;)

I've rebased this PR onto the new master which includes PR #16 . It's kept the original dates for the commits, but I guess it doesn't matter. It all builds now.

jaromil commented 6 years ago

cheers and welcome to frei0r :+1: