Skip to content

dnn: fix High-Level public API (cv::dnn::Model class)#18711

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
alalek:dnn_fix_model_public_api
Nov 17, 2020
Merged

dnn: fix High-Level public API (cv::dnn::Model class)#18711
opencv-pushbot merged 1 commit intoopencv:masterfrom
alalek:dnn_fix_model_public_api

Conversation

@alalek
Copy link
Copy Markdown
Member

@alalek alalek commented Oct 31, 2020 •
edited
Loading

relates #14780

  • proxy selected Net methods only (don't derive from Net directly)
  • default Model ctor is protected

This refactoring is conservative enough and it doesn't introduce changes in existed code from:

  • C++ tests
  • C++ samples
  • Python bindings tests (no Java tests, BTW)

Required for integration of #17570

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Nov 2, 2020

Am I right that we should proxy dnn::Net to introduce private fields like discussed here?

@alalek
Copy link
Copy Markdown
Member Author

alalek commented Nov 2, 2020

introduce private fields

No, all private fields should be hidden.
Also only one "Impl" should exist (no public Voc or similar in derived classes).
Users should have 1 smart pointer only in object instances.

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Nov 3, 2020

introduce private fields

No, all private fields should be hidden.
Also only one "Impl" should exist (no public Voc or similar in derived classes).
Users should have 1 smart pointer only in object instances.

So why we need to break this inheritance?

@alalek
Copy link
Copy Markdown
Member Author

alalek commented Nov 3, 2020

Because we should not have this inheritance exposing excessive Net internals (there is a comment about that in the patch).
Lets keep APIs simple, especially in "High-Level" case.

@alalek
Copy link
Copy Markdown
Member Author

alalek commented Nov 3, 2020

inheritance

There is still a question how to deal with algorithms with multiple DL networks (SiamRPN++).
But it is clean that inheritance from Net doesn't work in that case.

@wenqingzhang-gordon wenqingzhang-gordon mentioned this pull request Nov 6, 2020
Merged
12 tasks
@alalek
dnn: fix High-Level public API (cv::dnn::Model class)
23baf1a 
- proxy selected Net methods only (don't derive from Net directly)
- default Model ctor is protected
@opencv-pushbot opencv-pushbot merged commit 9acbfc6 into opencv:master Nov 17, 2020
@alalek alalek mentioned this pull request Nov 19, 2020
Merged
@alalek alalek mentioned this pull request Nov 27, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshabunin mshabunin mshabunin approved these changes

@dkurt dkurt dkurt approved these changes

Assignees

@dkurt dkurt

Labels

category: dnn RFC

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

4 participants

@alalek @dkurt @mshabunin @opencv-pushbot