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

Remove copies in image and video implementation #104

Merged
merged 1 commit into from
May 17, 2019
Merged

Remove copies in image and video implementation #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 14 additions & 2 deletions include/vcl/Image.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ namespace VCL {
*
* @param cv_img An OpenCV Mat that contains an image
*/
Image(const cv::Mat &cv_img);
Image(const cv::Mat &cv_img, bool copy=true);

/**
* Creates an Image object from an encoded buffer
Expand Down Expand Up @@ -112,11 +112,21 @@ namespace VCL {

/**
* Creates a new Image object from an existing Image object
* This will make a deep copy of the arrays.
*
* @param img An existing Image object
*/
Image(const Image &img);

/**
* Move constructor, needed to avoid copies of the arrays.
* noexcept is needed to let vectors grow and call the move
* instead of copy constructor.
*
* @param img An existing Image object
*/
Image(const Image &&img) noexcept;

/**
* Sets an Image object equal to another Image object
*
Expand Down Expand Up @@ -178,9 +188,11 @@ namespace VCL {
/**
* Gets an OpenCV Mat that contains the image data
*
* @param copy Specify if a deep copy of the cvmat will be made before
* returning the cvmat object.
* @return An OpenCV Mat
*/
cv::Mat get_cvmat() const;
cv::Mat get_cvmat(bool copy=true);

/**
* Gets the raw image data
Expand Down
19 changes: 12 additions & 7 deletions src/vcl/Image.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ Image::Image(const std::string &image_id)
_image->read(image_id);
}

Image::Image(const cv::Mat &cv_img)
Image::Image(const cv::Mat &cv_img, bool copy)
{
if ( cv_img.empty() ) {
throw VCLException(ObjectEmpty, "Image object is empty");
}

_image = new ImageData(cv_img);
_image = new ImageData(cv_img, copy);

}

Expand All @@ -77,21 +77,23 @@ Image::Image(const Image &img)
_image = new ImageData(*img._image);
}

Image::Image(const Image &&img) noexcept
{
_image = new ImageData(*img._image, false);
}

void Image::operator=(const Image &img)
{
ImageData *temp = _image;
_image = new ImageData(*img._image);
delete temp;
}


Image::~Image()
{
delete _image;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed. Is this never null? I think we talked about this but I am not remembering why there is no null check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is never null.

}



/* *********************** */
/* GET FUNCTIONS */
/* *********************** */
Expand Down Expand Up @@ -130,11 +132,14 @@ Image Image::get_area(const Rectangle &roi) const
return img_copy;
}

cv::Mat Image::get_cvmat() const
cv::Mat Image::get_cvmat(bool copy)
{
cv::Mat mat = _image->get_cvmat();

return mat.clone();
if (copy)
return mat.clone();
else
return mat;
}

void Image::get_raw_data(void* buffer, long buffer_size ) const
Expand Down
74 changes: 49 additions & 25 deletions src/vcl/ImageData.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@

#include "ImageData.h"


using namespace VCL;

/* *********************** */
Expand All @@ -45,6 +44,7 @@ using namespace VCL;
/* *********************** */
/* READ OPERATION */
/* *********************** */

ImageData::Read::Read(const std::string& filename, Image::Format format)
: Operation(format),
_fullpath(filename)
Expand All @@ -64,7 +64,8 @@ void ImageData::Read::operator()(ImageData *img)
img->_channels = img->_tdb->get_image_channels();
}
else {
img->copy_cv(cv::imread(_fullpath, cv::IMREAD_ANYCOLOR));
cv::Mat img_read = cv::imread(_fullpath, cv::IMREAD_ANYCOLOR);
img->shallow_copy_cv(img_read);
if ( img->_cv_img.empty() )
throw VCLException(ObjectEmpty, _fullpath + " could not be read, \
object is empty");
Expand All @@ -74,6 +75,7 @@ void ImageData::Read::operator()(ImageData *img)
/* *********************** */
/* WRITE OPERATION */
/* *********************** */

ImageData::Write::Write(const std::string& filename, Image::Format format,
Image::Format old_format, bool metadata)
: Operation(format),
Expand Down Expand Up @@ -128,7 +130,7 @@ void ImageData::Resize::operator()(ImageData *img)
if ( !img->_cv_img.empty() ) {
cv::Mat cv_resized;
cv::resize(img->_cv_img, cv_resized, cv::Size(_rect.width, _rect.height));
img->copy_cv(cv_resized);
img->shallow_copy_cv(cv_resized);
}
else
throw VCLException(ObjectEmpty, "Image object is empty");
Expand All @@ -152,7 +154,7 @@ void ImageData::Crop::operator()(ImageData *img)
if ( img->_cv_img.rows < _rect.height + _rect.y || img->_cv_img.cols < _rect.width + _rect.x )
throw VCLException(SizeMismatch, "Requested area is not within the image");
cv::Mat roi_img(img->_cv_img, _rect);
img->copy_cv(roi_img);
img->shallow_copy_cv(roi_img);
}
else
throw VCLException(ObjectEmpty, "Image object is empty");
Expand Down Expand Up @@ -192,7 +194,7 @@ void ImageData::Flip::operator()(ImageData *img)
cv::Mat dst = cv::Mat(img->_cv_img.rows, img->_cv_img.cols,
img->_cv_img.type());
cv::flip(img->_cv_img, dst, _code);
img->_cv_img = dst.clone();
img->shallow_copy_cv(dst);
}
else
throw VCLException(ObjectEmpty, "Image object is empty");
Expand Down Expand Up @@ -239,7 +241,7 @@ void ImageData::Rotate::operator()(ImageData *img)

cv::Mat dst;
cv::warpAffine(img->_cv_img, dst, r, bbox.size());
img->_cv_img = dst.clone();
img->shallow_copy_cv(dst);
}
}
else
Expand Down Expand Up @@ -269,9 +271,12 @@ ImageData::ImageData()
_image_id = "";
}

ImageData::ImageData(const cv::Mat &cv_img)
ImageData::ImageData(const cv::Mat &cv_img, bool copy)
{
copy_cv(cv_img);
if (copy)
deep_copy_cv(cv_img);
else
shallow_copy_cv(cv_img);

_format = Image::Format::NONE_IMAGE;
_compress = CompressionType::LZ4;
Expand Down Expand Up @@ -300,7 +305,6 @@ ImageData::ImageData(const std::string &image_id)
}
else
_tdb = NULL;

}

ImageData::ImageData(void* buffer, cv::Size dimensions, int cv_type)
Expand All @@ -318,26 +322,34 @@ ImageData::ImageData(void* buffer, cv::Size dimensions, int cv_type)
_tdb->set_compression(_compress);
}

