Skip to content

dnn(tflite): add 3rdparty flatbuffers with pre-generated schema#23274

Merged
opencv-pushbot merged 1 commit intoopencv:4.xfrom
alalek:dnn_flatbuffers_builtin
Feb 21, 2023
Merged

dnn(tflite): add 3rdparty flatbuffers with pre-generated schema#23274
opencv-pushbot merged 1 commit intoopencv:4.xfrom
alalek:dnn_flatbuffers_builtin

Conversation

@alalek
Copy link
Copy Markdown
Member

@alalek alalek commented Feb 18, 2023

relates #23161
https://github.com/google/flatbuffers/releases/tag/v23.1.21 (Apache 2.0)

TODO:

  • fix build issues (32-bit and static libraries)
  • fix tests
  • readme and licensing information
force_builders=Custom,Custom Win,Linux AVX2,Linux OpenCL,linux,windows,docs,Win32,linux32
build_image:Custom=ubuntu:16.04
build_shared:Custom=OFF
build_examples:Custom=OFF
build_shared:Custom Win=OFF
Xbuild_shared:Linux AVX2=OFF

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Feb 20, 2023

BTW, is there limitation why OpenCV does not support submodules instead?

@alalek
Copy link
Copy Markdown
Member Author

alalek commented Feb 20, 2023

We don't use submodules in the main OpenCV repositories.

endif()

if(HAVE_FLATBUFFERS)
ocv_option(OPENCV_TEST_DNN_TFLITE "Build test with TFLite" (TARGET ocv.3rdparty.flatbuffers))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we really need this option?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks! Updated handing of this option in tests. It should be more clear now (flatbuffers -> tflite)

Copy link
Copy Markdown
Contributor

@mshabunin mshabunin left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.

set(flatbuffers_VERSION "23.1.21")
ocv_install_3rdparty_licenses(flatbuffers "${OpenCV_SOURCE_DIR}/3rdparty/flatbuffers/LICENSE.txt")
ocv_add_external_target(flatbuffers "${OpenCV_SOURCE_DIR}/3rdparty/flatbuffers/include" "" "HAVE_FLATBUFFERS=1")
set(CUSTOM_STATUS_flatbuffers " Flatbuffers:" "builtin/3rdparty (${flatbuffers_VERSION})")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not like other libraries?

Suggested change
set(CUSTOM_STATUS_flatbuffers " Flatbuffers:" "builtin/3rdparty (${flatbuffers_VERSION})")
set(CUSTOM_STATUS_flatbuffers " Flatbuffers:" "build (${flatbuffers_VERSION})")

Or will we change others to the new style eventually (e.g. protobuf)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

flatbuffers is integrated as headers only (there is no compilation of the library). IMHO, build may confuse here.

@opencv-pushbot opencv-pushbot merged commit 5a22735 into opencv:4.x Feb 21, 2023
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants