Mercurial > octave-libtiff

changeset 31119:dbca50246dfc

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author magedrifaat <magedrifaat@gmail.com>
date Tue, 19 Jul 2022 19:45:06 +0200
parents f8be3654caef
children 46bb98cec195
files libinterp/dldfcn/__tiff__.cc
diffstat 1 files changed, 124 insertions(+), 91 deletions(-) [+]
line wrap: on
line diff
--- 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<octave_idx_type> 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