Skip to content

Conversation

vpv-csc
Copy link

@vpv-csc vpv-csc commented Aug 21, 2025

Fixes #9168 .

Changes proposed in this pull request:

  • Standard FLI/FLC files always have 8-bit color depth. This change helps avoid erroneously detecting some TIFFs as FLI/FLC files.
  • Update test files Tests/images/timeout-9139147ce93e20eb14088fe238e541443ffd64b3.fli and Tests/images/timeout-bff0a9dc7243a8e6ede2408d2ffa6a9964698b87.fli to claim to have 8-bit colour depth so that the tests related to them will still work.

Standard FLI/FLC files always have 8-bit color depth.
This change helps avoid erroneously detecting some TIFFs as FLI/FLC
files.

Update test files
Tests/images/timeout-9139147ce93e20eb14088fe238e541443ffd64b3.fli and
Tests/images/timeout-bff0a9dc7243a8e6ede2408d2ffa6a9964698b87.fli
to claim to have 8-bit colour depth so that the tests related to them
will still work.
@vpv-csc vpv-csc force-pushed the improve-fli-flic-detection branch from cd6114e to 583c33c Compare August 21, 2025 08:35
@radarhere
Copy link
Member

Just to link to documentation, https://www.fileformat.info/format/fli/egff.htm describes the header as

typedef struct _FlicHeader
{
  DWORD FileSize;       /* Total size of file */
  WORD  FileId;         /* File format indicator */
  WORD  NumberOfFrames; /* Total number of frames */
  WORD  Width;          /* Screen width in pixels */
  WORD  Height;         /* Screen height in pixels */
  WORD  PixelDepth;     /* Number of bits per pixel */
...
PixelDepth indicates the number of bits per pixel; the value of this field is always 08h.

@radarhere
Copy link
Member

Under 'Frequent errors', https://www.compuphase.com/flic.htm lists 'The colour depth is set to zero'. Is it worth also accepting zero as the value?

return (
len(prefix) >= 6
and i16(prefix, 4) in [0xAF11, 0xAF12]
and i16(prefix, 12) == 8 # 8-bit colour
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and i16(prefix, 12) == 8 # 8-bit colour
and i16(prefix, 12) == [0, 8] # depth

@vpv-csc
Copy link
Author

vpv-csc commented Sep 1, 2025

Is it worth also accepting zero as the value?

The file magic accepts only 8. Based on that, I'd say no, but I'll leave the decision to you (the maintainers).

We have received two ”false positive” TIFFs from our users. One of them has 0 as byte 12 and one of them has 1. Our problem would not then be completely fixed. But hypothetically there could be a similar TIFF file, which had 8 as byte 12, as well. With the 16-byte limit of prefix this can never be perfect.

@radarhere
Copy link
Member

I've created #9183 to also help the issue. If you don't feel that changing _accept is sufficient, then is that better than this? Or would you like both PRs?

@vpv-csc
Copy link
Author

vpv-csc commented Sep 4, 2025

I concentrated too much on _accept when trying to find a solution. #9183 actually fixes this issue as well, for both of our test files. This PR is not absolutely necessary. If you are worried about the 0 vs. 8 case, then I guess you can drop this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some TIFF files are identified as FLI/FLC files
2 participants