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

Wrong in defining module as returned value #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fengdh
Copy link

@fengdh fengdh commented Jul 31, 2015

compress: _lzo1x.compress(state),
decompress: _lzo1x.decompress(state),
  • state is not defined and unable to pass as argument
  • inside implementation of _lzo1x.compress()/decompress(), there are lot of reference to "this" which must be the _lzo1x itself. It is possible to redefine those this.* as local variable/functions, or you have to bind the function to _lzo1x.

```
compress: _lzo1x.compress(state),
decompress: _lzo1x.decompress(state),
```

 * state is not defined and unable to pass as argument
 * inside implementation of _lzo1x.compress()/decompress(), there are lot of reference to "this" which must be the _lzo1x itself. It is possible to redefine those this.* as local variable/functions, or you have to bind the function to _lzo1x.
@fengdh
Copy link
Author

fengdh commented Jul 31, 2015

By the way, I have tried with your new implementation but could not decompress data in my MDict dictionary file correctly (result byte length IS as expected, but data itself completely wrong). However my port of java-compress works well (https://github.com/fengdh/minilzo-decompress.js).

Would you please take my suggestions below into consideration?

  • Define your implementation as node/AMD-compatible module rather than through a global variable. https://github.com/umdjs/umd gives good example.
  • Add an optional property to state argument, which can be used to specify expected size to hold decompress data if known ahead.

@abraidwood
Copy link
Owner

I'm investigating the decompression problem

Your compressed files have different file sizes to the ones I get when I run minilzo (c-version) against the decompressed files, which is a little unusual.

@fengdh fengdh changed the title Wrong in return value define the module Wrong in defining module Aug 3, 2015
@fengdh fengdh changed the title Wrong in defining module Wrong in defining module as returned value Aug 3, 2015
@abraidwood
Copy link
Owner

I've checked in a fix for the problem you found with your test file & included it in my tests - I'd not added the second test file for some reason.

Now all my files & both your files pass my tests.

@fengdh
Copy link
Author

fengdh commented Aug 4, 2015

Your latest commit on Aug. 3 (fix loop problems) still does not work well for all my dictionary data, though this time it produced much more correct results.

I have uploaded a zip file contains test result against the real dictionary file ( https://github.com/fengdh/mdict-js/blob/master/lzo1x-test-for-Aug.3.zip).

  • Folder named "good" contains result being decompressed with my port of java-compress, all correct.
  • Folder named "bad" contains result being decompressed with lzo1x.js, partially corrupt.
  • block0@517-15583.lzo: original lzo-compressed data block
  • block0@517-15583.dat: decompressed result data
  • .jpg/.png: image resoource sliced from block0@517-15583.dat file, more information in "sliced files.txt"

Under "bad" folder, you can see that "-cover.jpg" corrupts in half while other png files just unrecognizable.

@abraidwood
Copy link
Owner

Thanks, I'll add those to my test set and work on fixes.

(by the way, I'm not sure that lzo compression would be of any use for jpg or png images & you may be better off filtering them out?)

@abraidwood
Copy link
Owner

Can you please upload the original lzo compressed versions of the files please? Then I can see where the decompression differs?

@fengdh
Copy link
Author

fengdh commented Aug 4, 2015

block0@517-15583.lzo in both folder "good" & "bad" is the same original lzo-compressed data block (not a real *.lzo file).

In fact, I have no real *.lzo file in hand, only lzo-compressed data block which is extracted from a large dictionary file.

@abraidwood
Copy link
Owner

So block0@517-15583 contains the other files?

@fengdh
Copy link
Author

fengdh commented Aug 4, 2015

block0@517-15583.lzo is ONE lzo-compressed data block, which contains other image resources (png/jpg) one by one.
In "sliced files.txt", there is information about how to slice those image files from result block0@517-15583.dat file.

@fengdh
Copy link
Author

fengdh commented Aug 4, 2015

I am sorry, you have to ignore the first 8-byte of my uploaded .lzo (data block) file to feed it to decompress() function. LZO compressed data follows compress type (4-byte) and checksum (4-byte).

@abraidwood
Copy link
Owner

Sure.

I've checked in another loop correction which I think produces the correct output.

I've tried decompression block0@517-15583.lzo with my lzo1x.js, your java based one and also the c based one (modified testmini.c / minilzo.c). In all cases, the output is the same 18911 bytes.

The block0@517-15583.dat file is 20480 bytes long. It has the same initial 18911 bytes that the decompression produces and then has padding zeros. Are you expecting the padding to be present in the decompressed output?

@fengdh
Copy link
Author

fengdh commented Aug 5, 2015

Yes, you're right.

In fact, my port of java-compress returns an Uint8Array which is a view upon underground ArrayBuffer with a shrinked range by calling Uint8Array#subarray(). According to MDN document, it is said that for TypedArray.prototype.subarray:

The subarray() method returns a new TypedArray on the same ArrayBuffer store and with the same element types as for this TypedArray object.
ArrayBuffer.prototype.slice will copy data to a new ArrayBuffer.

while for ArrayBuffer.prototype.subarray it is said that:

The slice() method returns a new ArrayBuffer whose contents are a copy of this ArrayBuffer's bytes from begin, inclusive, up to end, exclusive.

Uint8Array#subarray() seems to take lower overhead than ArrayBuffer#slice() that it works well for most case except through TypedArray.prototype.buffer directly. I forget to truncate that buffer before write it to a file.

I have confirmed that your latest commit on Aug. 4 (further loop correction) works well on all my dictionary data.

Thank you.

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

Successfully merging this pull request may close these issues.

2 participants