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

Inpaint node #1546

Merged
merged 2 commits into from
Feb 12, 2023
Merged

Inpaint node #1546

merged 2 commits into from
Feb 12, 2023

Conversation

harisreedhar
Copy link
Contributor

@harisreedhar harisreedhar commented Feb 10, 2023

Read more about opencv inpaint
chaiNNer-Inpaint-2023-2-8_20-49

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Great node idea. Thank you for adding this!

2 minor nits and then we can merge this :)


It's not gonna happen, but I wanted to mention it anyway: OpenCV also supports FSR and ShiftMap as part of xphoto, an unstable extension...

backend/src/nodes/nodes/image_utility/inpaint.py Outdated Show resolved Hide resolved
backend/src/nodes/nodes/image_utility/inpaint.py Outdated Show resolved Hide resolved
@harisreedhar
Copy link
Contributor Author

It's not gonna happen, but I wanted to mention it anyway: OpenCV also supports FSR and ShiftMap as part of xphoto, an unstable extension...

opencv-contrib have tons of extra image processing tools. Any plans on including that to dependency?

@RunDevelopment
Copy link
Member

RunDevelopment commented Feb 11, 2023

opencv-contrib have tons of extra image processing tools. Any plans on including that to dependency?

I mean, it would be nice. It has tons of extra image processing functionality, so we could potentially implement some cool nodes with it. Since opencv-contrib-python is a strict superset of plain opencv-python that we currently use, we wouldn't have to change any of our existing python code either.

The main problem is how OpenCV is distributed. Their README explains it best, but I'll quickly summarize it here. OpenCV has 2 optional features: GUI and unstable. But instead of the optional features being add-on packages, they distribute 4 variants of OpenCV for all possible combination of features. So we get opencv (GUI, no unstable), opencv-headless (no GUI, no unstable), opencv-contrib (GUI, unstable), and opencv-contrib-headless (no GUI, unstable).

Right now, chainner works with any of those variants, which is great for compatibility with system python. If we go and require a variant with unstable features (contrib), then we will run into compatibility issues. We can resolve those issues in 2 ways:

  1. We write nodes that use unstable features in a way that they "feature detect" what OpenCV variant is installed, and adjust accordingly.
  2. We change the dependency managing code to always install a contrib variant.

No matter which way we go, we'll have to change how we install OpenCV. You can only install one of the variants, so we can't blindly install our preferred variant (like we currently do).

@joeyballentine Thoughts?

@joeyballentine
Copy link
Member

@RunDevelopment This actually made me realize that the way we currently do it could cause a conflict, since if they already have a different one of the 4 versions, we're installing another and creating an issue.

It actually would make the most sense for us to run the uninstall command for the other three, and then install the contrib version. It would ensure the most compatibility as well as give us those extra features

@joeyballentine
Copy link
Member

Is this PR good now?

@joeyballentine joeyballentine merged commit 044005a into chaiNNer-org:main Feb 12, 2023
@harisreedhar harisreedhar deleted the inpaint-node branch February 17, 2023 12:44
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

3 participants