Skip to content

G-API: ONNX. Support for networks with three dimensional input.#18943

Merged
alalek merged 7 commits intoopencv:masterfrom
mpashchenkov:mp/onnx-padding
Jan 29, 2021
Merged

G-API: ONNX. Support for networks with three dimensional input.#18943
alalek merged 7 commits intoopencv:masterfrom
mpashchenkov:mp/onnx-padding

Conversation

@mpashchenkov
Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov commented Nov 27, 2020 •
edited
Loading

We can call infer with images (not tensors) for networks like this:
https://github.com/onnx/models/tree/master/vision/object_detection_segmentation/faster-rcnn#input-to-model

Fix for build on Windows (ort::Session on Windows expects wchar_t* instead char*).

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=Custom
buildworker:Custom=linux-1
build_image:Custom=ubuntu-onnx:20.04
test_modules:Custom=gapi
test_filter:Custom=*ONNX*

OrestChura
OrestChura reviewed Nov 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@OrestChura OrestChura left a comment

Choose a reason for hiding this comment

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

Very well!

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Dec 9, 2020

@mpashchenkov Friendly reminder.

OrestChura
OrestChura approved these changes Dec 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@OrestChura OrestChura left a comment

Choose a reason for hiding this comment

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

LGTM

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated
const int padded_h = std::ceil(new_h / div) * div;
const int padded_w = std::ceil(new_w / div) * div;
int type = -1;
// is_hwc - Mat has C*H, W dimensions
Copy link
Copy Markdown
Contributor

@OrestChura OrestChura Dec 14, 2020

Choose a reason for hiding this comment

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

This comment should be vice versa, right? if Mat is_not_hwc, i. e. CHW => dimensions [C*H, W, 1]

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Dec 18, 2020

Choose a reason for hiding this comment

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

Removed after "padding" discussion.

@dmatveev
Copy link
Copy Markdown
Contributor

dmatveev commented Dec 14, 2020

.cfgPadding({ cv::Size(custom_w1, custom_h1), cv::Size(custom_w2, custom_h2), ... }, { divisibility1, divisibility2, ... }).

What is divisibility? Do you have a formula how the resulting image size is calculated with padding?

@dmatveev dmatveev self-assigned this Dec 14, 2020
dmatveev
dmatveev reviewed Dec 14, 2020
View reviewed changes
mpashchenkov
mpashchenkov commented Dec 18, 2020
View reviewed changes
@mpashchenkov mpashchenkov changed the title G-API: ONNX. Adding padding in params. G-API: ONNX. Support for networks with three dimensional input. Dec 18, 2020
@mpashchenkov
Copy link
Copy Markdown
Contributor Author

mpashchenkov commented Dec 18, 2020

Removed padding from preprocessing after discussing possible troubles.

dmatveev
dmatveev reviewed Dec 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

Looks good but not sure if the std::string/std::wstring conversion is done right

@dmatveev dmatveev added this to the 4.5.2 milestone Jan 12, 2021
dmatveev
dmatveev reviewed Jan 12, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

"Approved with comments"

modules/gapi/include/opencv2/gapi/infer/onnx.hpp Outdated
#include <string>
#include <array>
#include <tuple> // tuple, tuple_size
#include <codecvt> // wstring_convert
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Jan 12, 2021

Choose a reason for hiding this comment

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

Please move it to .cpp if you only need it in .cpp.

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Jan 14, 2021

Choose a reason for hiding this comment

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

Tests use conversion to wstring on Windows.

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Jan 26, 2021

Choose a reason for hiding this comment

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

Then tests should include this, but not your generic public header.

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Jan 29, 2021

Choose a reason for hiding this comment

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

@alalek, Moved #include <codecvt> from onnx.hpp

@mpashchenkov mpashchenkov requested a review from alalek January 15, 2021 09:41
@mpashchenkov mpashchenkov requested a review from dmatveev January 22, 2021 15:19
dmatveev
dmatveev approved these changes Jan 26, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

Please avoid extra includes in public headers.

@mpashchenkov
Copy link
Copy Markdown
Contributor Author

mpashchenkov commented Jan 29, 2021

@alalek, could you merge this?

@alalek
Copy link
Copy Markdown
Member

alalek commented Jan 29, 2021

could you merge this?

You have ARs from @dmatveev : #18943 (comment)

@alalek alalek merged commit e250bae into opencv:master Jan 29, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
Merge pull request opencv#18943 from mpashchenkov:mp/onnx-padding
78e9a19 
G-API: ONNX. Support for networks with three dimensional input.

* Padding without tests

* Removed padding

* Some small fixes

* Added wstring_convert

* Alignment fix, m b

* Small fixes

* Moved include from onnx.hpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TolyaTalamanov TolyaTalamanov TolyaTalamanov left review comments

@dmatveev dmatveev dmatveev approved these changes

@OrestChura OrestChura OrestChura approved these changes

@alalek alalek Awaiting requested review from alalek

Assignees

@dmatveev dmatveev

Labels

category: g-api / gapi feature

Projects

None yet

Milestone

4.5.2

Development

Successfully merging this pull request may close these issues.

6 participants

@mpashchenkov @asmorkalov @dmatveev @alalek @TolyaTalamanov @OrestChura