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

Added Loading images from camera using CameraLoader and image display when qt is supported #30

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

Conversation

Mithul
Copy link

@Mithul Mithul commented Nov 30, 2015

You can load the images from the camera instead of a folder by specifying a -camera_id <camera_id> option to the script. The default camera_id for most OS is 0.

The images being processed can now be seen while processing with their captions(as the title of the window) if 'qt' is supported. Using qlua instead of th works.

This pull request is the solution for Issue #26 .

@karpathy
Copy link
Owner

karpathy commented Dec 1, 2015

Thank you for this PR, a few concerns before we merge:

  • Could you please exclude changes to .gitignore?
  • In eval.lua, could we do a (safer) lazy loading of the camera/misc.CameraLoader? i.e. you could move these parts to right before you instantiate the CameraLoader inside the if block.
  • Similarly, it seems that the display_image function should only be called in case we're actually using the camera loader?
  • Spurious -- break should be removed
  • I don't think the repeat ... until logic is backwards compatible? Would this result in an infinite loop?

In short, I would like this somewhat exotic feature to have minimal impact on the script. If the user would like to not use it, it would be nice to have identical behavior as before and with no errors. Almost there!

@Mithul
Copy link
Author

Mithul commented Dec 3, 2015

Hey, you are welcome.
I have made the changes, the repeat untill will only result in an infinite loop if camera_id is set(which is what is needed I think?). In other cases it would execute only once.

@SaMnCo
Copy link

SaMnCo commented Dec 10, 2015

That's a great addition. @karpathy when do you think it will be merged?

Thx

@karpathy
Copy link
Owner

I'm sorry I'm currently at a conference and have become unresponsive on several issues. I'll look at this when I get back early next week and we'll work to merge it in.

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.

3 participants