changeset 31189:6a9d985e7474

Tiff: fixed bug graphicsmagick error handler colliding with our handler * __tiff__.cc (F__set_errors_enabled__): fixed bug where the use of graphicsmagick functions set the LibTIFF error handler to its own handler. Added calls in all internal functions to reset handlers. * Tiff.m: modified expected error message in some tests to reflect new behavior.
author magedrifaat <magedrifaat@gmail.com>
date Thu, 25 Aug 2022 20:17:59 +0200
parents 66cf3c7c06e8
children a91f2f79e58c
files libinterp/corefcn/__tiff__.cc scripts/io/Tiff.m
diffstat 2 files changed, 78 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/__tiff__.cc	Tue Aug 23 21:56:46 2022 +0200
+++ b/libinterp/corefcn/__tiff__.cc	Thu Aug 25 20:17:59 2022 +0200
@@ -16,8 +16,7 @@
 
 #if defined (HAVE_TIFF)
 #  include <tiffio.h>
-TIFFErrorHandler tiff_default_error_handler = NULL;
-TIFFErrorHandler tiff_default_warning_handler = NULL;
+bool handlers_set = true;
 #endif
 
 namespace octave
@@ -106,6 +105,21 @@
     }
   };
 
+  void
+  set_internal_handlers (void)
+  {
+    if (handlers_set)
+      {
+        TIFFSetErrorHandler (verror_with_id);
+        TIFFSetWarningHandler (vwarning_with_id);
+      }
+    else
+      {
+        TIFFSetErrorHandler (NULL);
+        TIFFSetWarningHandler (NULL);
+      }
+  }
+
   // A map of tag names supported by matlab, there are some differences
   // than LibTIFF's names (e.g. Photometric vs PhotometricInerpretation)
   static const std::map<std::string, ttag_t> tag_name_map = {
@@ -182,12 +196,14 @@
 
   const TIFFField *get_fip_with_name (TIFF *tif, std::string& tag_name)
   {
-    const TIFFField *fip = TIFFFieldWithName (tif, tag_name.c_str ());
+    const TIFFField *fip = NULL;
+    if (tag_name_map.find (tag_name) != tag_name_map.cend ())
+      fip = TIFFFieldWithTag (tif, tag_name_map.at (tag_name));
     
     // If the tag name is not found, fallback to the names defined
-    // in our name map.
-    if (! fip && tag_name_map.find (tag_name) != tag_name_map.cend ())
-      fip = TIFFFieldWithTag (tif, tag_name_map.at (tag_name));
+    // in LibTIFF.
+    if (fip == NULL)
+      fip = TIFFFieldWithName (tif, tag_name.c_str ());
     
     return fip;
   }
@@ -206,7 +222,6 @@
       error ("The image file was closed");
   }
 
