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

[Curve] fixing precision issue with HUE mode on cubic curves, and cac… #10

Closed alcinos closed 7 years ago

alcinos commented 7 years ago

…hing map computation to avoid superfluous work

This PR essentially does two things:

ddennedy commented 7 years ago

This looks good, but you should increase the minor version in the plugin info especially since the hue curve parameter value is interpreted differently now. In the future, please make the first line of the commit message shorter to make it more readable. I do appreciate the detail in the body.

alcinos commented 7 years ago

I'm not sure what you mean by the "hue curve parameter value is interpreted differently". It is interpreted has it should be, this is just a bug fix, it is not intended to change the behaviour.

Here is a demo of the bug that was experienced before. With this input image: test and the melt command line

melt test.jpg -filter frei0r.curves 0=0.6 3=2 1=0-consumer avformat:o.gif

Which simply apply identity curve on the HUE space (using the cubic curve mechanism), we obtain test2-0

Notice how the color are distorted. This is NOT the intended behaviour, since the identity curve should produce the same image. Note that this doesn't happen if you supply the curve as a Bezier curve.

ddennedy commented 7 years ago

If a parameter value gives a different result than before, then it behaves differently regardless if it is a fix or not. People may have made a composition or script using a specific value that will be give a different result with this new version. When using MLT to serialize XML, it writes the frei0r plugin version number so one can know the values are for a specific version. Bump the version.

alcinos commented 7 years ago

Ok, thanks for the clarification! I pushed the modification and split the commit message to make it more readable.

Please tell me if anything has to be done before this gets merged :)