Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add normalize() #194

Merged
merged 1 commit into from
Apr 19, 2015
Merged

Add normalize() #194

merged 1 commit into from
Apr 19, 2015

Conversation

bkw
Copy link
Contributor

@bkw bkw commented Apr 16, 2015

Scratching my own itch, here is a simple histogram stretcher.
It uses vips-scale and none of the fancy other equalization techniques like vips_hist_equal, since it was simpler this way and gave the result I wanted ;-)

I experimented with implementing the optional exponent argument for log scaling, but since I couldn't get any useful results with that, I left it out for now.

I expect to have to rebase after #192 has been merged.
Looking forward to Your comments.

this.options.normalize = (typeof normalize === 'boolean') ? normalize : true;
return this;
};

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you add Sharp.prototype.normalise = Sharp.prototype.normalize; to keep with the greyscale/grayscale pattern of supporting both types of English :)

@lovell
Copy link
Owner

lovell commented Apr 16, 2015

This looks great, thank you, loving the tests too!

For colour images, vips_scale will seek the min/max value across all channels then scale all channels equally. This may produce better results in the LAB colour space. Are you able to try converting to VIPS_INTERPRETATION_LAB via a vips_colourspace operation before the vips_scale to see if this is the case?

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

added the alias and rebased to current judgement branch, now looking into the colorspace thing.

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

Just cargo-culting from other transformations only gave me an empty white image.
Here is what I tried: https://gist.github.com/bkw/0055c4f638328f6a4dc8
I did not transform back, since the sRGB transform happens right after that anyway.

Any hints as to what I'm missing?

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

I think I'll have to somehow limit the effect of vips_scale() to the Luminance-Channel only.

@lovell
Copy link
Owner

lovell commented Apr 16, 2015

The Luminance of LAB is clamped to a 0-100 range, hence the near-white output when scaled 0-255.

The vips_extract_band and vips_bandjoin2 methods are your friends for extracting/reattaching L and AB.

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

Thanks. But I guess I'm at a dead end with vips_scale then. Looking at the code, it always scales up so that max hits 255. I don't think I have the time right now to dive into the other methods like the vips_hist family, sorry.

I guess I'll let this rest then.

@lovell
Copy link
Owner

lovell commented Apr 16, 2015

vips_scale is a wrapper around methods such as vips_stats. Leave it with me and I'll see what I can do.

@lovell
Copy link
Owner

lovell commented Apr 16, 2015

The following seems to work, at least for images without transparency (probably need to extract alpha channel before LAB conversion then rejoin at the end).

    // Convert to LAB
    VipsImage *lab;
    if (vips_colourspace(image, &lab, VIPS_INTERPRETATION_LAB, NULL)) {
      return Error();
    }
    vips_object_local(hook, lab);
    // Split into L and AB
    VipsImage *luminance;
    if (vips_extract_band(lab, &luminance, 0, "n", 1, NULL)) {
      return Error();
    }
    vips_object_local(hook, luminance);
    VipsImage *chroma;
    if (vips_extract_band(lab, &chroma, 1, "n", 2, NULL)) {
      return Error();
    }
    vips_object_local(hook, chroma);
    // Normalise L in 0-255 range
    VipsImage *luminance255;
    if (vips_scale(luminance, &luminance255, NULL)) {
      return Error();
    }
    vips_object_local(hook, luminance255);
    // Normalise L in 0-100 range
    VipsImage *luminance100;
    if (vips_linear1(luminance255, &luminance100, 100.0 / 255.0, 0.0, NULL)) {
      return Error();
    }
    vips_object_local(hook, luminance100);
    // Join normalised L to AB
    VipsImage *normalized;
    if (vips_bandjoin2(luminance100, chroma, &normalized, NULL)) {
      return Error();
    }
    vips_object_local(hook, normalized);
    image = normalized;

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

Thanks! Thats so funny - my current attempt is almost exactly identical, even down to the name of normalized100 :-) With the minor difference that yours works while mine segfaults...

