Re: patch for gdk-pixbuf to read and write PNG tEXt chunks



Hi,

Owen Taylor <otaylor redhat com> writes:

> > +        creator = (const gchar *) g_object_get_data (G_OBJECT (pixbuf), 
> > +                                                     "tEXt::Software");
> > +        if (creator)
> > +                g_print ("%s was created by '%s'\n", argv[1], creator);
> 
> I don't like this -- better to add a real API:
> 
> Something along the lines of:
> 
>  G_CONST_RETURN char *gdk_pixbuf_get_option (GdkPixbuf *pixbuf,
>                                              const char *key);
> 
> And as part of this, you probably should hang all options off of
> a single object data key rather than using a bunch of object data
> keys.
> 
> Something like:
> 
>  void gdk_pixbuf_get_options (GdkPixbuf         *pixbuf,
>                               GdkPixbufOption ***options,
>                               gint              *n_options);
>  void gdk_pixbuf_free_options (GdkPixbufOption *options,
>                                gint             n_options);
> 
> Might be nice as well, though that could (should?) be added later.
> 
> The final comment is that I think that the code in io-png should
> translate the values of the keys to/from UTF-8 and error if
> the key value is not representable in ISO-8859-1.
> 
> (tEXt is specified to be in iso-8859-1, ugh. There is a "internationalized
> text segment in later versions of png, but we presumbaly don't want 
> to assume sufficiently new libpng.)

attached is a revised patch that addresses the encoding conversion and
introduces a GdkPixbufOption struct that is however only used internally.
It adds the gdk_pixbuf_get_option() API that you suggested. The helper
functions _gdk_pixbuf_set_options() and _gdk_pixbuf_free_options() are
not exported since I think the user shouldn't have to know anything about
the implementation. 

I have not addressed the error handling issues since I would prefer to
discuss this problem first and if necessary handle it in a seperate 
patch. At the moment there are a few places where gdk_pixbuf functions 
return FALSE without setting GError. Havoc mentioned that this might be
intentional as it indicates a bug in the application that is not 
recoverable and I think I agree with him. What do you think?


Salut, Sven

Index: gdk-pixbuf/gdk-pixbuf-io.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk-pixbuf/gdk-pixbuf-io.c,v
retrieving revision 1.60
diff -u -r1.60 gdk-pixbuf-io.c
--- gdk-pixbuf/gdk-pixbuf-io.c	2001/09/14 22:04:55	1.60
+++ gdk-pixbuf/gdk-pixbuf-io.c	2001/10/01 15:55:16
@@ -731,8 +731,8 @@
  * @Varargs: list of key-value save options
  *
  * Saves pixbuf to a file in @type, which is currently "jpeg" or
- * "png".  If @error is set, FALSE will be returned. Possible errors include those
- * in the #GDK_PIXBUF_ERROR domain and those in the #G_FILE_ERROR domain.
+ * "png".  If @error is set, FALSE will be returned. Possible errors include 
+ * those in the #GDK_PIXBUF_ERROR domain and those in the #G_FILE_ERROR domain.
  *
  * The variable argument list should be NULL-terminated; if not empty,
  * it should contain pairs of strings that modify the save
@@ -743,8 +743,13 @@
  *                  "quality", "100", NULL);
  * </programlisting>
  *
- * The only save parameter that currently exists is the "quality" field
- * for JPEG images; its value should be in the range [0,100].
+ * Currently only few parameters exist. JPEG images can be saved with a 
+ * "quality" parameter; its value should be in the range [0,100]. 
+ * Text chunks can be attached to PNG images by specifying parameters of
+ * the form "tEXt::key", where key is an ASCII string of length 1-79;
+ * the values being UTF-8 encoded strings. Note however that PNG text
+ * chunks are stored in ISO-8859-1 encoding, so you can only set texts
+ * that can be represented in this encoding.
  *
  * Return value: whether an error was set
  **/
Index: gdk-pixbuf/gdk-pixbuf-private.h
===================================================================
RCS file: /cvs/gnome/gtk+/gdk-pixbuf/gdk-pixbuf-private.h,v
retrieving revision 1.9
diff -u -r1.9 gdk-pixbuf-private.h
--- gdk-pixbuf/gdk-pixbuf-private.h	2001/06/26 20:56:32	1.9
+++ gdk-pixbuf/gdk-pixbuf-private.h	2001/10/01 15:55:16
@@ -128,8 +128,22 @@
                                         const GTimeVal         *current_time);
 };
       
-
 
 GdkPixbufAnimation* gdk_pixbuf_non_anim_new (GdkPixbuf *pixbuf);
+
+
+
+/*  key/value pairs that can be attached by the pixbuf loader  */
+
+typedef struct _GdkPixbufOption GdkPixbufOption;
+struct _GdkPixbufOption {
+        gchar *key;
+        gchar *value;
+};
+
+void _gdk_pixbuf_set_options  (GdkPixbuf        *pixbuf,
+                               GdkPixbufOption **options);
+void _gdk_pixbuf_free_options (GdkPixbufOption **options);
+
 
 #endif
