-
Notifications
You must be signed in to change notification settings - Fork 398
Simplify decode c api #144
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
Conversation
…ime. But if we do, this is how we do it...
The flow will now be: * cimbard_scan_extract_decode (loop until done) * cimbard_fountain_decode * cimbard_decompress_read (loop until done) This eliminates the awkward "make sure you keep the zstd compress memory block around until the decompress finishes, or else (:" footgun, and 2 api calls to boot. It *does* make testing slightly trickier, so we'll keep some internal/testing methods to help test the intermediate step (recover but don't decompress) (changes to the javascript code forthcoming...)
Not sure yet what to do with the zstd test page -- might just delete it. The *automated* js test, OTOH, needs updating...
|
|
||
| decompress_on_store<std::ofstream>(outpath, true)(filename, data); | ||
| int res = 1; | ||
| while ((res = cimbard_decompress_read(fileId, data.data(), data.size())) > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually using the full api here now.
| , "_cimbard_get_filename" | ||
| , "_cimbard_finish_copy" | ||
| , "_cimbard_get_decompress_bufsize" | ||
| , "_cimbard_decompress_read" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasm export list acts as a public api reference for the moment...
| { | ||
| if (_reassembled.empty()) | ||
| return nullptr; | ||
| return _reassembled.data(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing only, please
| if (!_sink->recover(id, buffer, size)) | ||
| return -2; | ||
| return 0; | ||
| int res = recover_contents(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notably, both decompress_read() and get_filename() will call recover_contents() (the result will be cached), since they both need to pull the data from the zstd stream. Definitionally, then, you can only call either after cimbard_fountain_decode() has finished reassembling the file.
| var height = window.innerHeight - 10; | ||
| Main.scaleCanvas(canvas, width, height); | ||
| Main.alignInvisibleClick(canvas); | ||
| Main.checkNavButtonOverlap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completely unrelated little bonus UI fix here. Hide the nav button if it overlaps the barcode.
| }, false); | ||
|
|
||
| window.addEventListener('resize', () => { | ||
| Main.resize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another UI tweak: listen for the resize event
| QUnit.config.reorder = false; | ||
| QUnit.config.testTimeout = 10000; | ||
|
|
||
| var Zstd = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected upside of changing the api: we're now testing more of the zstd code path in the JS test.
| } | ||
| // this needs to happen after decompress() completes | ||
| // currently decompress is sync, so it's fine. But... | ||
| Module._free(dataPtr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^
Happy to make this memory cleanup race go away.
For the first pass at the web decoder (#134), I introduced a new C api for decoding. It currently works like this:
cimbard_scan_extract_decode-> on independent threads, input rgb image data, output 1-N fountain bufferscimbard_fountain_decode-> on the (1) reassembly thread, to reassemble the fountain bufferscimbard_finish_copy-> on the reassembly thread, whencimbard_fountain_decodereports success. output one contiguous buffer of compressed file dataAnd then a set of zstd apis, which frankly could be ditched for an off-the-shelf zstd implementation:
cimbarz_init_decompress-> input the contiguous buffer of compressed data. Set state -- we decompress one file at a time.cimbarz_decompress_read-> read the next chunk of decompressed dataThis is changing for a few reasons:
Luckily, there's no real need to expose this intermediate compressed representation (short of for testing purposes -- but we can work around that). So we can collapse a few methods down. The new api is:
cimbard_scan_extract_decode-> on independent threads, input rgb image data, output 1-N fountain buffers. (same as before)cimbard_fountain_decode-> on the (1) reassembly thread, to reassemble the fountain buffers (same as before)cimbard_decompress_read-> on the reassembly thread, whencimbard_fountain_decodereports success. read the next chunk of decompressed data, or the first chunk (if it's the first call)So: we've collapsed the last 3 calls into 1, and gone from 5 to 3 as far as the main flow is concerned. Also, the intermediate buffer is now internal state, so the caller doesn't have to think about it too much.