libsai icon indicating copy to clipboard operation
libsai copied to clipboard

Reading pixel data troubles.

Open therahedwig opened this issue 5 years ago • 22 comments

So, I'd been poking at the example code and trying to see if I could get it running.

There's several parts to decrypting the data:

  1. Parsing through blocks for 32x32
  2. Checking if these blocks are activated.
  3. If active, reading the size of the upcoming rle stream.
  4. reading the RLE stream.
  5. Decoding the RLE stream
  6. manipulating the pixeldata into an appropriate format. (swizzling, converting premultiplied alpha, etc)
  7. pouring final pixel data into final pixelformat container.

1 and 2 have been easy enough, and I suspect 6 and 7(7 works... with undefined colors, but it works...) are easy enough as well... once the rest works.

Sometimes, when reading the RLE stream (step 4), I sometimes get a 'stack_buffer_overflow' asan crash, pointing at the saistream in libsai:

entry.Read(CompressedTile.data(), Size);

#3 0x7fe834021048 in sai::VirtualFileEntry::Read(void*, unsigned long) /home/wolthera/krita/src/plugins/impex/sai/3rdparty/libsai/source/sai.cpp:779

I am not sure if this is happening because the Read() function is buggy, the Readme algorithm is buggy, or because there's missing functionality in VirtualFileEntry to prevent reading out of line. (The Readme algorithm, at the very least, is outdated. It tries to treat VirtualFileEntry::Read as a boolean for the while loop, which doesn't work...)

I also suspect the RLE function given is buggy, given I have frequently gotten similar crashes with the write/DecompressedTile.data() buffer, but I am not sure whether this is because the algorithm doesn't make sense, or that it is being fed the wrong information.

I mean, I guess if I sit down with some graph paper and a big c++ data structures book I'll eventually figure this out, but I was wondering if there's anything obvious you see when looking at your own old notes?

therahedwig avatar Sep 01 '19 19:09 therahedwig

Sometimes, when reading the RLE stream (step 4), I sometimes get a 'stack_buffer_overflow' asan crash, pointing at the saistream in libsai:

Each RLE stream is prefixed with a uint16_t RLESize value that says how long the RLE stream after it is, which determines the next call to .Read(RLEBuffer, RLESize). It's possible there is some kind of mis-alignment going on in the interpretation of these bytes? Such as it reading bytes of the RLE stream and wrongly interpreting it as the size, and calling entry.Read(RLEData, SomeWildNumber).

The RLE function is a standard RLE decompression function, with the only difference really being that there is an channel-offset and 4-byte-stride where it writes its output bytes (Decompressing four separate R, G, B, A streams into a packed RGBA32 format).

Some things to make sure of:


LayerFile.Read(BlockMap.data(), (LayerHead.Bounds.Width / 32) * (LayerHead.Bounds.Height / 32));

Ensure that you are reading the boolean block-map size correctly as it might mis-align your further reads. Use the Bounds field and not the regular width and height! It's a little different:

struct LayerBounds
{
	std::int32_t X; // (X / 32) * 32
	std::int32_t Y; // (Y / 32) * 32
	std::uint32_t Width; // Width - 31
	std::uint32_t Height; // Height - 31
};

Channel++; // Move on to next channel
if( Channel >= 4 ) // skip all other channels besides the RGBA ones we care about
{
	for( std::size_t i = 0; i < 4; i++ )
	{
		std::uint16_t Size = LayerFile.Read<std::uint16_t>();
		LayerFile.Seek(LayerFile.Tell() + Size);
	}
	break;
}

Remember to skip the additional RLE streams after you get your RGBA data!


