Backward comp of color_profile and iptc_profile impossible

Post any defects you find in the released or beta versions of the ImageMagick software here. Include the ImageMagick version, OS, and any command-line required to reproduce the problem. Got a patch for a bug? Post it here.
Post Reply
jn0101
Posts: 40
Joined: 2007-06-16T01:36:07-07:00

Backward comp of color_profile and iptc_profile impossible

Post by jn0101 »

Hi,

This looks very much like my previous post, but read on :-)

I am using JMagick, which at several places use the ImageMagick API as it was
used in 2003, including the use of color_profile and iptc_profile members.

IMs changelog states:

2003-09-21 Cristy <yarrow@image...>
* Image profiles are now conveniently handled with a hashmap
structure. The color_profile and iptc_profile members of the Image
structure are still maintained for backwards compatibility, however,
we encourage you to use the new PutImageProfile(), GetImageProfile(),
and DeleteImageProfile() methods.


However JMagick not only *reads* but altso *writes* to the color_profile and iptc_profile members.

When saving file to disk later on the new hashtable structure for profiles is of course used,
therefore new values of color_profile and iptc_profile members are NOT stored.



For example in JMagick/src/magick/magick_MagickImage.c line 3376:
/*
* Class: magick_MagickImage
* Method: setColorProfile
* Signature: (Lmagick/ProfileInfo;)V
*/
JNIEXPORT void JNICALL Java_magick_MagickImage_setColorProfile
(JNIEnv *env, jobject self, jobject profile)
{
Image *image = (Image*) getHandle(env, self, "magickImageHandle", NULL);
if (image == NULL) {
throwMagickException(env, "Cannot obtain image handle");
return;
}
setProfileInfo(env, &image->color_profile, profile);
}


which calls JMagick/src/magick/jmagick.c line 732, which is shown in
viewtopic.php?f=2&t=9021&p=28005
but essentially sets these variables:

image.color_profile.name = <some string>
image.color_profile.info = <a byte array>
image.color_profile.length = <length of byte array>



As said these new values are ignored when the image is saved to disk later on.


So, backward compatibility of color_profile and iptc_profile seems impossible.


Only solution I can come up with would be to always check the old deprecated
members for new values and transfer them if they have changed.
Seems a little cumbersome to me.

Alternatively it should be stated that color_profile and iptc_profile and other
deprecated members are *read only* and state that any code that changes
them will be broken.



Jacob
jn0101
Posts: 40
Joined: 2007-06-16T01:36:07-07:00

Re: Backward comp of color_profile and iptc_profile impossible

Post by jn0101 »

I am conversely going to propose a change of JMagics code to the following:

/*
* Class: magick_MagickImage
* Method: setIptcProfile
* Signature: (Lmagick/ProfileInfo;)V
*/
JNIEXPORT void JNICALL Java_magick_MagickImage_setIptcProfile
(JNIEnv *env, jobject self, jobject profileObj)
{
Image *image = (Image*) getHandle(env, self, "magickImageHandle", NULL);
if (image == NULL) {
throwMagickException(env, "Cannot obtain image handle");
return;
}
// setProfileInfo() is broken, dont use
//setProfileInfo(env, &image->iptc_profile, profileObj);

char *name;
unsigned char *info;
int infoSize = 0;

if (profileObj == NULL) {
throwMagickException(env, "ProfileInfo cannot be null");
return;
}

name = getStringFieldValue(env, profileObj, "name", NULL);
info = getByteArrayFieldValue(env, profileObj, "info", NULL, &infoSize);

StringInfo* profilex = AcquireStringInfo(infoSize);
memcpy(profilex->datum, info, infoSize);
SetImageProfile(image,"iptc",profilex);
profilex=DestroyStringInfo(profilex);
}


And likewise for setColorProfile:
...
SetImageProfile(image,"icc",profilex);
...


Please confirm that this code is OK
(I am nok very good neither at C nor at IMs style of coding)

Jacob
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: Backward comp of color_profile and iptc_profile impossible

Post by magick »

Don't access the StringInfo members directly. Instead use
  • SetStringInfoDatum(profilex, info);
    SetImageProfile(image, name, profilex);
jn0101
Posts: 40
Joined: 2007-06-16T01:36:07-07:00

Re: Backward comp of color_profile and iptc_profile impossible

Post by jn0101 »

Thank you very much,
Ive comed to the following, much more robust code (I hope)

info = getByteArrayFieldValue(env, profileObj, "info", NULL, &infoSize);

fprintf(stderr, "infoSize = %d\n", infoSize);
fprintf(stderr, "info = %p\n", info);

if (info==NULL) {
RemoveImageProfile(image,"icc");
} else {
StringInfo* profilex;
profilex = AcquireStringInfo(infoSize);
SetStringInfoDatum(profilex, info);
SetImageProfile(image,"icc",profilex);
profilex=DestroyStringInfo(profilex);
}

As far as I understand the 'name' is totally obsolete.

Could you please confirm that the backward compatibility of color_profile and iptc_profile
members is limited to *read only* and any code that changes these members is broken.
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: Backward comp of color_profile and iptc_profile impossible

Post by magick »

A few years ago we pledged to maintain backwards compatibility. Any method/member we deprecate remains fully supported. If any deprecated method/member is mis-behavin' just post a bug report and we will fix it. Be sure to post a description of the problem and how we can reproduce it. However, in your case it is probably best to use the Set/GetImageProfile() methods because its a more robust approach to dealing with profiles.
Post Reply