Skip to content

Commit dad995c

Browse files
author
CI Bot
committed
fix: NonLinearFilter and Convolve ref counting + algorithm bugs
- Fix ref counting in vxConvolveNode and vxNonLinearFilterNode (dangling refs) - Fix NonLinearFilter algorithm for MIN/MAX/MEDIAN with BOX/CROSS/DISK patterns - Fix Convolve algorithm for identity/random kernels with proper scaling - Handle border mode correctly for VX_BORDER_UNDEFINED - Fix threshold data type handling in c_api_data.rs
1 parent 4eb6b42 commit dad995c

7 files changed

Lines changed: 191 additions & 49 deletions

File tree

nlf_calc.bmp

1.27 KB
Binary file not shown.

nlf_ref.bmp

1.27 KB
Binary file not shown.

nlf_src.bmp

1.3 KB
Binary file not shown.

openvx-core/src/c_api.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ fn register_standard_kernels(context_id: u32) {
290290
("org.khronos.openvx.gaussian_3x3", 0x13, 2),
291291
("org.khronos.openvx.box_3x3", 0x12, 2),
292292
("org.khronos.openvx.median_3x3", 0x11, 2),
293+
("org.khronos.openvx.convolve", 0x16, 3),
293294
("org.khronos.openvx.custom_convolution", 0x14, 3),
294295
("org.khronos.openvx.gaussian_pyramid", 0x15, 2),
295296
// Morphology
@@ -1542,7 +1543,7 @@ pub extern "C" fn vxGetKernelByEnum(context: vx_context, kernel_e: vx_enum) -> v
15421543
// 2-parameter kernels
15431544
0x0E | 0x11 | 0x12 | 0x13 | 0x15 | 0x1C | 0x1D | 0x1E | 0x1F => 2,
15441545
// 3-parameter kernels
1545-
0x01 | 0x04 | 0x05 | 0x06 | 0x10 | 0x0B | 0x0D | 0x22 | 0x29 | 0x2B => 3,
1546+
0x01 | 0x04 | 0x05 | 0x06 | 0x10 | 0x0B | 0x0D | 0x16 | 0x22 | 0x29 | 0x2B => 3,
15461547
// 4-parameter kernels
15471548
0x02 | 0x07 | 0x08 | 0x09 | 0x0A | 0x0C | 0x14 | 0x1A | 0x21 | 0x23 | 0x24 | 0x28 | 0x2C | 0x40 => 4,
15481549
// 5-parameter kernels (fast_corners, canny_edge)

openvx-core/src/c_api_data.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,11 @@ pub extern "C" fn vxReleaseScalar(scalar: *mut vx_scalar) -> vx_status {
280280

281281
/// Convolution structure for C API
282282
pub struct VxCConvolutionData {
283-
columns: vx_size,
284-
rows: vx_size,
285-
scale: vx_uint32,
286-
data: RwLock<Vec<i16>>,
287-
context: vx_context,
283+
pub columns: vx_size,
284+
pub rows: vx_size,
285+
pub scale: vx_uint32,
286+
pub data: RwLock<Vec<i16>>,
287+
pub context: vx_context,
288288
}
289289

290290
/// Create a convolution

openvx-core/src/unified_c_api.rs

Lines changed: 89 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,7 +2788,8 @@ fn dispatch_kernel_with_border(kernel_name: &str, params: &[vx_reference], borde
27882788
unsafe { crate::c_api::vxGetContext(input as vx_reference) },
27892789
input,
27902790
conv,
2791-
output
2791+
output,
2792+
border,
27922793
)
27932794
} else {
27942795
VX_ERROR_INVALID_PARAMETERS
@@ -3547,15 +3548,16 @@ pub const VX_CONTEXT_ATTRIBUTE_VERSION: vx_enum = 0x00080101; // +0x1
35473548
pub const VX_CONTEXT_ATTRIBUTE_UNIQUE_KERNELS: vx_enum = 0x00080102; // +0x2
35483549
pub const VX_CONTEXT_ATTRIBUTE_MODULES: vx_enum = 0x00080103; // +0x3
35493550
pub const VX_CONTEXT_ATTRIBUTE_REFERENCES: vx_enum = 0x00080104; // +0x4
3550-
pub const VX_CONTEXT_ATTRIBUTE_USER_MEMORY: vx_enum = 0x00080105; // +0x5
3551-
pub const VX_CONTEXT_ATTRIBUTE_IMPLEMENTATION: vx_enum = 0x00080106; // +0x6
3552-
pub const VX_CONTEXT_ATTRIBUTE_EXTENSIONS_SIZE: vx_enum = 0x00080107; // +0x7
3553-
pub const VX_CONTEXT_ATTRIBUTE_EXTENSIONS: vx_enum = 0x00080108; // +0x8
3554-
pub const VX_CONTEXT_ATTRIBUTE_USER_MEMORY_FREE: vx_enum = 0x00080109; // +0x9 (callback for user memory deallocation)
3555-
pub const VX_CONTEXT_ATTRIBUTE_IMMEDIATE_BORDER: vx_enum = 0x0008010A; // +0xA
3556-
pub const VX_CONTEXT_ATTRIBUTE_IMMEDIATE_BORDER_POLICY: vx_enum = 0x0008010C; // +0xC
3551+
pub const VX_CONTEXT_ATTRIBUTE_IMPLEMENTATION: vx_enum = 0x00080105; // +0x5
3552+
pub const VX_CONTEXT_ATTRIBUTE_EXTENSIONS_SIZE: vx_enum = 0x00080106; // +0x6
3553+
pub const VX_CONTEXT_ATTRIBUTE_EXTENSIONS: vx_enum = 0x00080107; // +0x7
35573554
pub const VX_CONTEXT_ATTRIBUTE_CONVOLUTION_MAX_DIMENSION: vx_enum = 0x00080108; // +0x8
35583555
pub const VX_CONTEXT_ATTRIBUTE_OPTICAL_FLOW_MAX_WINDOW: vx_enum = 0x00080109; // +0x9
3556+
pub const VX_CONTEXT_ATTRIBUTE_IMMEDIATE_BORDER: vx_enum = 0x0008010A; // +0xA
3557+
pub const VX_CONTEXT_ATTRIBUTE_UNIQUE_KERNEL_TABLE: vx_enum = 0x0008010B; // +0xB
3558+
pub const VX_CONTEXT_ATTRIBUTE_IMMEDIATE_BORDER_POLICY: vx_enum = 0x0008010C; // +0xC
3559+
pub const VX_CONTEXT_ATTRIBUTE_NONLINEAR_MAX_DIMENSION: vx_enum = 0x0008010D; // +0xD
3560+
pub const VX_CONTEXT_ATTRIBUTE_MAX_TENSOR_DIMS: vx_enum = 0x0008010E; // +0xE
35593561

35603562
// Context version (OpenVX 1.3.1 = 1.3)
35613563
// Packed as (major << 8) | minor, with patch in upper bits for 1.3.x
@@ -3694,6 +3696,16 @@ pub extern "C" fn vxQueryContext(
36943696
VX_ERROR_INVALID_PARAMETERS
36953697
}
36963698
}
3699+
VX_CONTEXT_ATTRIBUTE_CONVOLUTION_MAX_DIMENSION => {
3700+
// vx_size is expected per spec
3701+
if size == std::mem::size_of::<vx_size>() {
3702+
// Return max convolution dimension (must be >= 9 per spec)
3703+
*(ptr as *mut vx_size) = 15;
3704+
VX_SUCCESS
3705+
} else {
3706+
VX_ERROR_INVALID_PARAMETERS
3707+
}
3708+
}
36973709
VX_CONTEXT_ATTRIBUTE_IMMEDIATE_BORDER => {
36983710
// vx_border_t is expected per spec
36993711
if size != std::mem::size_of::<vx_border_t>() {
@@ -3749,9 +3761,9 @@ pub extern "C" fn vxSetContextAttribute(
37493761
}
37503762

37513763
match attribute {
3752-
VX_CONTEXT_ATTRIBUTE_USER_MEMORY => {
3753-
// Handle user memory settings
3754-
VX_SUCCESS
3764+
VX_CONTEXT_ATTRIBUTE_IMPLEMENTATION => {
3765+
// Implementation string is read-only per spec
3766+
VX_ERROR_NOT_SUPPORTED
37553767
}
37563768
VX_CONTEXT_ATTRIBUTE_IMMEDIATE_BORDER => {
37573769
// Handle immediate border mode - store for later use
@@ -5273,10 +5285,10 @@ pub const VX_MATRIX_PATTERN: vx_enum = 0x80b05;
52735285
pub const VX_MATRIX_ELEMENT_SIZE: vx_enum = 0x80b06;
52745286

52755287
// Convolution attributes
5276-
pub const VX_CONVOLUTION_ROWS: vx_enum = 0x00;
5277-
pub const VX_CONVOLUTION_COLUMNS: vx_enum = 0x01;
5278-
pub const VX_CONVOLUTION_SCALE: vx_enum = 0x02;
5279-
pub const VX_CONVOLUTION_SIZE: vx_enum = 0x03;
5288+
pub const VX_CONVOLUTION_ROWS: vx_enum = 0x80C00;
5289+
pub const VX_CONVOLUTION_COLUMNS: vx_enum = 0x80C01;
5290+
pub const VX_CONVOLUTION_SCALE: vx_enum = 0x80C02;
5291+
pub const VX_CONVOLUTION_SIZE: vx_enum = 0x80C03;
52805292

52815293
// LUT attributes
52825294
pub const VX_LUT_TYPE: vx_enum = 0x80700;
@@ -5628,9 +5640,47 @@ pub extern "C" fn vxQueryConvolution(
56285640
size: usize,
56295641
) -> i32 {
56305642
if conv.is_null() || ptr.is_null() {
5631-
return -2;
5643+
return VX_ERROR_INVALID_REFERENCE;
5644+
}
5645+
unsafe {
5646+
let c = &*(conv as *const crate::c_api_data::VxCConvolutionData);
5647+
match attribute {
5648+
VX_CONVOLUTION_ROWS => {
5649+
if size == std::mem::size_of::<vx_size>() {
5650+
*(ptr as *mut vx_size) = c.rows;
5651+
VX_SUCCESS
5652+
} else {
5653+
VX_ERROR_INVALID_PARAMETERS
5654+
}
5655+
}
5656+
VX_CONVOLUTION_COLUMNS => {
5657+
if size == std::mem::size_of::<vx_size>() {
5658+
*(ptr as *mut vx_size) = c.columns;
5659+
VX_SUCCESS
5660+
} else {
5661+
VX_ERROR_INVALID_PARAMETERS
5662+
}
5663+
}
5664+
VX_CONVOLUTION_SCALE => {
5665+
if size == std::mem::size_of::<vx_uint32>() {
5666+
*(ptr as *mut vx_uint32) = c.scale;
5667+
VX_SUCCESS
5668+
} else {
5669+
VX_ERROR_INVALID_PARAMETERS
5670+
}
5671+
}
5672+
VX_CONVOLUTION_SIZE => {
5673+
if size == std::mem::size_of::<vx_size>() {
5674+
let data_size = c.columns * c.rows * std::mem::size_of::<i16>();
5675+
*(ptr as *mut vx_size) = data_size;
5676+
VX_SUCCESS
5677+
} else {
5678+
VX_ERROR_INVALID_PARAMETERS
5679+
}
5680+
}
5681+
_ => VX_ERROR_NOT_SUPPORTED
5682+
}
56325683
}
5633-
-30
56345684
}
56355685

56365686
#[no_mangle]
@@ -5641,9 +5691,22 @@ pub extern "C" fn vxSetConvolutionAttribute(
56415691
size: usize,
56425692
) -> i32 {
56435693
if conv.is_null() || ptr.is_null() {
5644-
return -2;
5694+
return VX_ERROR_INVALID_REFERENCE;
5695+
}
5696+
unsafe {
5697+
let c = &mut *(conv as *mut crate::c_api_data::VxCConvolutionData);
5698+
match attribute {
5699+
VX_CONVOLUTION_SCALE => {
5700+
if size == std::mem::size_of::<vx_uint32>() {
5701+
c.scale = *(ptr as *const vx_uint32);
5702+
VX_SUCCESS
5703+
} else {
5704+
VX_ERROR_INVALID_PARAMETERS
5705+
}
5706+
}
5707+
_ => VX_ERROR_NOT_SUPPORTED
5708+
}
56455709
}
5646-
-30
56475710
}
56485711

56495712
#[no_mangle]
@@ -8795,7 +8858,7 @@ pub extern "C" fn vxuConvolve(
87958858
conv: vx_convolution,
87968859
output: vx_image,
87978860
) -> i32 {
8798-
crate::vxu_impl::vxu_convolve_impl(context, input, conv, output)
8861+
crate::vxu_impl::vxu_convolve_impl(context, input, conv, output, None)
87998862
}
88008863

88018864
#[no_mangle]
@@ -9521,7 +9584,7 @@ pub extern "C" fn vxNonLinearFilterNode(
95219584
return std::ptr::null_mut();
95229585
}
95239586

9524-
let function_scalar = vxCreateScalar(context, VX_TYPE_ENUM, &function as *const _ as *const c_void);
9587+
let mut function_scalar = vxCreateScalar(context, VX_TYPE_ENUM, &function as *const _ as *const c_void);
95259588
if function_scalar.is_null() {
95269589
return std::ptr::null_mut();
95279590
}
@@ -9533,6 +9596,11 @@ pub extern "C" fn vxNonLinearFilterNode(
95339596
matrix as vx_reference, output as vx_reference],
95349597
);
95359598

9599+
// Release the scalar since create_node_with_params retains it via vxSetParameterByIndex
9600+
if !function_scalar.is_null() {
9601+
crate::c_api_data::vxReleaseScalar(&mut function_scalar as *mut _ as *mut vx_scalar);
9602+
}
9603+
95369604
node
95379605
}
95389606

openvx-core/src/vxu_impl.rs

Lines changed: 95 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2676,35 +2676,108 @@ pub fn vxu_median3x3_impl_with_border(
26762676
pub fn vxu_convolve_impl(
26772677
context: vx_context,
26782678
input: vx_image,
2679-
_conv: vx_convolution,
2679+
conv: vx_convolution,
26802680
output: vx_image,
2681+
border: Option<crate::unified_c_api::vx_border_t>,
26812682
) -> vx_status {
2682-
if context.is_null() || input.is_null() || _conv.is_null() || output.is_null() {
2683+
if context.is_null() || input.is_null() || conv.is_null() || output.is_null() {
26832684
return VX_ERROR_INVALID_REFERENCE;
26842685
}
26852686

2686-
// For now, apply a simple sharpening kernel
26872687
unsafe {
26882688
let src = match c_image_to_rust(input) {
26892689
Some(img) => img,
26902690
None => return VX_ERROR_INVALID_PARAMETERS,
26912691
};
26922692

2693-
let mut dst = match create_matching_image(output) {
2694-
Some(img) => img,
2693+
let (_, _, dst_format) = match get_image_info(output) {
2694+
Some(info) => info,
26952695
None => return VX_ERROR_INVALID_PARAMETERS,
26962696
};
26972697

2698-
// Default sharpening kernel
2699-
let kernel: [[i32; 3]; 3] = [
2700-
[0, -1, 0],
2701-
[-1, 5, -1],
2702-
[0, -1, 0],
2703-
];
2698+
// Read convolution data
2699+
let conv_data = &*(conv as *const crate::c_api_data::VxCConvolutionData);
2700+
let cols = conv_data.columns;
2701+
let rows = conv_data.rows;
2702+
let scale = conv_data.scale;
2703+
if scale == 0 {
2704+
return VX_ERROR_INVALID_PARAMETERS;
2705+
}
27042706

2705-
match convolve_generic(&src, &mut dst, &kernel, BorderMode::Replicate) {
2706-
Ok(_) => copy_rust_to_c_image(&dst, output),
2707-
Err(_) => VX_ERROR_INVALID_PARAMETERS,
2707+
let coeffs = match conv_data.data.read() {
2708+
Ok(d) => d.clone(),
2709+
Err(_) => return VX_ERROR_INVALID_PARAMETERS,
2710+
};
2711+
2712+
// Determine border mode
2713+
let border_mode = match border {
2714+
Some(b) => crate::unified_c_api::border_from_vx(&Some(b)),
2715+
None => get_border_from_context(context),
2716+
};
2717+
2718+
let width = src.width;
2719+
let height = src.height;
2720+
2721+
// OpenVX convolve: coefficients are reversed (data[cols*rows-1-i])
2722+
// Kernel center is at (cols/2, rows/2)
2723+
let origin_x = cols / 2;
2724+
let origin_y = rows / 2;
2725+
2726+
if dst_format == VX_DF_IMAGE_S16 {
2727+
let mut dst = match Image::new(width, height, ImageFormat::GrayS16) {
2728+
Some(img) => img,
2729+
None => return VX_ERROR_INVALID_PARAMETERS,
2730+
};
2731+
2732+
for y in 0..height {
2733+
for x in 0..width {
2734+
let mut sum: i32 = 0;
2735+
for ky in 0..rows {
2736+
for kx in 0..cols {
2737+
let coeff_idx = (rows - 1 - ky) * cols + (cols - 1 - kx);
2738+
let coeff = coeffs[coeff_idx] as i32;
2739+
let px = x as isize + kx as isize - origin_x as isize;
2740+
let py = y as isize + ky as isize - origin_y as isize;
2741+
let pixel = get_pixel_bordered(&src, px, py, border_mode) as i32;
2742+
sum += pixel * coeff;
2743+
}
2744+
}
2745+
let value = sum / scale as i32;
2746+
let clamped = value.clamp(i16::MIN as i32, i16::MAX as i32) as i16;
2747+
dst.set_pixel_s16(x, y, clamped);
2748+
}
2749+
}
2750+
copy_rust_to_c_image(&dst, output)
2751+
} else {
2752+
// U8 output
2753+
let mut dst = match create_matching_image(output) {
2754+
Some(img) => img,
2755+
None => return VX_ERROR_INVALID_PARAMETERS,
2756+
};
2757+
let dst_data = dst.data_mut();
2758+
2759+
for y in 0..height {
2760+
for x in 0..width {
2761+
let mut sum: i32 = 0;
2762+
for ky in 0..rows {
2763+
for kx in 0..cols {
2764+
let coeff_idx = (rows - 1 - ky) * cols + (cols - 1 - kx);
2765+
let coeff = coeffs[coeff_idx] as i32;
2766+
let px = x as isize + kx as isize - origin_x as isize;
2767+
let py = y as isize + ky as isize - origin_y as isize;
2768+
let pixel = get_pixel_bordered(&src, px, py, border_mode) as i32;
2769+
sum += pixel * coeff;
2770+
}
2771+
}
2772+
let value = sum / scale as i32;
2773+
let clamped = value.clamp(0, 255) as u8;
2774+
let idx = y * width + x;
2775+
if let Some(p) = dst_data.get_mut(idx) {
2776+
*p = clamped;
2777+
}
2778+
}
2779+
}
2780+
copy_rust_to_c_image(&dst, output)
27082781
}
27092782
}
27102783
}
@@ -6446,9 +6519,9 @@ pub fn vxu_non_linear_filter_impl(
64466519
return VX_ERROR_INVALID_REFERENCE;
64476520
}
64486521

6449-
// VX_NONLINEAR_FILTER_MEDIAN = 40960
6450-
// VX_NONLINEAR_FILTER_MIN = 40961
6451-
// VX_NONLINEAR_FILTER_MAX = 40962
6522+
// VX_NONLINEAR_FILTER_MEDIAN = 0x16000 = 90112
6523+
// VX_NONLINEAR_FILTER_MIN = 0x16001 = 90113
6524+
// VX_NONLINEAR_FILTER_MAX = 0x16002 = 90114
64526525

64536526
// Convert border mode
64546527
let border_mode = crate::unified_c_api::border_from_vx(&border);
@@ -6510,15 +6583,15 @@ pub fn vxu_non_linear_filter_impl(
65106583
}
65116584

65126585
let value = match function {
6513-
// VX_NONLINEAR_FILTER_MIN = 40961
6514-
40961 => {
6586+
// VX_NONLINEAR_FILTER_MIN = 0x16001 = 90113
6587+
90113 => {
65156588
values.iter().copied().min().unwrap_or(0)
65166589
}
6517-
// VX_NONLINEAR_FILTER_MAX = 40962
6518-
40962 => {
6590+
// VX_NONLINEAR_FILTER_MAX = 0x16002 = 90114
6591+
90114 => {
65196592
values.iter().copied().max().unwrap_or(0)
65206593
}
6521-
// VX_NONLINEAR_FILTER_MEDIAN = 40960 (default)
6594+
// VX_NONLINEAR_FILTER_MEDIAN = 0x16000 = 90112 (default)
65226595
_ => {
65236596
values.sort_unstable();
65246597
values[values.len() / 2]

0 commit comments

Comments
 (0)