diff --git a/src/isyntax/isyntax.c b/src/isyntax/isyntax.c index 765b81b..ebd84c6 100644 --- a/src/isyntax/isyntax.c +++ b/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; } diff --git a/src/isyntax/isyntax.h b/src/isyntax/isyntax.h index 98fddb6..9b463a0 100644 --- a/src/isyntax/isyntax.h +++ b/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); diff --git a/src/isyntax/isyntax_reader.c b/src/isyntax/isyntax_reader.c index f2636be..b6cd6b2 100644 --- a/src/isyntax/isyntax_reader.c +++ b/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; } diff --git a/src/isyntax/isyntax_reader.h b/src/isyntax/isyntax_reader.h index 4ee14e7..9eccf57 100644 --- a/src/isyntax/isyntax_reader.h +++ b/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); diff --git a/src/isyntax_example.c b/src/isyntax_example.c index 5bba620..785c15d 100644 --- a/src/isyntax_example.c +++ b/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); } diff --git a/src/libisyntax.c b/src/libisyntax.c index eb90f70..b672625 100644 --- a/src/libisyntax.c +++ b/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); -} - - diff --git a/src/libisyntax.h b/src/libisyntax.h index 4fbdb0b..8f6a4d8 100644 --- a/src/libisyntax.h +++ b/src/libisyntax.h @@ -13,6 +13,7 @@ // is: libisyntax__(..), where 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);