Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CodeClean] fix svace issue #4462

Merged
merged 1 commit into from
May 23, 2024

Conversation

jaeyun-jung
Copy link
Collaborator

Code clean, fix svace issues.

  • check max number of tensors and prevent underflow.

@@ -727,7 +727,8 @@ gst_tensor_query_client_chain (GstPad * pad,
data_h = g_async_queue_timeout_pop (self->msg_queue,
self->timeout * G_TIME_SPAN_MILLISECOND);
if (data_h) {
self->requested_num--;
if (self->requested_num > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

need { } in lines 730 to 769?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, pushing buffer to downstream element occurs when a data processed from query-server in the queue.

Comment on lines +731 to 734
self->requested_num--;
ret = nns_edge_data_get_count (data_h, &num_data);

if (ret == NNS_EDGE_ERROR_NONE && num_data > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry,
Should code(line 732 ~ 769) be done when self->requested_num > 0 is TRUE ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually unnecessary. The param self->requested_num is required for limiting max input to query-server.
query-client handles the response from query-server by using msg-queue.

Copy link
Contributor

@songgot songgot left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -173,10 +173,15 @@ PYConverterCore::convert (GstBuffer *in_buf, GstTensorsConfig *config)

if (output) {
GstTensorInfo *_info;
unsigned int num_tensors = PyList_Size (output);
unsigned int num_tensors = (unsigned int) PyList_Size (output);
Copy link
Contributor

Choose a reason for hiding this comment

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

Return of PyList_Size coud be negative, so how about checking error ?

unsigned int num_tensors = NNS_TENSOR_SIZE_LIMIT + 1;
Py_ssize_t out_len = PyList_Size (output);
if (out_len >= 0)
   num_tensors = (unsigned int) out_len;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed param type (Py_ssize_t), please review the changes again.

@jaeyun-jung jaeyun-jung force-pushed the svace-prevent-underflow branch 3 times, most recently from c826e40 to 6bd4dcb Compare May 23, 2024 06:33
Copy link
Contributor

@jihochu jihochu left a comment

Choose a reason for hiding this comment

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

LGTM

@myungjoo
Copy link
Member

[  503s] [Starting] nnstreamer_filter_python3
[  504s] ==================================================
[  504s]     Test Group nnstreamer_filter_python3 Starts.
[  504s] [PASSED] 1:gst-launch of case 1
[  504s] [PASSED] 1:Compare 1
[  504s] [PASSED] 2:gst-launch of case 2
[  504s] [FAILED][Critical] 2:Golden test comparison
[  504s] [PASSED] 3:gst-launch of case 3
[  504s] [FAILED][Critical] 3:Golden test comparison
[  504s] [IGNORED] 4-1:gst-launch of case 4-1
[  504s] [PASSED] 4-2:Multithreaded python script as a filter (CV2)
[  504s] ==================================================
[  504s] [FAILED] Test Group nnstreamer_filter_python3 has failed cases (3)

Code clean, fix svace issues.
- check max number of tensors and prevent underflow.

Signed-off-by: Jaeyun Jung <jy1210.jung@samsung.com>
@jaeyun-jung jaeyun-jung merged commit fff02da into nnstreamer:main May 23, 2024
15 checks passed
@jaeyun-jung jaeyun-jung deleted the svace-prevent-underflow branch May 23, 2024 08:22
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.

None yet

4 participants