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.

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 dcv.core.image.Image #64

Open
ljubobratovicrelja opened this issue Oct 2, 2016 · 2 comments
Open

Remove dcv.core.image.Image #64

ljubobratovicrelja opened this issue Oct 2, 2016 · 2 comments

Comments

@ljubobratovicrelja
Copy link
Member

Proposition by @9il, started in #62.

Here are the relevant copy/pasted messages:

9il:

Do we really need Image type? Why?

ljubobratovicrelja:

As said in the description of the module in docs it is designed mainly to help with image I/O, but also to hold additional image metadata. Since it's data type is defined in runtime, it allows reading of unknown image format. Since Slice format is statically defined, we would have to expect certain image format when reading it, and if read image is not of expected format, we'd have to convert it. Also, Image contains additional metadata, e.g. color format (HSV, YUV, RGB etc.). And, in future Image should hold EXIF metadata.

Pipeline in DCV should be:

Image = dcv.core.image.Image
Slice = mir.ndslice.slice.Slice

LoadImage(path) --> dcv.core.image.Image
InspectAndAdoptImageFormat(Image) --> Slice
Processing(Slice) --> Slice
PackSliceToImage(Slice) --> Image
SaveImage(Image, path)

Long story short, we need image container with runtime defined data type, and additional image related metadata.

9il:

This is scripting language idioms. They are not good for D.

If you have processing, then you work with one, two, maximum three formats for processing.
They should have their own CT instantiations because performance reasons.
Then, when you want to save something, you can just call a function which accepts Slice, Metadata, and optionally RT/CT format.

The last one issue os reading. Yes, when we read something, the image format is unknown. But, as was said above, only beforehand image types are interesting. So, a user or library should define mapping, for example:

  • RT image type1 -> Alg1
  • RT image type2 -> Alg1
  • RT image type3 -> Alg2
  • Other RT image -> Error

It is not possible to eliminate this mapping. But rather hiding it in different classes implementations it is better how have an explicit way to do it and library helpers if required.

Please avoid any usage of classes (except already existing D libs, which can be replaced in future). Even async I/O can be performed without classes. D users like it because they are familiar with OOP. But this is bad practice for D. Structural programming is proper way to move forward with D.

@ljubobratovicrelja
Copy link
Member Author

ljubobratovicrelja commented Oct 2, 2016

Also, OpticalFlow and StereoMatching interfaces should be revisited here - both are in OOP design, and if Image is removed, those APIs would have to be changed. I personally really like the mixin approach @DmitryOlshansky used to design the RHT module, and would like to see it applied in other algorithm pipelines in DCV.

@timotheecour
Copy link
Collaborator

I agree with @9il , this is one of my biggest gripes with opencv: the cv::Mat all over their API's (instead of typesafe cv::Mat_<T>) has following implications:

  • unclear from function signature which element type /nb_dimensions are accepted by function
  • code will crash at runtime instead of compile time if element type /nb_dimensions mismatch
  • (maybe lesser issue) since less code is templated, results in larger libraries since types have to be compiled in

This was so bad that at google they use instead a typesafe wrapper: template<typename T, int C> class WImageC + friends, see http://physics.nyu.edu/grierlab/manuals/opencv/wimage_8hpp_source.html

We can instead have type traits if needed, eg:

void processCoolAlgo(S)(S image) if(isSlice!S);
// or using other traits if needed:
isImage2!S // could mean a slice with 2 dimensions (width, height)
isImage3!S // could mean a slice with 3 dimensions (channels, width, height)
isImage!S // could mean a slice with 2 or 3 dimensions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants