Re: gdk_pixbuf and comments ?

[Sven Neumann <sven gimp org>]

Hi,

according to the proposed "Thumbnail Managing Standard" information 
about the original image is supposed to be stored in tEXt chunks
of the PNG format:

http://www.informatik.uni-oldenburg.de/~pearl/thumbnail-spec/x56.html

Attached is a patch that extends the Gdk-Pixbuf PNG module to store
key/value pairs passed as options to gdk_pixbuf_save() into those
tEXt chunks. The PNG load routines read those fields and attach them
as object data to the GdkPixbuf. As the slightly modified apps in the
demos directory show, this seems to work reasonably well. The patch
is in no way considered final. There are a number of issues:

 - longjmp might clobber variables (-> make them volatile ?!)
 - I'm not sure how libpng handles compressed tEXt chunks. Will
   png_get_text() return them uncompressed or do we have to take
   care about this ourselves? Do we want to support compressed
   tEXt chunks at all since the PNG standard discourages its use
   (http://www.w3.org/TR/REC-png#C.tEXt)?
 - should we prefix the options to be stored as tEXt using for 
   example "tEXt::" and include that prefix in the data attached
   to the GdkPixbuf as well?

That should be enough food for thought...


Salut, Sven

Index: demos/testpixbuf-save.c
===================================================================
RCS file: /cvs/gnome/gtk+/demos/testpixbuf-save.c,v
retrieving revision 1.4
diff -u -r1.4 testpixbuf-save.c
--- demos/testpixbuf-save.c	2001/05/07 19:27:51	1.4
+++ demos/testpixbuf-save.c	2001/08/07 14:14:33
@@ -44,7 +46,10 @@
                         return;
                 }
 
-                if (!gdk_pixbuf_save (pixbuf, "foo.png", "png", &err, NULL)) {
+                if (!gdk_pixbuf_save (pixbuf, "foo.png", "png", 
+                                      &err,
+                                      "Software", "testpixbuf-save", 
+                                      NULL)) {
                         fprintf (stderr, "%s", err->message);
                         g_error_free (err);
                 }
Index: demos/testpixbuf-scale.c
===================================================================
RCS file: /cvs/gnome/gtk+/demos/testpixbuf-scale.c,v
retrieving revision 1.10
diff -u -r1.10 testpixbuf-scale.c
--- demos/testpixbuf-scale.c	2001/07/18 04:31:08	1.10
+++ demos/testpixbuf-scale.c	2001/08/07 14:14:33
@@ -63,6 +63,7 @@
 	GtkWidget *hbox, *label, *hscale;
 	GtkAdjustment *adjustment;
 	GtkRequisition scratch_requisition;
+        const gchar *creator;
         GError *error;
         
 	pixbuf_init ();
@@ -83,6 +84,11 @@
                 g_error_free (error);
 		exit(1);
 	}
