Skip to content
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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 8, 2023
Merged

[DIP] Make Resize API more consistent with OpenCV #154

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
609d19c
Init commit
meshtag Jun 2, 2023
b9f4347
Fix rounding problem in output dimension calculation
meshtag Jun 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions examples/DIPDialect/resize2D.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ bool testImplementation(int argc, char *argv[]) {
// Define memref container for image.
Img<float, 2> input(image);

intptr_t outputSize[2] = {100, 250}; // {image_rows, image_cols}
intptr_t outputSize[2] = {250, 100}; // {image_cols, image_rows}
std::vector<float> scalingRatios = {
4, 3}; // {row_scaling_ratio, col_scaling_ratio}
0.25, 0.1}; // {col_scaling_ratio, row_scaling_ratio}

// dip::Resize2D() can be called with either scaling ratios
// (Input image dimension / Output image dimension) for both dimensions or
// (Output image dimension / Input image dimension) for both dimensions or
// the output image dimensions.
// Note : Both values in output image dimensions and scaling ratios must be
// positive numbers.
Expand Down
14 changes: 9 additions & 5 deletions frontend/Interfaces/buddy/DIP/DIP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,15 @@ inline MemRef<float, 2> Rotate2D(Img<float, 2> *input, float angle,
// User interface for 2D Resize.
inline MemRef<float, 2> Resize2D(Img<float, 2> *input, INTERPOLATION_TYPE type,
std::vector<float> scalingRatios) {
if (!scalingRatios[0] || !scalingRatios[1]) {
if (scalingRatios[0] <= 0 || scalingRatios[1] <= 0) {
throw std::invalid_argument(
"Please enter non-zero values of scaling ratios.\n"
"Please enter positive values of scaling ratios.\n"
"Note : scaling ratio = "
"input_image_dimension / output_image_dimension\n");
"output_image_dimension / input_image_dimension\n");
}
std::reverse(scalingRatios.begin(), scalingRatios.end());
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

@taiqzheng taiqzheng Jun 5, 2023 •
edited
Loading

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); ?

Copy link
Member Author

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].

Copy link
Contributor

@taiqzheng taiqzheng Jun 6, 2023 •
edited
Loading

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.

scalingRatios[0] = 1 / scalingRatios[0];
meshtag marked this conversation as resolved.
Show resolved Hide resolved
scalingRatios[1] = 1 / scalingRatios[1];

intptr_t outputSize[2] = {
static_cast<unsigned int>(input->getSizes()[0] / scalingRatios[0]),
Expand All @@ -346,10 +349,11 @@ inline MemRef<float, 2> Resize2D(Img<float, 2> *input, INTERPOLATION_TYPE type,
// User interface for 2D Resize.
inline MemRef<float, 2> Resize2D(Img<float, 2> *input, INTERPOLATION_TYPE type,
intptr_t outputSize[2]) {
if (!outputSize[0] || !outputSize[1]) {
if (outputSize[0] <= 0 || outputSize[1] <= 0) {
throw std::invalid_argument(
"Please enter non-zero values of output dimensions.\n");
"Please enter positive values of output dimensions.\n");
}
std::reverse(outputSize, outputSize + 2);

std::vector<float> scalingRatios(2);
scalingRatios[1] = input->getSizes()[0] * 1.0f / outputSize[0];
Expand Down