ImageData::ImageData(const ImageData &img)
ImageData::ImageData(const ImageData &img, bool copy)
{
_format = img._format;
_compress = img._compress;
_image_id = img._image_id;

if ( !(img._cv_img).empty() )
copy_cv(img._cv_img);
if ( !(img._cv_img).empty() ) {
if (copy) {
deep_copy_cv(img._cv_img);
}
else {
shallow_copy_cv(img._cv_img);
}
}

if ( img._tdb != NULL )
_tdb = new TDBImage(*img._tdb);
else
_tdb = NULL;

int start;

if ( img._operations.size() > 0 ) {
std::shared_ptr<Operation> front = img._operations.front();
if (front->get_type() == OperationType::READ) {
start = 1;
copy_cv(cv::imread(img._image_id, cv::IMREAD_ANYCOLOR));
cv::Mat img_read = cv::imread(img._image_id, cv::IMREAD_ANYCOLOR);
shallow_copy_cv(img_read);
}
else
start = 0;
Expand All @@ -352,7 +364,7 @@ void ImageData::operator=(const ImageData &img)
TDBImage *temp = _tdb;

if ( !(img._cv_img).empty() )
copy_cv(img._cv_img);
deep_copy_cv(img._cv_img);
else {
_channels = img._channels;

Expand Down Expand Up @@ -381,7 +393,8 @@ void ImageData::operator=(const ImageData &img)
std::shared_ptr<Operation> front = img._operations.front();
if (front->get_type() == OperationType::READ) {
start = 1;
copy_cv(cv::imread(img._image_id, cv::IMREAD_ANYCOLOR));
cv::Mat img_read = cv::imread(img._image_id, cv::IMREAD_ANYCOLOR);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check if this hurts or not.

shallow_copy_cv(img_read);
}
else
start = 0;
Expand Down Expand Up @@ -421,8 +434,6 @@ int ImageData::get_type() const

cv::Size ImageData::get_dimensions()
{
// TODO: iterate over operations themsevles to determine
// image size, rather than performing the operations
if ( _operations.size() > 0 )
perform_operations();
return cv::Size(_width, _height);
Expand Down Expand Up @@ -545,8 +556,10 @@ std::vector<unsigned char> ImageData::get_encoded(Image::Format format,
if ( _cv_img.empty() ) {
if ( _tdb == NULL)
throw VCLException(ObjectEmpty, "No data to encode");
else
copy_cv(_tdb->get_cvmat());
else {
cv::Mat img = _tdb->get_cvmat();
shallow_copy_cv(img);
}
}

std::vector<unsigned char> buffer;
Expand Down Expand Up @@ -653,7 +666,8 @@ void ImageData::set_data_from_raw(void* buffer, long size)

void ImageData::set_data_from_encoded(const std::vector<unsigned char> &buffer)
{
copy_cv(cv::imdecode(buffer, cv::IMREAD_ANYCOLOR));
cv::Mat img_read = cv::imdecode(buffer, cv::IMREAD_ANYCOLOR);
shallow_copy_cv(img_read);
}

void ImageData::set_minimum(int dimension)
Expand All @@ -666,7 +680,6 @@ void ImageData::set_minimum(int dimension)
}
}


/* *********************** */
/* IMAGEDATA INTERACTION */
/* *********************** */
Expand Down Expand Up @@ -752,16 +765,28 @@ void ImageData::delete_object()
/* COPY FUNCTIONS */
/* *********************** */

void ImageData::copy_cv(const cv::Mat &cv_img)
void ImageData::deep_copy_cv(const cv::Mat &cv_img)
{
_channels = cv_img.channels();

_height = cv_img.rows;
_width = cv_img.cols;
_width = cv_img.cols;

_cv_type = cv_img.type();

_cv_img = cv_img.clone();
_cv_img = cv_img.clone(); // deep copy
}

void ImageData::shallow_copy_cv(const cv::Mat &cv_img)
{
_channels = cv_img.channels();

_height = cv_img.rows;
_width = cv_img.cols;

_cv_type = cv_img.type();

_cv_img = cv_img; // shallow copy
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a silly question but is this still thread safe? That would be hard to catch with our tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

none of this is thread safe. these changes do not affect that.

}

template <class T>
Expand Down Expand Up @@ -819,4 +844,3 @@ std::string ImageData::create_fullpath(const std::string &filename,
else
return filename + "." + ext;
}

20 changes: 13 additions & 7 deletions src/vcl/ImageData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,12 @@ namespace VCL {
ImageData();

/**
* Creates an ImageData object from the OpenCV Mat.
* Creates an ImageData object from the OpenCV Mat,
* making a deep copy.
*
* @param cv_img An OpenCV Mat that contains an image
*/
ImageData(const cv::Mat &cv_img);
ImageData(const cv::Mat &cv_img, bool copy=true);

/**
* Creates an ImageData object from the filename
Expand All @@ -400,7 +401,7 @@ namespace VCL {
*
* @param img A reference to an existing ImageData object
*/
ImageData(const ImageData &img);
ImageData(const ImageData &img, bool copy=true);

/**
* Sets an ImageData object equal to another ImageData object
Expand Down Expand Up @@ -643,18 +644,23 @@ namespace VCL {
*/
void delete_object();



private:
/* *********************** */
/* COPY FUNCTIONS */
/* *********************** */
/**
* Copies an OpenCV Mat into the ImageData OpenCV Mat
* Copies (deep copy) an OpenCV Mat into the ImageData OpenCV Mat
*
* @param cv_img An existing OpenCV Mat
*/
void deep_copy_cv(const cv::Mat &cv_img);

/**
* Copies (shallow copy) an OpenCV Mat into the ImageData OpenCV Mat
*
* @param cv_img An existing OpenCV Mat
*/
void copy_cv(const cv::Mat &cv_img);
void shallow_copy_cv(const cv::Mat &cv_img);

/**
* Copies the ImageData OpenCV Mat into a buffer
Expand Down
Loading