-
   void
   validate_tiff_get_field (bool status)
   {
@@ -2398,12 +2413,13 @@
     // force disable this behavior by adding 'c' flag to the mode
     mode = mode + 'c';
     
+    set_internal_handlers ();
+
     TIFF *tif = TIFFOpen (filename.c_str (), mode.c_str ());
     
     if (! tif)
       error ("Failed to open Tiff file\n");
     
-    // TODO(maged): check if there is more to be done for r+
     if (is_rplus && ! TIFFSetDirectory (tif, 0))
       error ("Failed to open Tiff file\n");
 
@@ -2428,6 +2444,8 @@
     octave_tiff_handle *tiff_handle
       = octave_tiff_handle::get_tiff_handle (args(0));
     
+    set_internal_handlers ();
+
     tiff_handle->close ();
 
     return octave_value_list ();
@@ -2454,6 +2472,8 @@
     
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     const TIFFField *fip;
@@ -2494,6 +2514,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     check_readonly (tif);
     
@@ -2557,6 +2579,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
     
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     // FIXME: add support for reading in YCbCr mode
@@ -2609,6 +2633,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     // FIXME: add support for reading in YCbCr mode
@@ -2648,6 +2674,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     // FIXME: add support for reading in YCbCr mode
@@ -2687,6 +2715,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     tiff_image_data image_data (tif);
@@ -2746,6 +2776,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     // Matlab (R2022a) requires row to be double
@@ -2836,6 +2868,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+    
     TIFF *tif = tiff_handle->get_file ();
 
     if (! args(1).is_double_type ())
@@ -2936,6 +2970,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     check_readonly (tif);
@@ -2983,6 +3019,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     check_readonly (tif);
@@ -3019,6 +3057,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     check_readonly (tif);
@@ -3054,6 +3094,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     
     bool is_tiled = static_cast<bool> (TIFFIsTiled (tif));
@@ -3076,6 +3118,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     
     if (TIFFIsTiled (tif))
@@ -3101,6 +3145,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     
     if (! TIFFIsTiled (tif))
@@ -3126,6 +3172,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     
     if (TIFFIsTiled (tif))
@@ -3181,6 +3229,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     
     if (! TIFFIsTiled (tif))
@@ -3245,6 +3295,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     uint16_t dir = TIFFCurrentDirectory (tif);
@@ -3269,6 +3321,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     bool is_last = TIFFLastDirectory (tif);
@@ -3292,6 +3346,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     bool is_last = TIFFLastDirectory (tif);
@@ -3320,6 +3376,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     uint16_t dir = args(1).uint16_scalar_value ();
@@ -3374,6 +3432,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
 
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
 
     if (! TIFFRewriteDirectory(tif))
@@ -3398,6 +3458,8 @@
       = octave_tiff_handle::get_tiff_handle (args(0));
     check_closed (tiff_handle);
     
+    set_internal_handlers ();
+
     TIFF *tif = tiff_handle->get_file ();
     
     if (! args(1).is_double_type () && ! args(1).is_uint32_type ()
@@ -3446,13 +3508,6 @@
          "Enables or Disables error output from LibTIFF")
   {
 #if defined (HAVE_TIFF)
-    // Get the default error handlers the first time this function is called
-    if (tiff_default_error_handler == NULL)
-      {
-        tiff_default_error_handler = TIFFSetErrorHandler (NULL);
-        tiff_default_warning_handler = TIFFSetWarningHandler (NULL);
-      }
-
     if (args.length () == 0)
       error ("No state argument provided");
 
@@ -3460,16 +3515,8 @@
       error ("Expected logical value as argument");
     
     // Set the error and warning handlers according to the bool parameter
-    if (args(0).bool_value ())
-      {
-        TIFFSetErrorHandler (tiff_default_error_handler);
-        TIFFSetWarningHandler (tiff_default_warning_handler);
-      }
-    else
-      {
-        TIFFSetErrorHandler (NULL);
-        TIFFSetWarningHandler (NULL);
-      }
+    handlers_set = args(0).bool_value ();
+    set_internal_handlers ();
 
     return octave_value_list ();
 #else
--- a/scripts/io/Tiff.m	Tue Aug 23 21:56:46 2022 +0200
+++ b/scripts/io/Tiff.m	Thu Aug 25 20:17:59 2022 +0200
@@ -1,5 +1,3 @@
-## PKG_ADD: __tiff_set_errors_enabled__ (false);
-
 classdef Tiff
   properties (Constant = true)
     TagID = __tiff_make_tagid__ ();
@@ -285,10 +283,10 @@
 %!testif HAVE_TIFF
 %!  function test_fn (filename)
 %!    img = Tiff (filename, "w");
-%!    fail ("getTag (img, \"ImageWidt\")", "Tiff tag not found");
-%!    fail ("getTag (img, 999999)", "Tiff tag not found");
-%!    fail ("setTag (img, \"ImageWidt\", 2)", "Tiff tag not found");
-%!    fail ("setTag (img, 999999, 2)", "Tiff tag not found");
+%!    fail ("getTag (img, \"ImageWidt\")");
+%!    fail ("getTag (img, 0xf423f)");
+%!    fail ("setTag (img, \"ImageWidt\", 2)");
+%!    fail ("setTag (img, 0xf423f, 2)");
 %!  endfunction
 %!  file_wrapper (@test_fn);
 
@@ -308,8 +306,7 @@
 %!    img = Tiff (filename, "w");
 %!    setTag (img, "ImageLength", 1);
 %!    setTag (img, "ImageWidth", 1);
-%!    fail ("setTag (img, struct (\"ImageLength\", 2, \"a\", 1, \"ImageWidth\", 2))",
-%!          "Tag a not found");
+%!    fail ("setTag (img, struct (\"ImageLength\", 2, \"a\", 1, \"ImageWidth\", 2))");
 %!    assert (getTag (img, "ImageLength"), 2);
 %!    assert (getTag (img, "ImageWidth"), 1);
 %!  endfunction
@@ -340,7 +337,7 @@
 %!testif HAVE_TIFF
 %!  fail ("Tiff (\"test.tif\", \"rh\")",
 %!        "Invalid mode for openning Tiff file: rh");
-%!  fail ("Tiff ([tempname() \".tif\"], \"r\")", "Failed to open Tiff file");
+%!  fail ("Tiff ([tempname() \".tif\"], \"r\")");
 
 ## test r+ mode modifies exisitng images
 %!testif HAVE_TIFF