Skip to content

add libjxl ICC and scRGB support #2815

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 28, 2022
Merged

add libjxl ICC and scRGB support #2815

merged 3 commits into from
May 28, 2022

Conversation

jcupitt
Copy link
Member

@jcupitt jcupitt commented May 21, 2022 •
edited
Loading

see #2568

@jcupitt jcupitt changed the title add ICC and scRGB support add libjxl ICC and scRGB support May 21, 2022
This was referenced May 21, 2022
Closed
Closed
@jcupitt jcupitt marked this pull request as draft May 21, 2022 12:01
@jcupitt jcupitt added this to the 8.13 milestone May 21, 2022
@jcupitt
Copy link
Member Author

jcupitt commented May 21, 2022 •
edited
Loading

There's a branch here with the start of exif and xmp support, but libjxl have not released this yet, so we should wait.

https://github.com/libvips/libvips/tree/add-libjxl-exif-xmp

@jcupitt jcupitt marked this pull request as ready for review May 21, 2022 13:03
@jcupitt
Copy link
Member Author

jcupitt commented May 23, 2022

Here's a test JPG with an ICC profile:

nina-small

And the same image converted to float JXL like this:

$ vips colourspace nina-small.jpg nina-scrgb.jxl scrgb

http://www.rollthepotato.net/~john/nina-scrgb.jxl

The output image is saved with these settings:

$ vipsheader nina-scrgb.jxl 
JxlBasicInfo:
    have_container = 0
    xsize = 1024
    ysize = 683
    bits_per_sample = 32
    exponent_bits_per_sample = 8
    intensity_target = 255
    min_nits = 0
    relative_to_max_display = 0
    linear_below = 0
    uses_original_profile = 1
    have_preview = 0
    have_animation = 0
    orientation = 1
    num_color_channels = 3
    num_extra_channels = 0
    alpha_bits = 0
    alpha_exponent_bits = 0
    alpha_premultiplied = 0
    preview.xsize = 0
    preview.ysize = 0
    animation.tps_numerator = 0
    animation.tps_denominator = 0
    animation.num_loops = 0
    animation.have_timecodes = 0
JxlPixelFormat:
    data_type = JXL_TYPE_FLOAT
    num_channels = 3
    endianness = 0
    align = 0

@f1ac
Copy link

f1ac commented May 24, 2022

scRGB/RGBE vips file w/o ICC now has correct gamma when converted.
Issue #2818 seems to be inverted, now every converted file is not XYB encoded.

I've also tried several roundtrip conversions for chromium test files from https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/web_tests/images/resources/jxl/ (jxl -> vips -> jxl):

  1. HDR file with embedded ICC, lossy conversion. Loses some metadata, but looks visually identical.
.\JxlInfo.exe .\test\pq_gradient_icc_lossless.jxl
Image Metadata:
 All default: False
 Extra fields: True
  Orientation: Identity
  Have intrinsic size: False
  Have preview: False
  Have Animation: False
 Floating point sample: False
 Bits per sample: 8
 Modular 16 bit buffer sufficient: True
 Number of extra channels: 0
 XYB encoded: False
 ColourEncoding:
  All default: False
  Embedded ICC: True
  Colour space: RGB
Tonemapping:
 All default: False
 Maximum intensity: 10000 nits
 Minimum intensity: 0 nits
 Linear mapping value is relative to max display brightness in nits: False
 Linear mapping below: 0 nits

 .\JxlInfo.exe .\test\pq_gradient_icc_lossless_converted.jxl
Image Metadata:
 All default: False
 Extra fields: False
 Floating point sample: False
 Bits per sample: 8
 Modular 16 bit buffer sufficient: True
 Number of extra channels: 0
 XYB encoded: False
 ColourEncoding:
  All default: False
  Embedded ICC: True
  Colour space: RGB
  1. HDR file with synthetic ICC, lossy conversion. It seems decoder converts synthetic JXL ICC to a real one. Loses some metadata, but looks visually identical.
 .\JxlInfo.exe .\test\pq_gradient_lossless.jxl
Image Metadata:
 All default: False
 Extra fields: True
  Orientation: Identity
  Have intrinsic size: False
  Have preview: False
  Have Animation: False
 Floating point sample: False
 Bits per sample: 8
 Modular 16 bit buffer sufficient: True
 Number of extra channels: 0
 XYB encoded: False
 ColourEncoding:
  All default: False
  Embedded ICC: False
  Colour space: RGB
  White point: D65
  Primaries: BT2100
  Have gamma: False
  Transfer function: PQ
  Rendering intent: Relative
Tonemapping:
 All default: False
 Maximum intensity: 10000 nits
 Minimum intensity: 0 nits
 Linear mapping value is relative to max display brightness in nits: False
 Linear mapping below: 0 nits

 .\JxlInfo.exe .\test\pq_gradient_lossless_converted.jxl
Image Metadata:
 All default: False
 Extra fields: False
 Floating point sample: False
 Bits per sample: 8
 Modular 16 bit buffer sufficient: True
 Number of extra channels: 0
 XYB encoded: False
 ColourEncoding:
  All default: False
  Embedded ICC: True
  Colour space: RGB

@jcupitt
Copy link
Member Author

jcupitt commented May 28, 2022

I'm still not sure if this is correct, but it's certainly better than what we had before.

I'm inclined to merge this so we can release libvips 8.13, and do another round of JXL improvements when the next version of libjxl is released. What do you think?

@f1ac
Copy link

f1ac commented May 28, 2022

Totally agree, I see no issues with this PR.

@jcupitt
Copy link
Member Author

jcupitt commented May 28, 2022

OK, let's do it! Thank you for pushing for this and for doing the testing.

@jcupitt jcupitt merged commit 6260a37 into master May 28, 2022
@richardmonette richardmonette mentioned this pull request Jun 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
8.13
Development

Successfully merging this pull request may close these issues.
2 participants
@jcupitt @f1ac