-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some basic questions about usage #47
Comments
Thanks, it definitely did take a while 🙂
Yeah, it's just calling out that Lines 572 to 587 in 60576e5
It's always rgba, the palette is used to decode and pass out the image in a standard. The palette is also available in the API as a potentially useful feature for image editors. Luna Paint lets you inspect an image for example:
The decoded pixel data gets transferred to a regular png-codec/src/decode/chunks/chunk_IDAT.ts Line 246 in 60576e5
I think JS uses LE by default?
PNGs can be encoded with 64 bits per pixel, if you don't handle 64 bit images, you can use Line 21 in 60576e5
I wouldn't expect you to want to do this for terminal images.
I tried to make the encode API as simple as possible to a noob 😉, you will probably just want to encode without any options and some light analysis will happen to pick the right bit depth, color type, etc.
The only reason the promises exist is for code splitting:
It only works off a full UInt8Array currently, I'm not sure stream decoding would give much since pngs are typically tiny? I considered progressive decoding but it seems really overkill considering the small size of pngs and the large bandwidth of most internet connections (plus my use case works off local or remote without streaming in vscode).
I'm guessing you mean #10? Yeah I'll find the motivation to do this at some point. I actually don't expect it to be that tough to decode APNGs, the hard parts are making a nice multi-image API that I'll want to use for future libs (gif?) and the UX side in Luna Paint would need a bit of work. |
Ah right, I did it likewise in the sixel lib and exposed the palette for whatever usage. Only issue there - sixels are colored immediately (printer mode), which might be misused by hi-res encoders to shift palette colors to get more colors in. Which makes the palette unstable during decoding and limits its usage afterwards (was not keen to create a history preserving data structure just for that). Thats also one of the reasons, why I dont want to re-encode a former sixel image back to sixel format for serialization (others being bad compression/quality ratio and high CPU consumption).
Well webassembly is always LE, JS uses native endianess. Learnt that the hard way during wasm adoption for the sixel lib, and basically removed all BE stuff, as the wasm-LE <--> js-BE bridging became a code nuisance. Imho not addressing BE arch is a safe bet for xterm.js, as there is not a single desktop grade BE system anymore. I think ibms POWER arch is the last serious BE domain. Though I find it weird, that LE won the race - BE is for example alot easier in dynamic memory handling for vector processing.
Ah right, overlooked that in the api file.
Sweet, thats all I need I guess. @stream decoder @decoding/promise So thanks for your detailed response. |
@Tyriar I did some first perf tests in a dummy project just testing decoding of a 6000x4000 pixel image 10 times with these results (numbers in msec):
The browser method uses the To get abit more insight some profiling data:
vs. upng:
vs. sixel:
For browser decoding I cannot show you more detailed profiling data, as devtools just shows |
Interesting, good to know! Tweaking perf hasn't been a big focus yet as it's been fast enough in my experience, 3 seconds seems like a lot but that's for 24 megapixels which a png would never be used for in practice, going down to 2.4 megapixels is still a crazy size for a png and that's down to ~300ms. I also have the option of doing this in a worker if needed. Since I haven't needed to focus on perf yet I expect there's probably some easy wins, like pulling variable declarations to the top of a function to reduce GC here and here, bit math tricks, etc. I also notice for upng most time was spent in |
Well I created it in chrome from the canvas back by blob export. Is there a way to check its compression level? This is what imagemagick has to say about the image:
Thats alot of stats, dunno where the real compression hides here. And yes, the image is quite big with 19MB, the sixel original is 25MB. |
Well imagemagick does not contain compression level, as stated here https://legacy.imagemagick.org/discourse-server/viewtopic.php?t=32574
Does that mean, it is uncompressed? The image has highly different colors, could it be that the browser decided not to compress it because of high entropy? Edit: Edit2: Edit3:
|
A first optimization possibility I have found is function convert16BitTo8BitData(data) {
const result = new Uint8Array(data.length);
for (let i = 0; i < result.length; i += 4) {
result[i+0] = data[i+0] >> 8;
result[i+1] = data[i+1] >> 8;
result[i+2] = data[i+2] >> 8;
result[i+3] = data[i+3] >> 8;
}
return result;
} conversion throughput
Furthermore this task would highly benefit from a wasm rewrite, as I'd expect it to boost throughput above 1GB/s (scalar), or with wasm-simd its a single To make this more explicit - conversion of 100M channel values (== 25M pixels in RGBA) takes:
Edit: Found another faster version in JS: function convert16BitTo8BitData(data, buffer) {
// FIXME: add proper tail handling
const result = buffer
? new Uint32Array(buffer, 0, data.length / 4)
: new Uint32Array(data.length / 4);
const view = new Uint32Array(data.buffer);
let t1, t2;
for (let i = 0, j = 0; i < result.length; i += 4, j += 8) {
t1 = view[j+0], t2 = view[j+1];
result[i+0] = (t1 & 0xFF00) >> 8 | (t1 & 0xFF000000) >> 16 | (t2 & 0xFF00) << 8 | (t2 & 0xFF000000);
t1 = view[j+2], t2 = view[j+3];
result[i+1] = (t1 & 0xFF00) >> 8 | (t1 & 0xFF000000) >> 16 | (t2 & 0xFF00) << 8 | (t2 & 0xFF000000);
t1 = view[j+4], t2 = view[j+5];
result[i+2] = (t1 & 0xFF00) >> 8 | (t1 & 0xFF000000) >> 16 | (t2 & 0xFF00) << 8 | (t2 & 0xFF000000);
t1 = view[j+6], t2 = view[j+7];
result[i+3] = (t1 & 0xFF00) >> 8 | (t1 & 0xFF000000) >> 16 | (t2 & 0xFF00) << 8 | (t2 & 0xFF000000);
}
return new Uint8Array(result.buffer, 0, data.length);
} Runs the test above in 170 ms (~580 MB/s), or in 125 ms (~800 MB/s) with inplace writing (avoids new memory alloc by setting the original data buffer as Edit2 - some wasm numbers:
|
The image magick output doesn't say anything about filters, the filter selection is the hardest part with optimizing pngs and if it's using filter none primarily (ie. just raw pixel data), it's probably not very optimized.
This is arguably not too important generally, I find 16-bit (ie. 64-bit per channel) png files are fairly rare. The TS change looks great though, want to make a PR? Testing is very comprehensive so if the tests pass I'm very confident nothing broke 🙂
Looks like it comes out to 8.7mb when saving with Luna Paint which seems pretty good, not sure how many colors it has as Luna Paint is struggling to inspect the file (lunapaint/vscode-luna-paint#145). Do you mean png indexed as in you needed to reduce it to 256 colors?
I haven't done much with wasm but I worry about the complexity it will bring both in the implementation and in consuming the library (where do the wasm files go, do they place nicely with bundlers, etc.), I always had issues when I tried to setup emscripten for example. |
Ic. Well I used the browser to create it, and it was pretty fast compared to what gimp did (<2s vs. >20s), prolly it just dumped pixel data with little additional effort. With is nice later on for term serialization because its fast, but at the same time created pngs are overly big which is bad when loading a serialize state back into the terminal. Hmm.
Oh I know, its just the first function I stumbled over, and was a quite low hanging fruit, too. I did not even bother yet to look at more involved code paths as from the profiling data (those take much more time than a sunday afternoon). Sure I can create a PR (but need to write the tail handling first 😸).
Yes 8.7MB is okish, that what I got from image magick's
Can set up a toy project with the wasm impl for |
Luna Paint uses the recommended quick filter selection per line as I think is recommended by the spec. More involved methods try to guess which filters would be better encoded by zlib by hopefully doing less than brute forcing every option
Yeah, Windows is always the tricky part 😄
Sure, with that I can answer those questions about setup/bundling. |
Oh well, this might explain why gimp took so long maybe brute forcing things. Early repo is up here: https://github.com/jerch/wasm-dummy (def. not yet working on windows, macos might work though). Gonna close the issue, as I have enough info to get something rolling with the lib. |
@Tyriar Added an early wasm generator, which should have proper TS typing and creates an inline wasm module. Works already for very simple source code. 😸 (see https://github.com/jerch/wasm-dummy/blob/a77bf141cb2720037ffbc4259ad520dc2f362bc3/src/inline.ts#L26) |
Never knew inline wasm was a thing. I'll have to play around with this some upcoming weekend, thanks 🙂 |
Oh well - real in-lining across all stages is not possible, as wasm needs (foreign) compilation. The generator is a source compiler, that has to run at a certain build step and replaces things with wasm bootstrap logic, which is fully embedded in the final bundle though. |
Thought that's how it was working after having a little look. Still might be nice to keep the wasm source close if it's just a small function. |
Yes I think so, too. It makes things much more straight forward for small c helpers for speed reasons. It prolly would also works for bigger sources, but no one really likes to code within a string of another language. Thus I dont see this for bigger wasm projects/additions. |
@Tyriar Wow this lib looks really nice, grats for getting it done that sophisticated, must have been alot of work. I def. will look into it.
First I have a few questions about proper usage:
decodePng
:Does that mean, that the decoder carries the raw data along, thus should be removed manually from the returned
IDecodedPng
to free some memory?IImage32
andIImage64
. Is the pixel data there always fully RGBA colored, or is there a catch with some image modes (e.g. paletted contains idx, thus needs another indirection to resolve to real colors)?IImage32
or would I have to do the color reduction afterwards? Dont want to go the 16bit channel route for now, as it just puts the memory under even more stress.Sorry for so many questions, plz dont feel bugged by that, I dont need detailled answers but just some yes/no or a code pointer here and there.
Btw #15 would be a killer feature - imho APNG is a very nice, but sadly totally underrated format.
The text was updated successfully, but these errors were encountered: