Skip to content

PyrDown: Fix bug #12961#13672

Merged
alalek merged 4 commits intoopencv:3.4from
arnaudbrejeon:bug_fix_12961
Jan 28, 2019
Merged

PyrDown: Fix bug #12961#13672
alalek merged 4 commits intoopencv:3.4from
arnaudbrejeon:bug_fix_12961

Conversation

@arnaudbrejeon
Copy link
Copy Markdown
Contributor

resolves #12961

This pullrequest changes

Force ARM implementation to use an unaligned int pointer.

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Did you compare performance impact?

@arnaudbrejeon
Copy link
Copy Markdown
Contributor Author

I did a couple of quick measures on my iPad. I run 100 successive cv::pyrDown and measured the time in milliseconds. May not be the most accurate but it gives us an idea of the results.
I cannot compare directly both implementations because with the images supported by both implementations, the modified code will not run.

I tried to evaluate the speed on similar-size images.
Current implementation:
520x520, 1 channel image: 306 ms
521x527, 1 channel image: 356 ms

New implementation (is crashing on current implementation):
521x521, 1 channel image: 333 ms

The new implementation is close to the current implementation for images of similar sizes. It makes me think the alignment code is behaving properly or at least doesn't drain performance much.
Let me know if you have other tests in mind.

@alalek

This comment has been minimized.

@arnaudbrejeon arnaudbrejeon changed the base branch from master to 3.4 January 25, 2019 18:27
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Well done! Thank you 👍

@alalek alalek merged commit d998e70 into opencv:3.4 Jan 28, 2019
@alalek alalek mentioned this pull request Feb 1, 2019
@arnaudbrejeon arnaudbrejeon deleted the bug_fix_12961 branch June 5, 2019 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants