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

Adding histogram equalization for images #435

Closed
wants to merge 1 commit into from

Conversation

codejaeger
Copy link
Contributor

@codejaeger codejaeger commented Feb 19, 2020

Description

Histogram equalization is a contrast enhancement algorithm for images whose pixel intensities are concentrated in small regions.
Currently this implementation also supports histogram equalization for images with multiple channels under the assumption the input channels are independent. Images with dependent channels (like RGB) should be be converted to an (YCbCr for RGB) independent color space before using histogram equalization.

Motivation

This PR is proposed as a solution to the competency test for GSoC 2020. I chose histogram equalization though it was not in the proposed topics pages because if accepted it could be a starting step to implementing important image processing algorithms like CLAHE for cases where histogram equalization might not give good results.

References

Application of histogram equalization on some images - Github link

Tasklist

  • Add basic tests
  • Ensure all CI builds pass
  • Add appropriate commenting
  • Review and approve
  • Updating PR Description

@codejaeger codejaeger requested review from mloskot and removed request for mloskot February 19, 2020 13:42
@codejaeger codejaeger changed the title Added histogram equalization for images Adding histogram equalization for images Feb 19, 2020
@mloskot
Copy link
Member

mloskot commented Feb 19, 2020

Thank you for the PR and attempt to propose new feature.

First, to save us all time and avoid nitpicking reviews (naming, formatting, whitespaces, style),
could you please auto-review your PR against our development guidelines linked here https://github.com/boostorg/gil/wiki/Guidelines (i.e. CONTRIBUTING.md and suggested #include ordering)?

Second, I'd strongly recommend you to read through review and comments posted to PR #430. This should give an idea how reviews run, what comments to expect.

Third, as you can read in comments to PR #430, please be aware that for the competency test itself it is enough to present C++ code, but to get your feature accepted, it usually takes more effort (discussing design and interface choices, code comments, detailed tests, documentation, histogram operations may better fit general purpose part of GIL and not necessarily the image_processing module, etc.).
IOW, you are not expected to get it all done as part of the competency test.

Meanwhile, you could also offer some more details on what algorithm the PR proposes (any thing that can be named, referenced, etc.), updating PR description is fine. Code comments and documentation will come later if you decided to continue working on it.

/cc @stefanseefeld

@codejaeger
Copy link
Contributor Author

Thanks @mloskot for the feedback . I will make the necessary changes according to the guidelines specified and will have a look at PR #430 .
As for

histogram operations may better fit general purpose part of GIL and not necessarily the image_processing module, etc.)

I was of the same opinion , but a bit unsure of how to implement that. If I can get some further insights on this, so that my changes conform to Boost:gil design choices I would be glad to make those changes.
I will also make the PR more descriptive regarding the proposed solution for the algorithm.

@mloskot
Copy link
Member

mloskot commented Feb 19, 2020

histogram operations may better fit general purpose part of GIL and not necessarily the image_processing module, etc.)

I was of the same opinion , but a bit unsure of how to implement that.

For example, core histogram utilities could live in include/boost/gil/histogram.hpp, then additional, more task-specific features could be offered as extension(s) in include/boost/gil/extension/histogram/*.hpp.

For example, if someone comes up with support for Boost.Histogram as output as well as input from/to GIL histogram utilities and algorithms, such features could be hosted in the extension.

/cc @stefanseefeld may have some insights on the bigger picture of code structuring

@mloskot
Copy link
Member

mloskot commented Feb 19, 2020

@codejaeger

Images with dependent channels (like RGB) can be be converted to YCbCr color space before using histogram equalization.

One more thing to that note, I think it would be good to clarify (eventually in comments/docs) the definition of (in)dependent channels.

For example, if it is equivalent to (or taken from) this one given in the RGB CHANNELS (IN)DEPENDENCE IN PHASE CONTRAST MICROPHOTOGRAPHY (alternative PDF), it may be worth to refer/cite that paper:

If the detection system is for example a standard photograph, each of the three channels (Red, Greenand Blue) carries different - partly **independent** - information which may be used for state definition.
(...)
In case of colour images are histogram functions normally computed **independently** for each colour channel, without considerations about relations between each other.

By the way, this also reminded me GIL-related question on StackOverflow, Histogram equalization upon RGB images? RGB ouput possible?, you may want to score some credits there :-)

@codejaeger
Copy link
Contributor Author

@mloskot
I went through the guidelines as suggested and have taken care of most of those trivial changes . I guess there could still be some issues which might come up while reviewing . I will add more advanced testcases shortly.

@mloskot
Copy link
Member

mloskot commented Feb 19, 2020

I will have a detailed look at later this week or weekend.

Meanwhile, as I explained in #430 (comment)

you are not expected to finish-polish the PR in order to count it for a completed competency test solution (...)
Although you can postpone addressing further reviews and completion of this PR for later (e.g. during GSoC), you are very welcome to continue working on to make it ready for merging. It's your call.

At this stage it's more important, I think, to gain basic experience about [Boost development[(https://www.boost.org/development/index.html) (right-hand side links), GIL itself and its development, Git & GitHub operations, and finally think about what you'd like to develop during GSoC (important for writing your proposal).

@codejaeger
Copy link
Contributor Author

At this stage it's more important, I think, to gain basic experience about [Boost development[(https://www.boost.org/development/index.html) (right-hand side links), GIL itself and its development, Git & GitHub operations, and finally think about what you'd like to develop during GSoC (important for writing your proposal).

Thanks @mloskot, I will do so right away.

@mloskot mloskot added the google-summer-of-code All items related to GSoC activities label Feb 22, 2020
@codejaeger codejaeger changed the title Adding histogram equalization for images [WIP]:Adding histogram equalization for images Mar 1, 2020
@codejaeger codejaeger closed this Mar 2, 2020
@codejaeger codejaeger reopened this Mar 2, 2020
@codejaeger
Copy link
Contributor Author

codejaeger commented Mar 2, 2020

Accidentally closed the PR by mistake,
it seems that this PR need refurnishing before it can be even actually accepted as a partial solution to the histogram issue in GIL. So , I think I will make the changes as proposed in the wiki page.

@mloskot
Copy link
Member

mloskot commented Mar 2, 2020

@codejaeger First, this PR is a competency test solution. If your project is accepted, you will have chance to work on it during GSoC and if not you will be welcome to continue working on it.
One way or another, this PR can be improved according to the suggestions on the Wiki page, later. I wouldn't worry about it now.

@mloskot mloskot added the status/work-in-progress Do NOT merge yet until this label has been removed! label Mar 10, 2020
@codejaeger codejaeger changed the title [WIP]:Adding histogram equalization for images Adding histogram equalization for images Mar 19, 2020
@mloskot
Copy link
Member

mloskot commented Aug 25, 2020

@codejaeger What are we going to do with this PR? Shall this be closed as replaced by the later PRs?

@codejaeger
Copy link
Contributor Author

codejaeger commented Aug 25, 2020

Yes @mloskot we can close this as the other PR #514 aims to provide the same feature and is the one I am currently working on.

@mloskot mloskot removed the status/work-in-progress Do NOT merge yet until this label has been removed! label Aug 25, 2020
@mloskot mloskot closed this Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
google-summer-of-code All items related to GSoC activities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants