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.

Sign up for GitHub

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

Jump to bottom

rocAL Tensor Video pipeline changes #19

Merged

LakshmiKumar23 merged 12 commits into LakshmiKumar23:lk/rocalTensor from fiona-gladwin:PR_tensor_video

Apr 17, 2023

Collaborator

fiona-gladwin commented Apr 6, 2023

Change instances of image to tensor
Add changes to VideoFileSource, VideoFileResize and SequenceReader API
Remove video loader module and structures associated with it.
Make use of Loader module for video pipeline.
Remove VideoDecoderConfig, VideoReaderConfig, VideoStorageType and VideoDecoderType
Modify Sequence Rearrange wrt tensor.
Fix the brightness and CMN augmentations to operate on sequences

fiona-gladwin and others added 9 commits

March 30, 2023 10:53


          Add changes to the video pipeline

f3c88fb

Change instances of image to tensor
Remove video loader module
Remove VideoDecoderConfig, VideoReaderConfig
Remove VideoStorageType and VideoDecoderType


          Add tensor support for Sequence Rearrange

965a2b6

Code clean up


          Add sequence frame num and time stamps loader

77fa9d3

All loader API call in master graph


          Add tensor support for sequence reader

b022943

Add sequence batch size variable to change the batch size based on sequence length


          Add support to allocate N*F ROI for sequences


          Add chnanges to process sequences brightness and CMN

db5b112


          Merge branch 'lk/rocalTensor' of https://github.com/LakshmiKumar23/MI…

495df3c

…VisionX into PR_tensor_video


          Minor changes

9246ab8


          Minor formatting changes

9982d45

fiona-gladwin requested review from rrawther and LakshmiKumar23

April 6, 2023 17:50

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/SequenceRearrange.cpp Outdated Show resolved Hide resolved

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/SequenceRearrange.cpp Outdated

-                  vxReleaseImage(&image);
+                  // Check for input parameters
+                  vx_tensor input;

Collaborator

rrawther Apr 7, 2023

looks like input is not initialized here. Please use the parameter corresponding to input and output

Collaborator Author

fiona-gladwin Apr 11, 2023

Done

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/SequenceRearrange.cpp Outdated

+                      return ERRMSG(VX_ERROR_INVALID_DIMENSION, "validate: SequenceRearrange: tensor: #0 dimensions=%lu (must be greater than or equal to 4)\n", in_num_tensor_dims);
+                  // Check for output parameters
+                  vx_tensor output;

Collaborator

rrawther Apr 7, 2023

output is not initialized

Collaborator Author

fiona-gladwin Apr 11, 2023

Done

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/SequenceRearrange.cpp Outdated

+                  STATUS_ERROR_CHECK(vxSetMetaFormatAttribute(metas[1], VX_TENSOR_DIMS, &tensor_dims, sizeof(tensor_dims)));
+                  STATUS_ERROR_CHECK(vxSetMetaFormatAttribute(metas[1], VX_TENSOR_DATA_TYPE, &tensor_type, sizeof(tensor_type)));
+                  STATUS_ERROR_CHECK(vxSetMetaFormatAttribute(metas[1], VX_TENSOR_FIXED_POINT_POSITION, &tensor_fixed_point_position, sizeof(tensor_fixed_point_position)));
+                  vxReleaseTensor(&input);

Collaborator

rrawther Apr 7, 2023

do not release tensor here. Just reference it for querying. See samples in amd_nn extensions for proper usage

Collaborator Author

fiona-gladwin Apr 11, 2023

Done

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/SequenceRearrange.cpp Outdated

+                  STATUS_ERROR_CHECK(vxSetMetaFormatAttribute(metas[1], VX_TENSOR_FIXED_POINT_POSITION, &tensor_fixed_point_position, sizeof(tensor_fixed_point_position)));
+                  vxReleaseTensor(&input);
+                  vxReleaseTensor(&output);
+                  vxReleaseParameter(&input_param);

