Protobuf parser for DNN module #9106

dkurt · 2017-07-06T11:47:31Z

This pullrequest changes

Use OpenCV's protocol buffers parser as optional parser for DNN module. Dependency is designed to use parser if it's available. So applications will start use it automatically after integration. It this way, Google's protobuf not link and not build. To disable this dependency set -Dopencv_dnn_USE_PROTOBUF=ON or disable parser by -DBUILD_opencv_protobuf_parser=OFF if it isn't used somewhere.
Require compiled caffe.proto and graph.proto by proto compiler (see protoc --descriptor_set_out).
caffe_importer logic was completely rewritten. tensorflow_importer was modified to support parser usage.
Minor changes of detection_output_layer were made to support both parsers.

Merge with contrib: opencv/opencv_contrib#1291
Merge with extra: opencv/opencv_extra#356

There are issues related to libprotobuf. This PR isn't solve them but it may help avoid them.
close opencv/opencv_contrib#823
close opencv/opencv_contrib#854
close opencv/opencv_contrib#1242
close #9549
http://answers.opencv.org/question/138047/opencv-32-includes-libmirprotobuf-and-protobuf-26-which-is-conflicting-with-protobuf-31/

vpisarev · 2017-07-17T22:16:23Z

@dkurt, I suggest you to remove the word "w i p" from the PR description. We all know that this is experimental patch; but without this keyword your PR will be automatically checked by buildbot as soon as you put new commits.

dkurt · 2017-07-18T12:24:24Z

@vpisarev, would it be OK if I made this usage as a single module @ opencv_contrib for a while? It let us:

add more parser-specific tests (origin protobuf will be required for a part of tests)
make softer migration: use parser if found
reduce protobuf dependency in other modules (currently read mode only)

vpisarev · 2017-07-21T15:24:16Z

@dkurt, I'm fine with it

vpisarev · 2017-10-10T11:01:58Z

@dkurt, let's merge the patch. But could you please fix the merge conflicts first?

alalek

CMake conditions should be re-worked, for example this doesn't work properly (without contrib at least):

cmake -DBUILD_PROTOBUF=OFF <opencv_dir>
make opencv_test_dnn # compilation errors

Previously it uses system external protobuf with protoc (required for updating pb file to new version).

How to switch on this "parser"?

cmake <opencv_dir>
make opencv_test_dnn # still using libprotobuf

USE_PROTOBUF is broken and doesn't work at all.

modules/dnn/misc/caffe/caffe_proto.pb
modules/dnn/misc/tensorflow/graph_proto.pb

Custom binary blobs should be avoided, because there is no way to maintain/track changes (no tools)
I believe there is no problem to write/store parsed "AST" as C++ code instead.

alalek · 2017-10-31T11:18:08Z

modules/dnn/CMakeLists.txt

+ocv_option(${the_module}_USE_PROTOBUF "Force use of Protobuf library instead OpenCV's protobuf parser" OFF)
+
+# One and only way if dnn module will be disabled - failed to link to system protobuf.
+if (DEFINED BUILD_PROTOBUF AND NOT BUILD_PROTOBUF)


if should be used without space in CMake here and below

alalek · 2017-10-31T11:23:24Z

modules/dnn/CMakeLists.txt

-    /wd4456 /wd4510 /wd4610 /wd4800
-    -wd858 -wd2196
-)
+if (${USE_PROTOBUF})


if(USE_PROTOBUF)

alalek · 2017-10-31T11:38:18Z

modules/dnn/CMakeLists.txt

-include(${OpenCV_SOURCE_DIR}/cmake/OpenCVFindLibProtobuf.cmake)
-if(NOT Protobuf_FOUND)
-  ocv_module_disable(opencv_dnn)
+ocv_option(${the_module}_USE_PROTOBUF "Force use of Protobuf library instead OpenCV's protobuf parser" OFF)


This name is "_USE_PROTOBUF" because "the_module" is not defined here (before ocv_add_module()).

Replaced under the ocv_add_module.

May I ask you to show version of system protobuf to reproduce it?

Added text descriptors parsing. So now we generate .pb from .proto using protoc. Then print it as text. I've found only one way to use python:

from google.protobuf import descriptor_pb2 msg = descriptor_pb2.FileDescriptorSet() with open('/path/to/example.pb', 'rb') as f: msg.ParseFromString(f.read()) print msg

alalek · 2017-10-31T11:44:09Z

modules/dnn/src/caffe/caffe_importer_v2.cpp

+            }
+            DictValue dict;
+            if (typeid(T) == typeid(int32_t) || typeid(T) == typeid(uint32_t) ||
+                typeid(T) == typeid(int64_t) || typeid(T) == typeid(uint64_t))


Use template specializations / type traits instead.
I believe this "if" must be handled by DictValue itself.

Thanks! Replaced to is_integer and is_specialized for non-integers but floats and doubles.

alalek · 2017-10-31T11:45:18Z

modules/dnn/src/caffe/caffe_importer_v2.cpp

+
+using namespace pb;
+
+class CaffeImporter : public Importer


Do we really need another one Caffe importer?

@alalek, Thanks! It's possible to use a single Caffe importer. I'll do it.

alalek · 2017-10-31T11:45:52Z

modules/dnn/src/caffe/caffe_importer_v2.cpp

+#ifndef HAVE_PROTOBUF
+#include <typeinfo>
+
+#include "../precomp.hpp"


precomp.hpp must be the first include without any #ifs

ribalda · 2017-11-22T14:00:12Z

Hi, have you taken a look to: protocolbuffers/protobuf#1489 ?

dkurt · 2018-10-08T13:26:53Z

I think we can close it for now.

dkurt force-pushed the protobuf_parser branch 8 times, most recently from 21f431a to 25b0325 Compare July 7, 2017 12:12

dkurt changed the title ~~Eliminated protobuf dependency from Caffe importer~~ Protobuf parser for DNN module Jul 7, 2017

dkurt force-pushed the protobuf_parser branch 6 times, most recently from 777799b to 20011c2 Compare July 11, 2017 10:26

dkurt force-pushed the protobuf_parser branch from 20011c2 to 8746371 Compare July 17, 2017 06:50

dkurt force-pushed the protobuf_parser branch from 8746371 to 19d205f Compare July 18, 2017 11:24

dkurt force-pushed the protobuf_parser branch 8 times, most recently from c72266e to d8f35d3 Compare July 31, 2017 07:29

dkurt force-pushed the protobuf_parser branch from d8f35d3 to c5cdd67 Compare August 1, 2017 07:28

dkurt mentioned this pull request Aug 2, 2017

Linking dnn with dll version of protobuf failed opencv/opencv_contrib#1242

Open

dkurt force-pushed the protobuf_parser branch from 84752ea to 6137871 Compare August 10, 2017 11:50

dkurt force-pushed the protobuf_parser branch from 6137871 to b27326e Compare August 21, 2017 18:36

dkurt force-pushed the protobuf_parser branch from b27326e to 4065806 Compare September 5, 2017 10:52

dkurt force-pushed the protobuf_parser branch from 4065806 to 09dd64f Compare September 19, 2017 08:58

dkurt force-pushed the protobuf_parser branch 4 times, most recently from b58841c to 3e22e30 Compare October 16, 2017 11:32

dkurt mentioned this pull request Oct 24, 2017

opencv_dnn produce errors when a thirdparty also links against protobuf opencv/opencv_contrib#823

Closed

dkurt force-pushed the protobuf_parser branch 3 times, most recently from e24c537 to b77f128 Compare October 27, 2017 13:24

alalek requested changes Oct 31, 2017

View reviewed changes

dkurt force-pushed the protobuf_parser branch 2 times, most recently from e2b14f2 to 0ead211 Compare November 1, 2017 11:55

dkurt force-pushed the protobuf_parser branch 2 times, most recently from c492db7 to 386dfc2 Compare November 21, 2017 10:15

basvandijk mentioned this pull request Nov 21, 2017

opencv: 3.3.0 -> 3.3.1 NixOS/nixpkgs#30946

Merged

8 tasks

dkurt force-pushed the protobuf_parser branch 2 times, most recently from 8ecc554 to bc525a1 Compare November 22, 2017 15:32

dkurt added 2 commits December 10, 2017 10:22

Protobuf parser for DNN module

a6c8fc6

Single Caffe importer

4aff11d

dkurt force-pushed the protobuf_parser branch from bc525a1 to 5a6af44 Compare December 10, 2017 09:33

Caffe/TensorFlow protobuf descriptors in header files

b71fd67

dkurt force-pushed the protobuf_parser branch from 5a6af44 to b71fd67 Compare December 10, 2017 17:27

alalek mentioned this pull request Jan 5, 2018

protobuf: build issue #10510

Closed

dkurt closed this Oct 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Protobuf parser for DNN module #9106

Protobuf parser for DNN module #9106

dkurt commented Jul 6, 2017 •

edited

Loading

vpisarev commented Jul 17, 2017

dkurt commented Jul 18, 2017

vpisarev commented Jul 21, 2017

vpisarev commented Oct 10, 2017

alalek left a comment

alalek Oct 31, 2017

dkurt Nov 1, 2017

alalek Oct 31, 2017

dkurt Nov 1, 2017

alalek Oct 31, 2017

dkurt Nov 1, 2017

alalek Oct 31, 2017

dkurt Nov 1, 2017

alalek Oct 31, 2017

dkurt Nov 21, 2017

alalek Oct 31, 2017

dkurt Nov 1, 2017

ribalda commented Nov 22, 2017

dkurt commented Oct 8, 2018

Protobuf parser for DNN module #9106

Protobuf parser for DNN module #9106

Conversation

dkurt commented Jul 6, 2017 • edited Loading

This pullrequest changes

vpisarev commented Jul 17, 2017

dkurt commented Jul 18, 2017

vpisarev commented Jul 21, 2017

vpisarev commented Oct 10, 2017

alalek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ribalda commented Nov 22, 2017

dkurt commented Oct 8, 2018

dkurt commented Jul 6, 2017 •

edited

Loading