changeset 31099:6fc4bf5e14e1

Cleaned up the interface
author magedrifaat <magedrifaat@gmail.com>
date Mon, 27 Jun 2022 01:31:36 +0200
parents 3cbd0d82167c
children 4c606686d4ef
files libinterp/dldfcn/__tiff__.cc
diffstat 1 files changed, 40 insertions(+), 101 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/dldfcn/__tiff__.cc	Mon Jun 27 00:52:49 2022 +0200
+++ b/libinterp/dldfcn/__tiff__.cc	Mon Jun 27 01:31:36 2022 +0200
@@ -10,11 +10,20 @@
 
 // TODO(maged): Tidy up the formatting to be consistant with octave
 
+void validate_tiff_get_field(bool status, void *p_to_free=NULL)
+{
+    if (!status)
+    {
+        if (p_to_free != NULL)
+            _TIFFfree(p_to_free);
+        error("Failed to read tag");
+    }
+}
+
 octave_value_list interpret_data(void *data, uint32_t count, TIFFDataType tag_datatype)
 {
     // TODO(maged): Find the correct way fo returning multivalues (Matrix for numericals?)
     octave_value_list ovl_data;
-    // TODO(maged): check if this should be row matrix or column matrix
     dim_vector arr_dims(1, count);
     if (tag_datatype == TIFF_BYTE || tag_datatype == TIFF_UNDEFINED)
     {
@@ -130,7 +139,7 @@
     }
     else if (tag_datatype == TIFF_IFD || tag_datatype == TIFF_IFD8)
     {
-        // TODO(maged): implement IFD datatype
+        // TODO(maged): implement IFD datatype?
         error("Unimplemented IFFD data type");
     }
     else
@@ -149,30 +158,13 @@
 
     // TIFFFieldReadCount returns VARIABLE for some scalar tags (e.g. Compression)
     // But TIFFFieldPassCount seems consistent
-    if (!TIFFFieldPassCount(fip))
-    {
-        // TODO(maged): test this function vs actual data type size
-        int type_size = TIFFDataWidth(TIFFFieldDataType(fip));
-        void *data = _TIFFmalloc(type_size);
-        if (TIFFGetField(tif, tag_ID, data))
-        {
-            tag_data_ovl = interpret_data(data, 1, TIFFFieldDataType(fip));
-            _TIFFfree(data);
-        }
-        else
-        {
-            _TIFFfree(data);
-
-            // TODO(maged): Give a better error message?
-            error("Failed to read tag");
-        }
-        
-    }
-    else
-    {
-        // TODO(maged): Implement support for variable read argument tags
-        error("Variable argument tags not implemented");
-    }
+    validate_tiff_get_field(!TIFFFieldPassCount(fip));
+    // TODO(maged): test this function vs actual data type size
+    int type_size = TIFFDataWidth(TIFFFieldDataType(fip));
+    void *data = _TIFFmalloc(type_size);
+    validate_tiff_get_field(TIFFGetField(tif, tag_ID, data), data);
+    tag_data_ovl = interpret_data(data, 1, TIFFFieldDataType(fip));
+    _TIFFfree(data);
 
     return tag_data_ovl;
 }
@@ -183,19 +175,10 @@
 
     uint32_t tag_ID = TIFFFieldTag(fip);
     TIFFDataType tag_datatype = TIFFFieldDataType(fip);
-    int type_size = TIFFDataWidth(tag_datatype);
     void *data;
-    if (TIFFGetField(tif, tag_ID, &data))
-    {
-        tag_data_ovl = interpret_data(data, array_size, tag_datatype);
-    }
-    else
-    {
-        // TODO(maged): Give a better error message?
-        error("Failed to read tag");
-    }
-
-    return tag_data_ovl;
+    validate_tiff_get_field(TIFFGetField(tif, tag_ID, &data));
+    
+    return interpret_data(data, array_size, tag_datatype);
 }
 
 octave_value_list get_field_data(TIFF *tif, const TIFFField *fip)
@@ -220,7 +203,7 @@
         case TIFFTAG_REFERENCEBLACKWHITE:
             tag_data_ovl = get_array_field_data(tif, fip, 6);
             break;
-        // TODO(maged): colormap is not always 3 channels, but libtiff hardcodes it as 3?
+        // colormap is not always 3 channels, but libtiff hardcodes it as 3
         case TIFFTAG_COLORMAP:
             {
                 uint16_t bits_per_sample;
@@ -231,23 +214,15 @@
                 
                 if (bits_per_sample > 24)
                 {
-                    // TODO(maged): is this the right response here?
                     error("Too high bit depth for a palette image");
                 }
 
                 uint32_t count = 1<<bits_per_sample;
                 uint16_t *red, *green, *blue;
-                if (TIFFGetField(tif, TIFFTAG_COLORMAP, &red, &green, &blue))
-                {
-                    tag_data_ovl(0) = octave_value(interpret_data(red, count, TIFFFieldDataType(fip)));
-                    tag_data_ovl(1) = octave_value(interpret_data(green, count, TIFFFieldDataType(fip)));
-                    tag_data_ovl(2) = octave_value(interpret_data(blue, count, TIFFFieldDataType(fip)));
-                }
-                else
-                {
-                    // TODO(maged): Give a better error message?
-                    error("Failed to read tag");   
-                }
+                validate_tiff_get_field(TIFFGetField(tif, TIFFTAG_COLORMAP, &red, &green, &blue));
+                tag_data_ovl(0) = octave_value(interpret_data(red, count, TIFFFieldDataType(fip)));
+                tag_data_ovl(1) = octave_value(interpret_data(green, count, TIFFFieldDataType(fip)));
+                tag_data_ovl(2) = octave_value(interpret_data(blue, count, TIFFFieldDataType(fip)));
             }
             break;
         case TIFFTAG_TRANSFERFUNCTION:
