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

update video rotation when doing detection #1124

Closed
wants to merge 5 commits into from

Conversation

BruceWang94
Copy link

@BruceWang94 BruceWang94 commented Oct 12, 2020

using scikit-video to check rotate information of the video.

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Enhanced video rotation handling and width/height adjustment in YOLOv5 detection pipeline.

📊 Key Changes

  • 🔄 Added support for video rotation metadata detection and correction during the detection process.
  • 📐 Modified video writer dimensions to accommodate rotated videos (90° or 270°).
  • 📦 Included scikit-video and ffmpeg dependencies in the requirements to facilitate video metadata operations.

🎯 Purpose & Impact

  • 🔍 Purpose: Enable accurate object detection in videos irrespective of their rotation metadata, ensuring the model correctly interprets the orientation.
  • 🎥 Impact for users:
    • Videos captured in non-standard orientations will be automatically adjusted, leading to better detection results.
    • No additional manual preprocessing is required by the user for rotated videos, simplifying the workflow.
    • Potential increase in resource usage due to additional rotation handling, but with the benefit of enhanced usability and broader video handling capabilities.

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 @BruceWang94, thank you for submitting a PR! To allow your work to be integrated as seamlessly as possible, we advise you to:

  • Verify your PR is up-to-date with origin/master. If your PR is behind origin/master update by running the following, 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 rebase 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

@glenn-jocher
Copy link
Member

@BruceWang94 thanks for the PR buddy. I've dealt with a lot of formats before, which makes me cautious here. We handle all of our dataloading with cv2 and PIL, so we are constrained a bit with how robust we can be to handling all formats under all conditions. The current formats accepted are:

yolov5/utils/datasets.py

Lines 20 to 22 in 00917a6

img_formats = ['.bmp', '.jpg', '.jpeg', '.png', '.tif', '.tiff', '.dng']
vid_formats = ['.mov', '.avi', '.mp4', '.mpg', '.mpeg', '.m4v', '.wmv', '.mkv']

Is this PR capable of handling different video formats? I suppose the largest use cases for rotated video will clearly be mobile device exports, but we have a vast fragmented marketed there in Android. If we could cover I suppose iOS and Android default format exports under landscape and vertical native (and both rotated) that would be ideal, though I understand that may be too much to ask.

@BruceWang94
Copy link
Author

@BruceWang94 thanks for the PR buddy. I've dealt with a lot of formats before, which makes me cautious here. We handle all of our dataloading with cv2 and PIL, so we are constrained a bit with how robust we can be to handling all formats under all conditions. The current formats accepted are:

yolov5/utils/datasets.py

Lines 20 to 22 in 00917a6

img_formats = ['.bmp', '.jpg', '.jpeg', '.png', '.tif', '.tiff', '.dng']
vid_formats = ['.mov', '.avi', '.mp4', '.mpg', '.mpeg', '.m4v', '.wmv', '.mkv']

Is this PR capable of handling different video formats? I suppose the largest use cases for rotated video will clearly be mobile device exports, but we have a vast fragmented marketed there in Android. If we could cover I suppose iOS and Android default format exports under landscape and vertical native (and both rotated) that would be ideal, though I understand that may be too much to ask.

Hi Glenn,

Honestly speaking, I test all the video formats, the code works well without any bugs. But to mention, I only have the mp4 format video which needs to be rotated during processing, the code and its functionality were working well. The output video is rotated in the right way.

There is another tricky point. I create a new env using conda to test this PR. However, due to the lib dependency, it needs to type conda install ffmpeg to install this library. I do not find any way to using pip

@glenn-jocher
Copy link
Member

@BruceWang94 oh that's interesting, thanks.

About conda, I used to use anaconda because I figured it would manage dependencies better, but in practice it usually seemed a bit behind pip releases, and caused additional problems, especially with cv2 and some other packages. I finally abandoned conda and now only use pip installs in a venv (starting from python 3.8).

So my procedure for installing YOLOv5 is much simplified now, its just create a 3.8 venv, and then just pip install -r requirements.txt with all the extras uncommented in the file, and it works well all the time.

@glenn-jocher
Copy link
Member

BTW I will review and try to merge this weekend when I get some free time, thanks again for the PR!

@glenn-jocher
Copy link
Member

@BruceWang94 would it be possible to do this with PIL or cv2, in order to not expand the dependencies?

@BruceWang94
Copy link
Author

@glenn-jocher I don't think so. cv2 cannot handle the metadata information of a video. There is closed issue mentioned on OpenCV repository. PIL is good at handling images, while I tried using PIL on each frame to get the exif before, but I failed.

@glenn-jocher
Copy link
Member

glenn-jocher commented Oct 22, 2020

@BruceWang94 yes makes sense, I don't believe video frames have EXIF info. What is your environment?

I tried this approach today but oddly get an ffmpeg error even after installing the 2 additional dependencies:

skvideo.io.ffprobe(file)

Traceback (most recent call last):
  File "/Users/glennjocher/Library/Application Support/JetBrains/Toolbox/apps/PyCharm-C/ch-0/202.7660.27/PyCharm CE.app/Contents/plugins/python-ce/helpers/pydev/_pydevd_bundle/pydevd_exec2.py", line 3, in Exec
    exec(exp, global_vars, local_vars)
  File "<input>", line 1, in <module>
  File "/Users/glennjocher/PycharmProjects/yolov5/venv/lib/python3.8/site-packages/skvideo/io/ffprobe.py", line 27, in ffprobe
    assert _HAS_FFMPEG, "Cannot find installation of real FFmpeg (which comes with ffprobe)."
AssertionError: Cannot find installation of real FFmpeg (which comes with ffprobe).

@glenn-jocher
Copy link
Member

UPDATE: I did a pip install ffprobe also, but see same error. I have these installed with pip install:

ffmpeg                 1.4
ffprobe                0.5
scikit-image           0.17.2
scikit-learn           0.19.2
scikit-video           1.1.11

@BruceWang94
Copy link
Author

@glenn-jocher Yes, this part is a tricky one. I added two libs scikit-video and ffmpeg into the requirement.txt. The interesting thing is that ffmpeg can be only installed by conda, not pip. Otherwise, the error will occur.

conda install ffmpeg will install another package freetype-2.10.4 . However, I do not find the same one using pip.

@glenn-jocher
Copy link
Member

/rebase

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale Stale and schedule for closing soon label Dec 27, 2020
@github-actions github-actions bot closed this Jan 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Stale and schedule for closing soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants