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

Developing a better compression system for the .fset files #34

Open
kalwalt opened this issue Jan 25, 2020 · 59 comments
Open

Developing a better compression system for the .fset files #34

kalwalt opened this issue Jan 25, 2020 · 59 comments
Assignees
Labels
C/C++ coding with C and C++ language code design enhancement New feature or request javascript all about javascript code NFT nft markerless technology

Comments

@kalwalt
Copy link
Owner

kalwalt commented Jan 25, 2020

I already posted this question in the NFT-Marker-Creator issue tracker. To keep tracks and find resources, solutions or anything else.

@kalwalt kalwalt added enhancement New feature or request javascript all about javascript code C/C++ coding with C and C++ language code design NFT nft markerless technology labels Jan 25, 2020
@kalwalt kalwalt self-assigned this Jan 25, 2020
@kalwalt
Copy link
Owner Author

kalwalt commented Mar 4, 2020

@nicolocarpignoli @Carnaux @ThorstenBux i think we could try to compress the files with https://github.com/kripken/lzma.js (it use LZMA compression algorithm) maybe packaged all together and then de-compressed inside jsartoolkit5? Do you think that it worth the try?

@ThorstenBux
Copy link

@kalwalt great idea. I was thinking about something like that but haven't started yet to look into possible options. Having that includes into jsartoolkit makes total sense to me.

@kalwalt
Copy link
Owner Author

kalwalt commented Mar 4, 2020

So do you think that it could be a good idea? i know that artoolkit5 has zip.c inside ARUtil but i think that the LZMA has a better compression. I will start with this library instead. I will see what i can do!

@ThorstenBux
Copy link

Yes good idea for sure.
Two things,
1 yes zip.c is included but I think what we can handle in JS is preferred to using c/wasm code.
2 Browsers might be handling decompress zipped artifacts automatically. That is one thing to consider before looking into doing it manually.

@kalwalt
Copy link
Owner Author

kalwalt commented Mar 4, 2020

Yes good idea for sure.

ok! 😄

Browsers might be handling decompress zipped artifacts automatically. That is one thing to consider before looking into doing it manually.

So doesn't it needed to decompress them? or are you saying that can be an issue?

@ThorstenBux
Copy link

Yes good idea for sure.

ok! 😄

Browsers might be handling decompress zipped artifacts automatically. That is one thing to consider before looking into doing it manually.

So doesn't it needed to decompress them? or are you saying that can be an issue?

Saying the browser can potentially decompress them automatically which is a great feature and could save you some work.

@kalwalt
Copy link
Owner Author

kalwalt commented Mar 4, 2020

Yes good idea for sure.

ok! smile

Browsers might be handling decompress zipped artifacts automatically. That is one thing to consider before looking into doing it manually.

So doesn't it needed to decompress them? or are you saying that can be an issue?

Saying the browser can potentially decompress them automatically which is a great feature and could save you some work.

i understood now, thank you!

@ThorstenBux
Copy link

https://stackoverflow.com/questions/32172704/is-gzip-automatically-decompressed-by-browser

@ThorstenBux
Copy link

Hi there, how are you.
I looked a bit into that today and found that compressing all artifacts (fset, fset3, iset) into one zip won't work as a browser can't extract one zip into multiple files.
However, what we can do. We can take the content of the artifact and encode them with base64. Then add them to a json like this:

{
 fset: <<base64>>,
fset3: <<base64>>,
iset: <<base64>>
}

And would have in one file which potentially could be zipped.
This would also have the extended benefit to easily add it to ARStudio.
The NFT creator would need to be modified to allow this.

@ThorstenBux
Copy link

@Carnaux what do you think?

@kalwalt
Copy link
Owner Author

kalwalt commented Apr 5, 2020

I think that we can do it, or at least we can try.

@ThorstenBux
Copy link

We can do it :). The base64 would work I'm already implementing it.

@kalwalt
Copy link
Owner Author

kalwalt commented Apr 5, 2020

Are you testing on the NFT-Marker-Creator? or directly on jsartoolkit5?

@ThorstenBux
Copy link

adding the implementation to jsartoolkit5. Once I have that working I change the Marker-Creator to create the JSON file.

@ThorstenBux
Copy link

I create the JSON manually for now.

@kalwalt
Copy link
Owner Author

kalwalt commented Apr 5, 2020

Ok this sounds good!

@ThorstenBux
Copy link

Had a longer experimentation with my idea yesterday. Whereas it works and would allow the files to be contains in one file it does increase the file size by 33% due to the base64 encoding. I think that is not the intention what we are aiming for. For arStudio I would like to have the files within one file but increasing the size by 1/3 isn't suitable either.
Will think about another solution.

