# HG changeset patch # User magedrifaat # Date 1658252706 -7200 # Node ID dbca50246dfcb7ef68bbfac80c79d198f5b82021 # Parent f8be3654caef06e6b45538e5c2c6c71449b4d75b Tiff read: changed logic to matrix operations instead of pointer math * __tiff__.cc(read_stripped_image): changed logic of reading strip data to the output matrix to rely on matrix operations instead of pointer math. diff -r f8be3654caef -r dbca50246dfc libinterp/dldfcn/__tiff__.cc --- a/libinterp/dldfcn/__tiff__.cc Sun Jul 17 17:59:38 2022 +0200 +++ b/libinterp/dldfcn/__tiff__.cc Tue Jul 19 19:45:06 2022 +0200 @@ -51,105 +51,138 @@ { typedef typename T::element_type P; - T img = T (dim_vector (image_data->height, image_data->width, - image_data->samples_per_pixel)); + T img; + + // For 1-bit and 4-bit images, each row must be byte aligned and padding + // is added to the end of the row to reach the byte mark. + // To facilitate reading the data, the matrix is defined with the padded + // size and the padding is removed at the end. + uint32_t padded_width = image_data->width; + uint8_t remove_padding = 0; + if ((image_data->bits_per_sample == 1 || image_data->bits_per_sample == 4) + && padded_width % 8 != 0) + { + padded_width += (8 - padded_width % 8) % 8; + remove_padding = 1; + } + + // The matrix dimensions are defined in the order that corresponds to + // the order of strip data read from LibTIFF. + // At the end, the matrix is permutated to the order expected by Octave + if (image_data->planar_configuration == PLANARCONFIG_CONTIG) + img = T (dim_vector (image_data->samples_per_pixel, padded_width, + image_data->height)); + else if (image_data->planar_configuration == PLANARCONFIG_SEPARATE) + img = T (dim_vector (padded_width, image_data->height, + image_data->samples_per_pixel)); + else + error ("Unsupported Planar Configuration"); + P *img_fvec = img.fortran_vec (); // Obtain the necessary data for handling the strips uint32_t strip_count = TIFFNumberOfStrips (tif); - tdata_t buf = _TIFFmalloc (TIFFStripSize (tif)); - if (! buf) - error ("Failed to allocate buffer for strip data"); - - uint32_t row_index = 0; - uint32_t row_size_in_bits = image_data->width - * image_data->bits_per_sample; - if (image_data->planar_configuration == PLANARCONFIG_CONTIG) - row_size_in_bits *= image_data->samples_per_pixel; - // According to the Tiff format specification, the row size is - // padded at least up to the next byte, so we add padding to - // complete the byte - row_size_in_bits += (8 - row_size_in_bits % 8) % 8; + + // Can't rely on StripByteCounts because in compressed images + // the byte count reflect the actual number of bytes stored + // in the file not the size of the uncompressed strip + int64_t strip_size; + uint64_t written_size = 0; + uint64_t image_size = padded_width * image_data->height + * image_data->samples_per_pixel + * sizeof (P); for (uint32_t strip = 0; strip < strip_count; strip++) { - // Can't rely on StripByteCounts because in compressed images - // the byte count reflect the actual number of bytes stored - // in the file not the size of the uncompressed strip - int64_t strip_bytes = TIFFReadEncodedStrip (tif, strip, buf, -1); - if (strip_bytes == -1) + // Read the strip data into the matrix directly + // TODO(maged): Are incorrect sized strips checked internally? + strip_size = TIFFReadEncodedStrip (tif, strip, img_fvec, -1); + + // Check if the strip read failed. + if (strip_size == -1) error ("Failed to read strip data"); - uint32_t rows_in_strip = strip_bytes * 8 / row_size_in_bits; - uint32_t plane_size = image_data->width * image_data->height; - for (uint32_t row_subindex = 0; - row_subindex < rows_in_strip; - row_subindex++) - for (uint32_t column = 0; - column < image_data->width; - column++) - { - uint32_t row_offset = row_size_in_bits / 8 - * row_subindex; - // TODO(maged): support arbitrary BitsPerSample? - if (image_data->bits_per_sample >= 8 - && image_data->bits_per_sample % 8 == 0) - { - // TODO(maged): clean up the math for both cases - if (image_data->planar_configuration == PLANARCONFIG_CONTIG) - for (uint16_t sample = 0; - sample < image_data->samples_per_pixel; sample++) - // The memory organization of fvec is inverted from - // what would be expected for a normal C-like array. - // It is treated as samples * columns * rows as - // opposed to rows * columns * samples. - img_fvec[sample * plane_size - + column * image_data->height - + row_index + row_subindex] - = ((P *)buf)[row_subindex * image_data->width - * image_data->samples_per_pixel - + column * image_data->samples_per_pixel - + sample]; - else - { - uint16_t channel_no = strip - * image_data->samples_per_pixel - / strip_count; - uint16_t corrected_row = (row_index + row_subindex) - % image_data->height; - img_fvec[channel_no * plane_size - + column * image_data->height - + corrected_row] - = ((P *)buf)[row_subindex * image_data->width - + column]; - } - } - else if (image_data->bits_per_sample == 4 - && image_data->samples_per_pixel == 1) - { - // TODO(maged): Check FillOrder for completeness - uint8_t nibble - = ((uint8_t *)buf)[row_offset + column / 2]; - // Extract the needed nibble from the byte - nibble = (column % 2 == 0? nibble >> 4: nibble) & 0x0F; - img_fvec[(row_index + row_subindex) - + column * image_data->height] = nibble; - } - else if (image_data->bits_per_sample == 1 - && image_data->samples_per_pixel == 1) - { - uint8_t byte - = ((uint8_t *)buf)[row_offset + column / 8]; - // Extract the needed bit from the byte - uint8_t bit = (byte >> (7 - column % 8)) & 0x01; - img_fvec[(row_index + row_subindex) - + column * image_data->height] = bit; - } - else - error ("Unsupported bit depth"); - } - row_index += rows_in_strip; + // Check if the size being read exceeds the bounds of the matrix + // In case of a corrupt image with more data than needed + if (written_size + strip_size > image_size) + error ("Strip data is larger than the image size"); + + if (image_data->bits_per_sample == 1) + { + if (image_data->samples_per_pixel != 1) + error ("Bi-Level images must have one channel only"); + + // The strip size is multiplied by 8 to reflect tha actual + // number of bytes written to the matrix since each byte + // in the original strip contains 8 pixels of data + strip_size *= 8; + + // Checking bounds again with the new size + if (written_size + strip_size > image_size) + error ("Strip data is larger than the image size"); + + // Iterate over the memory region backwards to expand the bits + // to their respective bytes without overwriting the read data + for (int64_t pixel = strip_size - 1; pixel >= 0; pixel--) + { + // TODO(maged): is it necessary to check FillOrder? + uint8_t bit_number = 7 - pixel % 8; + img_fvec[pixel] = (img_fvec[pixel / 8] >> bit_number) & 0x01; + } + break; + } + else if (image_data->bits_per_sample == 4) + { + if (image_data->samples_per_pixel != 1) + error ("4-bit images are only supported for grayscale"); + + // Strip size is multplied by as each byte contains 2 pixels + // and each pixel is later expanded into its own byte + strip_size *= 2; + + // Checking bounds again with the ne size + if (written_size + strip_size > image_size) + error ("Strip data is larger than the image size"); + + // Iterate over the memory region backwards to expand the nibbles + // to their respective bytes without overwriting the read data + for (int64_t pixel = strip_size - 1; pixel >= 0; pixel--) + { + uint8_t shift = pixel % 2 == 0? 4: 0; + img_fvec[pixel] = (img_fvec[pixel / 2] >> shift) & 0x0F; + } + break; + } + else if (image_data->bits_per_sample != 8 && + image_data->bits_per_sample != 16 && + image_data->bits_per_sample != 32 && + image_data->bits_per_sample != 64) + error ("Unsupported bit depth"); + + // Advance the pointer by the amount of bytes read + img_fvec = (P*)((uint8_t *)img_fvec + strip_size); + written_size += strip_size; } - _TIFFfree (buf); + + // The matrices are permutated back to the shape expected by Octave + // which is height x width x channels + Array perm (dim_vector (3, 1)); + if (image_data->planar_configuration == PLANARCONFIG_CONTIG) + { + perm(0) = 2; + perm(1) = 1; + perm(2) = 0; + } + else if (image_data->planar_configuration == PLANARCONFIG_SEPARATE) + { + perm(0) = 1; + perm(1) = 0; + perm(2) = 2; + } + + img = img.permute (perm); + + if (remove_padding) + img.resize (dim_vector (image_data->height, image_data->width)); return octave_value (img); } @@ -781,7 +814,7 @@ // Planar Configuration // SamplesPerPixel and bits_per_smaple // nargout and ycbcr - // ExtendedSamples? TransferFunction? GrayResponse? ColorMap? + // SampleFormat? ExtendedSamples? TransferFunction? GrayResponse? ColorMap? // What about floating point images? // Obtain all necessary tags