-
Notifications
You must be signed in to change notification settings - Fork 176
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
[DIP] Make Resize API more consistent with OpenCV #154
Conversation
Sorry, i misunderstood OpenCV doc, its resize() read params in [width, height] format, instead of [h, w]. I 'm really sorry for this mistake, later i will revise the initial func in OpenCV benchmark design. |
Shall i delete |
} | ||
std::reverse(scalingRatios.begin(), scalingRatios.end()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also adjust the OpenCV dimension(previous PR give [h, w], but it need [w, h]), so the dimension conversion is no longer required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understand, we need to feed dimensions in [h, w] format at the lower level and we capture input in [w, h] format from the higher level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benchmark code acquire inputs in [h, w], in the example <RowNum>
comes before <ColNum>
, so we capture high level inputs in [h, w] format:
./image-processing-resize-benchmark <image path> <Scale option> <RowNum> <ColNum> <InterpolationOption>
The lower level format is only determined by API(Buddy or OpenCV), so both of them need to convert [h, w] input in high level to [w, h] level in low level.
How about changing the benchmark code, i will do the [h, w] -> [w, h] conversion in the benchmark code, so the low level API get the [w, h] format. Also, the code in DIP.h dont need a switch anymore, the following code
return detail::Resize2D_Impl( input, type, {scalingRatios[1], scalingRatios[0]}, outputSize);
can change to
return detail::Resize2D_Impl( input, type, {scalingRatios[0], scalingRatios[1]}, outputSize);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the benchmark code to take input in the [w, h] format.
Applying std::reverse
makes it easier to read the code imo, because then we use scalingRatio[0]
to get outputSize[0]
and scalingRatio[1]
to get outputSize[1]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the benchmark code to take input in the [w, h] format.
Please have a review at the resize2d PR. The tasks mentioned above were in the new commits.
So does this proposed API :)
Okay |
This PR intends to make some changes in resize API to make it more consistent with OpenCV. Ref: buddy-compiler/buddy-benchmark#61 (comment)
@taiqzheng, can you confirm if these changes would help your work?