+
+        creator = (const gchar *) g_object_get_data (G_OBJECT (pixbuf), 
+                                                     "Software");
+        if (creator)
+                g_print ("%s was created by %s\n", argv[1], creator);
 
 	window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
 	gtk_signal_connect (GTK_OBJECT (window), "destroy",
Index: gdk-pixbuf/io-png.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk-pixbuf/io-png.c,v
retrieving revision 1.40
diff -u -r1.40 io-png.c
--- gdk-pixbuf/io-png.c	2001/07/18 04:25:03	1.40
+++ gdk-pixbuf/io-png.c	2001/08/07 14:14:33
@@ -186,12 +186,15 @@
 static GdkPixbuf *
 gdk_pixbuf__png_image_load (FILE *f, GError **error)
 {
+        GdkPixbuf *pixbuf;
 	png_structp png_ptr;
 	png_infop info_ptr, end_info;
+        png_textp text_ptr;
         gboolean failed = FALSE;
-	gint i, ctype, bpp;
+	gint i, ctype, bpp, num_text;
 	png_uint_32 w, h;
 	png_bytepp rows;
+        gchar **texts = NULL;
 	guchar *pixels;
 
 	png_ptr = png_create_read_struct (PNG_LIBPNG_VER_STRING,
@@ -255,17 +258,40 @@
 		rows[i] = pixels + i * w * bpp;
 
 	png_read_image (png_ptr, rows);
+
+        if (png_get_text (png_ptr, info_ptr, &text_ptr, &num_text)) {
+                texts = g_new (gchar*, 2 * num_text);
+                for (i = 0; i < num_text; i++) {
+                        texts[2*i]   = g_strdup (text_ptr[i].key);
+                        texts[2*i+1] = g_strdup (text_ptr[i].text);
+                }
+        }
+
 	png_destroy_read_struct (&png_ptr, &info_ptr, &end_info);
 	g_free (rows);
 
 	if (ctype & PNG_COLOR_MASK_ALPHA)
-		return gdk_pixbuf_new_from_data (pixels, GDK_COLORSPACE_RGB, TRUE, 8,
-						 w, h, w * 4,
-						 free_buffer, NULL);
+		pixbuf = gdk_pixbuf_new_from_data (pixels, GDK_COLORSPACE_RGB, TRUE, 8,
+                                                   w, h, w * 4,
+                                                   free_buffer, NULL);
 	else
-		return gdk_pixbuf_new_from_data (pixels, GDK_COLORSPACE_RGB, FALSE, 8,
-						 w, h, w * 3,
-						 free_buffer, NULL);
+		pixbuf = gdk_pixbuf_new_from_data (pixels, GDK_COLORSPACE_RGB, FALSE, 8,
+                                                   w, h, w * 3,
+                                                   free_buffer, NULL);
+
+        /* Attach tEXt chunks as object data */
+
+        if (pixbuf) {
+                for (i = 0; i < num_text; i++) {
+                        g_object_set_data_full (G_OBJECT (pixbuf), 
+                                                texts[2*i], texts[2*i+1],
+                                                (GDestroyNotify) g_free);
+                        g_free (texts[2*i]);
+                }
+        }
+        g_free (texts);
+
+        return pixbuf;
 }
 
 /* I wish these avoided the setjmp()/longjmp() crap in libpng instead
@@ -501,6 +527,8 @@
 {
         LoadContext* lc;
         png_uint_32 width, height;
+        png_textp png_text_ptr;
+        int num_text, i;
         int color_type;
         gboolean have_alpha = FALSE;
         gboolean failed = FALSE;
@@ -538,7 +566,18 @@
                 }
                 return;
         }
+
+        /* Extract tEXt chunks and attach them as object data */
         
+        if (png_get_text (png_read_ptr, png_info_ptr, &png_text_ptr, &num_text)) {
+                for (i = 0; i < num_text; i++) {
+                        g_object_set_data_full (G_OBJECT (lc->pixbuf),
+                                                png_text_ptr[i].key,
+                                                g_strdup (png_text_ptr[i].text),
+                                                (GDestroyNotify) g_free);
+                }
+        }
+
         /* Notify the client that we are ready to go */
 
         if (lc->prepare_func)
@@ -642,6 +681,7 @@
 {
        png_structp png_ptr;
        png_infop info_ptr;
+       png_textp text_ptr = NULL;
        guchar *ptr;
        guchar *pixels;
        int x, y, j;
@@ -651,23 +691,24 @@
        int w, h, rowstride;
        int has_alpha;
        int bpc;
+       int num_keys;
 
+       num_keys = 0;
+
        if (keys && *keys) {
-               g_warning ("Bad option name '%s' passed to PNG saver",
-                          *keys);
-               return FALSE;
-#if 0
-               gchar **kiter = keys;
-               gchar **viter = values;
-
-               
-               while (*kiter) {
-                       
-                       ++kiter;
-                       ++viter;
+               gchar **kiter;
+               int     len;
+
+               for (kiter = keys; *kiter; kiter++) {
+                       len = strlen (*kiter);
+                       if (len < 1 || len > 79) {
+                               g_warning ("Keys passed as option to PNG saver must be of length 1-79");
+                               return FALSE;
+                       }
+                       num_keys++;
                }
-#endif
        }
+
        data = NULL;
        
        bpc = gdk_pixbuf_get_bits_per_sample (pixbuf);
@@ -693,7 +734,21 @@
                png_destroy_write_struct (&png_ptr, (png_infopp) NULL);
                return FALSE;
        }
+
+       if (num_keys > 0) {
+               text_ptr = g_new0 (png_text, num_keys);
+               for (j = 0; j < num_keys; j++) {
+                       text_ptr[j].compression = -1;
+                       text_ptr[j].key         = keys[j];
+                       text_ptr[j].text        = values[j];
+                       text_ptr[j].text_length = (values[j] ? 
+                                                  strlen (values[j]) : 0);
+               }                       
+               png_set_text (png_ptr, info_ptr, text_ptr, num_keys);
+       }
+
        png_init_io (png_ptr, f);
+
        if (has_alpha) {
                png_set_IHDR (png_ptr, info_ptr, w, h, bpc,
                              PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
@@ -751,6 +806,9 @@
 
        png_write_end (png_ptr, info_ptr);
        png_destroy_write_struct (&png_ptr, (png_infopp) NULL);
+
+       if (num_keys > 0)
+               g_free (text_ptr);
 
        return TRUE;
 }