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

Ensure extractChannel sets a relevant colourspace #1257

Closed
jeremychone opened this issue Jun 8, 2018 · 13 comments
Closed

Ensure extractChannel sets a relevant colourspace #1257

jeremychone opened this issue Jun 8, 2018 · 13 comments

Comments

@jeremychone
Copy link

We have ML project, and we would like to transform a large set of image to black/white based on a colorband (the C band from the LHC) for now.

I played with the toColorband('lhc').bandbool(...) but it while the 'or' 'eor' operations seems to work, I do not see how I can go to bw base only on one band.

I assume we cloud ask the Buffer, and process it in JS, but we would like to avoid that given our volume of images to be processed.

@jeremychone
Copy link
Author

jeremychone commented Jun 8, 2018

Ok, I found it .extractChannel(0); which works.

But there is one thing I still do not understand.

const img = sharp('~data/frames/frame-00005.jpg');

const orgImg = img.clone()
await save(orgImg, 'test-_origin.jpeg');

let imgDist = img.clone().extractChannel(2);
await save(imgDist, 'test-01.jpeg');

imgDist = img.clone().toColorspace('lhc').extractChannel(2);
await save(imgDist, 'test-02.jpeg');

The test-01.jpeg and test-02.jpeg look very much the same (and even if I change the index). It feels that the toColorspace is not having an effect.

Am I missing something?

With further digging, it seems that 'lhc' is not supported (See below), and we cannot move to cmyk or multiband because of the 'no route' issue (perhaps related to #218)

From node sharp code

const colourspace = {
  multiband: 'multiband',
  'b-w': 'b-w',
  bw: 'b-w',
  cmyk: 'cmyk',
  srgb: 'srgb'
};

So, it seems there is no way, for now, to extract the C band from LHC. Let me know if I am missing something.

This lib and node-sharp's "chaining" model is amazing. We are planning to use openCV for some of the function, but would love to use node-sharp/vipslib for those kind of operations. Scale/Perform much better from node.

@lovell
Copy link
Owner

lovell commented Jun 9, 2018

Hello, does .toColorspace('lch').extractChannel(1) work? (note 'lch' rather than 'lhc')

If the result is being passed to OpenCV, it might be worth avoiding the (de)compression roundtrip by using raw output, which will produce a Buffer of raw chroma values.

clone() is only required for Stream-based input.

sharp('~data/frames/frame-00005.jpg')
  .toColorspace('lch')
  .extractChannel(1)
  .raw()
  .toBuffer()
  .then(data => { ... });

@jeremychone
Copy link
Author

First, thanks a lot for the response. Greatly appreciated.

Not sure where I got the 'lhc', the header file has it correct.

So, now, when I do

const img = sharp('~data/frames/frame-00005.jpg');

let imgDist = img.toColorspace('lch').extractChannel(1);
await save(imgDist, 'test-01.jpeg');

I get this: sRGB2scRGB: image must have at least 3 bands

The original image is color .jpg, so I assume it has the necessary bands.

@lovell
Copy link
Owner

lovell commented Jun 9, 2018

The attempt to save as a JPEG is forcing a colour conversion of the chroma (of LCH) to something suitable for JPEG output, which is failing. I suspect a code change is required to alter the "interpretation" when using extractChannel with JPEG output.

Does the same thing happen with raw output?

@jeremychone
Copy link
Author

jeremychone commented Jun 9, 2018

First, this works. So, you are right, it's the .extractChannel

let imgDist = img.toColorspace('lch')
await save(imgDist, 'test-01.jpeg'); // just a imgDist.toFile(...)

Now,

let imgDist = img.toColorspace('lch').raw().extractChannel(1);
imgDist.toBuffer();

or

let imgDist = img.raw().toColorspace('lch').extractChannel(1);
imgDist.toBuffer();

No matter where I put the raw() (after img, after toColorspace, after extractChannel) I get the same sRGB2scRGB: image must have at least 3 bands

@lovell
Copy link
Owner

lovell commented Jun 10, 2018

Thanks for checking, it looks like we'll need to modify sharp to internally reset the colourspace after extracting a single channel. I should be able to take a look at this in the next couple of weeks - happy to help with a PR too.

@lovell lovell added the bug label Jun 10, 2018
@lovell lovell changed the title question - how to save black/white based on a color band Ensure extractChannel sets a relevant colourspace Jun 10, 2018
@jeremychone
Copy link
Author

Thanks a lot, really appreciate.

I would love to be able to do a PR, but while I am pretty proficient in Java and javascript, C++ has been quite a long time for me, and while I have some interest to get back in, I would not trust my work yet.

Also, I am an expert in color, so, I might get things more confused than anything else.

@jeremychone
Copy link
Author

Btw, on a related comment above you made about clone. My current understanding if the api, is that we need to clone if we want to have different operations/output from the same source.

const img = sharp('~data/frames/frame-00005.jpg');

await save(img, 'test-_origin.jpeg');

const smallImage = img.clone().resize(512, 512);
await save(smallImage, "test-1024.jpeg");

const bwImage = img.greyscale(); // since it is last, I can change the loaded image.
await save(bwImage, "test-bw.jpeg");

Is that a correct assumption?

@lovell
Copy link
Owner

lovell commented Jun 19, 2018

Given the input is immutable, perhaps make your code a little clearer by creating new instances of sharp as required, a bit like the following:

const img = '~data/frames/frame-00005.jpg';

await save(sharp(img), 'test-_origin.jpeg');

const smallImage = sharp(img).resize(512, 512);
...

@lovell
Copy link
Owner

lovell commented Jun 19, 2018

Commit 94607b5 adds this scenario as a unit test and fixes it. This will be in v0.20.4.

@lovell lovell added this to the v0.20.4 milestone Jun 19, 2018
@jeremychone
Copy link
Author

Ha, interesting, I was planning to reuse some base transformations on a given image, this is why I thought the clone() was what I needed.

@lovell
Copy link
Owner

lovell commented Jun 20, 2018

v0.20.4 now available, thanks for reporting!

@lovell lovell closed this as completed Jun 20, 2018
@jeremychone
Copy link
Author

@lovell I have been swamped, but I will test it soon. Really appreciate the responsiveness and the quality of this lib. I will try to find time to blog about it. I really like the "instructions builder" approach.

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