Mercurial > octave-libtiff
changeset 31168:27ed758c1688
Tiff setTag: fixed bug for rational tags and special-case tags.
| author | magedrifaat <magedrifaat@gmail.com> |
|---|---|
| date | Fri, 12 Aug 2022 21:50:43 +0200 |
| parents | f91cd5ceaae6 |
| children | ae41e14bf5c7 |
| files | libinterp/corefcn/__tiff__.cc |
| diffstat | 1 files changed, 113 insertions(+), 68 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/__tiff__.cc Fri Aug 12 14:36:48 2022 +0200 +++ b/libinterp/corefcn/__tiff__.cc Fri Aug 12 21:50:43 2022 +0200 @@ -739,7 +739,7 @@ } case TIFF_RATIONAL: { - error ("TIFF_RATIONAL should have at least 2 elements but got only 1"); + retval = *(reinterpret_cast<float *> (data)); break; } case TIFF_SBYTE: @@ -774,7 +774,7 @@ } case TIFF_SRATIONAL: { - error ("TIFF_SRATIONAL should have at least 2 elements but got only 1"); + retval = *(reinterpret_cast<float *> (data)); break; } case TIFF_IFD: @@ -854,13 +854,10 @@ } case TIFF_RATIONAL: { - // TODO(maged): This is incorrect, LibTIFF already handles - // rationals and converts them to floats NDArray arr (arr_dims); - for (uint32_t i = 0; i < count; i+=2) + for (uint32_t i = 0; i < count; i++) { - arr(i / 2) = static_cast<float> ((reinterpret_cast<uint32_t *> (data))[i]) - / static_cast<float> ((reinterpret_cast<uint32_t *> (data))[i+1]); + arr(i) = (reinterpret_cast<float *> (data))[i]; } retval = octave_value (arr); break; @@ -928,10 +925,9 @@ case TIFF_SRATIONAL: { NDArray arr (arr_dims); - for (uint32_t i = 0; i < count; i+=2) + for (uint32_t i = 0; i < count; i++) { - arr(i / 2) = static_cast<float> ((reinterpret_cast<int32_t *> (data))[i]) - / static_cast<float> ((reinterpret_cast<int32_t *> (data))[i+1]); + arr(i) = (reinterpret_cast<float *> (data))[i]; } retval = octave_value (arr); break; @@ -1082,17 +1078,13 @@ uint32_t count = 1 << bits_per_sample; uint16_t *ch1, *ch2, *ch3; - if (samples_per_pixel == 1) - { - validate_tiff_get_field (TIFFGetField (tif, TIFFTAG_COLORMAP, &ch1)); - tag_data_ov = interpret_tag_data (ch1, count, - TIFFFieldDataType (fip)); - } + validate_tiff_get_field (TIFFGetField (tif, tag_id, + &ch1, &ch2, &ch3)); + if (ch2 == NULL) + tag_data_ov = interpret_tag_data (ch1, count, + TIFFFieldDataType (fip)); else - { - validate_tiff_get_field (TIFFGetField (tif, TIFFTAG_COLORMAP, - &ch1, &ch2, &ch3)); - + { OCTAVE_LOCAL_BUFFER (uint16NDArray, array_list, 3); dim_vector col_dims(count, 1); array_list[0] = interpret_tag_data (ch1, @@ -1189,12 +1181,15 @@ if (TIFFFieldPassCount (fip)) error ("Unsupported tag"); + // According to matlab, the value must be a scalar double + if (! tag_ov.is_scalar_type () || ! tag_ov.is_double_type ()) + error ("Tag value must be a scalar double"); + TIFFDataType tag_datatype = TIFFFieldDataType (fip); switch (tag_datatype) { case TIFF_BYTE: case TIFF_UNDEFINED: - // TODO(maged): check if matlab errors for long data type/range TIFFSetField (tif, tag_id, tag_ov.uint8_scalar_value ()); break; case TIFF_ASCII: @@ -1242,6 +1237,16 @@ } } + template <typename T> + void + set_array_field_helper (TIFF *tif, uint32_t tag_id, T data_arr) + { + const void *data_ptr = data_arr.data (); + + if (! TIFFSetField (tif, tag_id, data_ptr)) + error ("Failed to set tag"); + } + void set_array_field_data (TIFF *tif, const TIFFField *fip, octave_value tag_ov, uint32_t count) @@ -1249,51 +1254,64 @@ uint32_t tag_id = TIFFFieldTag (fip); TIFFDataType tag_datatype = TIFFFieldDataType (fip); - // TODO(maged): validate count? - octave_unused_parameter (count); + // Array type must be double in matlab + if (! tag_ov.is_double_type ()) + error ("Tag data must be double"); - void *data_ptr; + // Matlab checks number of elements not dimensions + if (tag_ov.array_value ().numel () != count) + error ("Expected an array with %u elements", count); switch (tag_datatype) { case TIFF_BYTE: case TIFF_UNDEFINED: - // TODO(maged): check if matlab errors for long data type/dimensions - // TODO(maged): should we resize/reshape? - data_ptr = tag_ov.uint8_array_value ().fortran_vec (); + set_array_field_helper<uint8NDArray> (tif, tag_id, + tag_ov.uint8_array_value ()); break; case TIFF_SHORT: - data_ptr = tag_ov.uint16_array_value ().fortran_vec (); + set_array_field_helper<uint16NDArray> (tif, tag_id, + tag_ov.uint16_array_value ()); break; case TIFF_LONG: - data_ptr = tag_ov.uint32_array_value ().fortran_vec (); + set_array_field_helper<uint32NDArray> (tif, tag_id, + tag_ov.uint32_array_value ()); break; case TIFF_LONG8: - data_ptr = tag_ov.uint64_array_value ().fortran_vec (); + set_array_field_helper<uint64NDArray> (tif, tag_id, + tag_ov.uint64_array_value ()); break; case TIFF_RATIONAL: - data_ptr = tag_ov.float_array_value ().fortran_vec (); + set_array_field_helper<FloatNDArray> (tif, tag_id, + tag_ov.float_array_value ()); break; case TIFF_SBYTE: - data_ptr = tag_ov.int8_array_value ().fortran_vec (); + set_array_field_helper<int8NDArray> (tif, tag_id, + tag_ov.int8_array_value ()); break; case TIFF_SSHORT: - data_ptr = tag_ov.int16_array_value ().fortran_vec (); + set_array_field_helper<int16NDArray> (tif, tag_id, + tag_ov.int16_array_value ()); break; case TIFF_SLONG: - data_ptr = tag_ov.int32_array_value ().fortran_vec (); + set_array_field_helper<int32NDArray> (tif, tag_id, + tag_ov.int32_array_value ()); break; case TIFF_SLONG8: - data_ptr = tag_ov.int64_array_value ().fortran_vec (); + set_array_field_helper<int64NDArray> (tif, tag_id, + tag_ov.int64_array_value ()); break; case TIFF_FLOAT: - data_ptr = tag_ov.float_array_value ().fortran_vec (); + set_array_field_helper<FloatNDArray> (tif, tag_id, + tag_ov.float_array_value ()); break; case TIFF_DOUBLE: - data_ptr = tag_ov.array_value ().fortran_vec (); + set_array_field_helper<NDArray> (tif, tag_id, + tag_ov.array_value ()); break; case TIFF_SRATIONAL: - data_ptr = tag_ov.float_array_value ().fortran_vec (); + set_array_field_helper<FloatNDArray> (tif, tag_id, + tag_ov.float_array_value ()); break; case TIFF_IFD: case TIFF_IFD8: @@ -1302,16 +1320,13 @@ default: error ("Unsupported tag data type"); } - - if (! TIFFSetField (tif, tag_id, data_ptr)) - error ("Failed to set tag"); } void set_field_data (TIFF *tif, const TIFFField *fip, octave_value tag_ov) { uint32_t tag_id = TIFFFieldTag (fip); - + // TODO(maged): find/create images to test the special tags switch (tag_id) { @@ -1338,22 +1353,28 @@ &bits_per_sample)) error ("Failed to obtain the bit depth"); + if (! tag_ov.is_double_type ()) + error ("Tag data must be double"); + // According to the format specification, this field should // be 8 or 16 only. if (bits_per_sample > 16) error ("Too high bit depth for a palette image"); - // TODO(maged): what is matlab behavior for wrong input dimensions uint32_t count = 1 << bits_per_sample; + + if (tag_ov.is_scalar_type () + || tag_ov.array_value ().dim1 () != count + || tag_ov.array_value ().dim2 () != 3) + error ("Expected matrix with %u rows and 3 columns", count); + NDArray array_data = tag_ov.array_value (); array_data *= UINT16_MAX; uint16NDArray u16_array = uint16NDArray (array_data); - uint16_t *data_ptr - = reinterpret_cast<uint16_t *> (u16_array.fortran_vec ()); - uint16_t *red = data_ptr; - uint16_t *green = red + count; - uint16_t *blue = green + count; - if (! TIFFSetField (tif, tag_id, red, green, blue)) + const uint16_t *data_ptr + = reinterpret_cast<const uint16_t *> (u16_array.data ()); + if (! TIFFSetField (tif, tag_id, data_ptr, data_ptr + count, + data_ptr + 2 * count)) error ("Failed to set tag"); break; } @@ -1371,9 +1392,16 @@ uint32_t count = 1 << bits_per_sample; - uint16_t *data_ptr - = reinterpret_cast<uint16_t *> (tag_ov.uint16_array_value () - .fortran_vec ()); + // An intermediate variable is required for storing the return of + // uint16_array_value so that the object remains in scope and the + // pointer returned from data () is not a dangling pointer + uint16NDArray data_arr = tag_ov.uint16_array_value (); + + if (data_arr.numel () != count) + error ("Tag data should be and array of %u elements", count); + + const uint16_t *data_ptr + = reinterpret_cast<const uint16_t *> (data_arr.data ()); if (samples_per_pixel == 1) { if (! TIFFSetField (tif, tag_id, data_ptr)) @@ -1392,25 +1420,35 @@ case TIFFTAG_DOTRANGE: case TIFFTAG_YCBCRSUBSAMPLING: { - // TODO(maged): what is matlab behavior for wrong dimensions uint16NDArray array_data = tag_ov.uint16_array_value (); + if (array_data.numel () != 2) + error ("Tag data should be an array with 2 elements"); + if (! TIFFSetField (tif, tag_id, array_data (0), array_data (1))) error ("Failed to set field"); break; } case TIFFTAG_SUBIFD: { - // TODO(maged): check if matlab supports setting this tag + // TODO(maged): Matlab doesnt error on setting this tag + // but returns 0 for getTag error ("Unsupported tag"); break; } case TIFFTAG_EXTRASAMPLES: { - // TODO(maged): check if matlab validates dimensions/values of this tag + uint16_t samples_per_pixel; + if (! TIFFGetFieldDefaulted (tif, TIFFTAG_SAMPLESPERPIXEL, + &samples_per_pixel)) + error ("Failed to obtain the number of samples per pixel"); + uint16NDArray data_array = tag_ov.uint16_array_value (); + if (data_array.numel () > samples_per_pixel - 3) + error ("Failed to set field, too many values"); + if (! TIFFSetField (tif, tag_id, - static_cast<uint16_t> (data_array.dim1 ()), - data_array.fortran_vec ())) + static_cast<uint16_t> (data_array.numel ()), + data_array.data ())) error ("Failed to set field"); break; } @@ -1502,7 +1540,8 @@ perm(2) = 0; strip_data = strip_data.permute (perm); - void *data_vec = strip_data.fortran_vec (); + uint8_t *data_u8 + = reinterpret_cast<uint8_t *> (strip_data.fortran_vec ()); if (image_data->bits_per_sample == 8 || image_data->bits_per_sample == 16 || image_data->bits_per_sample == 32 @@ -1512,7 +1551,7 @@ // can be smaller in size tsize_t strip_size = strip_data.numel () * image_data->bits_per_sample / 8; - if (TIFFWriteEncodedStrip (tif, strip_no, data_vec, strip_size) == -1) + if (TIFFWriteEncodedStrip (tif, strip_no, data_u8, strip_size) == -1) error ("Failed to write strip data to image"); } @@ -1526,7 +1565,6 @@ std::unique_ptr<uint8_t []> strip_ptr = std::make_unique<uint8_t []> (TIFFStripSize (tif)); uint8_t *strip_buf = strip_ptr.get (); - uint8_t *data_u8 = reinterpret_cast<uint8_t *> (data_vec); // According to the format specification, the row should be byte // aligned so the number of bytes is rounded up to the nearest byte uint32_t padded_width = (image_data->width + 7) / 8; @@ -1604,13 +1642,13 @@ // Octave indexing is 1-based while LibTIFF is zero-based tile_no--; - void *data_vec = tile_data.fortran_vec (); + uint8_t *data_u8 = reinterpret_cast<uint8_t *> (tile_data.fortran_vec ()); 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) { - if (TIFFWriteEncodedTile (tif, tile_no, data_vec, + if (TIFFWriteEncodedTile (tif, tile_no, data_u8, TIFFTileSize (tif)) == -1) error ("Failed to write tile data to image"); @@ -1625,7 +1663,6 @@ std::unique_ptr<uint8_t []> tile_ptr = std::make_unique<uint8_t []> (TIFFTileSize (tif)); uint8_t *tile_buf = tile_ptr.get (); - uint8_t *data_u8 = reinterpret_cast<uint8_t *> (data_vec); // Packing the pixel data into bits for (uint32_t row = 0; row < tile_height; row++) { @@ -1782,7 +1819,8 @@ if (row_per_strip > image_data->height) row_per_strip = image_data->height; - uint8_t *pixel_fvec = reinterpret_cast<uint8_t *> (pixel_data.fortran_vec ()); + uint8_t *pixel_fvec + = reinterpret_cast<uint8_t *> (pixel_data.fortran_vec ()); uint32_t strip_count = TIFFNumberOfStrips (tif); tsize_t strip_size; uint32_t rows_in_strip; @@ -1957,10 +1995,17 @@ void write_image (TIFF *tif, T pixel_data, tiff_image_data *image_data) { - // TODO(maged): matlab sets the remaining to zero for less width and height - // and issues a warning for larger widths but not for larger height? (we should error) - // and doesnt issue a warning for larger width for tiled images and produces zeros - // and errors for less or more number of channels + // This dimensions checking is intentially done in this non-homogeneous + // way for matlab compatibility. + // In Matlab R2022a, If the width or height of the input matrix is less + // than needed, the data is silently padded with zeroes to fit. + // If the width is larger than needed, a warning is issued and the excess + // data is truncated. If the height is larger than needed, no warning is + // issued and the image data is wrong. If the number of channels is less + // or more than needed an error is produced. + // We chose to deviate from matlab in the larger height case to avoid + // errors resulting from silently corrupting image data, a warning is + // produced instead. if ((image_data->samples_per_pixel > 1 && pixel_data.ndims () < 3) || (pixel_data.ndims () > 2 && image_data->samples_per_pixel != pixel_data.dim3 ()))