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

[Owen Taylor <otaylor redhat com>]

Sven Neumann <sven gimp org> writes:

> Hi,
> 
> following up to myself, here's a reworked patch. It features better cleanup
> on error, adds the proposed "tEXt::" prefix to the gdk_pixbuf_save parameters
> and to the gobject data and gives some info about the new PNG parameter in
> the inline docs.
> 
> There is one issue that I'm still unsure about: When a wrong parameter is
> passed to the PNG saver it gives a warning and returns FALSE without setting
> GError. This leads to a segfault later. I've copied this code from the 
> JPEG save routine, so if this is considered the wrong behaviour, we should
> fix it in other places too. We'd probably need a new GDK_PIXBUF_ERROR value
> then (GDK_PIXBUF_ERROR_BAD_OPTION ?!).

Sounds like a good idea to me. Returning FALSE does need to occur
if and only if the error is set.

> +        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.)

Regards,
                                        Owen