search

IntelLabs / vdms

VDMS: Your Favorite Visual Data Management System
MIT License
84 stars 31 forks source link

Remove copies in image and video implementation #104

Closed luisremis closed 5 years ago

luisremis commented 5 years ago

Optimized video and image code, avoiding copies of cvmats. This increase performance of video read and several image operations by up to 25%.

vishakha041 commented 5 years ago

I don't see new tests for checking the copy parameters. Are our current tests sufficient to make sure we are not losing image content anywhere? Our typical checks involve metadata verification. I guess that concern applies to the old code too but just thought of it now.

vishakha041 commented 5 years ago

What are the pros/cons of merging the ImageData/Image classes? That code was reviewed well back when and this change kind of breaks consistency across how video and descriptors were structured. Is that not true?

luisremis commented 5 years ago

What are the pros/cons of merging the ImageData/Image classes? That code was reviewed well back when and this change kind of breaks consistency across how video and descriptors were structured. Is that not true?

These changes have been moved to another branch and will be part of a future pull request

luisremis commented 5 years ago

I don't see new tests for checking the copy parameters. Are our current tests sufficient to make sure we are not losing image content anywhere? Our typical checks involve metadata verification. I guess that concern applies to the old code too but just thought of it now.

We have tests for both, the metadata an pixels. We have many tests that check every single pixel of the image and video and compare it with direct OpenCV results. The copies in the images are tests through the video implementation that is using those copy and move constructors.

All tests are passing.

luisremis commented 5 years ago

All comments applied, all tests passing, noticeable improvement on video performance by avoing copies. Same should be good images that is now avoiding multiple copies. looks good to go!