(The Readme algorithm, at the very least, is outdated. It tries to treat VirtualFileEntry::Read as a boolean for the while loop, which doesn't work...)

This is correct, Read returns a size_t, which will return 0 in the case that there is nothing left to read. It basically says "read until you aren't getting any new data" which I had used back in the day to possibly read masking data using the same procedure.

Wunkolo avatar Sep 01 '19 19:09 Wunkolo

This is correct, Read returns a size_t, which will return 0 in the case that there is nothing left to read. It basically says "read until you aren't getting any new data" which I had used back in the day to possibly read masking data using the same procedure.

This seems to always return 0 at the moment(because it is always reduced to 0 within that function' while loop). I noticed this as well with reading the tags... I am not sure how to check if there's still anything to read right now. Even removing the compression algorithm, I can't even check how many channels are encoded in total in a given block.

I can confirm I am reading the blockmap(otherwise I can't get the active blocks, right, and that part works in coherence with where the inactive/active blocks ought to be on my test files :) )

therahedwig avatar Sep 01 '19 21:09 therahedwig

I think the issue might be that at the time of writing, Read used to return the number of bytes read(return Size), while now(probably in my attempts at fixing that hellish iteration code) it now returns the number of bytes it didn't read(using that passed in Size argument as an in-place iterator) and then does the return Size which it tries to make reach 0. Elsewhere in my code, such as VirtualFileSystem::Read, it returns the number of bytes read. See if this fixes the iteration code

Wunkolo avatar Sep 01 '19 22:09 Wunkolo

Doesn't seem to work. I've pushed my parsing code to the branch.

either I test entry.Read(Size) as equal to 0, and then the while loop is immidiately broken, or I test it as above 0 and then I does go into the while loop, but eventually has a segmentation error.

EDIT: Nvm, found the problem: I had accidentally deleted the 'break' in the channels loop.

Still getting crashes with the huge old file I had emailed you though... All the formal test files seem to work, but not an actual working file... Pushed the changes.

therahedwig avatar Sep 02 '19 11:09 therahedwig

When VirtualFileEntry.Read doesn't die, it works!

But yeah, same huge file doesn't want to cooperate.

therahedwig avatar Sep 02 '19 13:09 therahedwig

I've implemented some basic rearchitecting and added a new sample Document.cpp that dumps layer images, so now I have a proper testbed to get some much cleaner code in and to fix up the bugs you've been running into. And to try and remove the SIMD code and have a more re-usable RLE decompression function and such. image

I've re-created the issue of it being able to work on trivial files, but not on files larger and more complex files such as the 20yo_thera.sai file you sent, particularly the crash around the RLE stream. I'll dig in more tomorrow, but check out some of the new patterns I put in place until then. Eventually, much of this code will settle into the main sai.hpp API.

Wunkolo avatar Sep 03 '19 03:09 Wunkolo

I've been traveling around today, but I did find when the crash starts.

when I create a 2249*2249 file, it won't crash, but a 2250x2250 file will. The raster layers of these files are 2304x2304 with width and height for the first, and 2464x2464 in width and height for the second. Maybe the format somehow changes between these sizes?

I did look at the code, it looks good, but I'll have to think a bit how to unify this with the stuff I've done.

therahedwig avatar Sep 03 '19 18:09 therahedwig

file_crash_at_large_sizes.zip

Here's the testfiles for that, btw.

therahedwig avatar Sep 03 '19 18:09 therahedwig

I replicated similar crashes going on that I'll investigate soon. It seems at some point on the much larger images it crashes from getting an RLE stream size-prefix with an obscene value much greater than 0x1000 which implies misaligned stream reading issues. I'm going to investigate what the "skipped RLE streams" are exactly rather than just skipping an arbitrary amount of RLE streams after the RGBA one: https://github.com/Wunkolo/libsai/blob/23d113074834ef065d7687a99cb6869476fa08a0/samples/Document.cpp#L235-L244

Wunkolo avatar Sep 03 '19 21:09 Wunkolo

Here's what I gathered from reverse engineering that part of the code again, which my code pretty much mirrors. It looks like the RLE streams are being interpreted right but I still don't know what the alternate streams are after the RGBA channels. image

Similarly, here's how masking data is written: image

And depending on the layer type, it looks like the unknown layer type 4 and type 7 write their data in the same format that the masks do.

image

Might be possible that it actually is 16-bits-per-channel and it stores the upper and lower bytes for each element in separately interleaved RLE streams.

Doesn't look like larger files have any special format or anything though, so the bug is still at large.

Wunkolo avatar Sep 04 '19 07:09 Wunkolo

I guess Layers unknown4 and unknown7 may have been intended to be single color or pattern layers. Both are available in other Japanese software like Clip studio paint and AZPainter.

My own investigations have led me to find that...

  • Sai determines layer size by taking ((current image dimension/32)*32+32).
  • An image of 2304 x 2304 leads to layer sizes of 2336 x 2336, and does not crash. File Size is 0.980 MiB
  • An image of 2336 x 2304 leads to a layer size of 2368 x 2336 and segment faults on block 500[56,6] of 5402, RLE Stream Size at 6925, File Size is 23.5 MiB.
  • An image of 2368 x 2304 leads to a layer size of 2400 x 2336, and segment faults on block 477[27, 6] of 5475, RLE Stream Size at 15422, File Size is 23.8 MiB
  • An image of 2249 x 2250 with a fully filled layer, leads to a layer size of 2272 x 2272, segment fault* on block 718[8, 10] of 5041 RLE, Stream Size at 603. File Size is 12.9 MiB
  • An image of 2249 x 2249 with a fully filled layer, leads to a layer size of 2272 x 2272, doesn't segment fault. The only interesting thing I can find here is that the majority of the block stream sizes here is 18, 16, 18, 18, while for all other test files I've got where the layer is filled with one color(including ones that load), the stream sizes are 16, 16, 16, 16. File Size is 0.932 MiB
  • Typically with a fully filled layer the first and last block of a row has completely different stream sizes, with the first row having 100, 100, 100, 100 on the first and last block and subsequent rows having 128, 128, 128, 128 for each block. The first and last block of the last row also have different sizes, but this seems to differ between documents.
  • The images that crash are waaaaaaay bigger, and yes, it is indeed that single raster layer doing the ballooning.

Either there is some kind of encryption nonsense relating to the image size, or there is a bug in sai's saving mechanism... related to the image size somehow???

I'm going to upload my crashing files onto dropbox and will mail you a link, there's a few that are unique, but most of them follow the above routine.

therahedwig avatar Sep 04 '19 13:09 therahedwig

Weirdly enough, I've got another weird crashing file, but this time around it's super tiny. It's a canvas of 512x512, with the different mask options toggled. The first three layers (ids 2, 3 and 8) load fine, and it crashes on layer id 6. But that layer has nothing unique to it...

testfile_09_masks_small.sai.zip

therahedwig avatar Sep 07 '19 15:09 therahedwig

The small one looks like a good test-subject. I'll walk through the file by-hand and see whats up with these RLE streams.

Wunkolo avatar Sep 07 '19 15:09 Wunkolo

And... I am just kinda getting the Document Program to spit out everything... 2250_2250_freezes.sai has data in the lorg tag: Unknown: ffffff83, Unknown4: ffffff83 for example... But outside of the 20yo_thera.sai(22c for Unknown) none of the other crashing testfiles have anything interesting in their lorg tag. I guess we're seeing multiple * peculiarities * in the sai format here.

I'm going to check now if there's anything in the canvas data we're missing regarding these files.

EDIT: The lorg tag is a bust, I'll describe what I've found in the tags thread.

therahedwig avatar Sep 07 '19 15:09 therahedwig

So, I brought up this issue during today's meeting, and Dmitry, the guy who did our PSD support pointed out that PSD has either RLE or ZIP compression. Maybe this is the case as well with sai?

therahedwig avatar Sep 09 '19 12:09 therahedwig

Sai is strictly RLE from what I've seen. The method it uses to write and read the pixel data is all RLE or RLE with a strided offset to fill in RGBA data.

Been a bit distracted with IRL stuff lately but when I used the Decrypt sample to fully decrypt the VFS of one of your smaller sample files, I hand-parsed through the offending layer file using 010-Editor and wasn't finding any issues with my mental model of how they work, leading me to believe it's a logic error in my code somewhere else.

Wunkolo avatar Sep 09 '19 16:09 Wunkolo

  • ...does not crash. ... File Size is 0.980 MiB
  • ...segment faults ... File Size is 23.5 MiB.
  • ...segment faults ... File Size is 23.8 MiB
  • ...segment fault ... File Size is 12.9 MiB
  • ...doesn't segment fault. ... File Size is 0.932 MiB
  • The images that crash are waaaaaaay bigger, and yes, it is indeed that single raster layer doing the ballooning.
  • I've got another weird crashing file, but this time around it's super tiny. It's a canvas of 512x512
  • testfile_09_masks_small.sai, 2.2MB

This just stood out to me

Blocks in the sai VFS are 4096 blocks. And a table-block occurs at every multiple of 512. Meaning every table+data block pair is 4096 * 512 = 2097152 bytes(2MiB).

testfile_09 crashes on layer 00000006. Which is not a large file at all, but is much further down the table+data block chain towards the end of the 2MiB span. I edited Tree.cpp to display the VFS block index where a file resides

├── [          32 09/07/19 07:13] #01.73851dcd1203b24d (Block Index: 3)
├── [          56 09/07/19 07:13] canvas (Block Index: 4)
├── [          36 09/07/19 07:13] laytbl (Block Index: 5)
├── [         256 09/07/19 07:13] layers (Block Index: 7)
│   ├── [      826840 09/07/19 07:13] 00000008 (Block Index: 9)
│   ├── [     1006035 09/07/19 07:13] 00000002 (Block Index: 211)
│   ├── [      148971 09/07/19 07:13] 00000003 (Block Index: 457)
│   ├── [      100818 09/07/19 07:13] 00000006 (Block Index: 494) << Crashes here!!
├── [          36 09/07/19 07:13] subtbl (Block Index: 6)
├── [         256 09/07/19 07:13] sublayers (Block Index: 8)
│   ├── [        9546 09/07/19 07:13] 00000009 (Block Index: 520)
│   ├── [       40334 09/07/19 07:13] 00000004 (Block Index: 523)
│   ├── [       17357 09/07/19 07:13] 00000005 (Block Index: 533)
│   ├── [       15379 09/07/19 07:13] 00000007 (Block Index: 538)
├── [       65548 09/07/19 07:13] thumbnail (Block Index: 542)

I believe the crash happens whenever there is a table-block boundary, when a file is positioned and sized in such a way for it to cross a table-block boundary and spans across two separate table-block domains(Uses up the 2MiB of one table block, and then starts again in another).

This would imply the issue is here, unfortunately 😔: https://github.com/Wunkolo/libsai/blob/17062462522d740713a4fef0649ae2a859ded7b3/source/sai.cpp#L683-L727

I'll have a dive sometime soon to finally sit down and do some real discrete mathematics to fix this up.

Wunkolo avatar Sep 09 '19 17:09 Wunkolo

I'm elated to hear you've found the likely cause even if the fix is super fiddly(though I am still somewhat confused how the sai format can just jump between 0.9 mb and 12.5 mb despite a single pixel in dimension difference, but whatever, if it loads it loads).

I'll proly spent coming week on getting everything loaded that I can load (which is... everything but huge raster layers and vector layers :) ) and maybe start making files for unittests(which would be like my current test files but teensy), so take your time for now.

therahedwig avatar Sep 09 '19 18:09 therahedwig

Did some research and found that the Flags field of the table-pages actually are block indexes of where the "next chunk" of a file is located like a singly linked list, meaning all the math is actually already pre-calculated and files have just "happened" to be sequential next to each other. This also means seeks are more expensive. Currently running into a different class of RLE stream issues as I get it up to speed with this new pattern. I've updated the readme documentation a bit to describe this

https://github.com/Wunkolo/libsai/blob/d0fbcace4cfdd443c32e5279d7b29a8623525749/source/sai.cpp#L785-L790

Wunkolo avatar Sep 22 '19 02:09 Wunkolo

Still trying to fix this issue. WHile I have updated the code into the "linked list" nature of the Table-Pages I'm still running into issues and I'm trying to hand-decipher some small sample files to ensure that my reading routine is correct. It matches up pretty 1:1 with the decompiled IDA-Pro code but I am still running into issues during the RLE stream issues when any single file spans across table-spans(needs multiple table-pages to describe it, any file within the Virtual File System bigger than 4096*511 bytes).

Wunkolo avatar Feb 01 '20 05:02 Wunkolo

Thanks for the update!

therahedwig avatar Feb 01 '20 14:02 therahedwig

Still looking into this. I've reverse engineered Sai's own reading routine and I'll probably just aim to mimic the patterns seen here in my own code. Except for the part where Sai uses it's cache, an optimization ill be doing at some point. image I figured I'm basically doing the same thing here but I'll look more into it. https://github.com/Wunkolo/libsai/blob/607ec7acb817b52a4d2bc2682fcd07c0a78b4f19/source/sai.cpp#L752-L794 Reverse engineering Sai from the source rather than spending more time in 010 editor will do more good on this issue.

Wunkolo avatar Feb 05 '20 03:02 Wunkolo

Issue is now resolved by #16!

Wunkolo avatar Apr 11 '23 03:04 Wunkolo