diff --git a/addons/twitcher/media/native/gif-lzw/lzw.gd b/addons/twitcher/media/native/gif-lzw/lzw.gd index d07626e2..0884c7fa 100644 --- a/addons/twitcher/media/native/gif-lzw/lzw.gd +++ b/addons/twitcher/media/native/gif-lzw/lzw.gd @@ -4,6 +4,12 @@ extends RefCounted var lsbbitpacker = preload("./lsbbitpacker.gd") var lsbbitunpacker = preload("./lsbbitunpacker.gd") +## GIF89a caps LZW codes at 12 bits, so the code table never grows past 4096 +## entries. [method compress_lzw] already honours this by emitting a Clear Code +## instead of adding entry 4096; the decompressor has to stop widening at the +## same point or it desynchronises from the encoder. +const MAX_CODE_SIZE: int = 12 + class CodeEntry: var sequence: PackedByteArray var raw_array: PackedByteArray @@ -206,8 +212,17 @@ func decompress_lzw(code_stream_data: PackedByteArray, min_code_size: int, color prevcode = code # Detect when we should increase current code size and increase it. + # + # The clamp to MAX_CODE_SIZE is load bearing. The decoder trails the + # encoder by exactly one table entry, so it reaches counter == 4096 while + # reading the last code the encoder wrote before it gave up and emitted a + # Clear Code. get_bits_number_for(4096) is 13, so without the clamp the + # next read takes 13 bits out of a stream still written in 12 — every + # code after that is shifted, a Clear Code is mis-detected, and the reader + # eventually asks for a code past the end of its own table. That surfaces + # as a null CodeEntry a few lines up. var new_code_size_candidate: int = get_bits_number_for(code_table.counter) if new_code_size_candidate > current_code_size: - current_code_size = new_code_size_candidate + current_code_size = mini(new_code_size_candidate, MAX_CODE_SIZE) return index_stream diff --git a/test/fixtures/gif/README.md b/test/fixtures/gif/README.md new file mode 100644 index 00000000..21cc676b --- /dev/null +++ b/test/fixtures/gif/README.md @@ -0,0 +1,21 @@ +# GIF fixtures + +## `lzw_full_code_table.gif` + +160x125, one frame, 256-entry greyscale global colour table, no transparency. + +Generated rather than captured. The pixel indices come from the LCG +`state = (state * 1103515245 + 12345) & 0x7FFFFFFF`, seeded at `12345`, taking +`(state >> 16) & 0xFF` — 20 000 near-random bytes. Incompressible input is the +whole point: a real-world image finds repeats early and never fills the LZW code +table, which is why this bug survived until a 318-frame 7TV emote hit it +(issue #132). The stream was then encoded by Twitcher's own `compress_lzw`, so +it is a stream the codebase must be able to read back. + +It is a valid GIF89a — Pillow decodes it and reproduces the index stream exactly. +Decoding it drives the LZW code table to its full 4096 entries and back through +six mid-stream Clear Codes, which is the boundary +`decompress_lzw` used to mishandle. + +`test_gif_reader.gd` regenerates the index stream from the same LCG, so the +fixture and its expected output cannot drift apart. diff --git a/test/fixtures/gif/lzw_full_code_table.gif b/test/fixtures/gif/lzw_full_code_table.gif new file mode 100644 index 00000000..30ac17d8 Binary files /dev/null and b/test/fixtures/gif/lzw_full_code_table.gif differ diff --git a/test/fixtures/gif/lzw_full_code_table.gif.import b/test/fixtures/gif/lzw_full_code_table.gif.import new file mode 100644 index 00000000..7d08f04f --- /dev/null +++ b/test/fixtures/gif/lzw_full_code_table.gif.import @@ -0,0 +1,13 @@ +[remap] + +importer="gif.animated.texture.plugin" +type="SpriteFrames" +uid="uid://b87setm3use05" +valid=false + +[deps] + +source_file="res://test/fixtures/gif/lzw_full_code_table.gif" + +[params] + diff --git a/test/unit/media/native/gif-lzw/test_lzw.gd b/test/unit/media/native/gif-lzw/test_lzw.gd new file mode 100644 index 00000000..b84291df --- /dev/null +++ b/test/unit/media/native/gif-lzw/test_lzw.gd @@ -0,0 +1,77 @@ +## Unit tests for the vendored GIF LZW codec. +## +## This file comes from [url=https://github.com/jegor377/godot-gdgifexporter]gdgifexporter[/url] +## and is the only piece of Twitcher that has to agree, bit for bit, with an +## encoder it does not control. The interesting failures are all boundary +## conditions in the flexible code size, so that is what these tests pin. +extends TwitcherTest + +const LZW_SCRIPT := preload("res://addons/twitcher/media/native/gif-lzw/lzw.gd") + +var _codec: RefCounted + + +func before_each() -> void: + super() + _codec = LZW_SCRIPT.new() + + +## A 256-entry palette, which is what every GIF frame Twitcher decodes uses. +func _palette() -> PackedByteArray: + var colors := PackedByteArray() + for index: int in 256: + colors.append(index) + return colors + + +## Near-random bytes from a fixed LCG. Incompressible input is the point: a +## smooth gradient finds repeats early and never fills the code table, so it +## would sail past the boundary these tests exist to cover. The seed is fixed so +## a failure is reproducible rather than flaky. +func _noise(count: int) -> PackedByteArray: + var out := PackedByteArray() + var state := 12345 + for _step: int in count: + state = (state * 1103515245 + 12345) & 0x7FFFFFFF + out.append((state >> 16) & 0xFF) + return out + + +func _round_trip(indices: PackedByteArray) -> PackedByteArray: + var colors := _palette() + var compressed: Array = _codec.compress_lzw(indices, colors) + return _codec.decompress_lzw(compressed[0], compressed[1], colors) + + +func test_round_trips_a_short_stream() -> void: + var indices := PackedByteArray([1, 1, 1, 2, 2, 3, 1, 1, 1, 2, 2, 3, 4]) + assert_eq(_round_trip(indices), indices) + + +func test_round_trips_a_stream_that_never_fills_the_code_table() -> void: + var indices := _noise(2000) + assert_eq(_round_trip(indices), indices) + + +## The regression test for the 12-bit ceiling. +## +## GIF89a caps LZW codes at 12 bits, so the code table stops at 4096 entries. +## [code]compress_lzw[/code] has always honoured that — it emits a Clear Code +## rather than adding entry 4096 — but [code]decompress_lzw[/code] used to widen +## straight off its own counter. The decoder trails the encoder by exactly one +## entry, so it hit counter == 4096 while reading the last code before that +## Clear, computed a 13-bit code size, and started reading 13 bits out of a +## stream still written in 12. Every code after that was shifted: a Clear Code +## was mis-detected, the table reset, and the reader then asked for a code past +## the end of it. In the editor that faulted with "Nonexistent function 'add' in +## base 'Nil'"; in a release build [code]decompress_lzw[/code] simply abandoned +## the frame and handed back a short buffer, which failed much later and much +## further away. +## +## 20 000 near-random indices fill and clear the table several times over, so +## this covers the boundary in both directions. +func test_round_trips_a_stream_that_fills_the_code_table() -> void: + var indices := _noise(20000) + var restored := _round_trip(indices) + assert_eq(restored.size(), indices.size(), "decode must not stop short") + assert_eq(restored, indices) diff --git a/test/unit/media/native/gif-lzw/test_lzw.gd.uid b/test/unit/media/native/gif-lzw/test_lzw.gd.uid new file mode 100644 index 00000000..bd447300 --- /dev/null +++ b/test/unit/media/native/gif-lzw/test_lzw.gd.uid @@ -0,0 +1 @@ +uid://dd0mb0itdhf23 diff --git a/test/unit/media/native/test_gif_reader.gd b/test/unit/media/native/test_gif_reader.gd new file mode 100644 index 00000000..fd1cb0f4 --- /dev/null +++ b/test/unit/media/native/test_gif_reader.gd @@ -0,0 +1,56 @@ +## End-to-end test for [GifReader] over a GIF that fills the LZW code table. +## +## [code]test_lzw.gd[/code] pins the codec directly. This suite covers the path +## that actually broke in the wild: a 7TV emote large enough to reach the 12-bit +## code ceiling would make [method GifReader.load_gif] return a [SpriteFrames] +## whose [code]get_frame_count()[/code] looked healthy while the frame textures +## were empty, so the failure surfaced far away from its cause. +extends TwitcherTest + +const FIXTURE := "res://test/fixtures/gif/lzw_full_code_table.gif" +const WIDTH := 160 +const HEIGHT := 125 + + +## Regenerates the pixel indices the fixture was built from. Same LCG and seed as +## the generator, so the assertion below is exact rather than a smoke test. +func _expected_indices() -> PackedByteArray: + var out := PackedByteArray() + var state := 12345 + for _step: int in WIDTH * HEIGHT: + state = (state * 1103515245 + 12345) & 0x7FFFFFFF + out.append((state >> 16) & 0xFF) + return out + + +func test_decodes_a_gif_that_fills_the_lzw_code_table() -> void: + var reader := GifReader.new() + var frames: SpriteFrames = reader.read(FIXTURE) + + assert_not_null(frames, "fixture missing or unreadable: %s" % FIXTURE) + if frames == null: + return + assert_eq(frames.get_frame_count(&"default"), 1) + + var texture: Texture2D = frames.get_frame_texture(&"default", 0) + assert_not_null(texture, "a decode that fails mid-frame leaves a null texture") + if texture == null: + return + + var image: Image = texture.get_image() + assert_eq(Vector2i(image.get_width(), image.get_height()), Vector2i(WIDTH, HEIGHT)) + + # The fixture's palette is grey ramp (i, i, i), so the red channel of a + # decoded pixel is the palette index the encoder wrote. + var expected := _expected_indices() + var mismatches := 0 + var first_mismatch := "" + for y: int in HEIGHT: + for x: int in WIDTH: + var actual: int = image.get_pixel(x, y).r8 + var want: int = expected[y * WIDTH + x] + if actual != want: + mismatches += 1 + if first_mismatch == "": + first_mismatch = "(%d, %d): got %d, want %d" % [x, y, actual, want] + assert_eq(mismatches, 0, "pixel mismatches, first at %s" % first_mismatch) diff --git a/test/unit/media/native/test_gif_reader.gd.uid b/test/unit/media/native/test_gif_reader.gd.uid new file mode 100644 index 00000000..7c646468 --- /dev/null +++ b/test/unit/media/native/test_gif_reader.gd.uid @@ -0,0 +1 @@ +uid://7ro5fqigpw4k