Clone of Gimp's Color Balance?

[mikeytown2]

Now I remember why I didn't like non scripting languages... constructors!

I think Gimp's Preserve Luminosity is broken, or at least it's suffering from rounding errors. did a quick compare with this image from wikipedia



GIMP - Shadows
-54, -16, 17
Preserve Luminosity checked


Plugin

ImageReader("wikipediamiamisunset200ke5.jpg",0,0,pixel_type="RGB32")
a=last.ConvertToYUY2()
ColorBalance([<"LOW Cyan-Red", -100.0, 100.0, -54.9>],[<"LOW Magenta-Green", -100.0, 100.0, -16.6>],[<"LOW Yellow-Blue", -100.0, 100.0, 17.4>], \
[<"MID Cyan-Red", -100.0, 100.0, 0.0>],[<"MID Magenta-Green", -100.0, 100.0, 0.0>],[<"MID Yellow-Blue", -100.0, 100.0, 0.0>], \
[<"HI Cyan-Red", -100.0, 100.0, 0.0>],[<"HI Magenta-Green", -100.0, 100.0, 0.0>],[<"HI Yellow-Blue", -100.0, 100.0, 0.0>])
ConvertToYUY2()
MergeLuma(a)

Both give this when not Preserving Luminosity




Edit:
Using Midtones in gimp doesn't give awful results with Preserve Luminosity Checked.
Interesting... http://www.larryjordan.biz/articles/lj_color.html. sounds like ColorYUV(), but with a nice looking GUI. Thanks Gavino for pointing this out in your 2nd post and to Mug Funky for pointing to a nice GUI. Looks like http://avisynth.org/mediawiki/Histogram#Color2_mode would be useful


Edit 2: CinePaint with same settings as Gimp

Looks like 16bit helps a little bit, but not a lot

[IanB]

Originally Posted by Gavino

... though in general I prefer not to repeat code as it makes it harder to change.

Yes, repeating code of any substance can be problematic. But the thing here is the inner loop code should be as trivial as you can possibly make it, sufficiently trivial that cloning it is also trivial.

If there needs to be a meaty block of code repeated in several places and performance is an issue then you can use the inline compiler directive. Appropriate use of the alias operator, "&", with the formal arguments to the routine can help bend the compiler to your will.

[Gavino]

Originally Posted by mikeytown2

Now I remember why I didn't like non scripting languages... constructors!

Don't be put off - the stuff in the constructor is basically the things that need only be done once per filter instance, rather than on every frame. That's not an issue with Gimp as it only has one 'frame' to process.

A further optimisation (this time shared with Gimp) is that some lookup tables are independent of the filter parameters and so are created just once as static and shared by all instances.

I think Gimp's Preserve Luminosity is broken, or at least it's suffering from rounding errors.
...Using Midtones in gimp doesn't give awful results with Preserve Luminosity Checked.

Could it be that Gimp is using PC-range in its RGB<->YUV conversions? I think this would show bigger differences at the extremes of the range and less on midtones, consistent with what you saw.

[mikeytown2]

0.2 Changes
Added in Preserve Luminosity and set it default true
Compiled in MS VC++ 2005 with 0 warnings

http://rapidshare.com/files/14064363...e_0.2.zip.html


Next step is to create a GimpSupportFunctions Class, do simple code optimizations, then go 16 bit/double with a rounding function at the end. This is fast, so MT doesn't appeal to me right now.

gimpcolorspace 2.5.3 gimp_rgb_to_hsl_int, gimp_rgb_to_l_int, gimp_hsl_to_rgb_int, gimp_hsl_value_int
http://www.sfr-fresh.com/unix/misc/g...mpcolorspace.c
http://www.sfr-fresh.com/unix/misc/g...mpcolorspace.h

CinePaint equivalent rgb_to_hls, hls_to_rgb, hls_value
http://www.sfr-fresh.com/unix/misc/c...t_funcs_area.c
Doesn't look 16bit

[mikeytown2]

0.21 Changes
Code cleanup, should be slightly faster
Added AvsP sliders Example to zip

http://rapidshare.com/files/14066935..._0.21.zip.html

Preserve Luminosity Eats CPU

[mikeytown2]

0.22 Changes
Took out rounding and unnecessary calls to floor, Pixels shouldn't go out of bounds (some might have before). Slight speed increase as a result.
Updated AvsP sliders Example

http://rapidshare.com/files/14073331..._0.22.zip.html

[mikeytown2]

