Adding better auto caps negotiation based on the ZED SDK API feat.#91
Adding better auto caps negotiation based on the ZED SDK API feat.#91
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements improved automatic capabilities negotiation for GStreamer ZED camera source elements (zedsrc and zedxonesrc), leveraging the ZED SDK API to dynamically detect supported resolutions, formats, and frame rates based on the connected camera model.
Changes:
- Adds AUTO resolution mode that selects the best resolution for requested FPS based on camera model capabilities
- Implements flexible output resolution negotiation with ranged caps for non-NVMM formats (BGRA, BGR, GRAY8)
- Adds
fixatecallback to handle caps negotiation during pipeline setup - Removes deprecated
async-image-retrievalproperty from zedsrc - Changes default camera FPS from 15 to 30 and default resolution to AUTO
- Updates SDK version requirement to 5.2
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gst-zedxone-src/gstzedxonesrc.h | Adds resolution and format tracking fields for flexible output negotiation |
| gst-zedxone-src/gstzedxonesrc.cpp | Implements AUTO resolution detection, ranged caps, multi-format support (BGRA/BGR/GRAY8), and dynamic FPS validation |
| gst-zed-src/gstzedsrc.h | Adds resolution and format tracking fields, removes async_image_retrieval property |
| gst-zed-src/gstzedsrc.cpp | Implements AUTO resolution detection, ranged caps, multi-format support, dynamic FPS validation, and removes async_image_retrieval property/parameter |
| README.md | Updates SDK version to 5.2, documents new default FPS (30), corrects coordinate-system enum name, adds new properties documentation |
| .ci/test.sh | Adds comprehensive test suites for AUTO resolution negotiation, FPS clamping, and downstream resize scenarios for both zedsrc and zedxonesrc |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adujardin
changed the title
| auto devices = sl::Camera::getDeviceList(); | ||
| if (devices.empty()) { | ||
| GST_WARNING("No ZED cameras detected, falling back to ZED2"); | ||
| return sl::MODEL::ZED2; |
There was a problem hiding this comment.
Why fall back to ZED 2 instead of returning an error?
| if (src->output_width > 0 && src->output_height > 0) { | ||
| gst_structure_fixate_field_nearest_int(structure, "width", src->output_width); | ||
| gst_structure_fixate_field_nearest_int(structure, "height", src->output_height); | ||
| } |
There was a problem hiding this comment.
It should not happen, but I think you should add an "else" branch to raise an error if the width or height is null
| auto devices = sl::CameraOne::getDeviceList(); | ||
| if (devices.empty()) { | ||
| GST_WARNING("No ZED X One cameras detected, falling back to ZED_XM"); | ||
| return sl::MODEL::ZED_XM; |
There was a problem hiding this comment.
Like for zedsrc, why a default model instead of an error?
| if (src->_outputWidth > 0 && src->_outputHeight > 0) { | ||
| gst_structure_fixate_field_nearest_int(structure, "width", src->_outputWidth); | ||
| gst_structure_fixate_field_nearest_int(structure, "height", src->_outputHeight); | ||
| } |
There was a problem hiding this comment.
an else to return an error if one or both are NULL?
- Force close() in finalize() for abnormal shutdown safety - Return MODEL::LAST instead of blind ZED2/ZED_XM fallback - Validate isCameraOne() on SN/ID matches (bugged getDeviceList) - Fail early when no mono camera found instead of opening stereo - Guard resolve_auto_resolution against no-camera-detected - Always fixate width/height unconditionally
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Boolean. Default: false | ||
| od-max-range : Maximum Detection Range | ||
| flags: readable, writable |
There was a problem hiding this comment.
Duplicate property documentation: od-max-range appears twice with different types (Boolean on line 438, Float on line 441). Remove the Boolean entry on lines 436-438 as the Float type is correct for a range property.
| Boolean. Default: false | |
| od-max-range : Maximum Detection Range | |
| flags: readable, writable |
No description provided.