Since vips_scale also casts to uchar, we'd lose quite a bit of resolution from using it. I'm working on leaving out vips_scale all together and use vips_stats and vips_linear1 on our own.
It still will take some time, but your example removes much of the api guesswork that takes me so long (I was counting bands from 1 and have been pulling my hair why I can't get to the b channel for example)

Thanks again, much appreciated. I hope to come back with something workable soonish, I'll take a look at alpha as well.

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

I updated the PR to work in LAB (but without using vips_scale). Could you take a look at style and such things?
I'll see what happens with alpha in the meantime.

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

Extracting the alpha and glueing it back on works (better than graphicsmagick, btw).
Currently my poc is very ugly, but it works.

Is there a way to programatically get the band number of an existing alpha channel, as a companion to HasAlpha(image) ?

Update: I found it - if (HasAlpha(image) alphaBand = image->Bands-1; :-)

@bkw
Copy link
Contributor Author

bkw commented Apr 16, 2015

rebased, squashed and force-pushed.

Alpha channels are preserved now, and input grayscale images no longer are forced to be sRGB on output. This may be controversial, since it changes existing behavior. I just didn't want to blow up greyscale images just by normalizing them.

@bkw bkw force-pushed the normalize branch 4 times, most recently from 2872043 to c379245 Compare April 16, 2015 23:08
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 97.34% when pulling c379245 on bkw:normalize into ba034a8 on lovell:judgement.

vips_object_local(hook, stats);
double min = *VIPS_MATRIX(stats, 0, 0);
double max = *VIPS_MATRIX(stats, 1, 0);
double f = 100.0 / (max - min);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible /0 error here. If min == max we can probably skip the rest of normalization.

@bkw
Copy link
Contributor Author

bkw commented Apr 17, 2015

sorry, I accidentally pushed my icc-greyscale experiments, pushed again.

@bkw
Copy link
Contributor Author

bkw commented Apr 17, 2015

I reverted back to converting everything to srgb and adjusted the normaize()-tests to no longer expect b_w output for grayscale input. For the time being I think this is the most pragmatic solution.

What do you think, @lovell ? Is this good to merge now? I'll wait with the squashing.

@lovell
Copy link
Owner

lovell commented Apr 17, 2015

This looks great, thank you Bernhard. Squash away!

Available as normalize() or normalise().
Normalization takes place in LAB place and thus should not change any
colors.

Existing alpha channels are preserved untouched by normalization.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 97.34% when pulling dce36e0 on bkw:normalize into ba034a8 on lovell:judgement.

@bkw
Copy link
Contributor Author

bkw commented Apr 18, 2015

ready.

lovell added a commit that referenced this pull request Apr 19, 2015
Add normalize() to use full luminance range.
@lovell lovell merged commit be39297 into lovell:judgement Apr 19, 2015
@lovell
Copy link
Owner

lovell commented Apr 19, 2015

Thank you Bernhard! Merged and will be included in v0.10.0 to be published in a few days time.

@lovell lovell added this to the v0.10.0 milestone Apr 19, 2015
@skedastik
Copy link
Contributor

Wow, this is exactly the other feature I need. Thanks, @bkw! 👍

@bkw
Copy link
Contributor Author

bkw commented Apr 19, 2015

Glad to help, @skedastik - you provided the other one I needed :-)
We don't happen to work on the same thing - query by image?

@skedastik
Copy link
Contributor

Query by image -- you got it. Here's to simultaneously reinventing the wheel. 🍺

@bkw
Copy link
Contributor Author

bkw commented Apr 20, 2015

I drink to that 🍻 !

@lovell
Copy link
Owner

lovell commented Apr 20, 2015

If I was creating an image hashing algorithm (I'm not), it would use a DCT-based method of extracting and sorting luminance frequencies from low to high, similar to phash, but arrange them in a hierarchical system like geohash to enable fast bucket-based searching/narrowing.

@bkw
Copy link
Contributor Author

bkw commented Apr 20, 2015

Pssst shush ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants