[gimp] libgimp: fix and workaround Exif.Photo.UserComment interpretation.



commit 96e25b781711490a1bbda22cfd5b5397520ea3ba
Author: Jehan <jehan girinstud io>
Date:   Thu Jan 20 16:47:04 2022 +0100

    libgimp: fix and workaround Exif.Photo.UserComment interpretation.
    
    Here are the changes:
    - Separate the check for comment contents and the one of whether or not
      it starts with "charset=Ascii ". Indeed in my tests, we still want to
      handle the empty string case or the "binary comment" case even without
      the charset prefix (currently it was both or none, so I encountered
      cases with a broken "binary comment" comment because the charset
      prefix was absent).
    - Add some #warning in order not to forget to remove the bogus "binary
      comment" test. Indeed after some digging in Exiv2 code, I could
      confirm this return value got removed in commit 9b510858 of Exiv2
      repository, i.e. since Exiv2 0.27.4. Now in my test case where I had a
      tag containing only 0s, Exiv2 returns an empty string, which is
      perfectly fine (and it's up to us to keep or ignore it).
    - Use gexiv2_metadata_try_get_tag_interpreted_string() instead of their
      deprecate non-try counterparses. Right now, I am just outputting any
      error message to stderr, as I'm not sure if this is the kind of errors
      we want to warn people about. I guess it would depend on which type of
      errors exactly are returned, so let's see if we encounter some case in
      the future. For now stderr is fine to detect these.

 libgimp/gimpimagemetadata.c | 72 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 55 insertions(+), 17 deletions(-)
---
diff --git a/libgimp/gimpimagemetadata.c b/libgimp/gimpimagemetadata.c
index 37847a7104..24161be0b1 100644
--- a/libgimp/gimpimagemetadata.c
+++ b/libgimp/gimpimagemetadata.c
@@ -89,23 +89,40 @@ gimp_image_metadata_interpret_comment (gchar *comment)
    * Let's remove that part and return NULL if there
    * is nothing else left as comment. */
 
-  if (comment && g_str_has_prefix (comment, "charset=Ascii "))
+  if (comment)
     {
-      gchar *real_comment;
-
-      /* Skip "charset=Ascii " (length 14) to find the real comment */
-      real_comment = comment + 14;
-      if (real_comment[0] == '\0' ||
-          ! g_strcmp0 (real_comment, "binary comment"))
+      if (g_str_has_prefix (comment, "charset=Ascii "))
         {
+          gchar *real_comment;
+
+          /* Skip "charset=Ascii " (length 14) to find the real comment */
+          real_comment = g_strdup (comment + 14);
           g_free (comment);
-          return NULL;
+          comment = real_comment;
         }
-      else
+
+#warning TODO: remove second "binary comment" condition when Exiv2 minimum requirement >= 0.27.4
+
+      if (comment[0] == '\0' ||
+          /* TODO: this second test is ugly as hell and should be
+           * removed once our Exiv2 dependency is bumped to 0.27.4.
+           *
+           * Basically Exiv2 used to return "binary comment" for some
+           * comments, even just a comment filled of 0s (instead of
+           * returning the empty string). This has recently be reverted.
+           * For now, let's ignore such "comment", though this is weak
+           * too. What if the comment actually contained the "binary
+           * comment" text? (which would be weird anyway and possibly a
+           * result of the previously bugged implementation, so let's
+           * accept the weakness as we can't do anything to distinguish
+           * the 2 cases)
+           * See commit 9b510858 in Exiv2 repository.
+           */
+          ! g_strcmp0 (comment, "binary comment"))
         {
-          real_comment = g_strdup (real_comment);
+          /* Removing an empty comment.*/
           g_free (comment);
-          return real_comment;
+          return NULL;
         }
     }
 
@@ -137,15 +154,36 @@ gimp_image_metadata_load_finish (GimpImage             *image,
 
   if (flags & GIMP_METADATA_LOAD_COMMENT)
     {
-      gchar *comment;
+      gchar  *comment;
+      GError *error = NULL;
 
-      comment = gexiv2_metadata_get_tag_interpreted_string (GEXIV2_METADATA (metadata),
-                                                            "Exif.Photo.UserComment");
-      comment = gimp_image_metadata_interpret_comment (comment);
+      comment = gexiv2_metadata_try_get_tag_interpreted_string (GEXIV2_METADATA (metadata),
+                                                                "Exif.Photo.UserComment",
+                                                                &error);
+      if (error)
+        {
+          /* XXX. Should this be rather a user-facing error? */
+          g_printerr ("%s: unreadable '%s' metadata tag: %s\n",
+                      G_STRFUNC, "Exif.Photo.UserComment", error->message);
+          g_clear_error (&error);
+        }
+      else if (comment)
+        {
+          comment = gimp_image_metadata_interpret_comment (comment);
+        }
 
       if (! comment)
-        comment = gexiv2_metadata_get_tag_interpreted_string (GEXIV2_METADATA (metadata),
-                                                              "Exif.Image.ImageDescription");
+        {
+          comment = gexiv2_metadata_try_get_tag_interpreted_string (GEXIV2_METADATA (metadata),
+                                                                    "Exif.Image.ImageDescription",
+                                                                    &error);
+          if (error)
+            {
+              g_printerr ("%s: unreadable '%s' metadata tag: %s\n",
+                          G_STRFUNC, "Exif.Image.ImageDescription", error->message);
+              g_clear_error (&error);
+            }
+        }
 
       if (comment)
         {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]