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

Double allocation without reason #62

Open
9il opened this issue Oct 2, 2016 · 12 comments
Open

Double allocation without reason #62

9il opened this issue Oct 2, 2016 · 12 comments

Comments

@9il
Copy link
Member

9il commented Oct 2, 2016

as!aType.slice.asImage allocates without reason, bacause asImage allocates data anyway. asImage is uses data too. In addition, it should be something like toImage.

@9il
Copy link
Member Author

9il commented Oct 2, 2016

So, it should be just as!aType.asImage

@ljubobratovicrelja
Copy link
Member

Also, to match the Slice API, I'd like to rename the dcv.core.image.Image.asType to as. I'll deal with this in the same PR - deprecate Image.asType in favor of Image.as.

@9il
Copy link
Member Author

9il commented Oct 2, 2016

Probably it should be asSlice.

@ljubobratovicrelja
Copy link
Member

Probably it should be asSlice.

There's already Image.sliced. Image.asType (or Image.as) would change underlying data to targeted type.

@9il
Copy link
Member Author

9il commented Oct 2, 2016

There's already Image.sliced. Image.asType (or Image.as) would change underlying data to targeted type.

OK

@9il
Copy link
Member Author

9il commented Oct 2, 2016

But would be this as be lazy?

@ljubobratovicrelja
Copy link
Member

Image has no range interface, so I believe having lazy as doesn't give us much. Remember, this is only a convenience if for some reason we'd like to convert image data to another type (change bit depth) without any further processing. For extensive processing needs, we should slice it and work with Slice object.

@9il
Copy link
Member Author

9il commented Oct 2, 2016

Do we really need Image type? Why?

@ljubobratovicrelja
Copy link
Member

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
Copy link
Member Author

9il commented Oct 2, 2016

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 is 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 then hiding it in different classes implementations it is better to 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

I understand everything you're saying, and I have tried to design the API without the Image as intermediate class at first, but didn't like the way it turned out. If you're willing to make a proposition how this API should look like, please do.

Created separate issue about this, see #64

@9il
Copy link
Member Author

9il commented Oct 2, 2016

Thanks, will do

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

No branches or pull requests

2 participants