@@ -268,20 +243,12 @@
                 uint16_t *ch1, *ch2, *ch3;
                 if (samples_per_pixel == 1)
                 {
-                    if (!TIFFGetField(tif, TIFFTAG_COLORMAP, &ch1))
-                    {
-                        // TODO(maged): Give a better error message?
-                        error("Failed to read tag");   
-                    }
+                    validate_tiff_get_field(TIFFGetField(tif, TIFFTAG_COLORMAP, &ch1));
                     tag_data_ovl(0) = octave_value(interpret_data(ch1, count, TIFFFieldDataType(fip)));
                 }
                 else
                 {
-                    if (!TIFFGetField(tif, TIFFTAG_COLORMAP, &ch1, &ch1, &ch3))
-                    {
-                        // TODO(maged): Give a better error message?
-                        error("Failed to read tag");   
-                    }
+                    validate_tiff_get_field(TIFFGetField(tif, TIFFTAG_COLORMAP, &ch1, &ch2, &ch3));
                     tag_data_ovl(0) = octave_value(interpret_data(ch1, count, TIFFFieldDataType(fip)));
                     tag_data_ovl(1) = octave_value(interpret_data(ch2, count, TIFFFieldDataType(fip)));
                     tag_data_ovl(2) = octave_value(interpret_data(ch3, count, TIFFFieldDataType(fip)));
@@ -294,46 +261,25 @@
         case TIFFTAG_YCBCRSUBSAMPLING:
             {
                 uint16_t tag_part1, tag_part2;
-                if (TIFFGetField(tif, tag_ID, &tag_part1, &tag_part2))
-                {
-                    tag_data_ovl(0) = octave_value(interpret_data(&tag_part1, 1, TIFFFieldDataType(fip)));
-                    tag_data_ovl(1) = octave_value(interpret_data(&tag_part2, 1, TIFFFieldDataType(fip)));
-                }
-                else
-                {
-                    // TODO(maged): Give a better error message?
-                    error("Failed to read tag");
-                }
+                validate_tiff_get_field(TIFFGetField(tif, tag_ID, &tag_part1, &tag_part2));
+                tag_data_ovl(0) = octave_value(interpret_data(&tag_part1, 1, TIFFFieldDataType(fip)));
+                tag_data_ovl(1) = octave_value(interpret_data(&tag_part2, 1, TIFFFieldDataType(fip)));
             }
             break;
         case TIFFTAG_SUBIFD:
             {
                 uint16_t count;
                 uint64_t *offsets;
-                if (TIFFGetField(tif, tag_ID, &count, &offsets))
-                {
-                    tag_data_ovl = interpret_data(offsets, count, TIFFFieldDataType(fip));
-                }
-                else
-                {
-                    // TODO(maged): Give a better error message?
-                    error("Failed to read tag");
-                }
+                validate_tiff_get_field(TIFFGetField(tif, tag_ID, &count, &offsets));
+                tag_data_ovl = interpret_data(offsets, count, TIFFFieldDataType(fip));
             }
             break;
         case TIFFTAG_EXTRASAMPLES:
             {
                 uint16_t count;
                 uint16_t *types;
-                if (TIFFGetField(tif, tag_ID, &count, &types))
-                {
-                    tag_data_ovl = interpret_data(types, count, TIFFFieldDataType(fip));
-                }
-                else
-                {
-                    // TODO(maged): Give a better error message?
-                    error("Failed to read tag");
-                }
+                validate_tiff_get_field(TIFFGetField(tif, tag_ID, &count, &types));
+                tag_data_ovl = interpret_data(types, count, TIFFFieldDataType(fip));
             }
             break;
         // TODO(maged): Do I just return bytes? pass it to some parser lib?
@@ -344,15 +290,8 @@
             {
                 uint16_t count;
                 void *data;
-                if (TIFFGetField(tif, tag_ID, &count, &data))
-                {
-                    tag_data_ovl = interpret_data(data, count, TIFF_BYTE);
-                }
-                else
-                {
-                    // TODO(maged): Give a better error message?
-                    error("Failed to read tag");
-                }
+                validate_tiff_get_field(TIFFGetField(tif, tag_ID, &count, &data));
+                tag_data_ovl = interpret_data(data, count, TIFF_BYTE);
             }
             break;
         default:
@@ -382,7 +321,7 @@
     
     // TODO(maged): Look into unwind action
     TIFF *tif = TIFFOpen(filename.c_str(), mode.c_str());
-    // TODO(maged): print a better error
+    
     if (!tif)
         error("Failed to open Tiff file\n");
 
@@ -433,7 +372,7 @@
         std::string tagName = args(1).string_value();
         fip = TIFFFieldWithName(tif, tagName.c_str());
         if (!fip)
-            error("Tiff tag not found\n");
+            error("Tiff tag not found");
         
         tag_ID = TIFFFieldTag(fip);
     }
@@ -443,7 +382,7 @@
         fip = TIFFFieldWithTag(tif, tag_ID);
         // TODO(maged): Handle other types of errors (e.g. unsupported tags)
         if (!fip)
-            error("Tiff tag not found\n");
+            error("Tiff tag not found");
     }