Add pixel_format to libisyntax api. libisyntax_tile_read() now takes an external buffer.

src/isyntax/isyntax.c

@@ -47,7 +47,6 @@
 
 */
 
-
 #include "common.h"
 #include "work_queue.h"
 #include "intrinsics.h"
@@ -1342,13 +1341,12 @@ static rgba_t ycocg_to_bgr(i32 Y, i32 Co, i32 Cg) {
 	return (rgba_t){ATMOST(255, B), ATMOST(255, G), ATMOST(255, R), 255};
 }
 
-static u32* convert_ycocg_to_bgra_block(icoeff_t* Y, icoeff_t* Co, icoeff_t* Cg, i32 width, i32 height, i32 stride) {
+static void convert_ycocg_to_bgra_block(icoeff_t* Y, icoeff_t* Co, icoeff_t* Cg, i32 width, i32 height, i32 stride, u32* out_bgra) {
 	i32 first_valid_pixel = ISYNTAX_IDWT_FIRST_VALID_PIXEL;
-	u32* bgra = (u32*)malloc(width * height * sizeof(u32)); // TODO: performance: block allocator
     i32 aligned_width = (width / 8) * 8;
 
 	for (i32 y = 0; y < aligned_width; ++y) {
-		u32* dest = bgra + (y * width);
+		u32* dest = out_bgra + (y * width);
         i32 i = 0;
 #if defined(__SSE2__) && defined(__SSSE3__)
 		// Fast SIMD version (~2x faster on my system)
@@ -1412,16 +1410,14 @@ static u32* convert_ycocg_to_bgra_block(icoeff_t* Y, icoeff_t* Co, icoeff_t* Cg,
 		Co += stride;
 		Cg += stride;
 	}
-	return bgra;
 }
 
-static u32* convert_ycocg_to_rgba_block(icoeff_t* Y, icoeff_t* Co, icoeff_t* Cg, i32 width, i32 height, i32 stride) {
+static void convert_ycocg_to_rgba_block(icoeff_t* Y, icoeff_t* Co, icoeff_t* Cg, i32 width, i32 height, i32 stride, u32* out_rgba) {
     i32 first_valid_pixel = ISYNTAX_IDWT_FIRST_VALID_PIXEL;
-    u32* rgba = (u32*)malloc(width * height * sizeof(u32)); // TODO: performance: block allocator
     i32 aligned_width = (width / 8) * 8;
 
     for (i32 y = 0; y < aligned_width; ++y) {
-        u32* dest = rgba + (y * width);
+        u32* dest = out_rgba + (y * width);
         i32 i = 0;
 #if defined(__SSE2__) && defined(__SSSE3__)
         // Fast SIMD version (~2x faster on my system)
@@ -1486,7 +1482,6 @@ static u32* convert_ycocg_to_rgba_block(icoeff_t* Y, icoeff_t* Co, icoeff_t* Cg,
         Co += stride;
         Cg += stride;
     }
-    return rgba;
 }
 
 #define DEBUG_OUTPUT_IDWT_STEPS_AS_PNG 0
@@ -1879,7 +1874,9 @@ u32 isyntax_idwt_tile_for_color_channel(isyntax_t* isyntax, isyntax_image_t* wsi
 	return invalid_edges;
 }
 
-u32* isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32 tile_x, i32 tile_y, block_allocator_t* ll_coeff_block_allocator, bool decode_rgb) {
+void isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32 tile_x, i32 tile_y,
+                       block_allocator_t* ll_coeff_block_allocator,
+                       u32* out_buffer_or_null, enum isyntax_pixel_format_t pixel_format) {
 	// printf("@@@ isyntax_load_tile scale=%d tile_x=%d tile_y=%d\n", scale, tile_x, tile_y);
 	isyntax_level_t* level = wsi->levels + scale;
 	ASSERT(tile_x >= 0 && tile_x < level->width_in_tiles);
@@ -2010,15 +2007,15 @@ u32* isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32
 				console_print_error("load: scale=%d x=%d y=%d  idwt time =%g  invalid edges=%x\n", scale, tile_x, tile_y, elapsed_idwt, invalid_edges);
 				// early out
 				release_temp_memory(&temp_memory);
-				return NULL;
+				return;
 			}
 		}
 	}
 
 	tile->is_loaded = true; // Meaning: it is now safe to start loading 'child' tiles of the next level
-	if (!decode_rgb) {
+	if (out_buffer_or_null == NULL) {
 		release_temp_memory(&temp_memory); // free Y, Co and Cg
-		return NULL;
+		return;
 	}
 
 	// For the Y (luminance) color channel, we actually need the absolute value of the Y-channel wavelet coefficient.
@@ -2031,8 +2028,21 @@ u32* isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32
 	i32 tile_height = block_height * 2;
 
 	i32 valid_offset = (first_valid_pixel * idwt_stride) + first_valid_pixel;
-	u32* bgra = convert_ycocg_to_bgra_block(Y + valid_offset, Co + valid_offset, Cg + valid_offset,
-											tile_width, tile_height, idwt_stride);
+    switch (pixel_format) {
+        case LIBISYNTAX_PIXEL_FORMAT_BGRA:
+            convert_ycocg_to_bgra_block(Y + valid_offset, Co + valid_offset, Cg + valid_offset, tile_width, tile_height,
+                                        idwt_stride, out_buffer_or_null);
+            break;
+
+        case LIBISYNTAX_PIXEL_FORMAT_RGBA:
+            convert_ycocg_to_rgba_block(Y + valid_offset, Co + valid_offset, Cg + valid_offset, tile_width, tile_height,
+                                        idwt_stride, out_buffer_or_null);
+            break;
+
+        default:
+            ASSERT(!"unknown pixel format!");
+            break;
+    }
 	isyntax->total_rgb_transform_time += get_seconds_elapsed(start, get_clock());
 
 	//		float elapsed_rgb = get_seconds_elapsed(start, get_clock());
@@ -2043,7 +2053,6 @@ u32* isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32
 	}*/
 
 	release_temp_memory(&temp_memory); // free Y, Co and Cg
-	return bgra;
 }
 
 

