Add pixel_format to libisyntax api. libisyntax_tile_read() now takes an external buffer.#24
Add pixel_format to libisyntax api. libisyntax_tile_read() now takes an external buffer.#24
Conversation
…an external buffer. isyntax_streamer.c is not adjusted for the changes.
| @@ -70,18 +66,16 @@ int main(int argc, char** argv) { | |||
| assert(libisyntax_cache_create("example cache", 2000, &isyntax_cache) == LIBISYNTAX_OK); | |||
There was a problem hiding this comment.
This gets optimized out in Release builds (we should place the call outside the assert).
There was a problem hiding this comment.
Fixed, with CHECK_LIBISYNTAX_OK() macro that calls assert separately.
FYI the code was failing tests in release, thank you for catching this, I will test with release too.
| 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); | ||
| assert(libisyntax_tile_read(isyntax, isyntax_cache, level, tile_x, tile_y, |
There was a problem hiding this comment.
Gets optimized out in Release builds.
There was a problem hiding this comment.
Fixed, with CHECK_LIBISYNTAX_OK().
|
|
||
| */ | ||
|
|
||
|
|
There was a problem hiding this comment.
There already is ASSERT() from common.h, should we use that instead or should we replace all ASSERT() with assert()?
There was a problem hiding this comment.
Fixed to use common.h ASSERT().
(I have slight preference to standard solutions, but it is best being consistent with the existing codebase. This file uses ASSERT(), so I'll do the same).
| } 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? |
There was a problem hiding this comment.
Yes, I think so, e.g. if the decompression of the codeblocks fails (for example, if trying to decompress corrupt
or garbage codeblock). I guess we should probably detect that and escalate that up the call chain?
There was a problem hiding this comment.
I think so too. I prefer to defer this problem to when we need to virtualize the filesystem access (see openslide/openslide#437 (comment), maybe will be needed soon). Then we can figure out what errors the virtual FS can return, how to propogate them, and how to expose them here.
So for now I'll leave it as a todo.
|
PTAL again, thank you. |
2 participants
The buffer allocation is now on the caller, as discussed in #4 (comment)
I tested with the other (wrong) format in isyntax_example, it worked with colors properly switched (from quick visual examination).
Note: isyntax_streamer.c is not adjusted for the changes.