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

Fix cv2.imwrite on non-ASCII paths #7139

Merged
merged 10 commits into from
Mar 25, 2022
Merged

Conversation

CCRcmcpe
Copy link
Contributor

@CCRcmcpe CCRcmcpe commented Mar 25, 2022

Previous PR #6979 fixed imread. But I discovered imwrite is also broken: when saving a image, Unicode characters in the path are converted to meaningless characters, or it does not save the image at all.
Also moved the custom imread to utils.general.

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Refactoring OpenCV image read/write functions for compatibility with non-latin filenames.

📊 Key Changes

  • Removed direct import cv2 statements from multiple files (detect.py, hubconf.py, and datasets.py).
  • Re-defined cv2.imread and cv2.imwrite functions with custom Chinese-friendly versions in utils/general.py.
  • Replaced original OpenCV image read/write calls with new custom functions across different modules.

🎯 Purpose & Impact

  • 👨‍💻 Developers: Simplifies codebase by centralizing custom OpenCV functions, reducing direct imports, and remapping functions in a single location.
  • 🌍 Global Users: Enhances functionality for users with non-latin filenames, offering broader international support for image file operations.
  • 🚀 Performance: Streamlined interoperability with OpenCV without altering performance or functionality for existing users with latin-based filenames.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

👋 Hello @CCRcmcpe, thank you for submitting a YOLOv5 🚀 PR! To allow your work to be integrated as seamlessly as possible, we advise you to:

  • ✅ Verify your PR is up-to-date with upstream/master. If your PR is behind upstream/master an automatic GitHub Actions merge may be attempted by writing /rebase in a new comment, or by running the following code, replacing 'feature' with the name of your local branch:
git remote add upstream https://github.com/ultralytics/yolov5.git
git fetch upstream
# git checkout feature  # <--- replace 'feature' with local branch name
git merge upstream/master
git push -u origin -f
  • ✅ Verify all Continuous Integration (CI) checks are passing.
  • ✅ Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." -Bruce Lee

@CCRcmcpe
Copy link
Contributor Author

image
image

@glenn-jocher
Copy link
Member

glenn-jocher commented Mar 25, 2022

@CCRcmcpe ok got it. What we should do then is this. Create the two functions in general.py and then assign them to cv2:

def imread(path):
    return cv2.imdecode(np.fromfile(path, np.uint8), cv2.IMREAD_COLOR)


def imwrite(path, img):
    try:
        cv2.imencode(Path(path).suffix, img)[1].tofile(path)
        return True
    except:
        return False


cv2.imread, cv2.imwrite = imread, imwrite

Then everywhere that imports cv2, change it to:

from utils.general import cv2

And finally delete all other changes from #6979 and this PR. Then everything should work well with a minimum of customizations to the code.

@glenn-jocher glenn-jocher changed the title Fix imwrite on non-ASCII paths Fix cv2.imwrite on non-ASCII paths Mar 25, 2022
@glenn-jocher glenn-jocher merged commit d115bbf into ultralytics:master Mar 25, 2022
@glenn-jocher
Copy link
Member

@CCRcmcpe PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐

@CCRcmcpe
Copy link
Contributor Author

@CCRcmcpe ok got it. What we should do then is this. Create the two functions in general.py and then assign them to cv2:

def imread(path):
    return cv2.imdecode(np.fromfile(path, np.uint8), cv2.IMREAD_COLOR)


def imwrite(path, img):
    try:
        cv2.imencode(Path(path).suffix, img)[1].tofile(path)
        return True
    except:
        return False


cv2.imread, cv2.imwrite = imread, imwrite

Then everywhere that imports cv2, change it to:

from utils.general import cv2

And finally delete all other changes from #6979 and this PR. Then everything should work well with a minimum of customizations to the code.

Great! I'm relatively new to python so I don't know can actually import like this.

@glenn-jocher
Copy link
Member

@CCRcmcpe it's a bit of an experimental approach, but yes you can overwrite imports, or most any other variable/method after they are defined.

BjarneKuehl pushed a commit to fhkiel-mlaip/yolov5 that referenced this pull request Aug 26, 2022
* Fix imwrite on non-ASCII paths

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update general.py

* Update __init__.py

* Update __init__.py

* Update datasets.py

* Update hubconf.py

* Update detect.py

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update general.py

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
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