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

Improve magickload's color handling #3625

Open
Dclipsham opened this issue Aug 25, 2023 · 6 comments
Open

Improve magickload's color handling #3625

Dclipsham opened this issue Aug 25, 2023 · 6 comments

Comments

@Dclipsham
Copy link

Dclipsham commented Aug 25, 2023

Describe the bug
The attached image (an EXIF 2.3.1 JPEG) , when attempting to transcode to JPG using:
vips magickload {inputfile} {outputfile}
Creates an output image that is unviable - everything goes grey. I have a theory this might be due to mishandling the CMYK color profile, but I'm no expert at all.

Similar results if just trying to transcode to PNG, although the output PNG file is too large to attach here

To Reproduce
Steps to reproduce the behavior:

  1. Use Image '(original_input)Apple_Group_with_Labels_V3.jpg'
  2. Use Configuration vips-dev-w64-all-8.14.2: vips magickload (original_input)Apple_Group_with_Labels_V3.jpg (jpg_output)Apple_Group_with_Labels_V3.jpg
  3. See issues in output image

Expected behavior
The image to transcode without such a stark difference in color

Actual behavior
The image transcodes with everything a deep gray hue

Screenshots
Input file, and output file attached
(original_input)Apple_Group_with_Labels_V3
(jpg_output)Apple_Group_with_Labels_V3

Environment
(please complete the following information)

  • OS: Windows 11
  • Vips: 8.14.2

Additional context
Add any other context about the problem here.

@Dclipsham Dclipsham added the bug label Aug 25, 2023
@jcupitt
Copy link
Member

jcupitt commented Aug 25, 2023

Hi @Dclipsham,

It works if you avoid imagemagick, fwiw. If I run:

$ vips colourspace Apple_Group_with_Labels_V3.jpg x.jpg srgb

I see:

x

So perhaps that's a workaround.

If I run:

$ vips magickload Apple_Group_with_Labels_V3.jpg x.v
$ vipsheader x.v
x.v: 2625x1837 uchar, 4 bands, cmyk, magickload

And then look at the x.v with nip2, it seems to be a CMYK image, but with K (black) set to 255 everywhere. I would guess there's some confusion between K and alpha at some point.

@jcupitt
Copy link
Member

jcupitt commented Aug 25, 2023

It looks like magickload is performing an approximate CMYK->RGB conversion, but tagging the result as CMYK and adding an alpha.

Could you explain why you need to use magickload? You should get better speed and accuracy with the libvips loaders.

@Dclipsham
Copy link
Author

Thank you for looking at this @jcupitt - As I understand it we're using magickload as a general purpose because of the wide variety of origin image formats we need to transcode, some of which IM has loaders for that libvips doesn't have natively

@jcupitt
Copy link
Member

jcupitt commented Aug 25, 2023

libvips has internal code for falling back to IM if there's no native loader. For example:

$ vips invert MARBLES.BMP x.jpg

Will automatically run magickload for you behind the scenes, since there's no native libvips BMP loader. You can see what libvips is doing with the VIPS_TRACE env var, for example:

$ VIPS_TRACE=1 vips invert MARBLES.BMP x.jpg
vips cache : magickload filename="MARBLES.BMP" out=((VipsImage*) 0x5581b7be46f0) flags=((VipsForeignFlags) VIPS_FOREIGN_PARTIAL) access=((VipsAccess) VIPS_ACCESS_SEQUENTIAL) -
vips cache : copy in=((VipsImage*) 0x5581b7be46f0) out=((VipsImage*) 0x5581b7bf2770) -
vips cache : invert in=((VipsImage*) 0x5581b7be46f0) out=((VipsImage*) 0x5581b7bf2550) -
vips cache+: cast in=((VipsImage*) 0x5581b7bf2550) out=((VipsImage*) 0x5581b7bf6a70) format=((VipsBandFormat) VIPS_FORMAT_UCHAR) -
vips cache : copy in=((VipsImage*) 0x5581b7bf6a70) out=((VipsImage*) 0x5581b7bf8990) -
vips cache : jpegsave_target in=((VipsImage*) 0x5581b7bf2550) target=((VipsTarget*) 0x5581b7bf4460) -

You can see it's running magickload, invert and jpegsave to execute this command, plus some copies and casts that turn into NOPs.

@jcupitt
Copy link
Member

jcupitt commented Aug 25, 2023

... the native loaders are usually faster and more memory efficient. For example, with a large JPG I see:

$ /usr/bin/time -f %M:%e vips magickload st-francis.jpg x.jpg
6317532:10.68
$ /usr/bin/time -f %M:%e vips copy st-francis.jpg x.jpg
614692:7.24

So the native loader is about 40% faster and needs 10x less memory.

@jcupitt jcupitt changed the title Poor color handling when transcoding with magickload Improve color handling with magickload Sep 11, 2023
@jcupitt jcupitt added enhancement and removed bug labels Sep 11, 2023
@jcupitt jcupitt changed the title Improve color handling with magickload Improve magickload's color handling Sep 11, 2023
@jcupitt
Copy link
Member

jcupitt commented Sep 11, 2023

I don't think there's a bug here, but we could probably improve magicload's colour handling.

Let's leave this open as an enhancement.

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