robert/qoicodec
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix backbuffer test

Browse Source 
The backbuffer indices were all kinds of wrong, and there was even a
state corruption bug.

Hash values were simply done wrong. I've added some value delta notes
to the compressed byte array, and then tracked all the state changes
next to the expected indices array. The last RGBA value also needed to
be included. Off by one, oops.

The state corruption exists as a side effect of malformed OP_INDEXs.
This is actually a bigger issue, and I'll need to add a new test for it.
Basically: A pixel produced by an OP_INDEX will hash to that index.
That's the definition of an OP_INDEX. What happens if an OP_INDEX picks
up a pixel that hashes to a different index? Because an OP_INDEX is
reading from the backbuffer array without modification, it doesn't need
to write. An otherwise valid encoder can then skip the write step. This
causes a missing buffer update when the pixel should land on another
index. The decoder is now in a corrupted state.
This commit is contained in:
Robert Garrett
2024-05-09 14:08:26 -05:00
parent b9aa3cf28f
commit 97ec583442
1 changed files with 23 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
src/main.rs
View File

@@ -509,29 +509,41 @@ mod test {
let compressed = [
QOI_OP_RGB, 0x10, 0x10, 0x10,
QOI_OP_RGBA, 0x20, 0x20, 0x20, 0x20,
QOI_OP_RGBA, 0x03, 0x01, 0x01, 0x04, // filler to populate backbuffer[1] for the next OP_INDEX
(QOI_OP_INDEX | 1),
(QOI_OP_DIFF | 0b0011_1111),
(QOI_OP_LUMA | 0b0011_111), 0b0011_1111,
(QOI_OP_DIFF | 0b0011_1111), // + Pix (1, 1, 1, 0)
(QOI_OP_LUMA | 0b0011_1111), 0b1111_1111, // + Pix (38, 31, 38)
(QOI_OP_RUN | 2),
QOI_OP_RGBA, 0xFF, 0xFF, 0xFF, 0xFF,
];
// these are the indices where we're expecting each pixel to land.
// Each pixel gets put into this backbuffer as it's en/de-coded.
// For RGB and RGBA, it'll simply assign a value into the index.
// For INDEX, the write can be skipped, and the expected index will be
// the same as the one in the op code. By definition, it has to be.
// the same as the one in the op code.
// *however* it's possible for an index to refer to a value that shouldn't
// hash to that location. The backbuffer initialization will cause this
// scenario.
//
// Without an OP_INDEX buffer write, this condition will cause codec corruption.
// If the Indexed value should have landed somewhere else, and that other location
// has a different value in it, then the next operation to reference that field
// will receive the wrong data.
// OP_DIFF & OP_LUMA need to consider the value in the backbuffer, as
// they'll be using it to compute the new pixel.
//
let indices = [
31, // Pix (16, 16, 16, 255)
37, // Pix (16, 16, 16, 255)
0, // Pix (32, 32, 32, 32)
1, // Pix (0, 0, 0, 0)
3, // current state: Pix (1, 1, 1, 0)
38, // current state: Pix(39, 32, 39, 0)
38, // run x1
38, // run x2
38, // run x3
1, // Pix (3, 1, 1, 4)
1, // Pix (3, 1, 1, 4)
16, // Pix (4, 2, 2, 4)
39, // Pix (42, 33, 40, 4)
39, // run x1
39, // run x2
39, // run x3
38, // final RGBA
];
let mut decoder = Decoder::new(compressed.into_iter());
let mut iters = 0;