Handle metadata rotation in pyav code.#13605
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe change modifies video frame decoding in 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api/latest/_input_impl/video_types.py`:
- Around line 294-296: The rotation calculation currently uses floor division
(frame.rotation // 90) which loses precision; replace it with true division and
round to the nearest quarter-turn: compute k = int(round(frame.rotation / 90))
and (optionally) normalize with k = k % 4, then pass that k into np.rot90
(keeping the existing sign usage k=-k) before copying the image; update the code
that sets k and calls np.rot90 accordingly (references: frame.rotation, k,
np.rot90, img).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7792472c-9f18-4f72-be1c-298cf43157c0
📒 Files selected for processing (1)
comfy_api/latest/_input_impl/video_types.py
| if frame.rotation != 0: | ||
| k = int(frame.rotation // 90) | ||
| img = np.rot90(img, k=-k, axes=(0, 1)).copy() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if this is a git repository and understand the structure
git rev-parse --show-toplevel 2>/dev/null && echo "Git repo found" || echo "Not a git repo"
# List the file to understand context
ls -la comfy_api/latest/_input_impl/video_types.py 2>/dev/null || echo "File not found at that path"
# Check git diff to see what changed in this file
git diff HEAD -- comfy_api/latest/_input_impl/video_types.py 2>/dev/null | head -100Repository: Comfy-Org/ComfyUI
Length of output: 189
🏁 Script executed:
# Find all Python files related to video or rotation
find . -type f -name "*.py" | xargs grep -l "frame.rotation\|rot90" 2>/dev/null | head -20Repository: Comfy-Org/ComfyUI
Length of output: 149
🏁 Script executed:
# Check for PyAV imports and understand the dependency
rg "import.*av|from.*av" --type py -A 2 -B 2 | head -50Repository: Comfy-Org/ComfyUI
Length of output: 2894
🏁 Script executed:
# Look at the actual file context around lines 294-296
cat -n comfy_api/latest/_input_impl/video_types.py 2>/dev/null | sed -n '285,305p'Repository: Comfy-Org/ComfyUI
Length of output: 1288
🏁 Script executed:
# Check if there's a requirements file or setup.py to see the PyAV version
find . -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" \) -exec grep -l "av\|pyav" {} \;Repository: Comfy-Org/ComfyUI
Length of output: 97
🏁 Script executed:
# Check the requirements files for PyAV version
cat requirements.txt | grep -i "av\|pyav"
cat pyproject.toml | grep -i "av\|pyav" -A 2 -B 2Repository: Comfy-Org/ComfyUI
Length of output: 340
🏁 Script executed:
# Check git log to see if this line was recently changed
git log --oneline -n 20 -- comfy_api/latest/_input_impl/video_types.pyRepository: Comfy-Org/ComfyUI
Length of output: 74
🏁 Script executed:
# Get the full diff for this file to see what changed
git diff HEAD~1 HEAD -- comfy_api/latest/_input_impl/video_types.py | grep -A 10 -B 10 "rotation"Repository: Comfy-Org/ComfyUI
Length of output: 90
🏁 Script executed:
# Check if there's a staged change vs committed change
git status comfy_api/latest/_input_impl/video_types.py
git diff --cached -- comfy_api/latest/_input_impl/video_types.pyRepository: Comfy-Org/ComfyUI
Length of output: 127
🏁 Script executed:
# Let's check the actual current state of the file more carefully
git show HEAD:comfy_api/latest/_input_impl/video_types.py | sed -n '285,305p'Repository: Comfy-Org/ComfyUI
Length of output: 1141
🏁 Script executed:
# Check what changes are in the current working directory
git diff -- comfy_api/latest/_input_impl/video_types.py | head -100Repository: Comfy-Org/ComfyUI
Length of output: 43
🏁 Script executed:
# Let's also verify numpy.rot90 behavior and semantics
python3 << 'EOF'
import numpy as np
# Test numpy.rot90 behavior with k parameter
test_arr = np.array([[1, 2], [3, 4]])
print("Original array:\n", test_arr)
print("\nrot90(k=1):\n", np.rot90(test_arr, k=1, axes=(0, 1)))
print("\nrot90(k=-1):\n", np.rot90(test_arr, k=-1, axes=(0, 1)))
print("\nrot90(k=2):\n", np.rot90(test_arr, k=2, axes=(0, 1)))
print("\nrot90(k=-2):\n", np.rot90(test_arr, k=-2, axes=(0, 1)))
EOFRepository: Comfy-Org/ComfyUI
Length of output: 211
🏁 Script executed:
# Check PyAV documentation or code to understand frame.rotation return type
# Search for any documentation or tests about frame.rotation
rg "frame\.rotation" --type py -B 3 -A 3 | head -80Repository: Comfy-Org/ComfyUI
Length of output: 845
🏁 Script executed:
# Check PyAV documentation about frame.rotation type and value range
# Let's look for any tests or usage patterns
rg "frame\.rotation\s*[=!<>]" --type py -B 2 -A 2Repository: Comfy-Org/ComfyUI
Length of output: 556
🏁 Script executed:
# Search for any PR or commit history that might have modified this line
git log --all --oneline -- comfy_api/latest/_input_impl/video_types.py | head -10Repository: Comfy-Org/ComfyUI
Length of output: 670
🏁 Script executed:
# Let's verify the exact current state one more time
git show HEAD:comfy_api/latest/_input_impl/video_types.py | sed -n '294,297p' | cat -ARepository: Comfy-Org/ComfyUI
Length of output: 294
🏁 Script executed:
# Check git blame to see when this line was last modified
git blame -L 294,296 comfy_api/latest/_input_impl/video_types.pyRepository: Comfy-Org/ComfyUI
Length of output: 415
🏁 Script executed:
# Let's see what the most recent commit changed
git show d1aa4df -- comfy_api/latest/_input_impl/video_types.py | grep -A 10 -B 10 "rotation" || echo "No rotation changes in that commit"Repository: Comfy-Org/ComfyUI
Length of output: 1164
🏁 Script executed:
# Let's check the previous version before the most recent commit
git show d1aa4df^:comfy_api/latest/_input_impl/video_types.py 2>/dev/null | sed -n '294,297p' || echo "Could not get previous version"Repository: Comfy-Org/ComfyUI
Length of output: 43
🏁 Script executed:
# Let's check what the code was before this "Fix" commit
git show d1aa4df^:comfy_api/latest/_input_impl/video_types.py 2>/dev/null | grep -A 5 -B 5 "rotation" || echo "No rotation in previous version"Repository: Comfy-Org/ComfyUI
Length of output: 93
🏁 Script executed:
# Let's understand PyAV frame.rotation better - check if there are any tests
rg "rotation" tests-unit/comfy_api_test/video_types_test.py -B 3 -A 3Repository: Comfy-Org/ComfyUI
Length of output: 43
🏁 Script executed:
# Let's check PyAV documentation or source about frame.rotation type
python3 << 'EOF'
# Test the mathematical behavior of the current code
test_values = [0, 45, 89, 90, 91, 179, 180, 181, 269, 270, 271, 359, 360]
print("Testing: k = int(round(frame.rotation // 90))")
print("frame.rotation | // 90 | round() | int()")
for val in test_values:
divided = val // 90
rounded = round(divided)
k = int(rounded)
print(f"{val:14} | {divided:5} | {rounded:7} | {k:4}")
print("\n\nCorrecting approach would be: k = int(round(frame.rotation / 90))")
print("frame.rotation | / 90 | round() | int()")
for val in test_values:
divided = val / 90
rounded = round(divided)
k = int(rounded)
print(f"{val:14} | {divided:5.2f} | {rounded:7} | {k:4}")
EOFRepository: Comfy-Org/ComfyUI
Length of output: 1297
🏁 Script executed:
# Let's verify what PyAV actually returns for frame.rotation
# Check if there are any examples or documentation
python3 << 'EOF'
# Simulate what the current code does
import math
# Test if the current approach is correct
test_cases = [0, 45, 89.5, 90, 90.5, 179.5, 180, 270, 359.5]
print("Current code: k = int(round(frame.rotation // 90))")
print("rotation | // 90 | round() | result")
for rot in test_cases:
step1 = rot // 90 # Floor division (integer)
step2 = round(step1)
step3 = int(step2)
print(f"{rot:7.1f} | {step1:5.0f} | {step2:7} | {step3}")
print("\n\nProposed fix: k = int(round(frame.rotation / 90))")
print("rotation | / 90 | round() | result")
for rot in test_cases:
step1 = rot / 90 # True division (float)
step2 = round(step1)
step3 = int(step2)
print(f"{rot:7.1f} | {step1:5.2f} | {step2:7} | {step3}")
EOFRepository: Comfy-Org/ComfyUI
Length of output: 776
🏁 Script executed:
# Check what the review comment really showed vs what's in the code
# The review showed k=-k but the actual code shows k=k
# Let's understand if there was a sign change
python3 << 'EOF'
# Test both approaches
test_rotations = [0, 90, 180, 270]
print("With k=k (current):")
for rot in test_rotations:
k = int(round(rot // 90))
print(f" rotation={rot}° -> k={k}")
print("\nWith k=-k (as shown in review):")
for rot in test_rotations:
k = int(round(rot // 90))
k = -k
print(f" rotation={rot}° -> k={k}")
EOFRepository: Comfy-Org/ComfyUI
Length of output: 295
Use true division instead of floor division for rotation normalization.
The code uses floor division (//) which loses precision before rounding: frame.rotation // 90 returns the floored value before round() is applied. For floating-point rotations like 89.5°, this produces the wrong quarter-turn. Change to frame.rotation / 90 so rounding operates on the actual quotient, not the floored integer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@comfy_api/latest/_input_impl/video_types.py` around lines 294 - 296, The
rotation calculation currently uses floor division (frame.rotation // 90) which
loses precision; replace it with true division and round to the nearest
quarter-turn: compute k = int(round(frame.rotation / 90)) and (optionally)
normalize with k = k % 4, then pass that k into np.rot90 (keeping the existing
sign usage k=-k) before copying the image; update the code that sets k and calls
np.rot90 accordingly (references: frame.rotation, k, np.rot90, img).
1 participant
No description provided.