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

[tflchef] Introduce finalize_ext_buffer and members #13699

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

seanshpark
Copy link
Contributor

This will add finalize_ext_buffer method and members to handle data outside of limits of flatbuffres offset.

This will add finalize_ext_buffer method and members to handle data
outside of limits of flatbuffres offset.

ONE-DCO-1.0-Signed-off-by: SaeHie Park <[email protected]>
@seanshpark
Copy link
Contributor Author

seanshpark commented Aug 19, 2024

@seanshpark seanshpark requested a review from a team August 19, 2024 08:33
Copy link
Contributor

@shs-park shs-park left a comment

Choose a reason for hiding this comment

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

I left some questions, PTAL.
Anyway the code LGTM.
=)

@@ -223,6 +225,11 @@ class ModelChef
std::vector<flatbuffers::Offset<::tflite::Operator>> _operator_vec;

std::string _graph_name;

// store Buffer data to external of FB and use (Buffer) offset/size fields
bool _ext_offset = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

There is nowhere to set this flag to true yet, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, will be added in another PR.

align16(result_size);
result_size += 16; // additional for safety

std::string result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, is there any reason to use string instead of vector<uint8_t>?
I was wondering if there might be cases where it is misrecognized as text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::string was referenced from tflite codes

  • from void Translator::AppendBufferData(std::string& result), from r2.16 branch (5bc9d26649cca274750ad3625bd93422617eed4b)
  • current HEAD is different; void Translator::AppendBufferData(absl::Cord& result)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!

@seanshpark seanshpark merged commit dad984b into Samsung:master Aug 19, 2024
9 checks passed
@seanshpark seanshpark deleted the tflchef_fin_ext_buff branch August 19, 2024 09:31
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.

2 participants