Camera improvements #2772
Camera improvements #2772
Conversation
Because I want to get the colorspace working even when the backend isn't _camera.
|
This pull request introduces 4 alerts when merging 86ffff3 into fb51ba4 - view on LGTM.com new alerts:
|
| if flip_code is not None: | ||
| image = cv2.flip(image, flip_code) | ||
|
|
||
| image = numpy.fliplr(image) |
There was a problem hiding this comment.
Can remove the dependency on numpy with some cv2 functions like in my cv2 camera module: https://github.com/pygame/parrotjoy/blob/master/parrotjoy/camera_cv.py#L46
There was a problem hiding this comment.
cv2 itself is dependent on numpy, so I figured it wouldn't hurt to do it like this.
src_py/camera.py
Outdated
| def _setup_opencv_legacy(): | ||
| global list_cameras, Camera, colorspace | ||
|
|
||
| from pygame import _camera_opencv_highgui |
There was a problem hiding this comment.
I don't think we should bother with this backend. It's very hard to actually use compared to cv2. I'd suggest removing it in this PR. I'm pretty sure I'm the only user of this, and the module is experimental... so we don't need to give any warning before removing it.
There was a problem hiding this comment.
I'm happy to remove this one, I just wanted to keep this PR easy on the backwards compat. I think it'll be good to remove though.
| except ImportError: | ||
| _camera = None | ||
|
|
||
| warnings.warn("The VideoCapture backend is not recommended and may be removed." |
There was a problem hiding this comment.
It's kind of nice to use on windows still. As a user, I'm planning on still using VideoCapture still. Not sure there's any other person on earth that applies to though.
So, let's remove VideoCapture support now?
There was a problem hiding this comment.
My impression is that more people are using this than the legacy opencv. Like, I saw an issue where MyreMylar showed off windows webcam support with this. And my Camera backend doesn't support python2. I guess the transition path would be to an OpenCV backend.
I should at least add in that the VideoCapture module doesn't support enumeration either, in the doc.
|
|
||
| list_cameras = _camera.list_cameras | ||
| Camera = _camera.Camera | ||
| def init(backend=None): |
There was a problem hiding this comment.
Can you please explain the advantage of a backend arg? For other modules we use environment variables to do this. I guess it's a bit easier to test with?
There was a problem hiding this comment.
The idea is that the backend arg and the get_backends() function will make the internal workings more transparent to users. Which is especially important for this module, since the backend choice impacts lots of things, like what modules need to be installed.
Why not an environment variable? With init(backend=), you can initialize multiple backends (at separate times) during program execution, or easily test if one crashes or not (which would be a bit clunkier with environment variables). It seems intuitive to me to have it in with the rest of the code being executed rather than an environment variable set at the beginning, especially since this module is so squirrely.
|
|
||
| return self.get_controls() | ||
|
|
||
| def get_controls(self): |
There was a problem hiding this comment.
Yeah, get_controls is sort of limited.
One problem with the cv2 control system is that controls are very different depending on camera/backend. Somehow keeping a set of well supported controls would be a nice feature to have.
There was a problem hiding this comment.
Definitely. Plus the mappings to whatever they're called in the native backends. But that's out of scope on this PR.
When I was experimenting with MSMF brightness control, I actually skipped around some of the OpenCV source to figure out how to do it.
The weird thing was that it was "sticky" between program runs, so it probably needs to get "unset" as a cleanup.
This gets the
pygame.cameramodule up to cross platform feature completeness.It includes:
opencv-python/import cv2system_profilercommandpygame.camera.get_backends(),pygame.camera.init(backend: str))PYGAME_CAMERAenvironment variable._camerabackend. (Except on windows python 2, where the _camera backend isn't built :/)The
pygame.cameramodule is one of our more "troubled" modules, and this doesn't fix everything I've seen that isn't quite right, but I think it's a large positive step.