Skip to content

Conversation

@humphd
Copy link
Contributor

@humphd humphd commented Jun 4, 2014

This isn't right yet, but it's close. I'm corrupting my data somehow, probably where I've changes the subarray/slice/set Uint8Array calls into copy/slice with Buffers. I could use a hand spotting my errors.

@humphd
Copy link
Contributor Author

humphd commented Jun 4, 2014

We missed each other on irc, but:

15:48 < humph> can you help me?
15:49 < humph> ack: https://github.com/js-platform/filer/pull/222
15:49 < humph> I need some eyes on my logic in the internal methods that chop 
               up bytes and truncate buffers, append, etc.
15:49 < humph> somehow I'm corrupting things
16:11 <@ack> kk, sec
16:21 <@ack> humph: i don't think slice is working like you expect
16:22 <@ack> in the original code, subarray creates a typed array view, but 
             it's backed by the same arraybuffer
16:23 <@ack> are you sure useTypedArrays is true?

Yeah, for sure I'm messing things up here. Can you help me figure out how this kind of stuff should work? I have 3 or 4 places where I do things like this:

-      var newData = new Uint8Array(length);
-      var bufferWindow = buffer.subarray(offset, offset + length);
-      newData.set(bufferWindow);
+      var newData = new Buffer(length);
+      var bufferWindow = buffer.slice(offset, offset + length);
+      bufferWindow.copy(newData);

And I think I'm botching it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileData.copy(newData);

@humphd
Copy link
Contributor Author

humphd commented Jun 7, 2014

I've got this all working now. Fixing the zip/unzip stuff sucked because I also had to patch 2 upstream modules:

modeswitch pushed a commit that referenced this pull request Jun 9, 2014
WIP - moving to Buffer internally from Uint8Array, not passing all tests yet
@modeswitch modeswitch merged commit 5cc39b1 into filerjs:develop Jun 9, 2014
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