Sped up exif_content_fix() considerably by splitting the one giant

loop into two much smaller & faster loops.

diff --git a/ChangeLog b/ChangeLog
index 4b6d301..27d8ef7 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,7 +1,9 @@
-2009-09-25  Dan Fandrich <dan@coneharvesters.com>
+2009-09-26  Dan Fandrich <dan@coneharvesters.com>
 
        * libexif/exif-data.c: Added more error log messages and improved
          a few data boundary checks.
+       * Sped up exif_content_fix() considerably by splitting the one giant
+         loop into two much smaller & faster loops.
 
 2009-09-24  Dan Fandrich <dan@coneharvesters.com>
 

diff --git a/NEWS b/NEWS
index dc26e3c..4c667d6 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -14,14 +14,17 @@ libexif-0.6.x:
   * Added remaining GPS tags from the EXIF 2.2 spec to the tag table
   * Fixed the interpretation of some tags as being optional in IFD 1
     (to match the EXIF 2.2 spec) which stops them from being erroneously
-    removed from a file
+    removed from a file when EXIF_DATA_OPTION_IGNORE_UNKNOWN_TAGS is set
   * Changed exif_tag_get_support_level_in_ifd() to return a value when possible
-    when the data type for the given tag is unknown
+    when the data type for the given EXIF data is unknown. This will cause
+    tags to be added or deleted when tag fixup is requested without a data
+    type being set.
   * Added support for writing Pentax and Casio type2 MakerNotes
   * Improved display of Pentax and Casio type2 MakerNotes
   * Completely fixed bug #1617997 to display APEX values correctly
   * Stopped some crashes due to read-beyond-buffer accesses in MakerNotes
   * Don't abort MakerNote parsing after the first invalid tag
+  * Sped up exif_content_fix()
 
 libexif-0.6.17 (2008-11-06):
   * Updated translations: cs, de, pl, sk, vi

diff --git a/libexif/exif-content.c b/libexif/exif-content.c
index 5304b85..a79d1f2 100644 (file)
--- a/libexif/exif-content.c
+++ b/libexif/exif-content.c
@@ -246,31 +246,71 @@ fix_func (ExifEntry *e, void *UNUSED(data))
        exif_entry_fix (e);
 }
 
