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

ImageDisplayerOCV #16

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

Conversation

avdmitry
Copy link
Contributor

@avdmitry avdmitry commented Nov 3, 2014

Hello, @nitishsrivastava

I implemented ImageDisplayer using OpenCV and tested it on mnist example with display & localizer true. I also removed CImg dependency and updated install file.

@nitishsrivastava
Copy link
Contributor

@avdmitry Thanks for the pull request. I tested it and there are some issues:

  • Normalization of float images: I used normalize(image, image, 0, 1, NORM_MINMAX); and 32FC3/32FC1 images.
  • waitKey(1) I had to use around WaitKey(30) for it to work, even then the display is not responsive and sometimes fails to update. There is also a distinctive dimming of the display window when it is being updated. This usually happens when a window becomes non-responsive. The whole need for WaitKey() seems a bit weird to me. I found the CImg GUI to be much better in this regard. I am considering keeping CImg for this purpose.

@avdmitry
Copy link
Contributor Author

@nitishsrivastava,

I took a look on these issues:

  • yes, normalization needed. I used CV_8UC because it has less size and should work faster. Internally OpenCV anyway will convert floating 0..1 value to 0..255 char value. I also got strange behaviour when I tried to use 32FC today. May be there is some bug in my code, I'll test it more.
  • I don't have such issue on my PC even with waitKey(1). I also think that waitKey approach is little bit strange, but as it should be inside ImageDisplayer it should be transparent by using this class.

I'll test this code more and try implement convenient ImageDisplayer, if I can't, let keep current implementation.

@avdmitry avdmitry force-pushed the ImageDisplayerOCV branch 10 times, most recently from b6102d2 to a1b8b30 Compare December 20, 2014 10:55
@avdmitry
Copy link
Contributor Author

@nitishsrivastava,

I switched to use 32FC3/32FC1 images and added normalization.
There was bug, which, I think, was the cause of second issue. I fixed it, now code should work correct.

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.

2 participants