From fecc723c0a2238ca1b15102727c39549a4e306c2 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sun, 5 Sep 2021 12:34:48 +1200 Subject: [PATCH 1/3] Fix issue with unpacked size computation This was an issue with any compression type that encodes a single scanline per chunk, the unpacked size would end up incorrect. Fixes #1137 Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/chunk.c | 67 ++++++++++++++++---------- src/lib/OpenEXRCore/internal_structs.h | 3 +- src/lib/OpenEXRCore/parse_header.c | 33 ++++++++++--- 3 files changed, 69 insertions(+), 34 deletions(-) diff --git a/src/lib/OpenEXRCore/chunk.c b/src/lib/OpenEXRCore/chunk.c index 3fce503a88..000bd61501 100644 --- a/src/lib/OpenEXRCore/chunk.c +++ b/src/lib/OpenEXRCore/chunk.c @@ -143,6 +143,42 @@ alloc_chunk_table ( /**************************************/ +static uint64_t +compute_chunk_unpack_size ( + int y, + int width, + int height, + int lpc, + const struct _internal_exr_part* part) +{ + uint64_t unpacksize = 0; + if (part->chan_has_line_sampling || height != lpc) + { + const exr_attr_chlist_t* chanlist = part->channels->chlist; + for (int c = 0; c < chanlist->num_channels; ++c) + { + const exr_attr_chlist_entry_t* curc = (chanlist->entries + c); + uint64_t chansz = ((curc->pixel_type == EXR_PIXEL_HALF) ? 2 : 4); + chansz *= ((uint64_t) width); + if (curc->x_sampling > 1) chansz /= ((uint64_t) curc->x_sampling); + chansz *= ((uint64_t) height); + if (curc->y_sampling > 1) + { + if (height > 1) + chansz /= ((uint64_t) curc->y_sampling); + else if ((y % ((int) curc->y_sampling)) != 0) + chansz = 0; + } + unpacksize += chansz; + } + } + else + unpacksize = part->unpacked_size_per_chunk; + return unpacksize; +} + +/**************************************/ + exr_result_t exr_read_scanline_chunk_info ( exr_const_context_t ctxt, int part_index, int y, exr_chunk_info_t* cinfo) @@ -335,28 +371,8 @@ exr_read_scanline_chunk_info ( } else { - uint64_t unpacksize = 0; - if (cinfo->height == lpc) - { - unpacksize = part->unpacked_size_per_chunk; - } - else - { - const exr_attr_chlist_t* chanlist = part->channels->chlist; - for (int c = 0; c < chanlist->num_channels; ++c) - { - const exr_attr_chlist_entry_t* curc = (chanlist->entries + c); - if (curc->y_sampling > 1 || curc->x_sampling > 1) - unpacksize += - (((uint64_t) (cinfo->height / curc->y_sampling)) * - ((uint64_t) (cinfo->width / curc->x_sampling)) * - ((curc->pixel_type == EXR_PIXEL_HALF) ? 2 : 4)); - else - unpacksize += - ((uint64_t) cinfo->height) * ((uint64_t) cinfo->width) * - ((curc->pixel_type == EXR_PIXEL_HALF) ? 2 : 4); - } - } + uint64_t unpacksize = compute_chunk_unpack_size ( + y, cinfo->width, cinfo->height, lpc, part); ++rdcnt; if (data[rdcnt] < 0 || @@ -846,8 +862,8 @@ exr_read_tile_chunk_info ( levelx, levely, cidx, - (int)tdata[4], - (int)unpacksize, + (int) tdata[4], + (int) unpacksize, fsize); } cinfo->packed_size = (uint64_t) tdata[4]; @@ -1292,7 +1308,8 @@ exr_write_scanline_chunk_info ( cinfo->sample_count_table_size = 0; cinfo->data_offset = 0; cinfo->packed_size = 0; - cinfo->unpacked_size = part->unpacked_size_per_chunk; + cinfo->unpacked_size = + compute_chunk_unpack_size (y, cinfo->width, cinfo->height, lpc, part); return EXR_UNLOCK_AND_RETURN_PCTXT (EXR_ERR_SUCCESS); } diff --git a/src/lib/OpenEXRCore/internal_structs.h b/src/lib/OpenEXRCore/internal_structs.h index b4eef68628..3e02205c5b 100644 --- a/src/lib/OpenEXRCore/internal_structs.h +++ b/src/lib/OpenEXRCore/internal_structs.h @@ -97,7 +97,8 @@ struct _internal_exr_part int32_t* tile_level_tile_size_y; uint64_t unpacked_size_per_chunk; - int32_t lines_per_chunk; + int16_t lines_per_chunk; + int16_t chan_has_line_sampling; int32_t chunk_count; uint64_t chunk_table_offset; diff --git a/src/lib/OpenEXRCore/parse_header.c b/src/lib/OpenEXRCore/parse_header.c index e4996d1fb3..a96c4a729a 100644 --- a/src/lib/OpenEXRCore/parse_header.c +++ b/src/lib/OpenEXRCore/parse_header.c @@ -2073,6 +2073,7 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) const exr_attr_chlist_t* channels = curpart->channels->chlist; uint64_t unpackedsize = 0; int64_t w; + int hasLineSample = 0; w = ((int64_t) dw.max.x) - ((int64_t) dw.min.x) + 1; @@ -2118,12 +2119,20 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) cunpsz = 2; else cunpsz = 4; - unpackedsize += - (cunpsz * - (uint64_t) (((uint64_t) tiledesc->x_size + (uint64_t) xsamp - 1) / (uint64_t) xsamp) * - (uint64_t) (((uint64_t) tiledesc->y_size + (uint64_t) ysamp - 1) / (uint64_t) ysamp)); + cunpsz *= + (uint64_t) (((uint64_t) tiledesc->x_size + (uint64_t) xsamp - 1) / (uint64_t) xsamp); + if (ysamp > 1) + { + hasLineSample = 1; + cunpsz *= + (uint64_t) (((uint64_t) tiledesc->y_size + (uint64_t) ysamp - 1) / (uint64_t) ysamp); + } + else + cunpsz *= (uint64_t) tiledesc->y_size; + unpackedsize += cunpsz; } curpart->unpacked_size_per_chunk = unpackedsize; + curpart->chan_has_line_sampling = ((int16_t) hasLineSample); } else { @@ -2146,7 +2155,6 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) return -1; } - curpart->lines_per_chunk = linePerChunk; for (int c = 0; c < channels->num_channels; ++c) { int32_t xsamp = channels->entries[c].x_sampling; @@ -2156,11 +2164,20 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) cunpsz = 2; else cunpsz = 4; - unpackedsize += - (cunpsz * (uint64_t) ((w + xsamp - 1) / xsamp) * - (uint64_t) ((linePerChunk + ysamp - 1) / ysamp)); + cunpsz *= (uint64_t) (w / xsamp); + cunpsz *= ((uint64_t) linePerChunk); + if (ysamp > 1) + { + hasLineSample = 1; + if (linePerChunk > 1) + cunpsz *= (uint64_t) (linePerChunk / ysamp); + } + unpackedsize += cunpsz; } + curpart->unpacked_size_per_chunk = unpackedsize; + curpart->lines_per_chunk = ((int16_t) linePerChunk); + curpart->chan_has_line_sampling = ((int16_t) hasLineSample); /* h = max - min + 1, but to do size / divide by round, * we'd do linePerChunk - 1, so the math cancels */ From 8fb45b236c9fc5440c540de89d6d8304de4f21d9 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sun, 19 Sep 2021 11:02:02 +1200 Subject: [PATCH 2/3] Simplify type casts Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/parse_header.c | 31 +++++++++++++----------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/lib/OpenEXRCore/parse_header.c b/src/lib/OpenEXRCore/parse_header.c index a96c4a729a..7718402f67 100644 --- a/src/lib/OpenEXRCore/parse_header.c +++ b/src/lib/OpenEXRCore/parse_header.c @@ -2072,10 +2072,10 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) const exr_attr_box2i_t dw = curpart->data_window; const exr_attr_chlist_t* channels = curpart->channels->chlist; uint64_t unpackedsize = 0; - int64_t w; + uint64_t w; int hasLineSample = 0; - w = ((int64_t) dw.max.x) - ((int64_t) dw.min.x) + 1; + w = (uint64_t) (((int64_t) dw.max.x) - ((int64_t) dw.min.x) + 1); if (curpart->tiles) { @@ -2112,20 +2112,18 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) for (int c = 0; c < channels->num_channels; ++c) { - int32_t xsamp = channels->entries[c].x_sampling; - int32_t ysamp = channels->entries[c].y_sampling; + uint64_t xsamp = (uint64_t) channels->entries[c].x_sampling; + uint64_t ysamp = (uint64_t) channels->entries[c].y_sampling; uint64_t cunpsz = 0; if (channels->entries[c].pixel_type == EXR_PIXEL_HALF) cunpsz = 2; else cunpsz = 4; - cunpsz *= - (uint64_t) (((uint64_t) tiledesc->x_size + (uint64_t) xsamp - 1) / (uint64_t) xsamp); + cunpsz *= (((uint64_t) tiledesc->x_size + xsamp - 1) / xsamp); if (ysamp > 1) { hasLineSample = 1; - cunpsz *= - (uint64_t) (((uint64_t) tiledesc->y_size + (uint64_t) ysamp - 1) / (uint64_t) ysamp); + cunpsz *= (((uint64_t) tiledesc->y_size + ysamp - 1) / ysamp); } else cunpsz *= (uint64_t) tiledesc->y_size; @@ -2136,7 +2134,7 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) } else { - int linePerChunk; + uint64_t linePerChunk; switch (curpart->comp_type) { case EXR_COMPRESSION_NONE: @@ -2157,20 +2155,19 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) for (int c = 0; c < channels->num_channels; ++c) { - int32_t xsamp = channels->entries[c].x_sampling; - int32_t ysamp = channels->entries[c].y_sampling; + uint64_t xsamp = (uint64_t) channels->entries[c].x_sampling; + uint64_t ysamp = (uint64_t) channels->entries[c].y_sampling; uint64_t cunpsz = 0; if (channels->entries[c].pixel_type == EXR_PIXEL_HALF) cunpsz = 2; else cunpsz = 4; - cunpsz *= (uint64_t) (w / xsamp); - cunpsz *= ((uint64_t) linePerChunk); + cunpsz *= w / xsamp; + cunpsz *= linePerChunk; if (ysamp > 1) { hasLineSample = 1; - if (linePerChunk > 1) - cunpsz *= (uint64_t) (linePerChunk / ysamp); + if (linePerChunk > 1) cunpsz *= linePerChunk / ysamp; } unpackedsize += cunpsz; } @@ -2179,9 +2176,7 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) curpart->lines_per_chunk = ((int16_t) linePerChunk); curpart->chan_has_line_sampling = ((int16_t) hasLineSample); - /* h = max - min + 1, but to do size / divide by round, - * we'd do linePerChunk - 1, so the math cancels */ - retval = (dw.max.y - dw.min.y + linePerChunk) / linePerChunk; + retval = (int32_t) ((w + linePerChunk - 1) / linePerChunk); } return retval; } From 72413dc360f049a98c284a86aa5914ffd61aa1c0 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sun, 19 Sep 2021 12:38:30 +1200 Subject: [PATCH 3/3] Fix previous commit, over zealous cleaning of code Signed-off-by: Kimball Thurston --- src/lib/OpenEXRCore/parse_header.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/OpenEXRCore/parse_header.c b/src/lib/OpenEXRCore/parse_header.c index 7718402f67..5e0fc72a2f 100644 --- a/src/lib/OpenEXRCore/parse_header.c +++ b/src/lib/OpenEXRCore/parse_header.c @@ -2134,7 +2134,7 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) } else { - uint64_t linePerChunk; + uint64_t linePerChunk, h; switch (curpart->comp_type) { case EXR_COMPRESSION_NONE: @@ -2176,7 +2176,8 @@ internal_exr_compute_chunk_offset_size (struct _internal_exr_part* curpart) curpart->lines_per_chunk = ((int16_t) linePerChunk); curpart->chan_has_line_sampling = ((int16_t) hasLineSample); - retval = (int32_t) ((w + linePerChunk - 1) / linePerChunk); + h = (uint64_t) dw.max.y - (uint64_t) dw.min.y + 1; + retval = (int32_t) ((h + linePerChunk - 1) / linePerChunk); } return retval; }