src/isyntax/isyntax.h

@@ -33,6 +33,7 @@ extern "C" {
 
 #include "common.h"
 //#include "platform.h"
+#include "libisyntax.h"
 #include "block_allocator.h"
 #include "work_queue.h"
 
@@ -393,7 +394,8 @@ void isyntax_set_work_queue(isyntax_t* isyntax, work_queue_t* work_queue);
 bool isyntax_open(isyntax_t* isyntax, const char* filename, bool init_allocators);
 void isyntax_destroy(isyntax_t* isyntax);
 void isyntax_idwt(icoeff_t* idwt, i32 quadrant_width, i32 quadrant_height, bool output_steps_as_png, const char* png_name);
-u32* isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32 tile_x, i32 tile_y, block_allocator_t* ll_coeff_block_allocator, bool decode_rgb);
+void isyntax_load_tile(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32 tile_x, i32 tile_y, block_allocator_t* ll_coeff_block_allocator,
+                       u32* out_buffer_or_null, enum isyntax_pixel_format_t pixel_format);
 u32 isyntax_get_adjacent_tiles_mask(isyntax_level_t* level, i32 tile_x, i32 tile_y);
 u32 isyntax_get_adjacent_tiles_mask_only_existing(isyntax_level_t* level, i32 tile_x, i32 tile_y);
 u32 isyntax_idwt_tile_for_color_channel(isyntax_t* isyntax, isyntax_image_t* wsi, i32 scale, i32 tile_x, i32 tile_y, i32 color, icoeff_t* dest_buffer);

src/isyntax/isyntax_reader.c

@@ -199,35 +199,39 @@ static isyntax_tile_children_t isyntax_openslide_compute_children(isyntax_t* isy
 }
 
 