Collaborator

rrawther Apr 7, 2023

Not required to release parameters and tensors

Collaborator Author

fiona-gladwin Apr 11, 2023

Done

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/SequenceRearrange.cpp Outdated

+                  SequenceRearrangeLocalData *data;
+                  STATUS_ERROR_CHECK(vxQueryNode(node, VX_NODE_LOCAL_DATA_PTR, &data, sizeof(data)));
+                  STATUS_ERROR_CHECK(releaseGraphHandle(node, data->handle, data->deviceType));
+                  free(data->newOrder);

Collaborator

rrawther Apr 7, 2023

please check the data validity before freeing

Collaborator Author

fiona-gladwin Apr 11, 2023

Done

rrawther reviewed

View reviewed changes

amd_openvx_extensions/amd_rpp/source/kernel_rpp.cpp Outdated Show resolved Hide resolved

rrawther reviewed

View reviewed changes

rocAL/rocAL/include/readers/video/video_reader.h Show resolved Hide resolved

rrawther reviewed

View reviewed changes

rocAL/rocAL/source/api/rocal_api_augmentation.cpp

                       std::shared_ptr<SequenceRearrangeNode> sequence_rearrange_node =  context->master_graph->add_node<SequenceRearrangeNode>({input}, {output});
-                      sequence_rearrange_node->init(new_order, new_sequence_length, sequence_length, context->master_graph->internal_batch_size());
+                      sequence_rearrange_node->init(new_order);

Collaborator

rrawther Apr 10, 2023

why is the sequence_length and new_sequence_length not needed anymore?

Collaborator Author

fiona-gladwin Apr 10, 2023

Previously we did not have the provision to store sequence length in ImageInfo
Now we can get the sequence length and new sequence length directly from the dims of input tensor and the new order vector length respectively.

rrawther reviewed

View reviewed changes

rocAL/rocAL/source/api/rocal_api_data_loaders.cpp

+                      info.set_color_format(color_format);
+                      info.set_tensor_layout(RocalTensorlayout::NFHWC);
+                      info.set_sequence_batch_size(sequence_length);
+                      info.set_max_shape();

Collaborator

rrawther Apr 10, 2023

what does set_max_shape() api do here?

Collaborator Author

fiona-gladwin Apr 10, 2023

Set max shape is to store the max width and height of the tensor wrt the layout passed.
It is set only only for the dataloaders i.e the first node.

rrawther reviewed

View reviewed changes

rocAL/rocAL/source/api/rocal_api_data_loaders.cpp

-              {
-                  Image* output = nullptr;
+                      unsigned stride) {
+                  rocalTensor* output = nullptr;

Collaborator

rrawther Apr 10, 2023

please move this after the context validity check

Collaborator Author

fiona-gladwin Apr 11, 2023

We are checking if the context is set and we throw the error and return the rocalTensor
Hence the output rocalTensor is initialized to nullptr before the validity check

rrawther reviewed

View reviewed changes

rocAL/rocAL/source/api/rocal_api_data_loaders.cpp Outdated Show resolved Hide resolved

rrawther requested changes

View reviewed changes

Collaborator

rrawther left a comment

@fiona-gladwin : please address review comments

fiona-gladwin added 3 commits

April 11, 2023 04:48


          Add check to pass only 5D tensors to SequenceRearrange

ac0c642


          Minor changes

2b5e03a

Move SequenceInfo back to video reader
Null ptr check
Variable name change


          Query params directly from parameters in SequenceRearrange

bb95d04

Collaborator Author

fiona-gladwin commented Apr 11, 2023

@fiona-gladwin : please address review comments

Hi @rrawther I have addressed all review comments. Please check

rrawther approved these changes

View reviewed changes

Collaborator

rrawther commented Apr 17, 2023

@fiona-gladwin : Please merge tot changes before making next PR

LakshmiKumar23 merged commit 0808d57 into LakshmiKumar23:lk/rocalTensor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet