[MagickWand] Potential regression when creating GIFs
Swiss army knife of image processing
Brought to you by:
bfriesen
Menu
▾
▴
1 Attachments
#735 [MagickWand] Potential regression when creating GIFs
Hello,
Thank you for all your work on the library, I really appreciate it :-)
I have been developing a language binding for GraphicsMagick. I had some examples that worked in version 1.3.39, but they no longer work in subsequent versions due to changes in the behavior of AppendImageToList(). I have modified the samples that use the Core API, but I am struggling to address the changes with the Wand API.
The code below works in 1.3.39 and produces a correct GIF:
#include <wand/wand_api.h>
int main(int argc, char **argv)
{
MagickWand *wand;
if (argc < 2) {
fprintf(stderr, "usage: %s <input files...>\n", argv[0]);
return 1;
}
InitializeMagick(*argv);
wand = NewMagickWand();
if (NULL == wand) {
fputs("error: failed to allocate a MagickWand*\n", stderr);
DestroyMagick();
return 1;
}
for (int i = 1; i < argc; i++) {
MagickBool success;
success = MagickReadImage(wand, argv[i]);
if (MagickFalse == success) {
ExceptionType severity;
char *message = MagickGetException(wand, &severity);
fprintf(stderr, "error: failed to read image: %s (%d)\n",
message, severity);
continue;
}
MagickSetImageDelay(wand, 100);
MagickSetImageDispose(wand, PreviousDispose);
}
// I thought adding the below would help, but seemingly not.
MagickResetIterator(wand);
MagickSetImageIterations(wand, 0);
MagickWriteImages(wand, "output.gif", MagickTrue);
DestroyMagickWand(wand);
DestroyMagick();
return 0;
}
Is there something that I'm missing, or has the change in behavior not addressed in the Wand API?
AppendImageToList() now always updates the images pointer to be the last image in the list. This helps immensely when appending more images. I see that there are both "wand->images" and "wand->image" so there are two references to the list. When the operation is completed, "wand->image" is set to the last image in the list and "wand->images" also refers to the last image in the list, but previously perhaps it referred to the first frame of the image list which was just appended.
I will investigate.
Ah, okay. I remember reading that the changes were for the better. It makes sense to avoid re-traversing the list each time.
Yea, as I understand it, after invoking MagickReadImage() (which calls AppendImageToList()), "wand->images" used to continue pointing to the first image in the list. This allowed MagickWriteImages() to correctly write the entire list.
A couple of solutions I came up with:
If I can be of any assistance, please let me know.
Thanks 😄
There appear to be three input PNG files. I am not sure if order matters, but can you please provide the exact argument list you are testing with?
Never mind. I can immediately see that the images are not being appended.
The strategy being used is not a documented feature of MagickReadImage(). Regardless, if this undocumented "feature" worked before, and no longer works now, it seems to be a form of regression.
There was no change to the MagickReadImage() code. These two lines of code do the work:
From this, one may determine that wand->image should always have been pointing to the last image in the list.
The problem seems to be in WriteImages() in that it starts writing from the current position (which is now the end of the list), rather than at the start of the list.
There is also the possibility of modifying MagickReadImageFile() to store the beginning of the list in wand->images, but this would also penalize your read algorithm.
After a bit more study, it appears that all functions currently using AppendImageToList() need to also update wand->images to point to the start of the list since that is what the other APIs expect. The behavior of AppendImageToList() was changed.
This issue is fixed by Mercurial changeset 17447:ee005758e1a6, which assures that functions using AppendImageToList() update wand->images to point to the beginning of the list. This fix is in today's GraphicsMagick snapshot tarball.
Thank you for fixing this.