Is cv::Mat class flawed by design?

Question

I work a lot with the OpenCV C++ interface and designed a number of classes which use Mat's as private resources.

Recently, I got concerned about the Mat class, as it always uses image data as shared resource unless I explicitly call clone. Even if I write const Mat I can't be sure that the imagedata does not get changed later from the outside.

So I need to clone to ensure encapsulation. But the problem with needing to explicitly clone a Mat is that it is often unnecessary and expensive. On the other hand I understand that the need for shared imagedata originates from roi selectors, and being able to write something like this: Mat m_small = m_big(my_roi).

My questions are:

1.) Should not the cv::Mat class be rather lazily cloned? So the user will not see Mat's as shared resource handlers from the outside. Should not the user explicitly instantiate a class called something like SharedMat when a real shared imagedata is needed?
2.) Do you have any better strategy than always cloning in case of cv::Mat as private resource for a class?

---

UPDATE: "you do not use Mat::clone() unless you plan to modify the data." (by Vadim Pisarevsky)
This idea has a problem. Consider the situation that you have this class:

class Res_handler{
public:
  const Mat emit_mat(){ return m_treasure; } // I argue you are compelled to clone here.
private:
  Mat m_treasure;
};

If you do not clone in this case you can write

Mat m_pirate = res_handler.emit_mat(); m_pirate = Scalar(0,0,0);

which causes a full black-out in the m_treasure inside the res_handler through the shared image data between m_pirate and m_treasure. :) So to avoid accidental modification of the inner m_treasure, you need to clone it.

On the other hand, this solution is also flawed:

const Mat m_pirate = res_handler.emit_mat(); 

because m_treasure can get modified too, so the content of m_pirate gets changed in the background, causing great headache to the pirate's programmer. :)

Answer 1

Yes, this is a poor design. Because Mat implements shared ownership internally, it’s not compatible with the standard way of choosing ownership policy, namely smart pointers. The basic issue is that data and ownership are orthogonal, and ought to be separated.

Because it is mutable, even a const Mat is more like a const shared_ptr<Mat>, with no way of describing that the contained Mat ought to be mutable, i.e., shared_ptr<const Mat>. This bears great similarity to the problems with final in Java, if you’re familiar.

I believe you can skirt these issues by wrapping Mat in a class that exposes the same interface as Mat, but which implements copy-on-write behaviour atop the default shared implementation.

Answer 2

[shameless ad] we now have answers.opencv.org, which is sort of StackOverflow for OpenCV-specific questions.

Now to the questions:

1. No. We do not see an efficient way to implement this. If you do, let's discuss it.

2. Yes. Do not use Mat::clone() unless you plan to modify the data. Reference counting takes care of proper deallocation of the data when it's no longer used.

Answer 3

To answer the OPs question:

Yes, most definitely! As a couple of people have pointed out: OpenCV gives you no possibility to describe a const reference to an image. This is indeed a flaw. "const cv::Mat&" is not what a c++ programmer would expect it to be and I have often found myself sprinkling clone() calls all over my code to the point where I lose the benefit of the data sharing to begin with.

To answer Vadims question of how to do it efficiently:

It is absolutely possible to do this efficiently, though not without an API change. Look how Qt abandoned the explicit sharing model it had before Qt 4(analogue to OpenCV's current model) to it's current implicit sharing (copy on write) with great success. Basically all function calls which mutate an object, or returns a reference which could later mutate the object have to "deref" it. That is make a copy if there are more than one references.

The cost of this is miniscule compared to the average cost of an image operation. It only becomes prohibitive if it has to be performed per pixel. That's why the class need to be separated into two. Much like cv::Mat and cv::Mat_. One that is responsible for the implicit sharing and copying, and one which is just a templated wrapper of a IplImage. Here is an example of what the API could look like(I have chosen overly explicit names for clarity):

// The following makes no unnecessary copies. Only a 
// couple of atomic increments and decrements.
const cv::Image img = cv::Image("lenna.bmp").toGray().brighter(0.3).inverted();

cv::Image copy(img);// Still no deep copy.

cv::ConstImageRef<char> src = img.constRef<char>();// Still no deep copy.

// This is where the copy(detach) happens.
// "img" is left untouched
cv::MutableImageRef<char> dst = copy.ref<char>();

// The following is as efficient as it has ever been.
for(int y = 0; y<dst.height(); y++)
    for(int x = 0; x<dst.width(); x++)
        dst.at(x, y) += src.at(x, y);

I realize there is too much OpenCV code floating around to make any radical changes and the window to make an API change with OpenCV 3 is closed, but I don't see why it shouldn't be possible to add a new improved interface.

Answer 4

Adding and expanding on Vadim's answer, here are some thoughts on the topic.

I have also used cv::Mat extensively, in many ways, and enjoyed its benefits.

A general truth in programming is that you have to balance different opposing needs of a projects. One of them is performance versus maintainability. And this was resolved once and for all with "Premature optimization is evil.". This approach is great, but many programmers just follow it blindly.

For image processing, performance is of paramount importance. Without it, many projects simply aren't feasible. So it is never premature to optimize when processing images. It is one of those very few fields where milliseconds count, where all you do is measured by both quality and speed. And, it may be hard to digest it if you come from C#, Java, or user interface design, but for this speed improvement, it's worth sacrificing some of the established practices of object oriented design.

If you go through the OpenCV's source code, you will see an incredible emphasis on optimization: SSE-based functions, NEON functions, pointer tricks, all kinds of algorithm curiosities, graphics processor implementations, OpenCL implementations, lookup tables, and many, many others which would be considered overkill, hard-to-mantain, or "premature optimization" in other types of projects.

A small change in your app architecture (like cv::Mat allocation strategy) can make a really huge difference when it comes to performance. Sharing an image on an embedded device, instead of cloning may make the difference between a great gadget and a dead-end proof-of-concept.

So, when Vadim said they do not see an efficient way to implement your suggested changes, he suggested that the performance penalties of those changes would not cover the benefits.

Such a project is harder to write and to maintain, but it is for good. And usually, the difficult part of an imaging project is writing the right algorithm. Encapsulating it is just the 1% work at the end.