From 7616daf5cb2dcdd8c1ace5a9a4d5f173877dcd1a Mon Sep 17 00:00:00 2001 From: Dirk Farin Date: Tue, 22 Oct 2024 20:16:32 +0200 Subject: [PATCH] fix various minor issues --- libheif/api/libheif/heif.cc | 10 ++--- libheif/bitstream.cc | 2 +- libheif/box.cc | 8 ++-- libheif/box.h | 18 ++++---- .../codecs/uncompressed/decoder_abstract.cc | 2 +- .../decoder_component_interleave.cc | 4 +- .../uncompressed/decoder_mixed_interleave.cc | 4 +- libheif/codecs/uncompressed/unc_boxes.cc | 10 ++--- libheif/image-items/grid.cc | 5 +++ libheif/image-items/grid.h | 2 +- libheif/image-items/tiled.cc | 45 ++++++++++++++----- libheif/image-items/tiled.h | 12 +++-- libheif/image-items/unc_image.cc | 2 + libheif/pixelimage.cc | 10 +++-- tests/conversion.cc | 4 ++ 15 files changed, 88 insertions(+), 50 deletions(-) diff --git a/libheif/api/libheif/heif.cc b/libheif/api/libheif/heif.cc index 103d66eb49..9980058f77 100644 --- a/libheif/api/libheif/heif.cc +++ b/libheif/api/libheif/heif.cc @@ -3317,7 +3317,7 @@ int heif_encoder_has_default(struct heif_encoder* encoder, } -static void set_default_options(heif_encoding_options& options) +void set_default_encoding_options(heif_encoding_options& options) { options.version = 7; @@ -3368,7 +3368,7 @@ heif_encoding_options* heif_encoding_options_alloc() { auto options = new heif_encoding_options; - set_default_options(*options); + set_default_encoding_options(*options); return options; } @@ -3396,7 +3396,7 @@ struct heif_error heif_context_encode_image(struct heif_context* ctx, heif_encoding_options options; heif_color_profile_nclx nclx; - set_default_options(options); + set_default_encoding_options(options); if (input_options) { copy_options(options, *input_options); @@ -3462,7 +3462,7 @@ struct heif_error heif_context_encode_grid(struct heif_context* ctx, // TODO: Don't repeat this code from heif_context_encode_image() heif_encoding_options options; heif_color_profile_nclx nclx; - set_default_options(options); + set_default_encoding_options(options); if (input_options) { copy_options(options, *input_options); @@ -3710,7 +3710,7 @@ struct heif_error heif_context_encode_thumbnail(struct heif_context* ctx, std::shared_ptr thumbnail_image; heif_encoding_options options; - set_default_options(options); + set_default_encoding_options(options); if (input_options != nullptr) { copy_options(options, *input_options); diff --git a/libheif/bitstream.cc b/libheif/bitstream.cc index bdaa31bfa5..9063718179 100644 --- a/libheif/bitstream.cc +++ b/libheif/bitstream.cc @@ -116,7 +116,7 @@ bool StreamReader_memory::read(void* data, size_t size) bool StreamReader_memory::seek(uint64_t position) { - if (position > m_length || position < 0) + if (position > m_length) return false; m_position = position; diff --git a/libheif/box.cc b/libheif/box.cc index 930eaa6886..3a788fec6a 100644 --- a/libheif/box.cc +++ b/libheif/box.cc @@ -1051,9 +1051,9 @@ std::string Box_Error::dump(Indent& indent) const sstr << indent << '\'' << fourcc_to_string(m_box_type_with_parse_error) << "' parse error: " << m_error.message << "\n"; sstr << indent << "fatality: "; switch (m_fatality) { - case parse_error_fatality::fatal: sstr << "fatal\n"; - case parse_error_fatality::ignorable: sstr << "ignorable\n"; - case parse_error_fatality::optional: sstr << "optional\n"; + case parse_error_fatality::fatal: sstr << "fatal\n"; break; + case parse_error_fatality::ignorable: sstr << "ignorable\n"; break; + case parse_error_fatality::optional: sstr << "optional\n"; break; } return sstr.str(); @@ -3643,7 +3643,7 @@ void Box_iref::overwrite_reference(heif_item_id from_id, uint32_t type, uint32_t { for (auto& ref : m_references) { if (ref.from_item_ID == from_id && ref.header.get_short_type() == type) { - assert(reference_idx >= 0 && reference_idx < ref.to_item_ID.size()); + assert(reference_idx < ref.to_item_ID.size()); ref.to_item_ID[reference_idx] = to_item; return; diff --git a/libheif/box.h b/libheif/box.h index 1bf5e2c41a..1d08b3a493 100644 --- a/libheif/box.h +++ b/libheif/box.h @@ -1293,15 +1293,15 @@ class Box_cclv : public Box Error parse(BitstreamRange& range, const heif_security_limits*) override; private: - bool m_ccv_primaries_valid; - int32_t m_ccv_primaries_x[3]; - int32_t m_ccv_primaries_y[3]; - bool m_ccv_min_luminance_valid; - uint32_t m_ccv_min_luminance_value; - bool m_ccv_max_luminance_valid; - uint32_t m_ccv_max_luminance_value; - bool m_ccv_avg_luminance_valid; - uint32_t m_ccv_avg_luminance_value; + bool m_ccv_primaries_valid = false; + int32_t m_ccv_primaries_x[3] {}; + int32_t m_ccv_primaries_y[3] {}; + bool m_ccv_min_luminance_valid = false; + uint32_t m_ccv_min_luminance_value = 0; + bool m_ccv_max_luminance_valid = false; + uint32_t m_ccv_max_luminance_value = 0; + bool m_ccv_avg_luminance_valid = false; + uint32_t m_ccv_avg_luminance_value = 0; }; diff --git a/libheif/codecs/uncompressed/decoder_abstract.cc b/libheif/codecs/uncompressed/decoder_abstract.cc index fd647ebe81..94913a93cd 100644 --- a/libheif/codecs/uncompressed/decoder_abstract.cc +++ b/libheif/codecs/uncompressed/decoder_abstract.cc @@ -83,7 +83,7 @@ void AbstractDecoder::processComponentRow(ChannelListEntry& entry, UncompressedB void AbstractDecoder::processComponentTileSample(UncompressedBitReader& srcBits, const ChannelListEntry& entry, uint64_t dst_offset, uint32_t tile_x) { - uint64_t dst_sample_offset = tile_x * entry.bytes_per_component_sample; + uint64_t dst_sample_offset = uint64_t{tile_x} * entry.bytes_per_component_sample; int val = srcBits.get_bits(entry.bits_per_component_sample); memcpy(entry.dst_plane + dst_offset + dst_sample_offset, &val, entry.bytes_per_component_sample); } diff --git a/libheif/codecs/uncompressed/decoder_component_interleave.cc b/libheif/codecs/uncompressed/decoder_component_interleave.cc index 5c20e284c9..955469fa0e 100644 --- a/libheif/codecs/uncompressed/decoder_component_interleave.cc +++ b/libheif/codecs/uncompressed/decoder_component_interleave.cc @@ -51,7 +51,7 @@ Error ComponentInterleaveDecoder::decode_tile(const HeifContext* context, uint32_t bytes_per_tile_row = (bits_per_component * entry.tile_width + 7) / 8; skip_to_alignment(bytes_per_tile_row, m_uncC->get_row_align_size()); - uint64_t bytes_per_tile = bytes_per_tile_row * entry.tile_height; + uint64_t bytes_per_tile = uint64_t{bytes_per_tile_row} * entry.tile_height; total_tile_size += bytes_per_tile; } @@ -82,7 +82,7 @@ Error ComponentInterleaveDecoder::decode_tile(const HeifContext* context, for (uint32_t y = 0; y < entry.tile_height; y++) { srcBits.markRowStart(); if (entry.use_channel) { - uint64_t dst_row_offset = (out_y0 + y) * entry.dst_plane_stride; + uint64_t dst_row_offset = uint64_t{(out_y0 + y)} * entry.dst_plane_stride; processComponentTileRow(entry, srcBits, dst_row_offset + out_x0 * entry.bytes_per_component_sample); } else { diff --git a/libheif/codecs/uncompressed/decoder_mixed_interleave.cc b/libheif/codecs/uncompressed/decoder_mixed_interleave.cc index 4187315b67..582a9425eb 100644 --- a/libheif/codecs/uncompressed/decoder_mixed_interleave.cc +++ b/libheif/codecs/uncompressed/decoder_mixed_interleave.cc @@ -47,7 +47,7 @@ Error MixedInterleaveDecoder::decode_tile(const HeifContext* context, uint32_t bits_per_row = entry.bits_per_component_sample * entry.tile_width; bits_per_row = (bits_per_row + 7) & ~7U; // align to byte boundary - tile_size += bits_per_row / 8 * entry.tile_height; + tile_size += uint64_t{bits_per_row} / 8 * entry.tile_height; } else { uint32_t bits_per_component = entry.bits_per_component_sample; @@ -60,7 +60,7 @@ Error MixedInterleaveDecoder::decode_tile(const HeifContext* context, uint32_t bits_per_row = bits_per_component * entry.tile_width; bits_per_row = (bits_per_row + 7) & ~7U; // align to byte boundary - tile_size += bits_per_row / 8 * entry.tile_height; + tile_size += uint64_t{bits_per_row} / 8 * entry.tile_height; } } diff --git a/libheif/codecs/uncompressed/unc_boxes.cc b/libheif/codecs/uncompressed/unc_boxes.cc index 1f5346ca06..c094dfe208 100644 --- a/libheif/codecs/uncompressed/unc_boxes.cc +++ b/libheif/codecs/uncompressed/unc_boxes.cc @@ -412,10 +412,10 @@ uint64_t Box_uncC::compute_tile_data_size_bytes(uint32_t tile_width, uint32_t ti if (m_profile != 0) { switch (m_profile) { case fourcc("rgba"): - return 4 * tile_width * tile_height; + return 4 * uint64_t{tile_width} * tile_height; case fourcc("rgb3"): - return 3 * tile_width * tile_height; + return 3 * uint64_t{tile_width} * tile_height; default: assert(false); @@ -433,7 +433,7 @@ uint64_t Box_uncC::compute_tile_data_size_bytes(uint32_t tile_width, uint32_t ti bytes_per_pixel += comp.component_bit_depth / 8; } - return bytes_per_pixel * tile_width * tile_height; + return bytes_per_pixel * uint64_t{tile_width} * tile_height; } default: assert(false); @@ -713,7 +713,7 @@ Error Box_cpat::parse(BitstreamRange& range, const heif_security_limits* limits) "Maximum Bayer pattern size exceeded."}; } - m_components.resize(m_pattern_width * m_pattern_height); + m_components.resize(size_t{m_pattern_width} * m_pattern_height); for (uint16_t i = 0; i < m_pattern_height; i++) { for (uint16_t j = 0; j < m_pattern_width; j++) { @@ -747,7 +747,7 @@ Error Box_cpat::write(StreamWriter& writer) const { size_t box_start = reserve_box_header_space(writer); - if (m_pattern_width * m_pattern_height != m_components.size()) { + if (m_pattern_width * size_t{m_pattern_height} != m_components.size()) { // needs to be rectangular return {heif_error_Usage_error, heif_suberror_Invalid_parameter_value, diff --git a/libheif/image-items/grid.cc b/libheif/image-items/grid.cc index 9ccc4d2ff3..17fdd8ab5f 100644 --- a/libheif/image-items/grid.cc +++ b/libheif/image-items/grid.cc @@ -143,15 +143,20 @@ std::string ImageGrid::dump() const } +extern void set_default_encoding_options(heif_encoding_options& options); + + ImageItem_Grid::ImageItem_Grid(HeifContext* ctx) : ImageItem(ctx) { + set_default_encoding_options(m_encoding_options); } ImageItem_Grid::ImageItem_Grid(HeifContext* ctx, heif_item_id id) : ImageItem(ctx, id) { + set_default_encoding_options(m_encoding_options); } diff --git a/libheif/image-items/grid.h b/libheif/image-items/grid.h index 22f23d4d7f..a2811020b8 100644 --- a/libheif/image-items/grid.h +++ b/libheif/image-items/grid.h @@ -138,7 +138,7 @@ class ImageItem_Grid : public ImageItem const ImageGrid& get_grid_spec() const { return m_grid_spec; } - void set_grid_spec(const ImageGrid& grid) { m_grid_spec = grid; m_grid_tile_ids.resize(grid.get_rows() * grid.get_columns()); } + void set_grid_spec(const ImageGrid& grid) { m_grid_spec = grid; m_grid_tile_ids.resize(size_t{grid.get_rows()} * grid.get_columns()); } const std::vector& get_grid_tiles() const { return m_grid_tile_ids; } diff --git a/libheif/image-items/tiled.cc b/libheif/image-items/tiled.cc index e0a69eb1c1..28c4b9f413 100644 --- a/libheif/image-items/tiled.cc +++ b/libheif/image-items/tiled.cc @@ -68,6 +68,27 @@ uint32_t nTiles_v(const heif_tiled_image_parameters& params) } +void Box_tilC::init_heif_tiled_image_parameters(heif_tiled_image_parameters& params) +{ + params.version = 1; + + params.image_width = 0; + params.image_height = 0; + params.tile_width = 0; + params.tile_height = 0; + params.compression_format_fourcc = 0; + params.offset_field_length = 40; + params.size_field_length = 24; + params.number_of_extra_dimensions = 0; + + for (uint32_t& dim : params.extra_dimensions) { + dim = 0; + } + + params.tiles_are_sequential = false; +} + + void Box_tilC::derive_box_version() { set_version(1); @@ -249,7 +270,7 @@ Error Box_tilC::parse(BitstreamRange& range, const heif_security_limits* limits) } -Error TildHeader::set_parameters(const heif_tiled_image_parameters& params) +Error TiledHeader::set_parameters(const heif_tiled_image_parameters& params) { m_parameters = params; @@ -271,7 +292,7 @@ Error TildHeader::set_parameters(const heif_tiled_image_parameters& params) } -Error TildHeader::read_full_offset_table(const std::shared_ptr& file, heif_item_id tild_id, const heif_security_limits* limits) +Error TiledHeader::read_full_offset_table(const std::shared_ptr& file, heif_item_id tild_id, const heif_security_limits* limits) { auto max_tiles = heif_get_global_security_limits()->max_number_of_tiles; @@ -286,8 +307,8 @@ Error TildHeader::read_full_offset_table(const std::shared_ptr& file, } -Error TildHeader::read_offset_table_range(const std::shared_ptr& file, heif_item_id tild_id, - uint64_t start, uint64_t end) +Error TiledHeader::read_offset_table_range(const std::shared_ptr& file, heif_item_id tild_id, + uint64_t start, uint64_t end) { const Error eofError(heif_error_Invalid_input, heif_suberror_Unspecified, @@ -328,20 +349,20 @@ Error TildHeader::read_offset_table_range(const std::shared_ptr& file, } -size_t TildHeader::get_header_size() const +size_t TiledHeader::get_header_size() const { assert(m_header_size); return m_header_size; } -uint32_t TildHeader::get_offset_table_entry_size() const +uint32_t TiledHeader::get_offset_table_entry_size() const { return (m_parameters.offset_field_length + m_parameters.size_field_length) / 8; } -std::pair TildHeader::get_tile_offset_table_range_to_read(uint32_t idx, uint32_t nEntries) const +std::pair TiledHeader::get_tile_offset_table_range_to_read(uint32_t idx, uint32_t nEntries) const { uint32_t start = idx; uint32_t end = idx+1; @@ -369,9 +390,9 @@ std::pair TildHeader::get_tile_offset_table_range_to_read(ui } -void TildHeader::set_tild_tile_range(uint32_t tile_x, uint32_t tile_y, uint64_t offset, uint32_t size) +void TiledHeader::set_tild_tile_range(uint32_t tile_x, uint32_t tile_y, uint64_t offset, uint32_t size) { - uint64_t idx = tile_y * nTiles_h(m_parameters) + tile_x; + uint64_t idx = uint64_t{tile_y} * nTiles_h(m_parameters) + tile_x; m_offsets[idx].offset = offset; m_offsets[idx].size = size; } @@ -388,7 +409,7 @@ void writevec(uint8_t* data, size_t& idx, I value, int len) } -std::vector TildHeader::write_offset_table() +std::vector TiledHeader::write_offset_table() { uint64_t nTiles = number_of_tiles(m_parameters); @@ -416,7 +437,7 @@ std::vector TildHeader::write_offset_table() } -std::string TildHeader::dump() const +std::string TiledHeader::dump() const { std::stringstream sstr; @@ -529,7 +550,7 @@ ImageItem_Tiled::add_new_tiled_item(HeifContext* ctx, const heif_tiled_image_par // Create header + offset table - TildHeader tild_header; + TiledHeader tild_header; tild_header.set_parameters(*parameters); tild_header.set_compression_format(encoder->plugin->compression_format); diff --git a/libheif/image-items/tiled.h b/libheif/image-items/tiled.h index 9af358b132..0909305e29 100644 --- a/libheif/image-items/tiled.h +++ b/libheif/image-items/tiled.h @@ -51,6 +51,8 @@ class Box_tilC : public FullBox Box_tilC() { set_short_type(fourcc("tilC")); + + init_heif_tiled_image_parameters(m_parameters); } bool is_essential() const override { return true; } @@ -72,6 +74,8 @@ class Box_tilC : public FullBox private: heif_tiled_image_parameters m_parameters; + + static void init_heif_tiled_image_parameters(heif_tiled_image_parameters& params); }; @@ -79,7 +83,7 @@ class Box_tilC : public FullBox #define TILD_OFFSET_SEE_LOWER_RESOLUTION_LAYER 1 #define TILD_OFFSET_NOT_LOADED 10 -class TildHeader +class TiledHeader { public: Error set_parameters(const heif_tiled_image_parameters& params); @@ -175,9 +179,9 @@ class ImageItem_Tiled : public ImageItem // --- tild - void set_tild_header(const TildHeader& header) { m_tild_header = header; } + void set_tild_header(const TiledHeader& header) { m_tild_header = header; } - TildHeader& get_tild_header() { return m_tild_header; } + TiledHeader& get_tild_header() { return m_tild_header; } uint64_t get_next_tild_position() const { return m_next_tild_position; } @@ -188,7 +192,7 @@ class ImageItem_Tiled : public ImageItem void get_tile_size(uint32_t& w, uint32_t& h) const override; private: - TildHeader m_tild_header; + TiledHeader m_tild_header; uint64_t m_next_tild_position = 0; uint32_t mReadChunkSize_bytes = 64*1024; // 64 kiB diff --git a/libheif/image-items/unc_image.cc b/libheif/image-items/unc_image.cc index f09673ac4b..568b1fba78 100644 --- a/libheif/image-items/unc_image.cc +++ b/libheif/image-items/unc_image.cc @@ -333,6 +333,8 @@ Result> ImageItem_uncompressed::add_unci const unciHeaders& headers = *genHeadersResult; + assert(headers.uncC); + if (headers.uncC) { file->add_property(unci_id, headers.uncC, true); } diff --git a/libheif/pixelimage.cc b/libheif/pixelimage.cc index f0dc302564..b2a15bbca1 100644 --- a/libheif/pixelimage.cc +++ b/libheif/pixelimage.cc @@ -232,8 +232,6 @@ bool HeifPixelImage::add_channel(heif_channel channel, uint32_t width, uint32_t bool HeifPixelImage::ImagePlane::alloc(uint32_t width, uint32_t height, heif_channel_datatype datatype, int bit_depth, int num_interleaved_components) // heif_chroma chroma) { - assert(width >= 0); - assert(height >= 0); assert(bit_depth >= 1); assert(bit_depth <= 128); @@ -788,7 +786,9 @@ Result> HeifPixelImage::rotate_ccw(int angle_deg out_img->add_channel(channel, out_plane_width, out_plane_height, plane.m_datatype, plane.m_bit_depth); - ImagePlane& out_plane = out_img->m_planes.find(channel)->second; + auto out_plane_iter = out_img->m_planes.find(channel); + assert(out_plane_iter != out_img->m_planes.end()); + ImagePlane& out_plane = out_plane_iter->second; if (plane.m_bit_depth <= 8) { plane.rotate_ccw(angle_degrees, out_plane); @@ -994,7 +994,9 @@ Result> HeifPixelImage::crop(uint32_t left, uint plane.m_datatype, plane.m_bit_depth); - ImagePlane& out_plane = out_img->m_planes.find(channel)->second; + auto out_plane_iter = out_img->m_planes.find(channel); + assert(out_plane_iter != out_img->m_planes.end()); + ImagePlane& out_plane = out_plane_iter->second; int bytes_per_pixel = plane.get_bytes_per_pixel(); plane.crop(plane_left, plane_right, plane_top, plane_bottom, bytes_per_pixel, out_plane); diff --git a/tests/conversion.cc b/tests/conversion.cc index 905bd04c34..21368d3f18 100644 --- a/tests/conversion.cc +++ b/tests/conversion.cc @@ -122,6 +122,10 @@ double GetPsnr(const HeifPixelImage& original, const HeifPixelImage& compressed, uint32_t h = original.get_height(channel); heif_chroma chroma = original.get_chroma_format(); + if (w == 0 || h == 0) { + return 0; + } + uint32_t orig_stride; uint32_t compressed_stride; const T* orig_p = (T*)original.get_plane(channel, &orig_stride);