nebgnahz / cv-rs

Rust wrapper for OpenCV (manual at this point)
https://nebgnahz.github.io/cv-rs/cv/
MIT License
203 stars 41 forks source link

How to use "flags" in Mat::image_encode? #95

Closed marhkb closed 5 years ago

marhkb commented 5 years ago

Here is an example of compressing a jpeg image https://stackoverflow.com/questions/40768621/python-opencv-jpeg-compression-in-memory

But Mat::image_encode only accepts a Vec of ImageWriteMode. So I cannot set a value for ImageWriteMode::JpegQuality

How can I use the method to set values? Do I miss something?

Pzixel commented 5 years ago

I'm not sure what's 90 stands for.

If you just want to pass ImageWriteMode::JpegQuality just use vec![ImageWriteMode::JpegQuality].

marhkb commented 5 years ago

This is the encoding quality. I might be wrong but I think that one must pass key-value pairs as flags. ImageWriteMode::JpegQuality is the key and 90 is the value. The value for ImageWriteMode::JpegQuality is in range from 0 to 100

EDIT: With your example I am not able to tell Mat::image_encode in which quality I want to encode the jpeg

Pzixel commented 5 years ago

It's not really useful to check python code because it may differ a lot.

It's better to find some C++ samples. I look at documentation

Format-specific save parameters encoded as pairs paramId_1, paramValue_1, paramId_2, paramValue_2, ... . The following parameters are currently supported:

For JPEG, it can be a quality ( CV_IMWRITE_JPEG_QUALITY ) from 0 to 100 (the higher is the better). Default value is 95. For WEBP, it can be a quality ( CV_IMWRITE_WEBP_QUALITY ) from 1 to 100 (the higher is the better). By default (without any parameter) and for quality above 100 the lossless compression is used. For PNG, it can be the compression level ( CV_IMWRITE_PNG_COMPRESSION ) from 0 to 9. A higher value means a smaller size and longer compression time. Default value is 3. For PPM, PGM, or PBM, it can be a binary format flag ( CV_IMWRITE_PXM_BINARY ), 0 or 1. Default value is 1.

It seems that current implementation is buggy and doesn't allow to pass non-default parameters.

So interface should be changed to Vec<(ImageWriteMode, i32)> with respective implementation change.

marhkb commented 5 years ago

This is exactly what I mean. :-) Another thing is: Can the method be changed to something like

pub fn image_encode<T>(&self, ext: &str, flags: &T) -> Result<Vec<u8>, Error> 
    where T: AsRef<[(ImageWriteMode, i32)]> + ?Sized
{
...
}

The reason is that I decode a video file and compressing each frame to a jpeg. So there is no need to allocate a new Vec on each frame.

Pzixel commented 5 years ago

As far as I understand the documentation, you may be able to pass more than one parameter, so you probably would like to write AsRef<[(ImageWriteMode, i32)]>. However, you will need to allocate vec anyway to create flat Vec<i32>. It's too ugly to use it as API so you will have to convert [(ImageWriteMode, i32)] -> Vec<i32> anyway.

You probably may abuse fact that tuple is very simple and treat it as x2 length of ints (of course, if you set repr(i32) on enum), but it's a bit hacky.

marhkb commented 5 years ago

Why not use a builder pattern for that?

struct Flags {
    key_values: Vec<i32>
}
impl Flags {
    fn new(key_values: Vec<i32>) -> Self { Self { key_values } }
}
impl AsRef<[i32]> for Flags {
    fn as_ref(&self) -> &[i32] {
        self.key_values.as_ref()
    }
}

struct FlagsBuilder {
    flags: Vec<i32>
}
impl FlagsBuilder {
    fn add(&mut self, key: ImageWriteMode, val: i32) -> &mut Self {
        self.flags.push(unsafe { ::std::mem::transmute(key) });
        self.flags.push(val);
        self
    }
    fn build(self) -> Flags { Flags::new(self.flags) }
}

Flags could then be passed to Mat::image_encode

EDIT: build method

Pzixel commented 5 years ago
  1. No, it won't be useful. You can always alloc 2-element array if you wish.
  2. You can mix different flags, for example use IMWRITE_JPEG_QUALITY and IMWRITE_JPEG_CHROMA_QUALITY simultaneously.
marhkb commented 5 years ago
  1. but can you pass that to the ffi code without modification?
  2. You can already do it with the current implementation and with the native C++ code. But I dont understand why this is an issue. Do these both two flags conflict?
Pzixel commented 5 years ago
  1. You have hacky solution that allows to do it alloc-less or straightforward solution that has an allocation
  2. They don't conflict.
marhkb commented 5 years ago

I somehow do not get your points and I am not sure if explained correctly what I mean.

 let flags = FlagBuilder::new()
    .add(ImageWriteMode::JpegQuality, 90)
    .add(ImageWriteMode::JpegChromaQuality, 20)
    ... // ad more flags if needed
    build()
mat.image_encode("jpg", &flags)

Isn't that exactly that what we want?

What would be your proposed solution?

Pzixel commented 5 years ago

Being said, I see it something like:

    pub fn image_encode(&self, ext: &str, flags: Vec<(ImageWriteMode, i32)>) -> Result<Vec<u8>, Error> {
        let ext = CString::new(ext)?;
        unsafe {
            let mut result: COption<CVec<u8>> = mem::zeroed();
            cv_imencode(ext.into_raw(), self.inner, flags.as_ptr(), flags.len()*2, &mut result);
            if result.has_value {
                Ok(result.value.unpack())
            } else {
                Err(CvError::UnknownError("Unable to convert this image to bytes".into()).into())
            }
        }
    }

so you just do

mat.image_encode(".jpg", vec![(ImageWriteMode::JpegQuality, 90) , (ImageWriteMode::JpegChromaQuality, 20)]

There could be optimization with AsRef<[T]> instead of Vec, but it's beside the main point.