Repository navigation
Accept step output images in video-state blocks so they work in embedded workflows #3296
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |
| IMAGE_KIND, | ||
| Selector, | ||
| StepSelector, | ||
| ) | ||
| from roboflow_workflows.execution_engine.entities.workload import ( | ||
| Discovery, | ||
|
|
@@ -113,7 +112,7 @@ class RateLimiterManifest(WorkflowBlockManifest): | |
| description="List of workflow steps to execute if the rate limit allows (i.e., sufficient time has passed since the last execution). These steps receive control flow only when the cooldown period has elapsed, enabling throttled downstream processing. If the cooldown period hasn't elapsed, these steps will not execute as the branch terminates. Each step selector references a block in the workflow that should execute when rate limiting permits.", | ||
| examples=[["$steps.upload"]], | ||
| ) | ||
| video_reference_image: Optional[Selector(kind=[IMAGE_KIND])] = Field( | ||
|
Collaborator
There was a problem hiding this comment. [P2] Keep video time advancing for calibration outputs This now allows Camera Calibration's output as flowchart TD
A["Video frames with advancing frame numbers"] --> B["Camera Calibration drops video metadata"]
B --> C["Fallback metadata: frame 0, FPS 30"]
C --> D["Rate limiter sees time 0 on every frame"]
D --> E["First frame: allow and save time 0"]
E --> F["Later frames: elapsed time = 0 - 0"]
F --> G["Positive cooldown never expires: suppress downstream steps"]
I reproduced this with frames 1, 3, and 5 at 1 FPS and a one-second cooldown. All three should pass. With calibration, only the first frame reached the downstream Cache Set; the grayscale control passed all three. The system-clock fallback does not help: reading the default metadata succeeds, so the limiter overwrites system time with zero without raising an exception. The metadata loss predates this PR; previously this connection failed selector validation. It now compiles but silently stops downstream processing after the first frame. Please preserve the original video metadata through calibration, or explicitly handle missing metadata in the limiter, and cover this with a multi-frame regression test.
Collaborator
Author
There was a problem hiding this comment. Fixed in 978e5c2 by keeping the metadata at the source. Camera Calibration now builds its output with Regression test: Not changed here: the rate limiter's fallback still never fires when an image has no video metadata. The |
||
| description="Optional reference to a video frame image to use for video-time-based timestamp generation. When provided, the rate limiter calculates timestamps based on video metadata (frame number and FPS) rather than system clock time. This is useful when processing video faster than real-time, ensuring rate limiting works correctly relative to video playback time rather than processing speed. If not provided (None), the block uses system clock time (datetime.now()). Only applicable for video processing workflows.", | ||
| examples=["$inputs.image"], | ||
| default=None, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Keep a stable video identity when accepting cropped images
This now allows a static-crop output to feed Cache Get/Set, including through an embedded workflow. However, static cropping generates a new crop ID on every frame, and
create_cropappends it tovideo_metadata.video_identifier. The cache uses that video identifier as its namespace, so successive frames from the same camera and the same crop region cannot read each other's cached values.flowchart TD A["Camera A, frame 1"] --> B["Same static crop: new ID crop-1"] B --> C["Cache Set writes under camera-A + crop-1"] D["Camera A, frame 2"] --> E["Same static crop: new ID crop-2"] E --> F["Cache Get reads camera-A + crop-2"] F --> G["Cache miss: previous value is in a different namespace"] C -. "Value remains here" .-> GI reproduced this by replacing grayscale with
absolute_static_crop@v1in the new embedded-cache test scenario. Three frames from one camera produced three cache misses and three retained namespaces. The grayscale control produced one namespace and hits after the first frame. On a long-running stream, the crop path also keeps adding namespaces until the cache blocks are closed.The crop behavior predates this PR; the newly enabled connection is what exposes it. Previously this workflow failed selector validation rather than running with disconnected state. Please preserve a stable source-video identity, with a stable crop-region identity where separate crop state is needed, and add a multi-frame regression test. The root image's parent ID alone is not necessarily a camera identity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 978e5c2.
create_crop/create_crop_from_tensortake an optionalvideo_crop_identifier. When it's set, it's used for the| crop:suffix in place ofcrop_identifier. The absolute and relative static crops pass their clamped pixel box (absolute_static_crop:<x_min>,<y_min>,<x_max>,<y_max>). The source video ID is unchanged, the crop region gets an ID that's the same on every frame, andparent_idis still a unique UUID per crop. Other callers (dynamic crop, slicer) don't change.Regression test:
test_embedded_cache_keeps_one_namespace_per_video_across_frames, parametrized over grayscale, absolute and relative static crop. It runs 3 frames fromcam-aand 1 fromcam-b, then asserts cache hits after the first frame and exactly one retained namespace per camera. Without the fix, both crop cases fail.