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

[Request] Remove OpenCV requirement from the core for 300MB #582

Open
m-roberts opened this issue Oct 27, 2022 · 3 comments · May be fixed by #588
Open

[Request] Remove OpenCV requirement from the core for 300MB #582

m-roberts opened this issue Oct 27, 2022 · 3 comments · May be fixed by #588

Comments

@m-roberts
Copy link
Contributor

m-roberts commented Oct 27, 2022

Problem or Possibility

Currently OpenCV is installed with the core of the SDK, but it appears to only be used by ImageFunctions.convert
Removing this dependency from the core would help to reduce install footprint for core functionality.

It is used in pitop.processing and pitop.camera, but - most importantly - it does the heavy lifting of the miniscreen's display_image function.

Proposed Solution

An OpenCV image is unlikely to be passed in without the cv2 dependency being installed, so it shouldn't even be necessary to invoke the convert function anyway.

Perhaps the display_image function could try to convert if the image format is PIL and raises an exception if it's an OpenCV image without the dependency installed?

Alternative Solutions

According to this, this operation should also work:

x = x[...,::-1].copy()

However, this does not have OpenCV's speed benefit. Perhaps this can be used as a fallback option if OpenCV is not available?

Additional Context

@m-roberts
Copy link
Contributor Author

m-roberts commented Nov 4, 2022

Turns out OpenCV adds an extra 171 packages accounting for ~300MB of the install size: https://pastebin.com/3czP6DWc

@m-roberts m-roberts linked a pull request Nov 4, 2022 that will close this issue
@m-roberts m-roberts changed the title [Request] Remove OpenCV requirement from the core [Request] Remove OpenCV requirement from the core for 300MB Nov 4, 2022
@hramrach
Copy link

Yes, the requirement to install 3 different image processing frameworks for the ability to power off the device is kind of lame.

I suspect this is due to bad design of the python libraries. Why do you need to import ImageFunctions.convert?

Can't the conversion function be a method on each image object? Then you would call the method of whichever library provided the image, transparently.

@m-roberts
Copy link
Contributor Author

m-roberts commented Jan 30, 2023

@jcapona @olivierwilkinson @angusjfw - any chance of someone taking a look at this? 🤔

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 a pull request may close this issue.

2 participants