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

ndslice + color = (mir / std.experimental).image #343

Closed
9il opened this issue Sep 17, 2016 · 20 comments
Closed

ndslice + color = (mir / std.experimental).image #343

9il opened this issue Sep 17, 2016 · 20 comments

Comments

@9il
Copy link
Member

9il commented Sep 17, 2016

Slices likeSlice!(2, RGB8*) are good but they are not always optimal for image processing and computer vision algorithms because

  • they does not allow vectorization for algorithms, which work with channels separately.
  • they are not cache friendly. Because average image size varies like CPU cache sizes, cache friendly channel splitting maybe very helpful.

Future image module should contain:

  • Types like RGBSlice!(N, ubyte*), which contains 3 Slice!(N, ubyte*), one per channel.
  • Lazy conversions between images with split and packed channels. This can be done in single template, thought.
  • Vectorised conversions (if possible) between images with split channels.

See also: color package, forum thread


sizediff_t [2]strides = raw.structure.strides[0 .. 2];
strides[] /= 3;

 auto image = Slice!(2, RGB8*)(sl.shape[0 .. 2], strides, sl.ptr);

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/37962268-ndslice-color-mir-std-experimental-image?utm_campaign=plugin&utm_content=tracker%2F18251717&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F18251717&utm_medium=issues&utm_source=github).
@9il
Copy link
Member Author

9il commented Sep 17, 2016

ping @timotheecour. You may be interesting in this issue

@ljubobratovicrelja
Copy link
Member

Slices likeSlice!(2, RGB8*) are good but they are not always optimal for image processing and computer vision algorithms because

I took a quick look at the color library, and recognized troubles you state here. There's also one big issue I can see for dcv usage: since there's a lot of meta info in color structures like RGB, there can be no simple casting from ubyte[] to say RGB8[]. So, if we'd have data from external source (like in DCV we would), this means we'd have to perform an extra copy for color manipulation, right? I see no other way to iterate through raw data and use functionalities from color library without the copy penal. Also, this is truly great way to structure data for any extensive color manipulation, but for simplest color conversion, I believe it's an overkill. Please correct me if I'm saying something off here.

Types like RGBSlice!(N, ubyte_), which contains 3 Slice!(N, ubyte_), one per channel.

I've thought about this before. Processing multi-channel images as Slice!(3, ubyte*), often can be done a channel at the time. So having a structure like RGBSlice can be really useful. But personally I think it's bit of an overkill - Why having separate structure? Cannot this be done with the present ndslice API? Also, in color conversion processing channel's often rely on each other - so separating channels here would not help, I believe.

Could you note what would be the key differences between RGBSlice and the Slice structure design?

Also, imho ImageSlice would be more appropriate name - we could have image data represented in different color formats in this structure, right?

@9il
Copy link
Member Author

9il commented Sep 17, 2016

We do not need to copy data for rgb8. The same raw data is valid.

I will modify packed slices so we can use them for images without additional types

@ljubobratovicrelja
Copy link
Member

We do not need to copy data for rgb8. The same raw data is valid.

Could you give me a simple example? Sorry, I haven't figured out how to do it - docs are really poor, and there are almost no examples in the library.. :(

I will modify packed slices so we can use them for images without additional types

That's awesome - that was my thought exactly! :)

@9il
Copy link
Member Author

9il commented Sep 17, 2016

RGB image is just sequence of RGB triplets. RGB8 is just single triplet

@ljubobratovicrelja
Copy link
Member

I thought you were referring to this structure. My point was, if we have Slice!(3, ubyte*), as raw data (matrix of ubyte rgb triplets), can we get Slice!(2, RGB8*) without copy? AFAICS, we cannot.

@9il
Copy link
Member Author

9il commented Sep 17, 2016

We can. the raw data is the same.

@9il
Copy link
Member Author

9il commented Sep 17, 2016

Will check when be at home

@9il
Copy link
Member Author

9il commented Sep 18, 2016

RGB8 has sizeof equal to 3 and alignof equal to 1. It is equivalent to

struct SimpleRGB8 { ubyte r,g,b; }

@9il
Copy link
Member Author

9il commented Sep 18, 2016

@ljubobratovicrelja The conversion code should look like:

// sl is type of Slice!(3, ubyte*)
auto image = Slice!(2, RGB8*)(sl.shape[0 .. 2], sl.structure.strides[0 .. 2], sl.ptr);

@ljubobratovicrelja
Copy link
Member

auto image = Slice!(2, RGB8*)(sl.shape[0 .. 2], sl.structure.strides[0 .. 2], sl.ptr);

Aaah, ok, I got it - all the time I've been trying the same and been getting gibberish. Strides are wrong, we should divide stride values by 3:

long [2]strides = raw.structure.strides[0 .. 2];
strides[] /= 3;

 auto image = Slice!(2, RGB8*)(sl.shape[0 .. 2], strides, sl.ptr);

Now it works. Great, thanks! :)

@9il
Copy link
Member Author

9il commented Sep 18, 2016

Aaah, ok, I got it - all the time I've been trying the same and been getting gibberish. Strides are wrong, we should divide stride values by 3:

Ah, thanks!

@TurkeyMan
Copy link

TurkeyMan commented Oct 9, 2016

Your opening comment describes splitting into planes as a goal. I would suggest that goal is strictly for bonus points. Most multimedia software will not find that acceptable (ie, textures). I think image processing of normal formats should come first, and be prioritised.

@TurkeyMan
Copy link

I'm having trouble following what you're actually trying to achieve throughout this thread... you're trying to reformat the image into planes?
It occurs to me that a type-safe way to do this might be to use R8[], G8[], B8[], instead of ubyte[]. You'll be able to leverage existing conversions that way, and operations will work naturally, including things like grey-scale conversion, or even conversion to other colourspaces:

  R8[] r; G8[] g; B8[] b;
  XYZ!float[] xyz;
  foreach(i; 0..N)
    xyz[i] = cast(XYZ!float)r[i] + cast(XYZ!float)g[i] + cast(XYZ!float)b[i];

That should even be equally efficient to an RGB8 conversion since it's just a 3x3 matrix multiply, and each plane will have 2 implicit zero colums, so the total work will be identical either way.

Most such operations should work by doing the operation 3 times and summing the result, and the implied zeroes should eliminate the redundant work, breaking-even.

@TurkeyMan
Copy link

TurkeyMan commented Oct 9, 2016

Also conversions from RGB8[] -> G8[] work as you expect, selecting the proper element automatically. It might save a lot of element selection logic; consider swizzled color, BGR8[] for instance.

@9il
Copy link
Member Author

9il commented Sep 9, 2018

no plan to add the package into the main Mir repo

@9il 9il closed this as completed Sep 9, 2018
@TurkeyMan
Copy link

TurkeyMan commented Sep 9, 2018

I could add this to Mir...

Infact, Mir's better-c mission is looking better to me every day.

@9il
Copy link
Member Author

9il commented Sep 9, 2018

I could add this to Mir...

Added you to the Mir organization https://github.com/libmir/mir-image

The main repo would be split into others repositories after a while.

@TurkeyMan
Copy link

The main repo would be split into others repositories after a while.

I'm not sure what you're talking about?

@9il
Copy link
Member Author

9il commented Sep 10, 2018

The main repo would be split into others repositories after a while.

I'm not sure what you're talking about?

Sparse tensors will be reworked and moved to mir-algorithm. Hoffman should be moved to a new repository, say mir-model or existing mir-neural. The main repository mir is legacy.

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

3 participants