Skip to content

Nvinfer picasso fixes#286

Merged
bwsw merged 32 commits intomainfrom
nvinfer-picasso-fixes
Mar 10, 2026
Merged

Nvinfer picasso fixes#286
bwsw merged 32 commits intomainfrom
nvinfer-picasso-fixes

Conversation

@bwsw
Copy link
Contributor

@bwsw bwsw commented Mar 10, 2026

Note

Medium Risk
Changes DeepStream-facing APIs and metadata plumbing: NvInfer ROI handling now converts/clamps rotated RBBoxes into DeepStream rect params, and Picasso callbacks now receive a unified OutputMessage with EOS routed differently for bypass sources. Risk is mitigated by extensive new unit/integration/Python test updates, but this is still behavior- and API-impacting for downstream callers.

Overview
NvInfer ROI API is changed from Rect to RBBox (Rust and Python), including a new conversion path that wraps rotated boxes to axis-aligned rects and clamps ROIs to frame bounds before writing NvDsObjectMeta.rect_params; adds many edge-case tests and updates all ROI construction in tests.

Picasso output/callback API is simplified by replacing EncodedOutput/BypassOutput with a single OutputMessage enum for both on_encoded_frame and on_bypass_frame, and by delivering bypass EOS via on_bypass_frame; updates the Rust engine/pipeline, PyO3 bindings, and Python tests/docs accordingly.

Also moves skia-safe to a workspace dependency (bumping to 0.93) across crates and adds/updates several internal KB guideline/reference markdown docs.

Written by Cursor Bugbot for commit d00bb7e. This will update automatically on new commits. Configure here.

bwsw added 27 commits October 17, 2025 11:35
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Clamped width/height not adjusted for shifted left/top
    • Fixed by computing the right/bottom edge as min(original_left + w, max_w) and deriving width as right_edge - clamped_left, with a .max(0.0) guard for boxes fully outside the frame.

Create PR

Or push these changes by commenting:

@cursor push fd547fc9ac
Preview (fd547fc9ac)
diff --git a/savant_deepstream/nvinfer/src/batch_meta_builder.rs b/savant_deepstream/nvinfer/src/batch_meta_builder.rs
--- a/savant_deepstream/nvinfer/src/batch_meta_builder.rs
+++ b/savant_deepstream/nvinfer/src/batch_meta_builder.rs
@@ -252,11 +252,11 @@
         // axis-aligned: as_ltwh always succeeds for angle=None/0
         let (left, top, w, h) = bbox.as_ltwh().unwrap();
         if max_w > 0.0 && max_h > 0.0 {
-            let left = left.max(0.0);
-            let top = top.max(0.0);
-            let clamped_w = w.min(max_w - left);
-            let clamped_h = h.min(max_h - top);
-            (left, top, clamped_w, clamped_h)
+            let clamped_left = left.max(0.0);
+            let clamped_top = top.max(0.0);
+            let clamped_w = ((left + w).min(max_w) - clamped_left).max(0.0);
+            let clamped_h = ((top + h).min(max_h) - clamped_top).max(0.0);
+            (clamped_left, clamped_top, clamped_w, clamped_h)
         } else {
             (left, top, w, h)
         }
@@ -552,4 +552,27 @@
         assert!(l + w <= 15.0 + 0.01, "right edge must not exceed max_w");
         assert!(t + h <= 15.0 + 0.01, "bottom edge must not exceed max_h");
     }
+
+    #[test]
+    fn rbbox_to_rect_params_negative_left_top_clamp() {
+        // center=(5,5), size=12x12 => ltwh=(-1,-1,12,12)
+        // visible region within 200x200 frame: (0,0) to (11,11) => w=11, h=11
+        let bbox = RBBox::new(5.0, 5.0, 12.0, 12.0, None);
+        let (l, t, w, h) = rbbox_to_rect_params(&bbox, 200.0, 200.0);
+        assert!((l - 0.0).abs() < 0.01, "left should be clamped to 0, got {l}");
+        assert!((t - 0.0).abs() < 0.01, "top should be clamped to 0, got {t}");
+        assert!((w - 11.0).abs() < 0.01, "width should be 11, got {w}");
+        assert!((h - 11.0).abs() < 0.01, "height should be 11, got {h}");
+    }
+
+    #[test]
+    fn rbbox_to_rect_params_fully_outside_frame() {
+        // entirely outside the frame: center=(-50,-50), size=10x10
+        let bbox = RBBox::new(-50.0, -50.0, 10.0, 10.0, None);
+        let (l, t, w, h) = rbbox_to_rect_params(&bbox, 200.0, 200.0);
+        assert!((l - 0.0).abs() < 0.01);
+        assert!((t - 0.0).abs() < 0.01);
+        assert!((w - 0.0).abs() < 0.01, "width should be 0 for fully outside box, got {w}");
+        assert!((h - 0.0).abs() < 0.01, "height should be 0 for fully outside box, got {h}");
+    }
 }

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Rotated ROI clamping uses visual 2-pixel inset
    • Replaced the get_visual_box call (which applies a hardcoded 2-pixel inset from frame edges) with direct clamping to [0, max_w] x [0, max_h], matching the axis-aligned branch logic.

Create PR

Or push these changes by commenting:

@cursor push 03527e6c43
Preview (03527e6c43)
diff --git a/savant_deepstream/nvinfer/src/batch_meta_builder.rs b/savant_deepstream/nvinfer/src/batch_meta_builder.rs
--- a/savant_deepstream/nvinfer/src/batch_meta_builder.rs
+++ b/savant_deepstream/nvinfer/src/batch_meta_builder.rs
@@ -28,7 +28,6 @@
     nvds_create_batch_meta, nvds_meta_api_get_type, GstBuffer, GstNvDsMetaType_NVDS_BATCH_GST_META,
     NvDsBatchMeta, NvDsFrameMeta,
 };
-use savant_core::draw::PaddingDraw;
 use savant_core::primitives::RBBox;
 use std::collections::HashMap;
 use std::ptr;
@@ -228,26 +227,25 @@
 ///
 /// For axis-aligned boxes the conversion is a straightforward center-to-ltwh.
 /// For rotated boxes `get_wrapping_bbox` produces the tight axis-aligned
-/// wrapper and `get_visual_box` clamps it to `[0, max_w] x [0, max_h]`.
+/// wrapper which is then clamped to `[0, max_w] x [0, max_h]`.
 fn rbbox_to_rect_params(bbox: &RBBox, max_w: f32, max_h: f32) -> (f32, f32, f32, f32) {
     let is_rotated = bbox.get_angle().is_some_and(|a| a.abs() > f32::EPSILON);
 
     if is_rotated {
         let wrapping = bbox.get_wrapping_bbox();
+        // wrapping is axis-aligned, as_ltwh always succeeds
+        let (left, top, w, h) = wrapping.as_ltwh().unwrap();
         if max_w > 0.0 && max_h > 0.0 {
-            let zero_padding = PaddingDraw::new(0, 0, 0, 0).unwrap();
-            match wrapping.get_visual_box(&zero_padding, 0, max_w, max_h) {
-                Ok(visual) => {
-                    // visual is axis-aligned (angle=None), as_ltwh always succeeds
-                    return visual.as_ltwh().unwrap();
-                }
-                Err(e) => {
-                    log::error!("get_visual_box failed for rotated ROI: {e}");
-                }
-            }
+            let left_clamped = left.max(0.0);
+            let top_clamped = top.max(0.0);
+            let right_clamped = (left + w).min(max_w);
+            let bottom_clamped = (top + h).min(max_h);
+            let clamped_w = (right_clamped - left_clamped).max(1.0);
+            let clamped_h = (bottom_clamped - top_clamped).max(1.0);
+            (left_clamped, top_clamped, clamped_w, clamped_h)
+        } else {
+            (left, top, w, h)
         }
-        // wrapping is axis-aligned, as_ltwh always succeeds
-        wrapping.as_ltwh().unwrap()
     } else {
         // axis-aligned: as_ltwh always succeeds for angle=None/0
         let (left, top, w, h) = bbox.as_ltwh().unwrap();

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared a fix for 1 of the 2 issues found in the latest run.

  • ✅ Fixed: Clamping logic allows rect to exceed frame bounds
    • Upper-bounded left_clamped/top_clamped to max_w-1/max_h-1 and derived width/height from clamped edges, ensuring the rect always stays within [0, max_w] x [0, max_h] even when the box center is beyond the frame.

Create PR

Or push these changes by commenting:

@cursor push dbc700b289
Preview (dbc700b289)
diff --git a/savant_deepstream/nvinfer/src/batch_meta_builder.rs b/savant_deepstream/nvinfer/src/batch_meta_builder.rs
--- a/savant_deepstream/nvinfer/src/batch_meta_builder.rs
+++ b/savant_deepstream/nvinfer/src/batch_meta_builder.rs
@@ -239,12 +239,12 @@
     };
 
     if max_w > 0.0 && max_h > 0.0 {
-        let left_clamped = left.max(0.0);
-        let top_clamped = top.max(0.0);
-        let right_clamped = (left + w).min(max_w);
-        let bottom_clamped = (top + h).min(max_h);
-        let clamped_w = (right_clamped - left_clamped).max(1.0);
-        let clamped_h = (bottom_clamped - top_clamped).max(1.0);
+        let left_clamped = left.max(0.0).min(max_w - 1.0);
+        let top_clamped = top.max(0.0).min(max_h - 1.0);
+        let right_clamped = (left + w).min(max_w).max(left_clamped + 1.0);
+        let bottom_clamped = (top + h).min(max_h).max(top_clamped + 1.0);
+        let clamped_w = right_clamped - left_clamped;
+        let clamped_h = bottom_clamped - top_clamped;
         (left_clamped, top_clamped, clamped_w, clamped_h)
     } else {
         (left, top, w, h)
@@ -650,4 +650,18 @@
             "visible height must be 7 (0 to 7), not original 10"
         );
     }