@kalwalt
Copy link
Owner Author

kalwalt commented Apr 6, 2020

Definetely has no sense, but nice idea, yes we have to think for another solution...

@Carnaux
Copy link

Carnaux commented Apr 16, 2020

I think I can make it work using ZLIB(minizip) in C to compress files from memory and save a zip file and then use the same lib to decompress it, I was trying it earlier this week, but I had to do other things. It is in my to-do list, I will keep you guys updated!

@ThorstenBux
Copy link

Thank you @Carnaux. One thing to keep in mind though is that on the client (I'm thinking mobile device) the power to decompress might be limited and it needs to be possible to compile it with EM.

One other idea is. If we could merge the three files into one with delimiters that are parsed on C side then the server can served a zipped version and the browser would do the unzipping for us automatically. However, I don't know the overhead of parsing of such a combined binary file.

@Carnaux
Copy link

Carnaux commented Apr 16, 2020

I will make some benchmarks of performance in PCs and Mobiles. The testing was already with emscripten, but was having some linking issues. When i was researching i found something about compressing the binary with RLE and then storing it as base32, maybe it works for the 3 in 1 idea.

@Carnaux
Copy link

Carnaux commented Apr 23, 2020

Hey guys, good news, I got the compression to work, in C and with emscripten. Soon I will create a new repo for you to test it. But the preliminary results show a reduction of about 27%(testing with the pinball image) from 1246KB to 1042Kb. It works this way:

I wrote two codes(separate files), one to compress and one to decompress.

The compression code:

1 - I read the marker files and convert the raw buffer to a Hex string

  <Buffer 61 62 63 .....> to "616263"

2 - Create an object with the contents and stringfy it

let obj = { iset : "hex string", fset: "hex string", fset3: "hex string" }
to
{iset : "hex string", fset: "hex string",fset3: "hex string"}

I'm using hex strings instead of the buffer, because, on the stringfy, the UTF-8 encoding may compromise part of the data.

3 - Just send it to the C function that compresses and creates a .bin file that we will use as marker file( We can change the extension to whatever name we want, like .arMarker, let's choose a name for it)

Then the decompression code:

1- Reads the .binMarker file and writes a temp bin file to Module memory.
I was having some issues with sending the contents as a string directly to the C code decompress it.

2- After reading the file and decompressing it we have the content string:

{iset : "hex string", fset: "hex string",fset3: "hex string"}

Now I'm writing the code to parse it and get each hex string and then load into artoolkit. I'm just worried about memory allocation, it works all fine, but it may cause problems in the future when merging with artoolkit or something like it.

@Carnaux
Copy link

Carnaux commented Apr 23, 2020

By the way, I'm testing with Node for now, and it all happens within ms, for now, its really fast

@ThorstenBux
Copy link

ThorstenBux commented Apr 23, 2020 via email

@kalwalt
Copy link
Owner Author

kalwalt commented Apr 29, 2020

@ThorstenBux @Carnaux i think to implement the decompression in jsartoolkit5, we should in loadNFTMarker in ARToolKitJS.cpp add a check if the extension is .zft start the decompression utility, and then load as usual the markers. That at least i would try to do...

@ThorstenBux
Copy link

ThorstenBux commented Apr 29, 2020 via email

@Carnaux
Copy link

Carnaux commented Apr 30, 2020

I will try it later today. @kalwalt which jsartoolkit repo I should use?

@kalwalt
Copy link
Owner Author

kalwalt commented Apr 30, 2020

I think we can start with https://github.com/artoolkitx/jsartoolkit5/ what do you think @ThorstenBux?

@ThorstenBux
Copy link

ThorstenBux commented Apr 30, 2020 via email

@Carnaux
Copy link

Carnaux commented May 6, 2020

@kalwalt @ThorstenBux Can you test if it builds?

https://github.com/Carnaux/jsartoolkit5/tree/compression

I'm getting an error because the command line is too long, I found a workaround, but it is only for windows, so I presume that you guys won't have the same issue if it is the case I will write a Windows condition on makem.js.

And I will create a submodule for Zlib as well.

@kalwalt
Copy link
Owner Author

kalwalt commented May 6, 2020

I would test @Carnaux, can't say so much for the issue you encountered.

@Carnaux
Copy link

Carnaux commented May 6, 2020

The test is just to tell if Linux/Mac has the same issue, or if it is a Windows exclusive issue.

@kalwalt
Copy link
Owner Author

kalwalt commented May 6, 2020

ok also i noted that decompressMarkers is empty, i understood that you need only a confirm of your issue.

@kalwalt
Copy link
Owner Author

kalwalt commented May 6, 2020

I get this error:
stderr: shared:ERROR: TOTAL_MEMORY=268435456: No such file or directory ("TOTAL_MEMORY=268435456" was expected to be an input file, based on the commandline arguments provided)

and looks at thiese lines:

-Oz -Wno-warn-absolute-paths-s TOTAL_MEMORY=268435456 -s USE_ZLIB=1 -s USE_LIBJPEG --memory-init-file 0 --bind -D HAVE_NFT -o /home/walter/kalwalt-github/jsartoolkit5/build/libar.bc
i think it is missed a space in the parsing.

@kalwalt
Copy link
Owner Author

kalwalt commented May 6, 2020

After fixing this i can not compile it, see the gist

@Carnaux
Copy link

Carnaux commented May 6, 2020

Thanks for the feedback!!!!!

@Carnaux
Copy link

Carnaux commented May 10, 2020

My suspicions were right, I'm getting memory issues, I tried with a struct, but I was getting an unreachable error. Can you guys give it a look? I will upload what I have so far. Testing with: nft_threejs_wasm.html, just to see if it loads or not.

@kalwalt
Copy link
Owner Author

kalwalt commented May 12, 2020

Haven't had the time to look at this, i hope soon to have the time... 🙂

@Carnaux
Copy link

Carnaux commented May 12, 2020

The decompression is working!! But i'm having issues with this clone of jsartoolkit5:

detectMarker error: -1

and

artoolkit.min.js:35 Assertion dst.type() == IMAGE_F32 failed in C:/wamp64/www/emscripten/emsdk/jsartoolkit5/emscripten/artoolkit5/lib/SRC/KPM/FreakMatcher/detectors/gaussian_scale_space_pyramid.cpp line 357: Destination image should be a float

@Carnaux
Copy link

Carnaux commented May 12, 2020

I've just seen that the camera_para.dat isn't loading

@kalwalt
Copy link
Owner Author

kalwalt commented May 12, 2020

that's a real strange error never encountered before, do you have made changes also in other part of the code other than in the loader?

@Carnaux
Copy link

Carnaux commented May 12, 2020

It was the two .dat files included, I don't know why they didn't work, but I got another one and it worked fine.

@Carnaux
Copy link

Carnaux commented May 12, 2020

I committed the last changes, and now we have the .zft file working!

@ThorstenBux
Copy link

ThorstenBux commented May 12, 2020 via email

@kalwalt
Copy link
Owner Author

kalwalt commented May 12, 2020

Super Daniel! that's a very nice news, and how is the performances?

@Carnaux
Copy link

Carnaux commented May 12, 2020

I only tested on my PC, but it is fast, the decompression code didn't cost anything. The memory issue before was a memory leak, I was saving the wrong data type to the marker files and that caused a memory leak somewhere in the loading of KPM data.

Please test it and send me feedback, I believe that there are some minor code enhancements, but it is all working.

@kalwalt
Copy link
Owner Author

kalwalt commented May 12, 2020

I only tested on my PC, but it is fast, the decompression code didn't cost anything. The memory issue before was a memory leak, I was saving the wrong data type to the marker files and that caused a memory leak somewhere in the loading of KPM data.

Please test it and send me feedback, I believe that there are some minor code enhancements, but it is all working.

I saw you comment in the other issue, I will test tomorrow for sure. See you soon with some data to compare. 🙂

@kalwalt
Copy link
Owner Author

kalwalt commented May 13, 2020

I tested on both Desktop and Mobile they work well on both. Tested with my Android (7.1.2) Wiko View model and the total time download of pinball.zft takes minimum 420 ms and maximum 507 ms on my device. I think is less than that the old approach. This at least is my impression, but i need to confirm this with some tests.

@kalwalt
Copy link
Owner Author

kalwalt commented May 13, 2020

With the old approach ( loading .fset3 .iset .fset) instead it takes minimun 537 and maximum 719 ms.

@ThorstenBux
Copy link

ThorstenBux commented May 13, 2020 via email

@kalwalt
Copy link
Owner Author

kalwalt commented May 13, 2020

What about the actual loading time of the marker? I mean download time is just one half of the equation. Get Outlook for iOShttps://aka.ms/o0ukef

I didn't make yet this comparison, but i should do indeed. Maybe ths evening 😃

@Carnaux
Copy link

Carnaux commented May 17, 2020

Do you think it is ready for a new PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C/C++ coding with C and C++ language code design enhancement New feature or request javascript all about javascript code NFT nft markerless technology
Projects
None yet
Development

No branches or pull requests

3 participants