Patch for fli.c
This discussion is connected to the gimp-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.
This is a read-only list on gimpusers.com so this discussion thread is read-only, too.
Patch for fli.c | David Capello | 09 Feb 23:25 |
Patch for fli.c | Sven Neumann | 10 Feb 13:11 |
Patch for fli.c | Sven Neumann | 10 Feb 13:19 |
Patch for fli.c | Sven Neumann | 11 Feb 08:35 |
Patch for fli.c | Sven Neumann | 12 Feb 08:58 |
77b6ce390802110425h37e823fa... | 07 Oct 20:26 | |
Fwd: Patch for fli.c | David Capello | 11 Feb 13:26 |
Patch for fli.c
Hello, I don't know if I should send patches to this list, but anyway, I think that it's a good place.
I have a very old patch for the GFLI plug-in. I never used GIMP to save or load FLIC animations, but I used the source code of that GFLI plug-ins for other program (http://www.aseprite.org/), so that is the way I found this bug.
The patch is for this specific file:
http://svn.gnome.org/svn/gimp/trunk/plug-ins/gfli/fli.c
It fixes two problems:
* loading of fli files that specified width=height=0 (that means 320x200)
* writing of differential color chunks (FLI_COLOR and FLI_COLOR_2).
I don't know if with the GIMP you can save a FLIC file with FLI_COLOR chunks anyway. So it's a little hard to test the effectiveness of the patch.
On the other hand, I think that the GFLI plug-in has other problems
(in the gfli.c file).
But I really don't know, this patch is only for the low-level routines in fli.c.
If somebody want to test reading/writing files with FLI_COLOR chunks, here you have one animation to test (in two formats): http://www.davidcapello.com.ar/gimp/flitest.flc http://www.davidcapello.com.ar/gimp/flitest.gif
Patch for fli.c
Hi,
On Sat, 2008-02-09 at 20:25 -0200, David Capello wrote:
On the other hand, I think that the GFLI plug-in has other problems (in the gfli.c file).
IMO we should remove this plug-in from the main distribution. It doesn't seem to be very useful for the main target audience and it appears to be buggy and unmaintained.
David, perhaps you want to take over maintainance of this plug-in and keep it updated in the plug-in registry?
Sven
Patch for fli.c
Hi,
On Sat, 2008-02-09 at 20:25 -0200, David Capello wrote:
Hello, I don't know if I should send patches to this list, but anyway, I think that it's a good place.
We would prefer if you opened one or even several bug reports at http://bugzilla.gnome.org/ with a description of the problems you see in the plug-in. You can then attach your patches there.
Sven
Patch for fli.c
Hi,
On Sat, 2008-02-09 at 20:25 -0200, David Capello wrote:
The patch is for this specific file: http://svn.gnome.org/svn/gimp/trunk/plug-ins/gfli/fli.c It fixes two problems:
* loading of fli files that specified width=height=0 (that means 320x200)
Where is this specified?
* writing of differential color chunks (FLI_COLOR and FLI_COLOR_2).
Again, where is this specified? I don't know the file format, and I don't think anyone else here has in-depth knowledge about it. So if you want us to accept patches, you better point us to the official documentation of the file format and explain what exactly the GIMP plug-in is not doing correctly here.
Sven
Fwd: Patch for fli.c
---------- Forwarded message ----------
From: David Capello
Date: Feb 11, 2008 10:25 AM
Subject: Re: [Gimp-developer] Patch for fli.c
To: Sven Neumann
Hi Sven,
On Feb 10, 2008 10:11 AM, Sven Neumann wrote:
On Sat, 2008-02-09 at 20:25 -0200, David Capello wrote:
On the other hand, I think that the GFLI plug-in has other problems (in the gfli.c file).
IMO we should remove this plug-in from the main distribution. It doesn't seem to be very useful for the main target audience and it appears to be buggy and unmaintained.
That could be (because the FLI/FLC formats are very old and rarely used in this time). But the current code is better than nothing.
David, perhaps you want to take over maintainance of this plug-in and keep it updated in the plug-in registry?
Sincerely I have a lot of work with Allegro Sprite Editor right now. So maybe in another time (or life :)
We would prefer if you opened one or even several bug reports at http://bugzilla.gnome.org/ with a description of the problems you see in the plug-in. You can then attach your patches there.
I don't think it should be necessary because the only problem that the user can see directly (that frames with changes in palette really don't change) is already documented in the same gfli.c file as a "Current disadvantage", literally it says: "doesn't support palette changes between two frames" :)
The problems are internal to the fli.c utility file (that I was using for my personal app), and happend when you're writing a file with color palette changes, but the Gimp doesn't support them (neither reading nor writing). So that is not a visible/reproducible bug.
The patch is for this specific file: http://svn.gnome.org/svn/gimp/trunk/plug-ins/gfli/fli.c It fixes two problems:
* loading of fli files that specified width=height=0 (that means 320x200)Where is this specified?
No, It isn't. Is like an "undocument" feature. I test it some time ago with an original FLI file created with the Autodesk Animator Pro. The FLI file couldn't be readed with the GIMP, but it was played with the Allegro library. So I searched in the code of the Allegro library and I found that patch. I'll try to find the Autodesk Animator Pro to create it again and upload the FLI file to test it.
* writing of differential color chunks (FLI_COLOR and FLI_COLOR_2).
Again, where is this specified? I don't know the file format, and I don't think anyone else here has in-depth knowledge about it. So if you want us to accept patches, you better point us to the official documentation of the file format and explain what exactly the GIMP plug-in is not doing correctly here.
Sorry for this, here you have a some documents:
http://www.ddj.com/architect/184408954 (official) http://www.martinreddy.net/gfx/anim/FLI.txt http://www.martinreddy.net/gfx/anim/FLC.txt http://www.compuphase.com/flic.htm
The GIMP doesn't fail because it doesn't support "palette changes between two frames", so the patch fix problems for an internal utility file (fli.c). In anycase the code is unreachable using the GIMP, so there aren't a visible problem right now.
Anyway I can explain you why the fli.c fails writing FLI_COLOR chunks. See the "fli_write_color" in the "fli.c" file http://svn.gnome.org/svn/gimp/trunk/plug-ins/gfli/fli.c
In that routine the first FLI_COLOR chunk (if old_cmap==NULL) is written right. Then, if some color in the palette changes (old_cmap != cmap) the problems appear.
int fli_write_color(..., unsigned char *old_cmap, unsigned char *cmap)
{
unsigned short num_packets;
...
num_packets=0;
if (old_cmap==NULL) {
/* This works fine (the GIMP just use this, nothing more) */
...
} else {
/* The next code is unreachable by the GIMP
(but it's used in my application)... */
/* Err... problems, two "num_packets" in the same routine... */
unsigned short num_packets, cnt_skip, cnt_col,
col_pos, col_start;
num_packets=0; col_pos=0;
do {
cnt_skip=0;
while ((col_pos0) {
num_packets++;
...
while (cnt_col>0) {
/* ..Here we are using the
col_start as a index for cmap...
a cmap is a color palette
of 256 RGB entries, that
is 768 bytes */
fli_write_char(f, cmap[col_start++]>>2);
fli_write_char(f, cmap[col_start++]>>2);
fli_write_char(f, cmap[col_start++]>>2);
cnt_col--;
}
}
} while (col_pos
...
return 0;
}
I hope that will be helpful.
Patch for fli.c
Hi,
On Sat, 2008-02-09 at 20:25 -0200, David Capello wrote:
The patch is for this specific file: http://svn.gnome.org/svn/gimp/trunk/plug-ins/gfli/fli.c It fixes two problems:
* loading of fli files that specified width=height=0 (that means 320x200) * writing of differential color chunks (FLI_COLOR and FLI_COLOR_2).
I have now applied this change to trunk. Thanks for your contribution.
Sven