OpenCV Mat to DynamicImage conversion

[rcastill]

I would like to be able to transform Mat to DynamicImage.

My specific use case for this functionality is using VideoCapture to get video or network stream and use it with images API.

This is more generally applicable to any use case that wants to interoperate between the 2 crates.

Draft

• Although this is a feature request, I already implemented a PoC of this in mat2image.
• Usage can be inspected in the example.
• It exposes 2 traits because of performance findings described in the README.

Is this something desirable?

Maybe improve mat2image and make it a PR to implement it into mainstream? or is it OK to live as a separate "plugin crate"?

I'd love some feedback on the implementation.

[fintelia]

I think this probably fits best as a plugin crate. But far as creating DynamicImages you should be able to simplify things. For instance, from_raw doesn't do any allocations so if the other side can produce a Vec of pixels then you can just do:

let v: Vec<u8> = ...; // Vec of pixel data in RGB order.
let img = DynamicImage::ImageRgb8(ImageBuffer::from_raw(width, height, v));

[HeroicKatora]

Re: ToImageUnsafe, what's unsafe about this? It doesn't name any invariants that the caller must uphold. Merely accessing unsafe methods internally doesn't necessitate something being unsafe itself. As long as all the required properties of all unsafe method calls are upheld the interface can be entirely safe. It seems to me that Mat::data() exists precisely so that a performant, safe encapsulation such as this matrix copy is possible. That said, I do not think it is correct because it does not follow the offset computation detailed in Mat's documentation.

The other way around also looks intriguing, finding a good way to safely expose Mat::new_size_with_data. It's tricky because it will contain a mutable pointer to another matrix without an associated lifetime. But perhaps similar to this:

struct ImageMat<'a>(Mat, PhantomData<&'a DynamicImage>);

trait AsMat: Sealed {
    fn as_mat(&mut self) -> ImageMat<'_>;
}

impl AsMat for image::RgbImage {
    fn as_mat(&mut self) -> ImageMat<'_> {
        let (rows, cols) = self.dimensions();
        let data = self.as_mut_ptr();
        let mat = Mat::new_rows_cols_with_data(rows, cols, CV_8UC3, data, Mat_AUTO_STEP);
        ImageMat(mat, PhantomData)
    }
}

//etc, additional impls for other image types

I'm still not sure if a conversion of ViewMut to Mat in a similar way would be total or fallible.

[rcastill]

@HeroicKatora it is actually much easier I'm afraid (glad).

There is data_bytes and data_bytes_mut, and they are safe.

Already experimented with it and there is no apparent performance hit.

Now my problem seems rather trivial but I'm finding many walls:

Opencv's Mat internal representation works with BGR. Since image does not provide BGR Pixel implementation it makes it difficult.

Now, I already went ahead and implemented Pixel for a custom Bgr type and it theoretically worked.

I say this because I was not able to save the buffer because my Bgr type does not implement PixelWithColorType and it can't because SealedPixelWithColorType is a private trait that is intended for downstream users not to do what I'm trying to do.

The solutions I see:

• Convert color format from BGR to RGB in opencv's Mat: should not allocate new Mat and rather do it in place to reduce overhead. At the moment I haven't been able to do this. I'll keep looking into it.
• Implement support for BGR in a fork of image (I saw a lot of references to BGR, but not the implementations I needed)

Now, what solutions am I not seeing?

[rcastill]

Just to clarify, the situation above is only a problem when trying to create an ImageBuffer using zero-copy.

This is not a problem when allocating DynamicImage and manually manipulating the pixels.

[HeroicKatora]

We can't provide a zero-copy path through the encoder in any case. Most decoders don't support the on-the-fly transpose and there is no interface to configure it. That's the reason Bgr: SealedPixelWithColorType was removed in 0.24 the first place, as silently doing the wrong thing is far less appealing than preparing the pixel matrix. In all likelihood there's a copy happening anyways to get pixels into a dense matrix layout (the 'unsafe' version in the mat2image fully ignores strides, afaik, and so do most Rust encoder libraries, so that the caller must first match the stride)—whereas most GPU buffers will require line strides padded to multiples of 256 bytes or even higher. And in that comparison the extra byteswap doesn't matter greatly anymore.

Handling image matrices in general is quite intricate, as it turns out. That's also why DynamicImage is as 'restricted' as the current implementation in both layouts and color variants, consumer complexity goes up a lot if it were more complex.