libvips · jcupitt · Sep 16, 2024 · Sep 22, 2024 · Sep 22, 2024 · Sep 25, 2024
diff --git a/ChangeLog b/ChangeLog
@@ -80,8 +80,12 @@ master
 - deprecate "cache" (use tilecache instead)
 - add tests for tokenisation
 - add "unpack_complex" option to vips_getpoint()
+- add zip level compression setting to vipsave [ruven]
 - add deflate compression level setting to tiffsave [ruven]
 - add "smart_deblock" to webpsave [goodusername123]
+- move vips_image_preeval(), vips_image_eval(), vips_image_posteval() into the
+  public API
+- jxlsave uses the new chunked API for low memory use for large images
 
 4/10/24 8.15.5
 

diff --git a/NOTES b/NOTES
@@ -0,0 +1,123 @@
+This branch adds chunked write for JXL save.
+
+It uses the old write by default, but the `chunked` flag enables a new chunked
+write path.
+
+- we should removed the old sequential write once we're done
+
+- should thread region prepare, test with
+
+	$ time vips gaussblur wtc.jpg x.jxl[chunked] 20 --vips-progress
+	real	0m42.860s
+	user	1m36.809s
+	sys	0m3.472s
+
+  so little overall threading and a lot of time is being spent in
+  single-threaded region prepare
+
+## sequential write
+
+- use data_at() to drive libvips? maybe make a region for each data_at()
+  and unref in buffer_release()
+
+- need a tilecache before it
+
+## sequential write
+
+- start sink_disc to generate scanlines, accumulate to a 2100 scanline
+  buffer
+
+	we know the first set of tiles will start at y == 0, so we don't
+	need to wait for data_at()
+VipsSemaphone tiles_available
+VipsSemaphone frame_complete
+VipsSemaphone tiles_written
+VipsSemaphone write_complete
+
+	need something like vips_semaphore_up_all()
+
+- scanlines come into the BG writer
+
+	- add to current write buffer
+
+	- have min(page_height, 2064) scanlines?
+
+- as soon as we have the next min(remaining page height, 2064) scanlines we
+  can write another line of tiles
+
+	- start a frame encode thread if we don't have one
+
+	- up on tiles_available
+
+		there can be many threads waiting in data_at(), we'll need to
+		up a lot
+
+		frame encode thread now encodes a line of tiles, then resets
+		scanline_buffer ready for the next set
+
+	- is this the final line of tiles?
+
+		- down on frame_complete ... ie. exit of the frame encode thread
+
+		- reset scanline_buffer for next frame
+
+	- frame not complete?
+
+		- down on tiles_written
+
+			data_at() has adjusted scanline_buffer ready for the
+			next set of tiles
+
+- frame encode thread:
+
+	- make the frame settings, set up stuff
+
+	- call JxlEncoderAddChunkedFrame() and block
+
+	- the call returns and the frame has been written
+
+	- signal on sem frame_complete
+
+	- return
+
+- in data_at() we need to:
+
+	- old scanlines? error
+
+	- next set of scanlines?
+
+		- down on write_complete
+
+			many data_at() will restart, we'll need a mutex
+
+			or maybe buffer_release can just up one and release a
+			single thread? that thread could move scanline_buffer,
+			then restart the rest?
+
+		- find new top of scanline_bytes ... there can be a 16 pixel
+		  overlap, and we'll need to copy some amount of pixels from
+		  the end to the start, and set scanline_y correctly
+
+		- up on tiles_written
+
+			sink_disc now fills data for the next set of tiles
+
+		- down on tiles_available
+
+		- and fall through to the case below
+
+	- in the current set of scanlines?
+
+		- add to n_threads, a counter tracking the number of threads
+		  running on this memory
+
+		- return a pointer into the memory
+
+- buffer_release
+
+	- decrement n_threads, hits zero?
+
+		- up on sem write_complete
+
+			** many threads can be blocked, we'll need to up a lot
+
diff --git a/libvips/foreign/jp2kload.c b/libvips/foreign/jp2kload.c
@@ -1068,6 +1068,9 @@ vips_foreign_load_jp2k_generate_tiled(VipsRegion *out,
 		// stop compiler warnings
 		VipsRect hit = { 0 };
 
+		// keep compilers happy
+		hit = (VipsRect) { 0 };
+
 		x = 0;
 		while (x < r->width) {
 			/* libvips tile position to opj base resolution coordinates.

diff --git a/libvips/foreign/jxlload.c b/libvips/foreign/jxlload.c
@@ -1,3 +1,3 @@
 /* load jpeg-xl
 *
 * 18/3/20
@@ -56,15 +56,12 @@
 #ifdef HAVE_LIBJXL
 
 #include <jxl/decode.h>
+#include <jxl/color_encoding.h>
 #include <jxl/thread_parallel_runner.h>
 
 #include "pforeign.h"
 
 /* TODO:
- *
- * - add metadata support
- *
- * - add animation support
  *
  * - add "shrink" option to read out 8x shrunk image?
  *
@@ -96,6 +93,7 @@
 	 */
 	JxlBasicInfo info;
 	JxlPixelFormat format;
+	JxlColorProfileTarget profile_target;
 	size_t icc_size;
 	uint8_t *icc_data;
 	size_t exif_size;
@@ -233,7 +231,8 @@
 static int
 vips_foreign_load_jxl_set_box_buffer(VipsForeignLoadJxl *jxl)
 {
-	if (!jxl->box_data || !jxl->box_size)
+	if (!jxl->box_data ||
+		!jxl->box_size)
 		return 0;
 
 	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS(jxl);
@@ -261,7 +260,8 @@
 static int
 vips_foreign_load_jxl_release_box_buffer(VipsForeignLoadJxl *jxl)
 {
-	if (!jxl->box_data || !jxl->box_size)
+	if (!jxl->box_data ||
+		!jxl->box_size)
 		return 0;
 
 	size_t remaining = JxlDecoderReleaseBoxBuffer(jxl->decoder);
@@ -296,8 +296,7 @@
 	jxl->bytes_in_buffer = bytes_read + bytes_remaining;
 
 #ifdef DEBUG_VERBOSE
-	printf("vips_foreign_load_jxl_fill_input: %zd bytes read\n",
-		bytes_read);
+	printf("vips_foreign_load_jxl_fill_input: %zd bytes read\n", bytes_read);
 #endif /*DEBUG_VERBOSE*/
 
 	return bytes_read;
@@ -386,11 +385,9 @@
 		info->exponent_bits_per_sample);
 	printf("    intensity_target = %g\n", info->intensity_target);
 	printf("    min_nits = %g\n", info->min_nits);
-	printf("    relative_to_max_display = %d\n",
-		info->relative_to_max_display);
+	printf("    relative_to_max_display = %d\n", info->relative_to_max_display);
 	printf("    linear_below = %g\n", info->linear_below);
-	printf("    uses_original_profile = %d\n",
-		info->uses_original_profile);
+	printf("    uses_original_profile = %d\n", info->uses_original_profile);
 	printf("    have_preview = %d\n", info->have_preview);
 	printf("    have_animation = %d\n", info->have_animation);
 	printf("    orientation = %d\n", info->orientation);
@@ -401,8 +398,7 @@
 	printf("    alpha_premultiplied = %d\n", info->alpha_premultiplied);
 	printf("    preview.xsize = %d\n", info->preview.xsize);
 	printf("    preview.ysize = %d\n", info->preview.ysize);
-	printf("    animation.tps_numerator = %d\n",
-		info->animation.tps_numerator);
+	printf("    animation.tps_numerator = %d\n", info->animation.tps_numerator);
 	printf("    animation.tps_denominator = %d\n",
 		info->animation.tps_denominator);
 	printf("    animation.num_loops = %d\n", info->animation.num_loops);
@@ -758,6 +754,7 @@
 		if (jxl->is_animated) {
 			int *delay = (int *) jxl->delay->data;
 
+			g_assert(jxl->delay->len >= jxl->frame_count);
 			vips_image_set_array_int(out, "delay", delay, jxl->frame_count);
 
 			/* gif uses centiseconds for delays
@@ -916,10 +913,8 @@
 			break;
 
 		case JXL_DEC_BASIC_INFO:
-			if (JxlDecoderGetBasicInfo(jxl->decoder,
-					&jxl->info)) {
-				vips_foreign_load_jxl_error(jxl,
-					"JxlDecoderGetBasicInfo");
+			if (JxlDecoderGetBasicInfo(jxl->decoder, &jxl->info)) {
+				vips_foreign_load_jxl_error(jxl, "JxlDecoderGetBasicInfo");
 				return -1;
 			}
 #ifdef DEBUG
@@ -948,11 +943,11 @@
 			break;
 
 		case JXL_DEC_COLOR_ENCODING:
+			/* FIXME ... call JxlDecoderGetColorAsEncodedProfile() as a
+			 * fallback.
+			 */
 			if (JxlDecoderGetICCProfileSize(jxl->decoder,
-#ifndef HAVE_LIBJXL_0_9
-					&jxl->format,
-#endif
-					JXL_COLOR_PROFILE_TARGET_DATA, &jxl->icc_size)) {
+					jxl->profile_target, &jxl->icc_size)) {
 				vips_foreign_load_jxl_error(jxl, "JxlDecoderGetICCProfileSize");
 				return -1;
 			}
@@ -965,15 +960,37 @@
 				return -1;
 
 			if (JxlDecoderGetColorAsICCProfile(jxl->decoder,
-#ifndef HAVE_LIBJXL_0_9
-					&jxl->format,
-#endif
 					JXL_COLOR_PROFILE_TARGET_DATA,
 					jxl->icc_data, jxl->icc_size)) {
+				vips_foreign_load_jxl_error(jxl, "JxlDecoderGetColorAsICCProfile");
+				return -1;
+			}
+
+			/* Tinkering with output transforms. Experiment with this plus
+			 *
+			 *		https://sneyers.info/hdrtest/
+			 *
+			 * and see if we can get values out of SDR range
+			 */
+
+			JxlColorEncoding enc = {
+				.color_space = JXL_COLOR_SPACE_RGB,
+				.white_point = JXL_WHITE_POINT_D65,
+				.primaries = JXL_PRIMARIES_SRGB,
+				.transfer_function = JXL_TRANSFER_FUNCTION_SRGB,
+				.rendering_intent = JXL_RENDERING_INTENT_RELATIVE,
+			};
+			if (JxlDecoderSetPreferredColorProfile(jxl->decoder, &enc)) {
+				vips_foreign_load_jxl_error(jxl,
+					"JxlDecoderSetOutputColorProfile");
+				return -1;
+			}
+			if (JxlDecoderSetDesiredIntensityTarget(jxl->decoder, 10000)) {
 				vips_foreign_load_jxl_error(jxl,
-					"JxlDecoderGetColorAsICCProfile");
+					"JxlDecoderSetDesiredIntensityTarget");
 				return -1;
 			}
+
 			break;
 
 		case JXL_DEC_FRAME:
@@ -1001,6 +1018,12 @@
 
 			jxl->frame_count++;
 
+			/* This is the last frame, we can stop right here.
+			 */
+			if (h.is_last ||
+				!jxl->info.have_animation)
+				status = JXL_DEC_SUCCESS;
+
 			break;
 
 		default:
@@ -1059,16 +1082,14 @@
 
 	JxlDecoderRewind(jxl->decoder);
 	if (JxlDecoderSubscribeEvents(jxl->decoder,
-				JXL_DEC_FRAME | JXL_DEC_FULL_IMAGE)) {
-		vips_foreign_load_jxl_error(jxl,
-			"JxlDecoderSubscribeEvents");
+			JXL_DEC_FRAME | JXL_DEC_FULL_IMAGE)) {
+		vips_foreign_load_jxl_error(jxl, "JxlDecoderSubscribeEvents");
 		return -1;
 	}
 
 	if (vips_foreign_load_jxl_fill_input(jxl, 0) < 0)
 		return -1;
-	JxlDecoderSetInput(jxl->decoder,
-		jxl->input_buffer, jxl->bytes_in_buffer);
+	JxlDecoderSetInput(jxl->decoder, jxl->input_buffer, jxl->bytes_in_buffer);
 
 	if (jxl->n > 1) {
 		if (vips_image_generate(t[0],
@@ -1170,10 +1191,8 @@
 		!(jxl->source = vips_source_new_from_file(file->filename)))
 		return -1;
 
-	if (VIPS_OBJECT_CLASS(vips_foreign_load_jxl_file_parent_class)->build(object))
-		return -1;
-
-	return 0;
+	return VIPS_OBJECT_CLASS(vips_foreign_load_jxl_file_parent_class)->
+		build(object);
 }
 
 const char *vips__jxl_suffs[] = { ".jxl", NULL };
@@ -1241,19 +1260,15 @@
 vips_foreign_load_jxl_buffer_build(VipsObject *object)
 {
 	VipsForeignLoadJxl *jxl = (VipsForeignLoadJxl *) object;
-	VipsForeignLoadJxlBuffer *buffer =
-		(VipsForeignLoadJxlBuffer *) object;
+	VipsForeignLoadJxlBuffer *buffer = (VipsForeignLoadJxlBuffer *) object;
 
 	if (buffer->buf)
 		if (!(jxl->source = vips_source_new_from_memory(
-				  VIPS_AREA(buffer->buf)->data,
-				  VIPS_AREA(buffer->buf)->length)))
+			VIPS_AREA(buffer->buf)->data, VIPS_AREA(buffer->buf)->length)))
 			return -1;
 
-	if (VIPS_OBJECT_CLASS(vips_foreign_load_jxl_file_parent_class)->build(object))
-		return -1;
-
-	return 0;
+	return VIPS_OBJECT_CLASS(vips_foreign_load_jxl_file_parent_class)->
+		build(object);
 }
 
 static gboolean
@@ -1324,11 +1339,8 @@
 		g_object_ref(jxl->source);
 	}
 
-	if (VIPS_OBJECT_CLASS(vips_foreign_load_jxl_source_parent_class)
-			->build(object))
-		return -1;
-
-	return 0;
+	return VIPS_OBJECT_CLASS(vips_foreign_load_jxl_source_parent_class)->
+		build(object);
 }
 
 static void