Multiple bugs with PNG cartridges #2042
Description
Hi,
Very cool console you have! I like it very much! I only have issues with the PNG files. The way how TIC-80 currently handles them is unacceptable for several reasons:
- First, it's not supported everywhere, only in studio and in surf (but not from command line for example).
- Second, it does not work on systems with case-sensitive file systems (due to the LoadPngCart call depends on file extension!)
- Third, and this is the biggest problem, a real no-go for me, is the way how TIC-80 embeds data generates CORRUPT png files! Tools are refusing to resize the image because of that, or worse, they might corrupt the cartridge data silently.
Here's a proposed solution that fixes all of these problems at once and as a side effect improves performance a lot. This renders the use of LoadPngCart completely unnecessary, it Just Works(tm) and gets the cartridge from a png file without actually decoding the data:
diff --git a/src/cart.c b/src/cart.c
index 9be32c7..1c6079d 100644
--- a/src/cart.c
+++ b/src/cart.c
@@ -87,6 +87,28 @@ void tic_cart_load(tic_cartridge* cart, const u8* buffer, s32 size)
#define LOAD_CHUNK(to) memcpy(&to, ptr, MIN(sizeof(to), chunk->size ? retro_le_to_cpu16(chunk->size) : TIC_BANK_SIZE))
+ // check if the cartridge buffer is a PNG file
+ if (!memcmp(buffer, "\x89PNG", 4))
+ {
+ s32 siz;
+ const u8* ptr = buffer + 8;
+ while (ptr < end)
+ {
+ siz = ((ptr[0] << 24) | (ptr[1] << 16) | (ptr[2] << 8) | ptr[3]) + 12;
+ if (!memcmp(ptr + 4, "caRt", 4))
+ {
+ buffer = ptr + 8;
+ size = siz;
+ end = buffer + size;
+ break;
+ }
+ ptr += siz;
+ }
+ // error, no TIC-80 cartridge chunk in PNG???
+ if (ptr >= end)
+ return;
+ }
+
// load palette chunk first
{
const u8* ptr = buffer;The advantage is, this patch is extremely simple, dependency-free and it does not require to decode the png at all! It just iterates through png chunks without interpreting them.
I've chosen the chunk magic to be caRt. This isn't some foolishness, rather selected in accordance to the PNG Specification section 9.8. "Use of private chunks", and means ancillary, private, safe-to-copy chunk.
As for creating such VALID png files, here's an extremely simple command line tool to do that (I've written it deliberately in a way so that you can reuse the tic2png function in TIC-80). Again, this is totally dependency-free (note no png libraries used!), and does not require to decode nor to encode png data, all it does is just manipulating png chunks. It simply inserts a caRt chunk with the cartridge data as the last chunk (well, the one before the IEND chunk). It also takes care of the case if the png already had a caRt chunk.
/*
* tic2png.c
*
* Copyright (C) 2022 bzt (bztsrc@gitlab) MIT license
*
* @brief Small dependency-free tool to embed a .tic cartridge into a .png file
* Compilation: gcc tic2png.c -o tic2png
*/
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
/* the png specification is very strict, *do not* change uppercase/lowercase */
#define TICMAGIC "caRt"
/**
* Add a tic file into a png chunk
*/
int tic2png(uint8_t *tic, size_t ticsize, uint8_t **png, size_t *pngsize)
{
int i, j;
uint32_t crc, crc_table[256], size;
uint8_t *buf, *in, *out, *end, *ptr = tic;
size_t s;
if(!tic || !png || !*png || memcmp(*png, "\x89PNG", 4) || !pngsize || *pngsize < 8+12+12) return 0;
end = *png + *pngsize;
buf = malloc(*pngsize + ticsize + 12);
if(!buf) return 0;
s = 8; in = *png; out = buf; memcpy(out, in, 8); in += 8; out += 8;
/* copy all chunks, except tic chunk */
while(in < end - 12 && memcmp(in + 4, "IEND", 4)) {
size = ((in[0] << 24) | (in[1] << 16) | (in[2] << 8) | in[3]) + 12;
if(memcmp(in + 4, TICMAGIC, 4)) { memcpy(out, in, size); out += size; s += size; }
in += size;
}
/* add new tic chunk */
s += 4;
*out++ = (ticsize >> 24) & 0xff; *out++ = (ticsize >> 16) & 0xff; *out++ = (ticsize >> 8) & 0xff; *out++ = (ticsize >> 0) & 0xff;
for(i = 0; i < 256; i++) {
crc = (uint32_t)i; for(j = 0; j < 8; j++) { crc = crc & 1 ? 0xedb88320L ^ (crc >> 1) : (crc >> 1); crc_table[i] = crc;
}
crc = 0xffffffff;
for(i = 0; i < 4; i++, s++, out++) { *out = TICMAGIC[i]; crc = crc_table[(crc ^ *out) & 0xff] ^ (crc >> 8); }
for(; ticsize; ticsize--, s++, ptr++, out++) { *out = *ptr; crc = crc_table[(crc ^ *out) & 0xff] ^ (crc >> 8); }
crc ^= 0xffffffff; s += 4;
*out++ = (crc >> 24) & 0xff; *out++ = (crc >> 16) & 0xff; *out++ = (crc >> 8) & 0xff; *out++ = (crc >> 0) & 0xff;
/* add end chunk */
memcpy(out, in, 12);
s += 12;
/* replace original png buffer with the new one */
free(*png);
*png = buf;
*pngsize = s;
return 1;
}
/**
* Read in one file into memory
*/
uint8_t *readfile(char *fn, size_t *size)
{
FILE *f;
uint8_t *buf = NULL;
f = fopen(fn, "rb");
if(f) {
fseek(f, 0, SEEK_END);
*size = ftell(f);
fseek(f, 0, SEEK_SET);
buf = malloc(*size);
if(!buf) { fclose(f); fprintf(stderr, "tic2png: unable to allocate memory\r\n"); exit(1); }
fread(buf, 1, *size, f);
fclose(f);
} else {
fprintf(stderr, "tic2png: unable to read %s\r\n", fn);
exit(1);
}
return buf;
}
/**
* Main procedure
*/
int main(int argc, char **argv)
{
FILE *f;
int ret;
size_t ticsize, pngsize, outsize;
uint8_t *tic, *png, *out;
/* check arguments */
if(argc < 3) {
printf("tic2png - by bzt MIT\r\n\r\n %s <.tic file> <.png file>\r\n", argv[0]);
return 1;
}
/* read in files */
tic = readfile(argv[1], &ticsize);
png = readfile(argv[2], &pngsize);
/* add tic to png */
if(!tic2png(tic, ticsize, &png, &pngsize)) {
fprintf(stderr, "tic2png: unable to add png chunk\r\n");
return 1;
}
/* save new png file */
f = fopen(argv[2], "wb");
if(f) {
fwrite(png, 1, pngsize, f);
fclose(f);
} else {
fprintf(stderr, "tic2png: unable to write %s\r\n", argv[2]);
return 1;
}
return 0;
}All these are licensed under MIT, feel free to use them in TIC-80.
Cheers,
bzt
Metadata
Metadata
Assignees
Labels
Projects
Milestone
Relationships
Development
Participants
Activity
joshgoebel commentedon Dec 5, 2022
Could you please provide additional details here? This should not be the case. The algorithm that we use to embed the cartridge data in the 6 least significant bits of a 24-bit image should not create corrupt files.
bztsrc commentedon Dec 5, 2022
Sure. Try running TIC-80 on Linux, and have cartridges with
.PNG,.Pngand.pngextension. Won't work.That's PIOC-8, and not how TIC-80 works. TIC-80 stores 3 bits per channel (which is, unlike 2 bits noticeable by naked eye), and furthermore into a 32-bit image. Even if you don't resize the image (which obviously corrupts data silently), tools might decide to simply clear the RGB channel or just store random bytes there if the A channel is 0 (because in that case the pixel won't be visible anyway), meaning you'll get data corruption in the cartridge data at random bytes (believe me, I had to debug such a steganography bug, you wouldn't want to do that, it's a particularly nasty bug to catch).
There are other serious issues with this method too: you have to encode and decode the entire PNG image all the time, which is time consuming and wastes precious resources. Also your storage capacity is seriously limited, and TIC-80 already run out of that (hence the need for that hackish, already obsoleted CHUNK_CODE_ZIP, but if you really want to store wasm bytecode in CHUNK_BINARY chunks, no storage space will be enough).
And doing this makes absolutely no sense: the PNG Specification is pretty clear that you're allowed to put private chunks into images, these will be kept by various tools and editors (thanks to the safe-to-copy bit, so resizing, alpha etc. isn't a problem any more because cartridge data will be kept untouched), plus it allows storing cartridges up to 2^31 bytes, meaning all that Lua compression hassle can be easily avoided, and there's plenty of space for wasm bytecode too.
I understand that PICO-8 uses stegano, but that's not a reason to keep using a bad solution in TIC-80, right? Especially when the simpler solution so much much better (smaller code, lot faster, more stupid-proof).
Cheers,
bzt
joshgoebel commentedon Dec 5, 2022
Same concept, different number of bits. :-) Your original message makes it sound like WE save corrupt PNG files that other tools can't read... what you're saying here is that other tools can CORRUPT our hidden data, which is an entirely different matter.
I think that's just a price you pay for doing it the "cool" way... not a problem on most newer hardware.
This is a much better point to me... I was wondering where all that extra data was going to go...
It might also be this is merely a carry over from the GIF days...did GIFs have data channels?
Some tools do strip these without asking, but I do agree for many cases binary data might be safer here than hidden in the main image...
I don't have a super strong opinion here. I think the existing encoding concept is kind of cool and the retro brain of mine lines the idea of "official" carts... once you start opening the carts with Photoshop and saving them, all bets are off... that's not an official cart anymore - one shouldn't expect it to work.
What kind of PNG do we save now if someone has 256kb of code or a 256kb BINARY chunk?. Are you saying it simply blows up and doesn't work. that we're already over the limit?
joshgoebel commentedon Dec 5, 2022
256 x 256 * 32 bit = 256kb total, ugh...
joshgoebel commentedon Dec 5, 2022
Your first two issues definitely sound like bugs/deficiencies we should fix, the last would really need a decision by @nesbox. But it does sound like we at least need a much larger cartridge bitmap if we're going to keep encoding data inside the graphic. So that alone may settle things.
bztsrc commentedon Dec 5, 2022
I think simply looking for PNG magic instead of the file extension would do the trick.
See? But actually worse. 240 x 136 and one byte per pixel only. That's 32640 bytes, no more, hardly enough for a single CHUNK_CODE bank...
Agreed. This is a change in png cartridge format that breaks compatibility (although it would be possible to use both solutions: if there's a
caRtchunk, use that, otherwise fallback to the stegano method. On save only use the new chunk method. This way old cartridges would keep working.)Cheers,
bzt
joshgoebel commentedon Dec 5, 2022
Not sure what you mean, our cartridge PNGs are 256x256... and I thought you said 32-bit (24-bit + alpha)... so that's 256kb total (before storing any images, of course even less if we count available for encoding)... still very little though, not sure how it'd be working now for larger carts... maybe people just use .TIC and not .PNG?
The project is very into backwards compatibility, so that complexity probably isn't fully going away anytime soon even if we switched.
bztsrc commentedon Dec 5, 2022
I can only see 240 x 136 images here and here. Am I looking at the wrong place?
Not sure how it works exactly, but with one cart byte per carriage pixel that's 64k, and if all 3 bits used in every channel, then 2562564*3/8 = 98k. But hard to tell, the code is extremely messy. Why there is a loop and a trinary? It should be as simple as
pixelchannel = (pixelchannel & ~mask) | (newbits & mask), where mask is 0b11 or 0b111. Anyway, still very little, not going to be future proof, agreed.That's okay. If new cartridges are saved with chunks instead of stegano, it would be perfectly fine with me.
Cheers,
bzt
joshgoebel commentedon Dec 6, 2022
That's just a screen shot, not a cartridge. We had a whole thread about it: #755
There is some way to export cartridge PNGs...
bztsrc commentedon Dec 6, 2022
Yeah, also someone really should convert all those screenshots on the website into png cartridges.
Anyway, here's a patch that adds
caRtchunk support to TIC-80.Unrelated: I think src/studio/screens/console.c should be refactored a bit, on save it encodes / decodes the png multiple times without any good reason.
Cheers,
bzt
bztsrc commentedon Dec 6, 2022
Concerning that, here's a patch (git diff fails here because of the indentation, actually the change in onLoadCommandConfirmed is straightforward and a lot smaller than the diff suggests):
Cheers,
bzt
bztsrc commentedon Dec 7, 2022
@joshgoebel
I've added a neat feature to my converter. Besides of converting PICO-8 cartridges, it can now convert between
.ticand.tic.pngback and forth. It is blasing fast, just try feeding any .tic to the web based converter. But when used as a CLI tool, then you can mass-convert any number of.ticfiles into PNGs in batch.Here are some examples from the website that I've converted. Should be 100% pixel correct to the version saved by the TIC-80 console:
These are all containing both
caRtchunk and steganography encoded data, so fully backward and forward compatible.Cheers,
bzt
bztsrc commentedon Dec 8, 2022
I've added a PR for this.
git pushdoesn't work for me on github, so I had to use the webeditor, pity me!Anyway, there's no reason to keep this ticket open, but @nesbox I'd like to hear your opinion. I've created a PR and a tool to mass convert cartridges, what else is needed to make this happen?
Cheers,
bzt