Looking at CinePaint

I think i'll use color_balance_row_float and its friends in order to support greater then 8bit color.
http://www.sfr-fresh.com/unix/misc/c...olor_balance.c


PS
Is there an easy threading class that I can plug-in? OpenMP sounds interesting, but Microsoft Visual Studio Professional 2005 isn't exactly cheap and going GCC means I have to use AviSynth C plugins.
http://gcc.gnu.org/projects/gomp/

Last time I played with threads we used JAVA at school, which if I remember correctly has threads built in.

[IanB]

Hint 1 :- Compare the quality of the asm code generated by this style

r = *(srcp + w+2);
...
r_n = r_lookup[r];
...
*(dstp + w + 2)	= r_n;

with the asm code generated by this style

dstp[w+2] = r_lookup[[srcp[w+2]];
...

Hint 2 :- For the preserve_luminosity case you currently use the same r_n = r_lookup[r]; byte lookup table as for the vanilla case. For no speed cost you could use a higher precision scaled unsigned short (16bit) version of the 256 element table and adjust the scaling constants in the gimp_rgb_to_hsl_int to match. Further as gimp_rgb_to_hsl_int is returning 3 32bit ints anyway you could super scale those results as well for no cost and similarly adjust the constants in gimp_hsl_to_rgb_int to match.


Hint 3 :- In all gimp_rgb_to_hsl_int, gimp_hsl_to_rgb_int and supporting routine you are using type double where type float has more than enough precision and the divides are considerably faster.

[mikeytown2]

hint 1: Nice trick with killing the pointers, didn't see it (arrays, what?). After I looked at the alpha channel, I finally realized it actually wasn't that complicated. And yeah I agree that the compiler should generate better code doing it this way.

hint 2: Need some time to think this one through.

hint 3: double to float gave a big speed boost in the hsl functions, thanks for the tip!


0.23 Changes
More speed

http://rapidshare.com/files/14092257..._0.23.zip.html


PS I used CinePaint in 8bit with the example above, going 16 bit makes the function behave completely differently & Preserve Luminosity doesn't work; thus comparisons of settings is useless. 16bit is so much better though!

[IanB]

42.5f is equivalent to (float)42.5 but less typing

As for hint 1 the compiler should generate identical code but for some dumb reason using the array reference is better than the equivalent pointer arithmetic. I guess the array reference was part of some bench mark and the compiler writers put bonus effort into that case.

And yes hint 2 involves a little thought re building the transfer tables.

[mikeytown2]

0.24 changes
scaled preserve_luminosity code by 64 (256*64=16384; 360*64=23040)
changed (float) to f; thanks for the tip IanB!

http://rapidshare.com/files/14096952..._0.24.zip.html


0.3 branch should have 16bit or float internals. Going camping so it might be done in a week or 2, unless someone wants to pick it up.

@IanB
I didn't touch the transfer tables... I believe they have to change in order to go from 8 bit to xx bit. All I did was scale by 64 what the hsl functions pass around; which should help seeing how I'm rounding down.

[mikeytown2]

Got cinepaint 0.22-1 (run "apt-get update" before following readme) working on my box with andLinux, decided to do some comparisons.

Settings
S: 100,100,100
M: 100,100,100
H: -100,100,100
Lum - false

- 8bit int

- 16bit int

- 16bit OpenEXR Half Float

32-bit IEEE Float & 16-bit Fixed Point 0-2.0 don't work correctly. I can't see much of any difference between 16bit int
and 16bit float so i'll go with 16bit int; should be faster.

[mikeytown2]

Using the same settings as above I get this in GIMP and this plugin...

I have a feeling that gimp's calculations are wrong. I've looked into the math and they both seem to be doing the same thing... any ideas? Cinepaint's internals look like they would run slow compared to Gimp's.

ps whats the standard way of getting internal calculations of your own plugin's, for debuging/curiosity purposes? I'm used to outputting to a GUI or command line, haven't done graphics before; using MSVC++2005 Express.


EDIT:
found it... it has to do with low and mid

[Gavino]

Originally Posted by mikeytown2

whats the standard way of getting internal calculations of your own plugin's, for debuging/curiosity purposes?

Probably the simplest way would be to write to a log file.
Perhaps flushing buffers every frame (or every x frames) so that you can look at it on-the-fly.

found it... it has to do with low and mid

Don't keep us in suspense, tell us more

[mikeytown2]

Originally Posted by Gavino

Don't keep us in suspense, tell us more

working on a solution that gives the user an option to choose between gimp and cinepaint... by looking at the code you can see how it differers.

void ColorBalance::Transfer8BitInit(bool clone_gimp) 
{
	int y = clone_gimp ? 0 : 255;
	for (int i = 0; i < 256; i++)
	{
		double low = (1.075 - 1 / ((double) i / 16.0 + 1));
		double mid = 2.0/3.0 * (1 - SQR (((double) i - 127.0) / 127.0));

		shadows8_sub[255 - i]			= low;
		shadows8_add[i]				= clone_gimp ? low : mid; 

		midtones8_sub[i]			= mid;
		midtones8_add[i]			= mid;
		
		highlights8_sub[i]			= clone_gimp ? low : mid;
		highlights8_add[y - i]			= low;
	}
}

[mikeytown2]

The above code is close to what I actually used. highlights8_add with a -i didn't work that well...

0.25 Changes
Added clone_gimp option, default true; false means it behaves like cinepaint
Started to add in 16bit int code, don't try it, bad idea.

http://rapidshare.com/files/14270429..._0.25.zip.html

[Gavino]

Originally Posted by mikeytown2

0.25 Changes
Added clone_gimp option, default true; false means it behaves like cinepaint

according to the code, default is false:

args[11].AsBool(false)

Also, I'm a bit puzzled by the Cinepaint transfer functions. It seems that adding a given value to shadows and adding to midtones will both produce exactly the same result. Similarly for subtracting from midtones and subtracting from highlights. Surely this doesn't make sense?

[mikeytown2]

Originally Posted by Gavino

according to the code, default is false:

args[11].AsBool(false)

Also, I'm a bit puzzled by the Cinepaint transfer functions. It seems that adding a given value to shadows and adding to midtones will both produce exactly the same result. Similarly for subtracting from midtones and subtracting from highlights. Surely this doesn't make sense?

Thanks for pointing that out!

did a quick comparison and yes you are correct. CinePaint doesn't quite add up...

ColorYUV(BlankClip(pixel_type="YUY2"),showyuv=true).ConvertToRGB()
a=last

x1=a.ColorBalance(0,0,0,0,0,0,-100,0,0,false,false,false)
y1=a.ColorBalance(0,0,0,-100,0,0,0,0,0,false,false,false)

x2=a.ColorBalance(0,0,0,0,0,0,-100,0,0,false,true,false)
y2=a.ColorBalance(0,0,0,-100,0,0,0,0,0,false,true,false)

v1=a.ColorBalance(0,100,0,0,0,0,0,0,0,false,false,false)
w1=a.ColorBalance(0,0,0,0,100,0,0,0,0,false,false,false)

v2=a.ColorBalance(0,100,0,0,0,0,0,0,0,false,true,false)
w2=a.ColorBalance(0,0,0,0,100,0,0,0,0,false,true,false)

i=StackHorizontal(Compare(x1,y1).Subtitle("CinePaint", align=5),Compare(x2,y2).Subtitle("GIMP", align=5))
j=StackHorizontal(Compare(v1,w1).Subtitle("CinePaint", align=5),Compare(v2,w2).Subtitle("GIMP", align=5))

StackVertical(i,j)

0.26 Changes
Fixed Default Value - Thanks Gavino!

http://rapidshare.com/files/14296739..._0.26.zip.html

[zee944]

mikeytown2,

I'm trying to use your plugin - but when I try to load ColorBalance.dll, I get this message:

Avisynth open failure:
LoadPlugin: unable to load "E:\ColorBalance.dll"

...while E:\ColorBalance.dll exists, of course. I can easily load other plugins like this:

LoadPlugin("E:\mt_masktools[2.0a30].dll")
LoadPlugin("E:\MaskTools[1.5.8].dll")
LoadPlugin("E:\RemoveGrain[1.0].dll")
LoadPlugin("E:\GiCoCu.dll")
LoadPlugin("E:\ColorBalance.dll")

The script still stops at ColorBalance (line 5).

I've tried it with Avisynth 2.5.7 first, then installed 2.5.8 beta. Still the same. Perhaps ColorBalance needs other plugins to load first?

[kemuri-_9]

Originally Posted by zee944

I've tried it with Avisynth 2.5.7 first, then installed 2.5.8 beta. Still the same. Perhaps ColorBalance needs other plugins to load first?

or it's dynamically linking to libraries you don't have.

VC++ dynamically links to external libraries by default, need to specify it to staticly link for easier distribution.