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

fixe Marr-Hildreth Hash kernel error #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leslie-wang
Copy link

Signed-off-by: Leslie Qi Wang wqyuwss@gmail.com

Based on https://github.com/aetilius/pHash/blob/master/src/pHash.cpp#L610, and https://web.fe.up.pt/~campilho/PDI/NOTES/EdgeDetection.pdf. Seems kernel should be -1 * a/2 or a/-2.

Thanks for the nice work. It is the only one having all image hashing algorithm!

Signed-off-by: Leslie Qi Wang <wqyuwss@gmail.com>
1. move kernel calculation into Marr-Hildreth constructor
2. add normalize function if needed
3. use golang native RGB to gray conversion method

Signed-off-by: Leslie Qi Wang <wqyuwss@gmail.com>
gray.Set(x, y, color.Gray{lum})
for y := img.Bounds().Min.Y; y < img.Bounds().Max.Y; y++ {
for x := img.Bounds().Min.X; x < img.Bounds().Max.X; x++ {
gray.Set(x, y, img.At(x, y))
Copy link
Owner

Choose a reason for hiding this comment

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

Thank you 😄 I didn't know that color.GrayModel uses the same luminance approach, I assumed that it was just averaging them out.

func grayModel(c Color) Color {
    if _, ok := c.(Gray); ok {
        return c
    }
    r, g, b, _ := c.RGBA()

    // These coefficients (the fractions 0.299, 0.587 and 0.114) are the same
    // as those given by the JFIF specification and used by func RGBToYCbCr in
    // ycbcr.go.
    //
    // Note that 19595 + 38470 + 7471 equals 65536.
    //
    // The 24 is 16 + 8. The 16 is the same as used in RGBToYCbCr. The 8 is
    // because the return value is 8 bit color, not 16 bit color.
    y := (19595*r + 38470*g + 7471*b + 1<<15) >> 24

    return Gray{uint8(y)}
}

@ajdnik
Copy link
Owner

ajdnik commented Apr 27, 2022

@leslie-wang Thank you for all the improvements, if you can update the unit tests to reflect these new changes, I'll gladly merge this to master. And I'm sorry for how long this process took.

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

2 participants