[PATCH 0/1] MR9486: windowscodecs: correct GIF LZW compression initialization
The GIF LZW compression algorithm was incorrectly initializing the code size to 8 bits regardless of the actual color depth. This caused issues with images that have fewer colors requiring smaller initial code sizes. The fix calculates the proper initial code size based on the actual number of colors in the image, using the formula: minimum code size = max(2, ceil(log2(color_count))). This ensures proper compression for images with small color palettes and maintains compatibility with GIF decoders that expect correct LZW initialization parameters. Log: Fixed GIF compression for images with small color palettes Signed-off-by: Jiajin Cui <cuijiajin(a)uniontech.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486
From: Jiajin Cui <cuijiajin(a)uniontech.com> The GIF LZW compression algorithm was incorrectly initializing the code size to 8 bits regardless of the actual color depth. This caused issues with images that have fewer colors requiring smaller initial code sizes. The fix calculates the proper initial code size based on the actual number of colors in the image, using the formula: minimum code size = max(2, ceil(log2(color_count))). This ensures proper compression for images with small color palettes and maintains compatibility with GIF decoders that expect correct LZW initialization parameters. Log: Fixed GIF compression for images with small color palettes Signed-off-by: Jiajin Cui <cuijiajin(a)uniontech.com> --- dlls/windowscodecs/gifformat.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/dlls/windowscodecs/gifformat.c b/dlls/windowscodecs/gifformat.c index 4fde4f81bba..989ae3f3568 100644 --- a/dlls/windowscodecs/gifformat.c +++ b/dlls/windowscodecs/gifformat.c @@ -1415,7 +1415,7 @@ static inline int read_byte(struct input_stream *in, unsigned char *byte) return 0; } -static HRESULT gif_compress(IStream *out_stream, const BYTE *in_data, ULONG in_size) +static HRESULT gif_compress(IStream *out_stream, const BYTE *in_data, ULONG in_size, int color_bits) { struct input_stream in; struct output_stream out; @@ -1429,7 +1429,7 @@ static HRESULT gif_compress(IStream *out_stream, const BYTE *in_data, ULONG in_s out.gif_block.len = 0; out.out = out_stream; - init_code_bits = suffix = 8; + init_code_bits = suffix = max(2, color_bits); if (IStream_Write(out.out, &suffix, sizeof(suffix), NULL) != S_OK) return E_FAIL; @@ -1563,12 +1563,24 @@ static HRESULT WINAPI GifFrameEncode_Commit(IWICBitmapFrameEncode *iface) hr = IStream_Write(This->encoder->stream, gif_palette, sizeof(gif_palette), NULL); if (hr == S_OK) { + int color_bits = 1; + while ((1 << color_bits) < This->colors) color_bits++; + /* Image Data */ - hr = gif_compress(This->encoder->stream, This->image_data, This->width * This->height); + hr = gif_compress(This->encoder->stream, This->image_data, This->width * This->height, color_bits); if (hr == S_OK) This->committed = TRUE; } } + else if (hr == S_OK) + { + int color_bits = 1; + while ((1 << color_bits) < This->encoder->colors) color_bits++; + + hr = gif_compress(This->encoder->stream, This->image_data, This->width * This->height, color_bits); + if (hr == S_OK) + This->committed = TRUE; + } } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9486
Dmitry Timoshkov (@dmitry) commented about dlls/windowscodecs/gifformat.c:
/* Image Data */ - hr = gif_compress(This->encoder->stream, This->image_data, This->width * This->height); + hr = gif_compress(This->encoder->stream, This->image_data, This->width * This->height, color_bits); if (hr == S_OK) This->committed = TRUE; } } + else if (hr == S_OK) + { + int color_bits = 1; + while ((1 << color_bits) < This->encoder->colors) color_bits++; + + hr = gif_compress(This->encoder->stream, This->image_data, This->width * This->height, color_bits); + if (hr == S_OK) + This->committed = TRUE; + }
It should be possible to avoid the code duplication, for instance by writing palette only when necessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_122498
I think this also needs a test. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_122573
I guess that's this part of [the spec](https://www.w3.org/Graphics/GIF/spec-gif89a.txt):
ESTABLISH CODE SIZE
The first byte of the Compressed Data stream is a value indicating the minimum number of bits required to represent the set of actual pixel values. Normally this will be the same as the number of color bits. Because of some algorithmic constraints however, black & white images which have one color bit must be indicated as having a code size of 2. This code size value also implies that the compression codes must start out one bit longer.
Not sure how we would go about testing this. The initial code size is encoded in the stream, so (unless the decoder assumes this value) the file should decode properly regardless. I wouldn't expect it to be visible through WIC. I think the test would have to parse the image data itself. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_122604
On Mon Nov 17 20:39:34 2025 +0000, Esme Povirk wrote:
I guess that's this part of [the spec](https://www.w3.org/Graphics/GIF/spec-gif89a.txt):
ESTABLISH CODE SIZE
The first byte of the Compressed Data stream is a value indicating the minimum number of bits required to represent the set of actual pixel values. Normally this will be the same as the number of color bits. Because of some algorithmic constraints however, black & white images which have one color bit must be indicated as having a code size of 2. This code size value also implies that the compression codes must start out one bit longer. Not sure how we would go about testing this. The initial code size is encoded in the stream, so (unless the decoder assumes this value) the file should decode properly regardless. I wouldn't expect it to be visible through WIC. I think the test would have to parse the image data itself. On the other hand IWICBitmapFrameEncode::SetPixelFormat() sets the format to GUID_WICPixelFormat8bppIndexed, so it's unclear how the LZW compressor would get an image with bpp different from 8.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_122608
On Mon Nov 17 20:39:34 2025 +0000, Dmitry Timoshkov wrote:
On the other hand IWICBitmapFrameEncode::SetPixelFormat() sets the format to GUID_WICPixelFormat8bppIndexed, so it's unclear how the LZW compressor would get an image with bpp different from 8. While "color bits" is never defined, I think it's implied here that this is based on the size of the palette. The decompressed data is always 8 bits.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_122610
On Mon Nov 17 20:49:54 2025 +0000, Esme Povirk wrote:
While "color bits" is never defined, I think it's implied here that this is based on the size of the palette. The decompressed data is always 8 bits. This makes sense, thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_122708
This merge request was closed by Dmitry Timoshkov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486
This should be superseded by https://gitlab.winehq.org/wine/wine/-/merge_requests/9923 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9486#note_127673
participants (6)
-
Dmitry Timoshkov (@dmitry) -
Dmitry Timoshkov (@dmitry) -
Esme Povirk (@madewokherd) -
Jiajin Cui -
Jiajin Cui (@jin-king1) -
Nikolay Sivov (@nsivov)