feat(slicing): slice_video — cut clips by time range with optional square resize#6
Conversation
… resize Cuts a video into one MP4 (bytes) per (start, end) second range, reusing read_frames_exact for decoding. With size set, each frame is center-cropped to a square and resized to size×size for models that expect square input. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new ChangesVideo Slicing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_slicing.py (1)
31-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for empty-slice output.
Current tests only exercise slices that contain frames. Given
slice_video's documented contract of returningb""for slices with no frames (and the related bug found in_encode_clipwhensizeis set), consider adding a case likeslice_video(video, [(2.0, 2.5)])(past the 1s source duration) assertingclips == [b""], both with and withoutsize.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_slicing.py` around lines 31 - 44, Add a test case in test_slicing.py for the empty-slice contract of slice_video, using a range past the source duration so it produces no frames. Extend the existing slice_video coverage in test_slice_returns_one_clip_per_range or a new test to assert clips == [b""] for something like [(2.0, 2.5)], and repeat the assertion with size set as well to cover the _encode_clip path when resizing is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@simple_video_utils/slicing.py`:
- Around line 22-28: The empty-slice behavior in _encode_clip is bypassed when
size is set, causing empty frame sequences to still produce container bytes.
Update _encode_clip so the empty-frames guard runs before any size-based
dimension setup or av.open() work, and return b"" whenever frames is empty
regardless of size. Keep the existing size handling for non-empty clips, but
ensure the contract described by _encode_clip and the slice path remains
consistent.
---
Nitpick comments:
In `@tests/test_slicing.py`:
- Around line 31-44: Add a test case in test_slicing.py for the empty-slice
contract of slice_video, using a range past the source duration so it produces
no frames. Extend the existing slice_video coverage in
test_slice_returns_one_clip_per_range or a new test to assert clips == [b""] for
something like [(2.0, 2.5)], and repeat the assertion with size set as well to
cover the _encode_clip path when resizing is enabled.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e507718-b146-49e4-a324-54664867d15a
📒 Files selected for processing (3)
README.mdsimple_video_utils/slicing.pytests/test_slicing.py
- size=None now remuxes (stream copy) instead of decode+re-encode: fast and lossless, cutting on the keyframe at/before start (may include a little pre-roll). A resize (size set) still re-encodes since it must touch pixels. - Empty-slice returns b"" in both paths (the encode guard ran too late when size was set; the copy path checks the header duration). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- slice_video yields per clip (returns a lazy generator) so a long slice list never holds every clip in memory. - Copy (no re-encode) not just when size is None but also when the source is already size x size — checked via the container header. - Remux honors the stream's start_time offset (pts is on the absolute timeline). - end < start raises ValueError up front in both paths (was silently b"" on copy). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Decide copy-vs-encode once from video_metadata (copy when size is None, or the source is already size x size and unrotated). - Single streaming loop; each clip yielded lazily. - Every slice must satisfy 0 <= start <= end <= duration; out-of-range raises ValueError instead of silently returning empty bytes. - Remux honors the stream start_time; rotated square sources re-encode so the copy and resize paths never disagree on orientation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b69343a. Configure here.
- Remove the codec parameter (always h264) — an unused knob. - Consolidate the duplicated "no frames" raise: helpers return b"" and slice_video decides in one place. - Reject zero/negative-length slices (end <= start) so the copy and encode paths agree, plus a test for it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Adds
slice_video, requested for the sign-inference gateway so the cut+transform lives in the shared lib instead of being reimplemented per service.read_frames_exactfor time-range decoding; encodes each clip to MP4 in memory.sizecenter-crops each frame to a square then resizes tosize×size; omit it to keep the source resolution.bytesat that index (order preserved).Tests generate a video and assert clip count, default resolution, and 256×256 output.
ruff checkclean.🤖 Generated with Claude Code
Note
Low Risk
New optional API in a utility module with validation and tests; no changes to auth, persistence, or existing frame APIs beyond reuse of
read_frames_exact.Overview
Adds
slice_videoinsimple_video_utils/slicingso callers can export one in-memory MP4 per(start, end)second range, yielded lazily to avoid holding many clips at once.Without
size(or when the source is already a matching square and unrotated), clips are stream-copied via PyAV remux—fast and lossless, with cuts aligned to the keyframe at or beforestart. Withsize, the encode path usesread_frames_exactfor the window, center-crops to a square, resizes, and re-encodes to H.264. Invalid slices (out of range, zero length, or no frames) raiseValueError.The README gains a Slice into Clips example;
tests/test_slicing.pycovers multiple ranges, default resolution, square resize, the copy fast-path, and validation errors.Reviewed by Cursor Bugbot for commit b13c786. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests