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

Made changes for AM62A_QNX for single cam and tidl demo #91

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Akshay-Abdar
Copy link

Made necessary changes to support single camera application and TIDL based classification demo on AM62A_QNX.

@@ -84,6 +84,7 @@ link_directories(${TARGET_FS}/usr/lib/aarch64-linux
${CMAKE_LIBRARY_PATH}/lib
${VISION_APPS_LIBS_PATH}
${EDGEAI_LIBS_PATH}
${PROJECT_SOURCE_DIR}/../../psdkqa/qnxfs/tilib
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use absolute path here
This might fail if repository is kept in a different directory

avcodec
# avformat
# avutil
# avcodec
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are this commented?

@@ -26,7 +26,7 @@ set(SRC_FILES
src/tiovx_pixelwise_multiply_module.c
src/tiovx_pixelwise_add_module.c
src/tiovx_lut_module.c
src/codec_input_demuxer.c
#src/codec_input_demuxer.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this comment

@@ -45,7 +45,10 @@ if ("${TARGET_OS}" STREQUAL "QNX")
if ("${TARGET_CPU}" STREQUAL "A72" OR "${TARGET_CPU}" STREQUAL "A53")
list(APPEND
SRC_FILES
src/omx_decode_module.c)
# src/omx_decode_module.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the comment

#include "tiovx_display_module.h"
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is bit confusing
Just add capture headers separately

#define TIOVX_MODULES_DEFAULT_CAPTURE_SENSOR "SENSOR_SONY_IMX390_UB953_D3"

Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@@ -268,6 +269,7 @@ void tiovx_capture_init_cfg(TIOVXCaptureNodeCfg *node_cfg)
node_cfg->usecase_option = TIOVX_SENSOR_MODULE_FEATURE_CFG_UC0;
node_cfg->enable_error_detection = 0;
sprintf(node_cfg->target_string, TIVX_TARGET_CAPTURE1);
//sprintf(node_cfg->target_string, TIVX_TARGET_CAPTURE2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@@ -347,7 +349,6 @@ vx_status tiovx_capture_init_node(NodeObj *node)
sensor_params->sensorInfo.raw_params.height,
VX_DF_IMAGE_UYVY);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

?

{
if(sensorObj->usecase_option == TIOVX_SENSOR_MODULE_FEATURE_CFG_UC1)
{
if(sensorObj->usecase_option == TIOVX_SENSOR_MODULE_FEATURE_CFG_UC1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@@ -288,7 +289,7 @@ vx_status tiovx_sensor_init(SensorObj *sensorObj)
if(0 != sensor_init_status)
{
TIOVX_MODULE_ERROR("Error initializing sensor %s \n", sensorObj->sensor_name);
status = VX_FAILURE;
//status = VX_FAILURE; // commented forcefully
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@Akshay-Abdar Akshay-Abdar force-pushed the am62a_qnx branch 3 times, most recently from bfc15bb to 5eda3d3 Compare November 25, 2024 08:10
link_directories(${TARGET_FS}/usr/lib/aarch64-linux
${TARGET_FS}/usr/lib
${CMAKE_LIBRARY_PATH}/usr/lib
${CMAKE_LIBRARY_PATH}/lib
${VISION_APPS_LIBS_PATH}
${EDGEAI_LIBS_PATH}
)
${QNX_LIB_PATH}

Choose a reason for hiding this comment

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

can this break linux builds ? psdkqa exists only for QNX SDK

{
.init_node = tiovx_sde_init_node,
.create_node = tiovx_sde_create_node,
.post_verify_graph = NULL,
.delete_node = tiovx_sde_delete_node,
.get_cfg_size = tiovx_sde_get_cfg_size,
.get_priv_size = tiovx_sde_get_priv_size
},
},

Choose a reason for hiding this comment

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

Please remove all trailing spaces, new line differences

* OF THE POSSIBILITY OF SUCH DAMAGE.
*
*/
#ifndef _qnx_DISPLAY_MODULE

Choose a reason for hiding this comment

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

is this newly added macro ? try to use CAPS for macro

#define MSC_OUTPUT_WIDTH MSC_INPUT_WIDTH/2
#define MSC_OUTPUT_HEIGHT MSC_INPUT_HEIGHT/2
#endif

Choose a reason for hiding this comment

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

can we use else condition here ?

@@ -109,11 +120,18 @@
#define DCC_VISS TIOVX_MODULES_IMAGING_PATH"/imx390/linear/dcc_viss.bin"
#define DCC_LDC TIOVX_MODULES_IMAGING_PATH"/imx390/linear/dcc_ldc.bin"

#define POST_PROC_OUT_WIDTH (1280)
#define POST_PROC_OUT_HEIGHT (720)
#define POST_PROC_OUT_WIDTH (1920)

Choose a reason for hiding this comment

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

do we need QNX -AM62A flag here, to avoid breaking previous code ?

#define TIDL_IO_CONFIG_FILE_PATH "/opt/model_zoo/ONR-OD-8200-yolox-nano-lite-mmdet-coco-416x416/artifacts/detslabels_tidl_io_1.bin"
#define TIDL_NETWORK_FILE_PATH "/opt/model_zoo/ONR-OD-8200-yolox-nano-lite-mmdet-coco-416x416/artifacts/detslabels_tidl_net.bin"
#endif

#if defined(SOC_AM62A)

Choose a reason for hiding this comment

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

Please add QNX check also, /tifs path is relvant only for QNX. This would break AM62A-Linux build

@@ -128,6 +146,7 @@ vx_status app_modules_capture_dl_display_test(int argc, char* argv[])
GraphObj graph;
Pad *input_pad = NULL, *output_pad[4] = {NULL, NULL, NULL, NULL}, *post_proc_in_img_pad = NULL;
Buf *inbuf = NULL;
Buf *outbuf = NULL;

Choose a reason for hiding this comment

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

if outbuf varible is used only for AM62A-QNX usecase, add flags here as well. It may throw unused varible error for other QNX platforms otherwise


status = tiovx_modules_verify_graph(&graph);

#if !defined(SOC_AM62A)

Choose a reason for hiding this comment

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

QNX flag is also oneeded

viss_node = tiovx_modules_add_node(&graph, TIOVX_VISS, (void *)&viss_cfg);

output_pad[0] = &viss_node->srcs[0];

Choose a reason for hiding this comment

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

can this change break Linux/J7 build ?

@Akshay-Abdar Akshay-Abdar force-pushed the am62a_qnx branch 11 times, most recently from 509b8e2 to a64bc2b Compare November 28, 2024 09:25
@@ -184,16 +187,23 @@ if ("${TARGET_OS}" STREQUAL "QNX")
tiipc-usr
ti-udmalld
ti-pdk
ti-csirxlld

Choose a reason for hiding this comment

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

csirx, fvid2 libraries are QNX specific for AM62A only, for Jacinto these are on R5. Please add AM62A check here

#include "tiovx_capture_module.h"
#include "tiovx_aewb_module.h"
#if !defined(SOC_AM62A)

Choose a reason for hiding this comment

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

are these exluded header files needed for AM62A-Linux ? if yes, please add QNX flag as well

int32_t qnx_display_render_buf(Buf *tiovx_buffer)
{

static uint32_t j=0;

Choose a reason for hiding this comment

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

can you rename i,j varibales to something more relevant ? so that the variable names reflect their usage here

@@ -50,6 +50,16 @@ endif()
endif()
endif()

if ("${TARGET_OS}" STREQUAL "QNX")
if ("${TARGET_CPU}" STREQUAL "A72" OR "${TARGET_CPU}" STREQUAL "A53")

Choose a reason for hiding this comment

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

will this affect Jacinto ? why are we adding A72 core here ?

@Akshay-Abdar Akshay-Abdar force-pushed the am62a_qnx branch 2 times, most recently from 6cb82ea to 159113d Compare December 3, 2024 08:22
@@ -201,7 +201,9 @@ int32_t create_input_block(GraphObj *graph, InputBlock *input_block)
sprintf(sensor_name, "SENSOR_OV2312_UB953_LI");
format_pixel_container = TIVX_RAW_IMAGE_16_BIT;
format_msb = 9;
v4l2_pix_format = v4l2_fourcc('B','G','I','0');
#if defined(TARGET_OS_LINUX)
v4l2_pix_format = v4l2_fourcc('B','G','I','0');
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrong indentation

@@ -46,7 +46,10 @@ if ("${TARGET_CPU}" STREQUAL "A72" OR "${TARGET_CPU}" STREQUAL "A53")
list(APPEND
SRC_FILES
src/omx_encode_module.c
src/omx_decode_module.c)
src/omx_decode_module.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be protected for AM62A

#endif

#endif

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this empty lines

@Akshay-Abdar Akshay-Abdar force-pushed the am62a_qnx branch 3 times, most recently from c07634a to 6bed5e7 Compare December 3, 2024 11:15
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.

3 participants