Possible bug: OpenCV Mat's inner mutability + Rust's aliasing optimization => problems?

[fzyzcjy]

trafficstars

Hi thanks for this binding! I am thinking about the following situation: We know multiple Mat can share the same underlying uchar* buffer. Therefore, we can write:

fn copy(dst: &mut [u8], src: &[u8]) {
  for i in 0..dst.len() { dst[i] = src[i]; }
}

fn main() {
  let a = Mat::zeros(...);
  let b = use_some_opencv_function_that_makes_a_and_b_share_same_underlying_buffer(a);
  copy(a.data_bytes_mut(), b.data_bytes()); // OOPS!
}

With assumptions on aliasing, copy function's dst and src must be two different memory areas, so Rust may optimize based on this assumption. But we know that is not true. So problem occurs!

related: https://doc.rust-lang.org/nomicon/aliasing.html

[twistedfall]

Yes, you're right, under such usage scenario the aliasing rules will be broken. I don't think there is a solution for that at the moment that can be applied automatically though.

[fzyzcjy]

Ah :( So what should we do?

[twistedfall]

For now, be careful when writing the code :) In the future those function that create a reference to the Mat (like Mat::roi) should be identified and marked manually and the they should return something like MatRef<'m> instead of just Mat.

[fzyzcjy]

For now, be careful when writing the code :)

Ok then...

In the future those function that create a reference to the Mat (like Mat::roi) should be identified and marked manually and the they should return something like MatRef<'m> instead of just Mat.

Sounds reasonable! This sounds really nontrivial though... We may have to look at each function to see it.

[twistedfall]

This sounds really nontrivial though...

It is nontrivial indeed

[AntoniosBarotsis]

Hello!

I've ran into a problem where a color conversion seems to be producing artifacts after the first run (the code is ran indefinitely until stopped for now) and I think it might be related to this issue.

I want to turn an image from BGR to RGB and display it in a window. Notice how I am passing img to the imshow function, NOT img_rgb which is the one that should be mutated, img is immutable.

pub fn show_in_window(img: &opencv::prelude::Mat) {
  let mut img_rgb = opencv::prelude::Mat::default();

  opencv::imgproc::cvt_color(img, &mut img_rgb, opencv::imgproc::COLOR_BGR2RGB, 0)
    .expect("BGR to RGB conversion.");

  opencv::highgui::imshow("test", &img).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");
}

I get this as a result

Commenting out the cvt_color call and keeping everything else the same shows this:

which has no artifacts.

I'm a bit out of my waters on this one but it seems to me that this is the same problem you are describing here. If that is the case, is there something I could do to fix this?

[twistedfall]

That's really weird, cvt_color should keep the source image completely unchanged. Is this the actual code you're using or is it the trimmed down version? I can't imagine this specific snippet of code to produce such results. Maybe if possible you could provide a full program and a sample image that exhibits this problem so I can run it locally and check?

[AntoniosBarotsis]

Thanks for the very swift reply! That is the whole code, the images are streamed from a ROS node with a webcam so it's not too easy to run locally.

I did figure that making a clone however might fix things and it did.

pub fn show_in_window(img: &Mat) {
  let img_clone = img.clone();
  let mut img_rgb = Mat::default();

  opencv::imgproc::cvt_color(&img_clone, &mut img_rgb, opencv::imgproc::COLOR_BGR2RGB, 0)
    .expect("BGR to RGB conversion.");

  opencv::highgui::imshow("test", &img_rgb).expect("open window");
  // opencv::highgui::imshow("test", &img_clone).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");
}

I'm not sure if this makes it more clear to you, if not I could try to get a more concrete example. Weirdly enough, this seemed fine when reading images from disc.

[AntoniosBarotsis]

Hm, looking at the source code, this indeed seems to not have a shared buffer since it's 2 separate arguments. Maybe this is unrelated then? Should I move this to a separate issue?

[twistedfall]

In this second snippet if you could show all 3 images: img, img_clone and img_rgb, are they all ok? Generally saying the broken image that you initially posted does really look like a memory access problem, but yeah, I can't imagine it happening because of cvt_color function. And additionally, the RGB2BGR conversion can actually happen in-place, so it wouldn't have produced that kind of artifacts.

[AntoniosBarotsis]

pub fn show_in_window(img: &Mat) {
  let img_clone = img.clone();
  let mut img_rgb = Mat::default();

  opencv::imgproc::cvt_color(&img_clone, &mut img_rgb, opencv::imgproc::COLOR_BGR2RGB, 0)
    .expect("BGR to RGB conversion.");

  opencv::highgui::imshow("img", &img).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");

  opencv::highgui::imshow("img_clone", &img_clone).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");
  
  opencv::highgui::imshow("img_rgb", &img_rgb).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");
}

gives:

While

pub fn show_in_window(img: &Mat) {
  let mut img_rgb = Mat::default();

  opencv::imgproc::cvt_color(&img, &mut img_rgb, opencv::imgproc::COLOR_BGR2RGB, 0)
    .expect("BGR to RGB conversion.");

  opencv::highgui::imshow("img", &img).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");
  
  opencv::highgui::imshow("img_rgb", &img_rgb).expect("open window");
  let _res = opencv::highgui::wait_key(0).expect("keep window open");
}

[twistedfall]

You're saying that image is coming from some external source and when you read the same image from the disk you can't reproduce the problem, right? If so, it looks like the source of the image is somehow problematic, so I suggest we look there.

[AntoniosBarotsis]

That could be the issue but it's weird that cloning it seems to solve the issue.

I do want to add that for me, cloning is fine as the resolution is pretty low so it shouldn't add too much overhead. At this point, I'm not too sure if this is indeed an issue with this crate since there are so many moving parts to get this specific use case to break successfully.

I think that explaining all of the involved code would just go a bit too much off-topic. I will reply here if I figure something out.

Again, really appreciate your responses and the bindings themselves!

[AntoniosBarotsis]

The image is only really passed around in 3 areas:

• Pulled from ROS
• Converted to the OpenCV format
• The snippets I pasted above

I tried doing an extra clone in all 3 of these but only the color conversion clone seemed to remove the artifacts. This is pretty elusive since I also can't see how the color change could possibly cause an issue with this. We are using version 0.76.4 of this crate (due to a dependency on cv-bridge-rs), I'm curious if something has changed/been fixed since?

[twistedfall]

The cv-bridge-rs crate uses the unsafe function new_rows_cols_with_data() to create a Mat in as_cvmat(): https://github.com/OmkarKabadagi5823/cv-bridge-rs/blob/main/src/cv_image.rs#LL198C17-L198C17 This makes the resulting Mat a reference to the data contained in CvImage. As soon as the CvImage is destroyed the memory is freed and Mat is now referring to that freed memory. The artifacts you're seeing could really likely be caused by this. I suggest you try calling a clone() right after you call as_cvmat(), it should fix the issue.

[AntoniosBarotsis]

This indeed fixes it, thanks a lot and sorry for the issue I guess since this crate wasn't at fault :)