Skip to content
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

performance: make memory re-usable instead of allocating new memory for every image decoded #70

Open
trusktr opened this issue Jun 5, 2020 · 4 comments

Comments

@trusktr
Copy link

trusktr commented Jun 5, 2020

Maybe it also applies to encoding too, but I just looked at the decoder code.

These lines:

https://github.com/eugeneware/jpeg-js/blob/a2f7080781a5539c65c2b42927cae28f1f051be5/lib/decoder.js#L1095-L1097

If we allow the user to send in an ArrayBuffer, and as long as the size of it is big enough to fit the image data, then jpeg-js could simply write data to it.

This part will be 400% faster on the 2nd...Nth invocations of decode() based on similar stuff I've done in other projects.

Use case: this is useful, for example, when processing frames of a live video feed, frame by frame as they stream in.

@trusktr trusktr changed the title idea: make memory re-usable instead of allocating new memory or every image decoded performance: make memory re-usable instead of allocating new memory or every image decoded Jun 5, 2020
@trusktr trusktr changed the title performance: make memory re-usable instead of allocating new memory or every image decoded performance: make memory re-usable instead of allocating new memory for every image decoded Jun 5, 2020
@trusktr
Copy link
Author

trusktr commented Jun 5, 2020

Also this line is expensive:

https://github.com/eugeneware/jpeg-js/blob/a2f7080781a5539c65c2b42927cae28f1f051be5/lib/decoder.js#L1079

We could allow the outside to pass in a UInt8Array and just use it.

The outside can pass an ArrayBuffer in which case the new UInt8Array will re-use the same memory, but the jpeg-js inner code does not know which range of data in the ArrayBuffer to use.

@patrickhulce
Copy link
Collaborator

patrickhulce commented Jun 5, 2020

Thanks for filing @trusktr!

I'm not opposed to letting the user pass in a bufferlike object to use for writing the data out 👍 that seems fine but I definitely wouldn't want to make it required by default.

Use case: this is useful, for example, when processing frames of a live video feed, frame by frame as they stream in.

FWIW, I would strongly advise against trying to use jpeg-js for this. It's slower than pretty much every other option you have regardless of your environment. For this reason, I also don't really want to bend over backwards to make minor malloc optimizations when the decode itself is so slow, synchronous, and holding all pixels in memory anyway.

@trusktr
Copy link
Author

trusktr commented Jun 5, 2020

No prob!

I was going to try it in a separate thread. I found that an offscreen canvas in a worker, drawing images from a Blob of jpeg data works well. Still curious to try it out though. So when I do, I'll fork and make those small mods.

@trusktr
Copy link
Author

trusktr commented Jun 5, 2020

Also wanted to try it in WASM (in a worker). I've been hearing of WASM reaching speeds only negligibly slower than native in some cases lately. Looks like someone forked the decoder to AssemblyScript at some point: https://github.com/glovas/assemblyscript-jpeg-decoder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants