Skip to content

Fix process_video and add test case #2011

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.

Sign up for GitHub

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

Open
cdeil wants to merge 3 commits into
base: develop
Choose a base branch
from
+34 −10
Open

Fix process_video and add test case #2011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83c3a05
Fix process_video and add test case
cdeil Nov 7, 2025
d91cf18
Simplify test_video.py temp file, itsa OK on Windows letsa hope
cdeil Nov 7, 2025
0500549
fix(pre_commit): 🎨 auto format pre-commit hooks
pre-commit-ci[bot] Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions supervision/utils/video.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,29 +230,24 @@ def callback(scene: np.ndarray, index: int) -> np.ndarray:
```
"""
source_video_info = VideoInfo.from_video_path(video_path=source_path)
max_frames = max_frames or source_video_info.total_frames
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

Using the 'or' operator here will incorrectly handle the case when max_frames is 0. If a user passes max_frames=0 (meaning process zero frames), this line will replace it with source_video_info.total_frames instead of respecting the 0 value.

Consider using an explicit None check instead, such as:
max_frames = source_video_info.total_frames if max_frames is None else max_frames

Suggested change
max_frames = max_frames or source_video_info.total_frames
max_frames = (
source_video_info.total_frames if max_frames is None else max_frames
)

Copilot uses AI. Check for mistakes.
if source_video_info.total_frames is not None and max_frames is not None:
max_frames = min(max_frames, source_video_info.total_frames)

video_frames_generator = get_video_frames_generator(
source_path=source_path, end=max_frames
)
with VideoSink(target_path=target_path, video_info=source_video_info) as sink:
total_frames = (
min(source_video_info.total_frames, max_frames)
if max_frames is not None
else source_video_info.total_frames
)
for index, frame in enumerate(
tqdm(
video_frames_generator,
total=total_frames,
total=max_frames,
disable=not show_progress,
desc=progress_message,
)
):
result_frame = callback(frame, index)
sink.write_frame(frame=result_frame)
else:
for index, frame in enumerate(video_frames_generator):
result_frame = callback(frame, index)
sink.write_frame(frame=result_frame)


class FPSMonitor:
Expand Down
29 changes: 29 additions & 0 deletions test/utils/test_video.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import cv2
import numpy as np

from supervision.utils.video import process_video


def create_test_video(path, num_frames, width=20, height=10):
fourcc = cv2.VideoWriter_fourcc(*"mp4v")
out = cv2.VideoWriter(path, fourcc, 1.0, (width, height))

for _ in range(num_frames):
frame = np.zeros((height, width, 3), dtype=np.uint8)
out.write(frame)

out.release()


def test_process_video_max_frames_exceeds_total_frames(tmp_path):
source_path = tmp_path / "source.mp4"
target_path = tmp_path / "target.mp4"

create_test_video(str(source_path), num_frames=5)

process_video(
Copy link
Member

@Borda Borda Jan 5, 2026

Choose a reason for hiding this comment

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

Any assertion or other validation, then it runs?

source_path=str(source_path),
target_path=str(target_path),
callback=lambda frame, _: frame,
max_frames=10,
)
Comment on lines +24 to +29
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

This test lacks assertions to verify the expected behavior. The test should validate that:

  1. The target video file was created successfully
  2. The video was processed without raising an exception
  3. The target video contains the expected number of frames (5, not 10)

Consider adding assertions such as checking that the target file exists and verifying the frame count of the output video.

Copilot uses AI. Check for mistakes.