Index: gdk-pixbuf/gdk-pixbuf.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk-pixbuf/gdk-pixbuf.c,v
retrieving revision 1.45
diff -u -r1.45 gdk-pixbuf.c
--- gdk-pixbuf/gdk-pixbuf.c	2001/08/07 17:49:09	1.45
+++ gdk-pixbuf/gdk-pixbuf.c	2001/10/01 15:55:16
@@ -1,3 +1,4 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*- */
 /* GdkPixbuf library - Basic memory management
  *
  * Copyright (C) 1999 The Free Software Foundation
@@ -479,6 +480,62 @@
         }
 }
 
+
+
+/**
+ * gdk_pixbuf_get_option:
+ * @pixbuf: a #GdkPixbuf
+ * @key: a nul-terminated string.
+ * 
+ * Looks up @key in the list of options that may have been attached to the
+ * @pixbuf when it was loaded. 
+ * 
+ * Return value: the value associated with @key. This is a nul-terminated 
+ * string that should not be freed or %NULL if @key was not found.
+ **/
+G_CONST_RETURN gchar *
+gdk_pixbuf_get_option (GdkPixbuf   *pixbuf,
+                       const gchar *key)
+{
+        GdkPixbufOption **options;
+
+        g_return_val_if_fail (GDK_IS_PIXBUF (pixbuf), NULL);
+        g_return_val_if_fail (key != NULL, NULL);
+  
+        options = g_object_get_qdata (G_OBJECT (pixbuf), 
+                                      g_quark_from_static_string ("gdk_pixbuf_options"));
+        if (options) {
+                while (*options) {
+                        if (strcmp ((*options)->key, key) == 0)
+                                return (*options)->value;
+                        options++;
+                }
+        }
+        
+        return NULL;
+}
+
+void
+_gdk_pixbuf_set_options (GdkPixbuf        *pixbuf,
+                         GdkPixbufOption **options)
+{
+        g_object_set_qdata_full (G_OBJECT (pixbuf), 
+                                 g_quark_from_static_string ("gdk_pixbuf_options"),
+                                 options, 
+                                 (GDestroyNotify) _gdk_pixbuf_free_options);
+}
+
+void
+_gdk_pixbuf_free_options (GdkPixbufOption **options)
+{
+        while (*options) {
+                g_free ((*options)->key);
+                g_free ((*options)->value);
+                options++;
+        }
+}
+
+
 
 /* Include the marshallers */
 #include <glib-object.h>
Index: gdk-pixbuf/gdk-pixbuf.h
===================================================================
RCS file: /cvs/gnome/gtk+/gdk-pixbuf/gdk-pixbuf.h,v
retrieving revision 1.62
diff -u -r1.62 gdk-pixbuf.h
--- gdk-pixbuf/gdk-pixbuf.h	2001/09/16 23:54:37	1.62
+++ gdk-pixbuf/gdk-pixbuf.h	2001/10/01 15:55:16
@@ -286,6 +286,12 @@
                                                                               const GTimeVal         *current_time);
 
 
+
+
+G_CONST_RETURN gchar * gdk_pixbuf_get_option (GdkPixbuf   *pixbuf,
+                                              const gchar *key);
+
+
  
 #include <gdk-pixbuf/gdk-pixbuf-loader.h>
 #include <gdk-pixbuf/gdk-pixbuf-enum-types.h>
Index: gdk-pixbuf/io-png.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk-pixbuf/io-png.c,v
retrieving revision 1.41
diff -u -r1.41 io-png.c
--- gdk-pixbuf/io-png.c	2001/08/30 07:21:13	1.41
+++ gdk-pixbuf/io-png.c	2001/10/01 15:55:16
@@ -182,17 +182,47 @@
 	g_free (pixels);
 }
 
+static GdkPixbufOption **
+png_text_to_pixbuf_options(png_textp text_ptr,
+                           gint      num_texts)
+{
+        GdkPixbufOption **options;
+        gchar *value;
+        gint   i, n;
+
+        options = g_new0 (GdkPixbufOption *, num_texts + 1);
+        for (i = 0, n = 0; i < num_texts; i++) {
+                value = g_convert (text_ptr[i].text, -1, 
+                                   "UTF-8", "ISO-8859-1", 
+                                   NULL, NULL, NULL);
+                if (value) {
+                        options[n] = g_new (GdkPixbufOption, 1);
+                        options[n]->key = g_strconcat ("tEXt::", 
+                                                       text_ptr[i].key, 
+                                                       NULL);
+                        options[n]->value = value;
+                        n++;
+                }
+        }
+        
+        return options;
+}
+
 /* Shared library entry point */
 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;
 	png_uint_32 w, h;
 	png_bytepp rows;
 	guchar *pixels;
+        gint    num_texts;
+        GdkPixbufOption **options = NULL;
 
 	png_ptr = png_create_read_struct (PNG_LIBPNG_VER_STRING,
                                           error,
@@ -255,17 +285,30 @@
 		rows[i] = pixels + i * w * bpp;
 
 	png_read_image (png_ptr, rows);
+
+        if (png_get_text (png_ptr, info_ptr, &text_ptr, &num_texts))
+                options = png_text_to_pixbuf_options (text_ptr, num_texts);
+
 	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);
+
+        if (options) {
+                if (pixbuf)
+                        _gdk_pixbuf_set_options (pixbuf, options);
+                else
+                        _gdk_pixbuf_free_options (options);
+        }
+
+        return pixbuf;
 }
 
 /* I wish these avoided the setjmp()/longjmp() crap in libpng instead
@@ -501,6 +544,8 @@
 {
         LoadContext* lc;
         png_uint_32 width, height;
+        png_textp png_text_ptr;
+        int num_texts;
         int color_type;
         gboolean have_alpha = FALSE;
         gboolean failed = FALSE;
@@ -538,7 +583,16 @@
                 }
                 return;
         }
+
+        /* Extract tEXt chunks and attach them as pixbuf options */
         
+        if (png_get_text (png_read_ptr, png_info_ptr, &png_text_ptr, &num_texts)) {
+                GdkPixbufOption **options;
+                options = png_text_to_pixbuf_options (png_text_ptr, num_texts);
+                if (options)
+                        _gdk_pixbuf_set_options (lc->pixbuf, options);
+        }
+
         /* Notify the client that we are ready to go */
 
         if (lc->prepare_func)
@@ -653,32 +707,58 @@
 {
        png_structp png_ptr;
        png_infop info_ptr;
+       png_textp text_ptr = NULL;
        guchar *ptr;
        guchar *pixels;
-       int x, y, j;
+       int x, y;
+       int i, j;
        png_bytep row_ptr;
        png_bytep data;
        png_color_8 sig_bit;
        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 < 6 || strncmp (*kiter, "tEXt::", 6)) {
+                                g_warning ("Bad option name '%s' passed to PNG saver",
+                                          *kiter);
+                               return FALSE;
+                       }
+                       len -= 6;
+                       if (len < 1 || len > 79) {
+                               g_warning ("tEXt keys passed as option to PNG saver must be of length 1-79");
+                               return FALSE;
+                       }
+                       num_keys++;
                }
-#endif
        }
+
+       if (num_keys > 0) {
+               text_ptr = g_new0 (png_text, num_keys);
+               for (i = 0; i < num_keys; i++) {
+                       text_ptr[i].compression = PNG_TEXT_COMPRESSION_NONE;
+                       text_ptr[i].key         = keys[i]+6;
+                       text_ptr[i].text        = g_convert (values[i], -1, 
+                                                            "ISO-8859-1", "UTF-8", 
+                                                            NULL, &text_ptr[i].text_length, 
+                                                            NULL);
+                       if (!text_ptr[i].text) {
+                               g_warning ("Value for key '%s' can not be represented in ISO-8859-1 encoding as specified for PNG tEXt chunks.", keys[i]);
+                               return FALSE;
+                               
+                       }
+               }
+       }        
+
        data = NULL;
        
        bpc = gdk_pixbuf_get_bits_per_sample (pixbuf);
@@ -704,7 +784,13 @@
                png_destroy_write_struct (&png_ptr, (png_infopp) NULL);
                return FALSE;
        }
+
+       if (num_keys > 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,
@@ -762,6 +848,12 @@
 
        png_write_end (png_ptr, info_ptr);
        png_destroy_write_struct (&png_ptr, (png_infopp) NULL);
+
+       if (num_keys > 0) {
+               for (i = 0; i < num_keys; i++)
+                       g_free (text_ptr[i].text);
+               g_free (text_ptr);
+       }
 
        return TRUE;
 }
Index: demos/testpixbuf-save.c
===================================================================
RCS file: /cvs/gnome/gtk+/demos/testpixbuf-save.c,v
retrieving revision 1.7
diff -u -r1.7 testpixbuf-save.c
--- demos/testpixbuf-save.c	2001/09/07 21:49:33	1.7
+++ demos/testpixbuf-save.c	2001/10/01 15:55:16
@@ -35,7 +35,10 @@
                         return;
                 }
 
-                if (!gdk_pixbuf_save (pixbuf, "foo.png", "png", &err, NULL)) {
+                if (!gdk_pixbuf_save (pixbuf, "foo.png", "png", 
+                                      &err,
+                                      "tEXt::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.11
diff -u -r1.11 testpixbuf-scale.c
--- demos/testpixbuf-scale.c	2001/08/23 15:26:44	1.11
+++ demos/testpixbuf-scale.c	2001/10/01 15:55:16
@@ -63,6 +63,7 @@
 	GtkWidget *hbox, *label, *hscale;
 	GtkAdjustment *adjustment;
 	GtkRequisition scratch_requisition;
+        const gchar *creator;
         GError *error;
         
 	pixbuf_init ();
@@ -82,6 +83,10 @@
                 g_error_free (error);
 		exit(1);
 	}
+
+        creator = gdk_pixbuf_get_option (pixbuf, "tEXt::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",




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