Conversation
OpenCV's FFmpeg backend applies a video's display rotation to decoded frames and to the reported width and height. The PyAV fallback ignored it, so portrait phone videos were read sideways without OpenCV. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2601 +/- ##
=======================================
Coverage 91% 91%
=======================================
Files 78 78
Lines 11388 11408 +20
=======================================
+ Hits 10387 10407 +20
Misses 1001 1001 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The non-quarter-turn behavior lacks regression coverage, and the changelog modification date is stale.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds PyAV fallback support for video display rotation, matching OpenCV behavior.
Changes:
- Rotates decoded frames and reports upright dimensions.
- Adds fallback and integration tests.
- Documents the fix in the changelog.
Review scores: Code 5/5 · Tests 4/5 · Docs 4/5
File summaries
| File | Description |
|---|---|
src/supervision/_cv2/_video.py |
Applies and caches display rotation metadata. |
tests/cv2/test_video.py |
Tests fallback rotation and OpenCV parity. |
tests/utils/test_video.py |
Tests public video APIs with rotated input. |
docs/changelog.md |
Adds the user-facing fix entry. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| - Removed, as scheduled for `supervision-0.31.0`: `sv.ByteTrack` (use `ByteTrackTracker` from the `trackers` package instead); the `supervision.keypoint` module (use `supervision.key_points`); `create_tiles` and `overlay_image` in `supervision.utils.image`; `ensure_cv2_image_for_annotation`, `ensure_pil_image_for_annotation`, and `ensure_cv2_image_for_processing` in `supervision.utils.conversion`; `validate_keypoint_confidence` and `validate_keypoints_fields` in `supervision.validators`; the `normalized_xyxy` argument of `sv.denormalize_boxes` (use `xyxy`); the `supervision.dataset.utils` import path for `sv.mask_to_rle`/`sv.rle_to_mask` (import from `supervision.detection.utils.converters` instead); `sv.LMM` and `Detections.from_lmm` (use `sv.VLM`/`Detections.from_vlm`); and the legacy `MeanAveragePrecision` in `supervision.metrics.detection` (use `supervision.metrics.mean_average_precision.MeanAveragePrecision`, exposed as `sv.metrics.MeanAveragePrecision`). See [Deprecated](deprecated.md) for the full list. [#2582](https://github.com/roboflow/supervision/pull/2582) | ||
|
|
||
| - `sv.VideoInfo.from_video_path`, `sv.get_video_frames_generator` and `sv.process_video` now turn a rotated video upright when OpenCV is not installed, as they already do with OpenCV. Phones usually store a portrait clip as landscape frames and record the turn in the container's display matrix. OpenCV's FFmpeg backend applies that turn to every frame it decodes and to the width and height it reports, but the OpenCV-free fallback ignored it, so a portrait phone video came back sideways, with its width and height swapped: models ran on sideways frames and `sv.VideoSink` saved the output sideways. The fallback now applies quarter and half turns, the same angles OpenCV applies. Videos without a display rotation, and every read with OpenCV installed, are unchanged. [#2601](https://github.com/roboflow/supervision/pull/2601) |
Comment on lines
+169
to
+170
| pytest.param(180, 2, id="half-turn"), | ||
| pytest.param(0, 0, id="no-rotation"), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Without OpenCV installed, a portrait video recorded on a phone is read sideways. With OpenCV installed, the same calls return it upright:
Phones usually store a portrait clip as landscape frames and record the turn in the container's display matrix. OpenCV's FFmpeg backend reads that matrix (
CAP_PROP_ORIENTATION_AUTOis on by default), turns every decoded frame upright, and swapsCAP_PROP_FRAME_WIDTHandCAP_PROP_FRAME_HEIGHTto match. The PyAV fallback insupervision/_cv2/_video.pyreturnedframe.to_ndarray()and the stream's coded size as they are, so it ignored the matrix.This PR applies the rotation in the fallback the way OpenCV does:
_VideoCapture.readturns each frame byVideoFrame.rotation, which PyAV reads from the same display matrix (available since PyAV 14.2, the minimum Supervision requires)._VideoCapture.getswaps the width and height for a quarter turn. PyAV only exposes the matrix on decoded frames, so_rotationdecodes the first frame from a second handle and caches the angle._frame_countalready opens a second handle in the same way, and this leaves the capture's position untouched.Motivation and Context
The fallback is what runs for anyone without
opencv-python:sv.VideoInfo.from_video_path,sv.get_video_frames_generatorandsv.process_videoall go through it. Phone footage is a common input for these, and without this fix models ran on sideways frames andsv.VideoSinkwrote the annotated output sideways. The image path already handles the equivalent case: the fallbackimreadapplies EXIF orientation as OpenCV does.Changes Made
src/supervision/_cv2/_video.py: new_quarter_turnshelper,_VideoCapture._rotation, and rotation handling in_VideoCapture.getand_VideoCapture.read.tests/cv2/test_video.py:test_fallback_capture_turns_rotated_video_uprightwrites a 32×16 clip with a display rotation of 90, -90, 180 or 0 degrees (VideoStream.set_display_rotation) and expects the upright frame and size.test_fallback_capture_matches_opencv_display_rotationcompares the size and first frame withcv2.VideoCapture, and is skipped without OpenCV.tests/utils/test_video.py:test_video_info_and_frames_follow_display_rotationchecksVideoInfo.from_video_pathandget_video_frames_generatoron a rotated clip. It passes with OpenCV today, and it runs against the fallback in the OpenCV-free CI jobs, wheretests/cv2is skipped.docs/changelog.md: entry under Unreleased.Out of scope: the writer. OpenCV's
VideoWriterdoesn't write a display matrix either, so the output of both backends is already the same.Testing
I checked parity against
cv2.VideoCapture(opencv-python 5.0.0) for rotations of 90, -90, 180 and 270 degrees, for 45 degrees, and for matrices with a horizontal or vertical flip. The fallback's size and first frame matched OpenCV's exactly in every case.New tests on
develop, with OpenCV installed:Without OpenCV (
sys.modules["cv2"] = None, as in the OpenCV-free CI jobs):With this change:
mypyon the changed source and test files reports the same errors ondevelopand on this branch.docs/changelog.mdunderUnreleased(skip for lint/type/format-only or pure doc changes)Additional Notes
The test fixtures use
VideoStream.set_display_rotation, which PyAV added in 17.1.uv.lockpins 17.1.0 and 18.0.0, so CI has it. The fix itself only needsVideoFrame.rotation, which PyAV has had since 14.2.🤖 Generated with Claude Code