+/*!
+ * Check if this entry is unknown and if so, delete it.
+ * \note Be careful calling this function in a loop. Deleting an entry from
+ * an ExifContent changes the index of subsequent entries, as well as the
+ * total size of the entries array.
+ */
+static void
+remove_not_recorded (ExifEntry *e, void *UNUSED(data))
+{
+       ExifIfd ifd = exif_entry_get_ifd(e) ;
+       ExifContent *c = e->parent;
+       ExifDataType dt = exif_data_get_data_type (c->parent);
+       ExifTag t = e->tag;
+
+       if (exif_tag_get_support_level_in_ifd (t, ifd, dt) ==
+                        EXIF_SUPPORT_LEVEL_NOT_RECORDED) {
+               exif_log (c->priv->log, EXIF_LOG_CODE_DEBUG, "exif-content",
+                               "Tag 0x%04x is not recorded in IFD '%s' and has therefore been "
+                               "removed.", t, exif_ifd_get_name (ifd));
+               exif_content_remove_entry (c, e);
+       }
+
+}
+
 void
 exif_content_fix (ExifContent *c)
 {
        ExifIfd ifd = exif_content_get_ifd (c);
        ExifDataType dt;
        ExifEntry *e;
-       unsigned int i;
+       unsigned int i, num;
 
-       if (!c) return;
+       if (!c)
+               return;
 
        dt = exif_data_get_data_type (c->parent);
 
-       /* First of all, fix all existing entries. */
+       /*
+        * First of all, fix all existing entries.
+        */
        exif_content_foreach_entry (c, fix_func, NULL);
 
        /*
-        * Then check for existing tags that are not allowed and for
-        * non-existing mandatory tags.
+        * Go through each tag and if it's not recorded, remove it. If one
+        * is removed, exif_content_foreach_entry() will skip the next entry,
+        * so do the loop again from the beginning if this happens to ensure
+        * they're all checked. This could be avoided if we stop relying on
+        * exif_content_foreach_entry.
         */
-       for (i = 0; i <= 0xffff; i++) {
-               /* Index using an int because t might only be 16 bits */
-               const ExifTag t = (ExifTag) i;
-               switch (exif_tag_get_support_level_in_ifd (t, ifd, dt)) {
-               case EXIF_SUPPORT_LEVEL_MANDATORY:
-                       if (exif_content_get_entry (c, t)) break;
+       do {
+               num = c->count;
+               exif_content_foreach_entry (c, remove_not_recorded, NULL);
+       } while (num != c->count);
+
+       /*
+        * Then check for non-existing mandatory tags and create them if needed
+        */
+       num = exif_tag_table_count();
+       for (i = 0; i < num; ++i) {
+               const ExifTag t = exif_tag_table_get_tag (i);
+               if (exif_tag_get_support_level_in_ifd (t, ifd, dt) ==
+                       EXIF_SUPPORT_LEVEL_MANDATORY) {
+                       if (exif_content_get_entry (c, t))
+                               /* This tag already exists */
+                               continue;
                        exif_log (c->priv->log, EXIF_LOG_CODE_DEBUG, "exif-content",
                                        "Tag '%s' is mandatory in IFD '%s' and has therefore been added.",
                                        exif_tag_get_name_in_ifd (t, ifd), exif_ifd_get_name (ifd));
@@ -278,20 +318,6 @@ exif_content_fix (ExifContent *c)
                        exif_content_add_entry (c, e);
                        exif_entry_initialize (e, t);
                        exif_entry_unref (e);
-                       break;
-               case EXIF_SUPPORT_LEVEL_NOT_RECORDED:
-                       e = exif_content_get_entry (c, t);
-                       if (!e) break;
-                       exif_log (c->priv->log, EXIF_LOG_CODE_DEBUG, "exif-content",
-                                       "Tag '%s' is not recorded in IFD '%s' and has therefore been "
-                                       "removed.", exif_tag_get_name_in_ifd (t, ifd),
-                                       exif_ifd_get_name (ifd));
-                       exif_content_remove_entry (c, e);
-                       break;
-               case EXIF_SUPPORT_LEVEL_OPTIONAL:
-               case EXIF_SUPPORT_LEVEL_UNKNOWN:
-               default:
-                       break;
                }
        }
 }

diff --git a/libexif/exif-tag.c b/libexif/exif-tag.c
index 91b31cc..400c86f 100644 (file)
--- a/libexif/exif-tag.c
+++ b/libexif/exif-tag.c
@@ -857,15 +857,6 @@ static const struct {
 
 /* For now, do not use these functions. */
 
-/*! \internal */
-ExifTag      exif_tag_table_get_tag  (unsigned int n);
-
-/*! \internal */
-const char  *exif_tag_table_get_name (unsigned int n);
-
-/*! \internal */
-unsigned int exif_tag_table_count    (void);
-
 ExifTag
 exif_tag_table_get_tag (unsigned int n)
 {

diff --git a/libexif/exif-tag.h b/libexif/exif-tag.h
index 33e38ed..cd71f64 100644 (file)
--- a/libexif/exif-tag.h
+++ b/libexif/exif-tag.h
@@ -259,6 +259,19 @@ const char     *exif_tag_get_title       (ExifTag tag);
 /*! \deprecated Use #exif_tag_get_description_in_ifd instead */
 const char     *exif_tag_get_description (ExifTag tag);
 
+
+/* For now, do not use these functions. */
+
+/*! \internal */
+ExifTag      exif_tag_table_get_tag  (unsigned int n);
+
+/*! \internal */
+const char  *exif_tag_table_get_name (unsigned int n);
+
+/*! \internal */
+unsigned int exif_tag_table_count    (void);
+
+
 /* Don't use these definitions. They are here for compatibility only. */
 
 /*! \deprecated Use EXIF_TAG_PRINT_IMAGE_MATCHING instead. */