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

Add JPEG‑XL Support to libcupsfilters (WoC 4.0 Contribution) #82

Open
wants to merge 53 commits into
base: master
Choose a base branch
from

Conversation

TitikshaBansal
Copy link

Add JPEG‑XL Support to libcupsfilters

This pull request adds initial support for the JPEG‑XL image format to libcupsfilters. With these changes, cups‑filters can now detect, decode, and convert JPEG‑XL files into PDF and raster formats, thereby enabling printing via CUPS.

Note: This pull request does not include modifications for MIME type definitions; those changes will be submitted in a separate pull request.


Overview

JPEG‑XL Detection

  • Function Updated: is_jpegxl() in image-jpeg-xl.c. Checks for both standard JPEG‑XL signature (\x00\x00\x00\x0C\x4A\x58\x4C\x20) and the JPEG‑XL stream signature (\xFF\x0A).

Decoding Implementation

  • New Functions:
    • _cfImageReadJPEGXL()
    • _cfImageOpenJPEGXL()
  • Core Decoder Function:
    • cf_image_create_from_jxl_decoder()
      • Sets up a JxlPixelFormat requesting 16‑bit per channel output.
      • Processes the decoder input until the full image is decoded.
      • Detailed logging has been added via fprintf(stderr, ...) to log file size, status codes, and key variable values for debugging.

Build System Updates

  • Linking: The build configuration now uses pkg-config to link against libjxl.
  • Integration: The JPEG‑XL code integrates with existing filters for PDF and raster conversion.

Testing and Verification

  • Decoder Verification: The JPEG‑XL support was verified using the djxl tool and by testing conversions through cups‑filters.
  • Output Validation: Output PDF and raster files were inspected using utilities like rasterview to ensure correct color depth and page dimensions.
  • Debug Logging: Added extensive logging in the JPEG‑XL functions to assist in diagnosing any issues.

Future Work

  • Error Handling: Enhance error handling and add more comprehensive unit tests.
  • Optimizations: Further optimize performance and extend support for additional JPEG‑XL profiles.
  • MIME Type Integration: Develop and submit a separate PR for updating MIME type definitions for JPEG‑XL.

Background

This contribution was developed as part of the WoC 4.0 event organized by IIIT Kalyani. I have enjoyed working on integrating JPEG‑XL support into cups‑filters and am eager to continue contributing to this project even after the event concludes.

GitHub Repository: https://github.com/TitikshaBansal/libcupsfilters
Branch: features/jpegxl-support


Thank you for reviewing my contribution. I look forward to your feedback and further improvements.

Best regards,
Titiksha Bansal

Added an inclusion of your new header for JPEG‑XL support.
In the block that checks the header for PNG, JPEG, and TIFF, added a new branch for JPEG‑XL.
We add an #ifdef HAVE_LIBJXL branch that calls _cfImageReadJPEGXL() if is_jpegxl() returns true.
Now, whether the user is converting to PDF (via imagetopdf.c) or to Raster (via imagetoraster.c), they call cfImageOpen() (and thus cfImageOpenFP()) which now supports JPEG‑XL.
Created a header file for your JPEG‑XL functions.
Implemented the JPEG‑XL–specific functions.
In this the extra parameters (primary, secondary, saturation, hue, lut) are passed but not used because the JPEG‑XL decoder (via libjxl) handles the uncompression and preserves full quality.
Implementation for cfImageCreateFromJxlDecoder(). 
This function uses the libjxl API to extract the frame header, compute the required output buffer size (requesting 16‑bit output for full quality), allocate memory for the decoded pixels, and then fill in a cf_image_t structure with the image properties.
Reverted the changes made earlier in the file.
Reverted all the changes made earlier.
Updated file to handle the errors encountered while make.
Made changes to handle errors during make
Updated file to handle errors encountered while make
@tillkamppeter
Copy link
Member

Also do not use fprintf(stderr, ...). Please use the log function instead. See the other cupsfilters/image*.c files.

@@ -28,12 +27,14 @@ is_jpegxl(const unsigned char *header, size_t len)
}

/*
* cf_image_create_from_jxl_decoder() - Create a cf_image_t from a libjxl decoder.
* cfImageCreateFromJxlDecoder() - Create a cf_image_t from a libjxl decoder.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you changing the name in the comment?

The scheme for the name, underscore-separated lowercase is correct, as the function is static.

@@ -38,12 +38,17 @@


#include "image-private.h"


Copy link
Member

@tillkamppeter tillkamppeter Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have too many 2-line spacings. Do only 2-line spacings

  • Between the implementations of 2 functions
  • Once between header comment and first #include, between #includes and macro definitions, between macro definitions and data types, between data types and functions
  • Do not do 2-line spacings withing functions or withing data types

Especially do not change the wihitespace in files which were already there before your PR.


// Process input until full image is decoded.
/* Process input until full image is decoded. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For comments please use // ... and not /* ... */ to align with the other files of libcupsfilters (and with OpenPrinting's code style).

@tillkamppeter
Copy link
Member

Please check the file DEVELOPING.md for information about coding style. Especially do the naming of functions as follows:

### Functions

Functions with a global scope have a lowercase prefix followed by capitalized
words, e.g., `cfDoThis`, `cfDoThat`, `cfDoSomethingElse`, etc.  Private
global functions begin with a leading underscore, e.g., `_cfDoThis`,
`_cfDoThat`, etc.

Functions with a local scope are declared static with lowercase names and
underscores between words, e.g., `do_this`, `do_that`, `do_something_else`, etc.

In short:

  • Public API of libcupsfilters: cfDoThis()
  • libcupsfilters-internal function used in more than one file: _cfDoThat()
  • static function (only used in the same file): do_something_else()

@@ -1,4 +1,6 @@
ACLOCAL_AMFLAGS = -I m4
AM_CPPFLAGS = $(shell pkg-config --cflags libjxl)


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this. All needed variables are set in configure.ac. Do not call pkg-config in Makefile.am.

* JPEG‑XL stream signature.
*/
static int
_cf_is_jpegxl(const unsigned char *header, size_t len)
Copy link
Member

@tillkamppeter tillkamppeter Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used in the file image.c, therefore do not declare it as static and also name it to _cfIsJPEGXL().

Please always test-build when doing any code changes!

@tillkamppeter
Copy link
Member

What is now missing is:

  • Please check each function whether
    • it is used only in the file where it is defined/implemented: In this case declare it static and give to it an underscore-separated lowercase name. Do not declare such functions in any of the *.h files.
    • it is used only inside libcupsfilters but also in other files than the one where it got defined/implemented: Do not declare it static, name it in camelCase, starting with _cf, and declare it in a private *.h file (image-*.h, *-private.h).
    • Your work should not contain any new function for the public API, so do not name any of your functions in camelCase starting with cf.
  • Please make sure not to change the whitespace in any file which existed before your PR.

Please always test-build after each code change!!

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

Successfully merging this pull request may close these issues.

2 participants