DamonOehlman / filestream

W3C File Reader API streaming interfaces
30 stars 11 forks source link

Optimization Potential #2

Closed DamonOehlman closed 10 years ago

DamonOehlman commented 10 years ago

Currently I make sure data is pushed into a node compatible Buffer at the stream interface boundaries. Would be interesting to see if this is actually required, and whether just using a Uint8Array would suffice.

I've had a quick look at this, in combination with testing with the rtc-dcstream module but has witnessed a few strange effects so I'm leaving it as is for now...

feross commented 10 years ago

You can't return Uint8Array because node streams require buffers. They actually do a Buffer.isBuffer check.

However, if you want to avoid the array copy that comes with calling new Buffer in your code here, then you can use typedarray-to-buffer.

I'm planning to use filestream in create-torrent to make it work in the browser with browserify, so thanks for writing this excellent module. Let me know if you'd like me to send a PR to add typedarray-to-buffer.

DamonOehlman commented 10 years ago

OK - I think this is all done, but let me know if I've missed anything :)

feross commented 10 years ago

All you changes look great, except for the one use of instanceof Buffer which will not work reliably in the browser. I left a comment on the specific line. Read this for context.

Hope your holidays were great! Thanks for merging :+1:

DamonOehlman commented 10 years ago

Thanks @feross - made that change. And, yes holidays were awesome :)