+
+    /// Box entirely beyond the right/bottom edge must be clamped inside the
+    /// frame, not produce `left > max_w`.
+    #[test]
+    fn rbbox_to_rect_params_box_beyond_frame_bounds() {
+        let bbox = RBBox::ltwh(200.0, 150.0, 50.0, 40.0).unwrap();
+        let (l, t, w, h) = rbbox_to_rect_params(&bbox, 100.0, 100.0);
+        assert!(l + w <= 100.0 + 0.01, "right edge must not exceed max_w");
+        assert!(t + h <= 100.0 + 0.01, "bottom edge must not exceed max_h");
+        assert!(l >= 0.0 && l < 100.0, "left must be within [0, max_w)");
+        assert!(t >= 0.0 && t < 100.0, "top must be within [0, max_h)");
+        assert!(w >= 1.0, "width must be at least 1");
+        assert!(h >= 1.0, "height must be at least 1");
+    }
 }

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Explicit drop after conditional is a no-op
    • Removed the redundant explicit drop(view) and renamed the parameter to _view to suppress the unused variable warning while preserving the RAII lifetime semantics.

Create PR

Or push these changes by commenting:

@cursor push 74a9e70726
Preview (74a9e70726)
diff --git a/savant_deepstream/picasso/src/pipeline/bypass.rs b/savant_deepstream/picasso/src/pipeline/bypass.rs
--- a/savant_deepstream/picasso/src/pipeline/bypass.rs
+++ b/savant_deepstream/picasso/src/pipeline/bypass.rs
@@ -10,7 +10,7 @@
 pub(crate) fn process_bypass(
     source_id: &str,
     mut frame: VideoFrameProxy,
-    view: SurfaceView,
+    _view: SurfaceView,
     callbacks: &Arc<Callbacks>,
 ) {
     frame.set_transcoding_method(VideoFrameTranscodingMethod::Copy);
@@ -25,7 +25,4 @@
     if let Some(cb) = &callbacks.on_bypass_frame {
         cb.call(OutputMessage::VideoFrame(frame));
     }
-    // `view` (SurfaceView) is dropped here after the callback returns,
-    // keeping the underlying GstBuffer alive during callback execution.
-    drop(view);
 }

@bwsw bwsw merged commit 3c0ab3b into main Mar 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant