---------- Forwarded message ----------
From: David Capello <[EMAIL PROTECTED]>
Date: Feb 11, 2008 10:25 AM
Subject: Re: [Gimp-developer] Patch for fli.c
To: Sven Neumann <[EMAIL PROTECTED]>

Hi Sven,

On Feb 10, 2008 10:11 AM, Sven Neumann <[EMAIL PROTECTED]> 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)

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

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;
        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 {
                        while ((col_pos<256) &&
                                 (old_cmap[col_pos*3+0]==cmap[col_pos*3+0]) &&
                                 (old_cmap[col_pos*3+1]==cmap[col_pos*3+1]) &&
                                 (old_cmap[col_pos*3+2]==cmap[col_pos*3+2])) {
                                cnt_skip++; col_pos++;
                        /* In the next line col_start must be "col_pos*3".
                           col_pos goes from 0 to 255 at most...
                           col_start goes from 0 to 767 (see below...) */
                        while ((col_pos<256) &&
                                 !((old_cmap[col_pos*3+0]==cmap[col_pos*3+0]) &&
                                 (old_cmap[col_pos*3+1]==cmap[col_pos*3+1]) &&
                                 (old_cmap[col_pos*3+2]==cmap[col_pos*3+2]))) {
                                cnt_col++; col_pos++;
                        if (cnt_col>0) {
                                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);
                } while (col_pos<256);
        return 0;

I hope that will be helpful.
Gimp-developer mailing list

Reply via email to