-static uint32_t* isyntax_openslide_idwt(isyntax_cache_t* cache, isyntax_t* isyntax, isyntax_tile_t* tile,
-                                        bool return_rgb) {
+static void isyntax_openslide_idwt(isyntax_cache_t* cache, isyntax_t* isyntax, isyntax_tile_t* tile,
+                                   uint32_t* pixels_buffer, enum isyntax_pixel_format_t pixel_format) {
     if (tile->tile_scale == 0) {
-        ASSERT(return_rgb); // Shouldn't be asking for idwt at level 0 if we're not going to use the result for pixels.
-        return isyntax_load_tile(isyntax, &isyntax->images[isyntax->wsi_image_index],
+        ASSERT(pixels_buffer != NULL); // Shouldn't be asking for idwt at level 0 if we're not going to use the result for pixels.
+        isyntax_load_tile(isyntax, &isyntax->images[isyntax->wsi_image_index],
                                  tile->tile_scale, tile->tile_x, tile->tile_y,
-                                 &cache->ll_coeff_block_allocator, /*decode_rgb=*/true);
+                                 &cache->ll_coeff_block_allocator,
+                                 pixels_buffer, pixel_format);
+        return;
     }
 
-    if (return_rgb) {
+    if (pixels_buffer != NULL) {
         // TODO(avirodov): if we want rgb from tile where idwt was done already, this could be cheaper if we store
         //  the lls in the tile. Currently need to recompute idwt.
-        return isyntax_load_tile(isyntax, &isyntax->images[isyntax->wsi_image_index],
-                                 tile->tile_scale, tile->tile_x, tile->tile_y,
-                                 &cache->ll_coeff_block_allocator, /*decode_rgb=*/true);
+        isyntax_load_tile(isyntax, &isyntax->images[isyntax->wsi_image_index],
+                          tile->tile_scale, tile->tile_x, tile->tile_y,
+                          &cache->ll_coeff_block_allocator,
+                          pixels_buffer, pixel_format);
+        return;
     }
 
     // If all children have ll coefficients and we don't need the rgb pixels, no need to do the idwt.
-    ASSERT(!return_rgb && tile->tile_scale > 0);
+    ASSERT(pixels_buffer == NULL && tile->tile_scale > 0);
     isyntax_tile_children_t children = isyntax_openslide_compute_children(isyntax, tile);
     if (children.child_top_left->has_ll && children.child_top_right->has_ll &&
         children.child_bottom_left->has_ll && children.child_bottom_right->has_ll) {
-        return NULL;
+        return;
     }
 
     isyntax_load_tile(isyntax, &isyntax->images[isyntax->wsi_image_index],
                       tile->tile_scale, tile->tile_x, tile->tile_y,
-                      &cache->ll_coeff_block_allocator, /*decode_rgb=*/false);
-    return NULL;
+                      &cache->ll_coeff_block_allocator,
+                      /*pixels_buffer=*/NULL, /*pixel_format=*/0);
 }
 
 static void isyntax_make_tile_lists_add_parent_to_list(isyntax_t* isyntax, isyntax_tile_t* tile,
@@ -318,7 +322,8 @@ static void isyntax_make_tile_lists_by_scale(isyntax_t* isyntax, int start_scale
     }
 }
 
-uint32_t* isyntax_read_tile_bgra(isyntax_t* isyntax, isyntax_cache_t* cache, int scale, int tile_x, int tile_y) {
+void isyntax_tile_read(isyntax_t* isyntax, isyntax_cache_t* cache, int scale, int tile_x, int tile_y,
+                       uint32_t* pixels_buffer, enum isyntax_pixel_format_t pixel_format) {
     // TODO(avirodov): more granular locking (some notes below). This will require handling overlapping work, that is
     //  thread A needing tile 123 and started to load it, and thread B needing same tile 123 and needs to wait for A.
     benaphore_lock(&cache->mutex);
@@ -328,10 +333,9 @@ uint32_t* isyntax_read_tile_bgra(isyntax_t* isyntax, isyntax_cache_t* cache, int
     isyntax_tile_t *tile = &level->tiles[level->width_in_tiles * tile_y + tile_x];
     // printf("=== isyntax_openslide_load_tile scale=%d tile_x=%d tile_y=%d\n", scale, tile_x, tile_y);
     if (!tile->exists) {
-        uint32_t* rgba = malloc(isyntax->tile_width * isyntax->tile_height * 4);
-        memset(rgba, 0xff, isyntax->tile_width * isyntax->tile_height * 4);
+        memset(pixels_buffer, 0xff, isyntax->tile_width * isyntax->tile_height * 4);
         benaphore_unlock(&cache->mutex);
-        return rgba;
+        return;
     }
 
     // Need 3 lists:
@@ -372,9 +376,9 @@ uint32_t* isyntax_read_tile_bgra(isyntax_t* isyntax, isyntax_cache_t* cache, int
     }
     for (ITERATE_TILE_LIST(tile, idwt_list)) {
         if (tile == idwt_list.tail) {
-            result = isyntax_openslide_idwt(cache, isyntax, tile, /*return_rgb=*/true);
+            isyntax_openslide_idwt(cache, isyntax, tile, pixels_buffer, pixel_format);
         } else {
-            isyntax_openslide_idwt(cache, isyntax, tile, /*return_rgb=*/false);
+            isyntax_openslide_idwt(cache, isyntax, tile, /*pixels_buffer=*/NULL, /*pixel_format=*/0);
         }
     }
 
@@ -409,5 +413,4 @@ uint32_t* isyntax_read_tile_bgra(isyntax_t* isyntax, isyntax_cache_t* cache, int
     }
 
     benaphore_unlock(&cache->mutex);
-    return result;
 }

src/isyntax/isyntax_reader.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "isyntax.h"
+#include "libisyntax.h"
 #include "benaphore.h"
 
 typedef struct isyntax_tile_list_t {
@@ -21,7 +22,9 @@ typedef struct isyntax_cache_t {
     int allocator_block_height;
 } isyntax_cache_t;
 
-uint32_t* isyntax_read_tile_bgra(isyntax_t* isyntax, isyntax_cache_t* cache, int scale, int tile_x, int tile_y);
+// TODO(avirodov): can this ever fail?
+void isyntax_tile_read(isyntax_t* isyntax, isyntax_cache_t* cache, int scale, int tile_x, int tile_y,
+                       uint32_t* pixels_buffer, enum isyntax_pixel_format_t pixel_format);
 
 void tile_list_init(isyntax_tile_list_t* list, const char* dbg_name);
 void tile_list_remove(isyntax_tile_list_t* list, isyntax_tile_t* tile);

src/isyntax_example.c

@@ -6,14 +6,13 @@
 #define STB_IMAGE_WRITE_IMPLEMENTATION
 #include "third_party/stb_image_write.h"  // for png export
 
-
+#define CHECK_LIBISYNTAX_OK(_libisyntax_call) do { \
+    isyntax_error_t result = _libisyntax_call;     \
+    assert(result == LIBISYNTAX_OK);               \
+} while(0);
 
 #define LOG_VAR(fmt, var) printf("%s: %s=" fmt "\n", __FUNCTION__, #var, var)
 
-uint32_t bgra_to_rgba(uint32_t val) {
-    return ((val & 0xff) << 16) | (val & 0x00ff00) | ((val & 0xff0000) >> 16) | (val & 0xff000000);
-}
-
 void print_isyntax_levels(isyntax_t* isyntax) {
     int wsi_image_idx = libisyntax_get_wsi_image_index(isyntax);
     LOG_VAR("%d", wsi_image_idx);
@@ -67,21 +66,19 @@ int main(int argc, char** argv) {
         LOG_VAR("%d", tile_height);
 
         isyntax_cache_t *isyntax_cache = NULL;
-        assert(libisyntax_cache_create("example cache", 2000, &isyntax_cache) == LIBISYNTAX_OK);
-        assert(libisyntax_cache_inject(isyntax_cache, isyntax) == LIBISYNTAX_OK);
+        CHECK_LIBISYNTAX_OK(libisyntax_cache_create("example cache", 2000, &isyntax_cache));
+        CHECK_LIBISYNTAX_OK(libisyntax_cache_inject(isyntax_cache, isyntax));
 
-        uint32_t *pixels = NULL;
-        assert(libisyntax_tile_read(isyntax, isyntax_cache, level, tile_x, tile_y, &pixels) == LIBISYNTAX_OK);
+        // RGBA is what stbi expects.
+        uint32_t *pixels_rgba = malloc(tile_width * tile_height * 4);
+        CHECK_LIBISYNTAX_OK(libisyntax_tile_read(isyntax, isyntax_cache, level, tile_x, tile_y,
+                                                 pixels_rgba, LIBISYNTAX_PIXEL_FORMAT_RGBA));
 
-        // convert data to the correct pixel format (bgra->rgba).
-        for (int i = 0; i < tile_height * tile_width; ++i) {
-            pixels[i] = bgra_to_rgba(pixels[i]);
-        }
         printf("Writing %s...\n", output_png);
-        stbi_write_png(output_png, tile_width, tile_height, 4, pixels, tile_width * 4);
+        stbi_write_png(output_png, tile_width, tile_height, 4, pixels_rgba, tile_width * 4);
         printf("Done writing %s.\n", output_png);
 
-        libisyntax_tile_free_pixels(pixels);
+        free(pixels_rgba);
         libisyntax_cache_destroy(isyntax_cache);
     }
 

src/libisyntax.c

@@ -303,15 +303,15 @@ void libisyntax_cache_destroy(isyntax_cache_t* isyntax_cache) {
 }
 
 isyntax_error_t libisyntax_tile_read(isyntax_t* isyntax, isyntax_cache_t* isyntax_cache,
-                                     int32_t level, int64_t tile_x, int64_t tile_y, uint32_t** out_pixels) {
+                                     int32_t level, int64_t tile_x, int64_t tile_y,
+                                     uint32_t* pixels_buffer, int32_t pixel_format) {
+    if (pixel_format <= _LIBISYNTAX_PIXEL_FORMAT_START || pixel_format >= _LIBISYNTAX_PIXEL_FORMAT_END) {
+        return LIBISYNTAX_INVALID_ARGUMENT;
+    }
+    // TODO(avirodov): additional vaidations, e.g. tile_x >= 0 && tile_x < isyntax...[level]...->width_in_tiles.
+
     // TODO(avirodov): if isyntax_cache is null, we can support using allocators that are in isyntax object,
     //  if is_init_allocators = 1 when created. Not sure is needed.
-    *out_pixels = isyntax_read_tile_bgra(isyntax, isyntax_cache, level, tile_x, tile_y);
+    isyntax_tile_read(isyntax, isyntax_cache, level, tile_x, tile_y, pixels_buffer, pixel_format);
     return LIBISYNTAX_OK;
 }
-
-void libisyntax_tile_free_pixels(uint32_t* pixels) {
-    free(pixels);
-}
-
-

src/libisyntax.h

@@ -13,6 +13,7 @@
 //   is: libisyntax_<object>_<action>(..), where <object> is omitted for the object representing the isyntax file.
 // - Functions don't check for null/out of bounds, for efficiency. (they may assert, but that is implementation detail).
 // - Booleans are represented as int32_t and prefxied with 'is_' or 'has_'.
+// - Enums are represented as int32_t and not the enum type. (enum type size forcing is C23 or C++11).
 // - Const applied to pointers is used as a signal that the object will not be modified.
 // - Prefer int even for unsigned types, see java rationale.
 
@@ -25,6 +26,13 @@ typedef int32_t isyntax_error_t;
 // One of the arguments passed to a function is invalid.
 #define LIBISYNTAX_INVALID_ARGUMENT 2
 
+enum isyntax_pixel_format_t {
+  _LIBISYNTAX_PIXEL_FORMAT_START = 0x100,
+  LIBISYNTAX_PIXEL_FORMAT_RGBA,
+  LIBISYNTAX_PIXEL_FORMAT_BGRA,
+  _LIBISYNTAX_PIXEL_FORMAT_END,
+};
+
 typedef struct isyntax_t isyntax_t;
 typedef struct isyntax_image_t isyntax_image_t;
 typedef struct isyntax_level_t isyntax_level_t;
@@ -64,12 +72,12 @@ void            libisyntax_cache_destroy(isyntax_cache_t* isyntax_cache);
 
 
 //== Tile API ==
-// Note: pixels are in BGRA layout.
-// Note: must use libisyntax_tile_free_pixels() to free out_pixels.
-// TODO(avirodov): are the pixels guaranteed to survive libisyntax_close()? Alternatively, change API to
-//  allow user to supply the buffer, as long as the user can compute the size.
+// Reads a tile into a user-supplied buffer. Buffer size should be [tile_width * tile_height * 4], as returned by
+// `libisyntax_get_tile_width()`/`libisyntax_get_tile_height()`. The caller is responsible for managing the buffer
+// allocation/deallocation.
+// pixel_format is one of isyntax_pixel_format_t.
 isyntax_error_t libisyntax_tile_read(isyntax_t* isyntax, isyntax_cache_t* isyntax_cache,
-                                     int32_t level, int64_t tile_x, int64_t tile_y, uint32_t** out_pixels);
-void            libisyntax_tile_free_pixels(uint32_t* pixels);
+                                     int32_t level, int64_t tile_x, int64_t tile_y,
+                                     uint32_t* pixels_buffer, int32_t pixel_format);