Skip to content

libjpeg replaces non-EXIF APP1 sections with empty EXIF ones #11

@glebignatieff

Description

@glebignatieff

Description

APP1 sections are not EXIF only, but also XMP. Currently, libjpeg leaves an empty EXIF section instead of copy-pasting XMP section.

Before copy:

JPEG APP1 (8188 bytes):
  + [XMP directory, 8159 bytes]
  | About = DJI Meta Data
  | ModifyDate = 2022-04-22
  | CreateDate = 2022-04-22
...

After copy:

JPEG APP1 (20 bytes):
  ExifByteOrder = MM
  + [IFD0 directory with 0 entries]

MWE

extern "C"
{
#include <libjpeg/jpeg-data.h>
}

auto main() -> int
{
    auto jpeg_data = ::jpeg_data_new_from_file("source.JPG");
    auto exif_data = ::jpeg_data_get_exif_data(jpeg_data);
    ::jpeg_data_set_exif_data(jpeg_data, exif_data);
    ::jpeg_data_save_file(jpeg_data, "dest.JPG");
    return 0;
}

Suggested solution

  1. In jpeg_data_load_data() check if EXIF buffer is NULL and if it is, then fall back to default, i.e. generic instead of app1.

    exif/libjpeg/jpeg-data.c

    Lines 241 to 242 in 4cd915d

    s->content.app1 = exif_data_new_from_data (
    d + o - 4, len + 4);
  2. In jpeg_data_save_data() check if generic buffer size is 0, if it is, then handle as EXIF, otherwise fallback to default.
I can't upload a file with the patch for some reason, so here it is.
diff --git a/libjpeg/jpeg-data.c b/libjpeg/jpeg-data.c
index a64d362..a2528d4 100644
--- a/libjpeg/jpeg-data.c
+++ b/libjpeg/jpeg-data.c
@@ -25,6 +25,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <errno.h>
+#include <stdbool.h>

 /* This refers to the exif-i18n.h file from the "exif" package and is
  * NOT to be confused with the libexif/i18n.h file.
@@ -45,6 +46,11 @@ struct _JPEGDataPrivate
 	ExifLog *log;
 };

+static bool is_exif_section(JPEGSection * section)
+{
+    return section->marker == JPEG_MARKER_APP1 && section->content.generic.size == 0;
+}
+
 JPEGData *
 jpeg_data_new (void)
 {
@@ -78,7 +84,7 @@ jpeg_data_append_section (JPEGData *data)
 		s = realloc (data->sections,
 			     sizeof (JPEGSection) * (data->count + 1));
 	if (!s) {
-		EXIF_LOG_NO_MEMORY (data->priv->log, "jpeg-data",
+		EXIF_LOG_NO_MEMORY (data->priv->log, "jpeg-data",
 				sizeof (JPEGSection) * (data->count + 1));
 		return;
 	}
@@ -142,19 +148,23 @@ jpeg_data_save_data (JPEGData *data, unsigned char **d, unsigned int *ds)
 		case JPEG_MARKER_EOI:
 			break;
 		case JPEG_MARKER_APP1:
-			ed = NULL;
-			exif_data_save_data (s.content.app1, &ed, &eds);
-			if (!ed) break;
-			CLEANUP_REALLOC (*d, sizeof (char) * (*ds + 2));
-			(*d)[*ds + 0] = (eds + 2) >> 8;
-			(*d)[*ds + 1] = (eds + 2) >> 0;
-			*ds += 2;
-			CLEANUP_REALLOC (*d, sizeof (char) * (*ds + eds));
-			memcpy (*d + *ds, ed, eds);
-			*ds += eds;
-			free (ed);
-			break;
-		default:
+            if (is_exif_section(&s)) {
+                ed = NULL;
+                exif_data_save_data(s.content.app1, &ed, &eds);
+                if (!ed) {
+                    break;
+                }
+                CLEANUP_REALLOC(*d, sizeof(char) * (*ds + 2));
+                (*d)[*ds + 0] = (eds + 2) >> 8;
+                (*d)[*ds + 1] = (eds + 2) >> 0;
+                *ds += 2;
+                CLEANUP_REALLOC(*d, sizeof(char) * (*ds + eds));
+                memcpy(*d + *ds, ed, eds);
+                *ds += eds;
+                free(ed);
+                break;
+            }
+        default:
 			CLEANUP_REALLOC (*d, sizeof (char) *
 					(*ds + s.content.generic.size + 2));
 			(*d)[*ds + 0] = (s.content.generic.size + 2) >> 8;
@@ -238,10 +248,12 @@ jpeg_data_load_data (JPEGData *data, const unsigned char *d,

 			switch (s->marker) {
 			case JPEG_MARKER_APP1:
-				s->content.app1 = exif_data_new_from_data (
-							d + o - 4, len + 4);
-				break;
-			default:
+                s->content.app1 = exif_data_new_from_data(d + o - 4, len + 4);
+                if (s->content.app1->data != NULL) {
+                    break;
+                }
+                exif_data_unref(s->content.app1);
+            default:
 				s->content.generic.data =
 						malloc (sizeof (char) * len);
 				if (!s->content.generic.data) {
@@ -393,9 +405,11 @@ jpeg_data_free (JPEGData *data)
 			case JPEG_MARKER_EOI:
 				break;
 			case JPEG_MARKER_APP1:
-				exif_data_unref (s.content.app1);
-				break;
-			default:
+                if (is_exif_section(&s)) {
+                    exif_data_unref(s.content.app1);
+                    break;
+                }
+            default:
 				free (s.content.generic.data);
 				break;
 			}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions