feat(utils): add prefetch to get_video_frames_generator#2273
Open
Ace3Z wants to merge 6 commits into
Open
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (79%) is below the target coverage (95%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2273 +/- ##
=======================================
- Coverage 84% 84% -0%
=======================================
Files 70 70
Lines 9946 9993 +47
=======================================
+ Hits 8339 8377 +38
- Misses 1607 1616 +9 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in prefetch parameter to get_video_frames_generator to overlap frame decoding with CPU-bound consumers by decoding frames on a background thread and buffering them in a bounded Queue. This targets the FPS bottleneck raised in #1411 while keeping the default (prefetch=0) behavior unchanged.
Changes:
- Extend
get_video_frames_generator(..., prefetch: int = 0)with a threaded prefetch path whenprefetch > 0. - Add internal
_prefetched_frames_generatorthat drives the existing synchronous generator from a daemon thread and yields frames from a queue. - Add a regression test ensuring prefetched output matches the synchronous frame sequence exactly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/supervision/utils/video.py |
Adds prefetch option and a new internal queued background-reader generator. |
tests/utils/test_video.py |
Adds a test asserting prefetched iteration matches the synchronous generator frame-for-frame. |
Adds an opt-in prefetch: int = 0 parameter. When > 0, frames are decoded in a background thread and buffered in a bounded queue, letting a CPU-bound consumer overlap with decode I/O. Default 0 keeps the original synchronous behaviour unchanged. The threaded path drives the existing sync generator on a daemon thread and pumps frames through a Queue(maxsize=prefetch). No new dependencies. Closes roboflow#1411.
Ace3Z
force-pushed
the
feat/get-video-frames-generator-prefetch
branch
from
bd3f788 to
ba1f44f
Compare
Contributor
Author
|
Friendly ping. @Borda, would you have a moment to take a look? Happy to address any feedback. |
Borda
changed the title
get_video_frames_generator
Resolved conflict in tests/utils/test_video.py: kept 3 prefetch tests from PR branch, accepted -> None return type annotation from develop. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Widen reader exception catch from Exception to BaseException; only propagate Exception subclasses via sentinel (non-Exception BaseException signals clean stop from consumer's perspective) - Move sentinel push into finally block so it always runs, including on abnormal thread exit — prevents consumer deadlock on frame_queue.get() - Replace blocking get() with get(timeout=0.5) + thread.is_alive() liveness check; consumer exits cleanly if reader dies without delivering sentinel - Add thread.join(timeout=2.0) in generator finally so VideoCapture is deterministically released after stop_event; mirrors process_video pattern - Wrap re-raised reader exception in RuntimeError with chained context so consumer traceback includes both reader and consumer call stacks --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Add parametrized test verifying stride/start/end parameters are forwarded correctly through the prefetch path vs the sync path (stride=2, start/end slicing, combined stride+start+end) - Add prefetch=1 test covering maximum backpressure / minimum queue size - Fix spurious mid-sentence line break in get_video_frames_generator Returns: docstring section --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…strings - Add ValueError for prefetch < 0 (negative values previously silently fell through to synchronous mode with no diagnostic) - Add memory footprint note to prefetch docstring: each frame = width x height x 3 bytes; directs users to VideoInfo.from_video_path() for sizing - Add prefetch=8 usage example to the Examples block showing threaded mode - Add Raises section documenting ValueError for negative prefetch - Add internal docstring to _prefetched_frames_generator explaining sentinel protocol (None=EOF, Exception instance=reader error) - Update 3 new prefetch test docstrings to Scenario/Expected Google style, matching the format of the 14 pre-existing tests in the same file - Add CHANGELOG entry for prefetch parameter under Unreleased > Added --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Comment on lines
+355
to
+357
| except BaseException as exc: | ||
| if isinstance(exc, Exception): | ||
| sentinel = exc |
Comment on lines
+270
to
+271
| Raises: | ||
| ValueError: If `prefetch` is negative. |
| assert all(frame.shape == (480, 640, 3) for frame in frames) | ||
|
|
||
|
|
||
| def test_get_video_frames_generator_prefetch_matches_sync(dummy_video_path): |
| assert np.array_equal(a, b) | ||
|
|
||
|
|
||
| def test_get_video_frames_generator_prefetch_propagates_decode_errors(tmp_path): |
| list(get_video_frames_generator(missing_path, prefetch=4)) | ||
|
|
||
|
|
||
| def test_get_video_frames_generator_prefetch_early_termination(dummy_video_path): |
Comment on lines
+227
to
+228
| with pytest.raises((Exception, RuntimeError)): | ||
| list(get_video_frames_generator(missing_path, prefetch=4)) |
| - Fixed [#2382](https://github.com/roboflow/supervision/pull/2382): `sv.Detections.get_anchors_coordinates` now uses oriented bounding box corners (`data["xyxyxyxy"]`) when OBB data is present, instead of falling back to the axis-aligned envelope. Anchors on rotated detections now lie on the oriented body rather than drifting to the envelope. Non-OBB detections and `Position.CENTER_OF_MASS` (which requires a mask) are unaffected. | ||
|
|
||
| ### Added | ||
| - `sv.get_video_frames_generator` now accepts `prefetch: int = 0` ([#2273](https://github.com/roboflow/supervision/pull/2273)). When `> 0`, frames are decoded on a background daemon thread and buffered in a bounded queue, overlapping I/O with consumer processing. Default `0` preserves the existing synchronous behaviour. |
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.
Closes #1411.
@LinasKo asked for a worked threading example that produces a real FPS improvement for the
get_video_frames_generatorpath. This PR adds an opt-inprefetch: int = 0argument: when> 0, frames are decoded in a background thread and buffered in a bounded queue, so a CPU-bound consumer can overlap with decode I/O.Default stays
0(synchronous, behaviour unchanged). The threaded path is a thin wrapper that drives the existing sync generator on onedaemon=Truethread and pushes frames through aQueue(maxsize=prefetch). No new dependencies, ~30 added lines invideo.py. Pattern matches the reader-thread already inprocess_videofurther down the same file.Benchmark
150-frame 1080p h.264 video, fixed CPU consumer simulated with
time.sleep:Decode alone on this video is ~10 ms/frame, so the speed-up is largest when the consumer cost is roughly decode-bound. Heavier consumers asymptote to no benefit, which is the right behaviour.
Test
test_get_video_frames_generator_prefetch_matches_syncruns the generator twice on the same dummy video withprefetch=0andprefetch=4and asserts the two outputs are frame-for-frame identical. Fullpytest src/ tests/is green (1859 passed). Pre-commit clean.