fasterthanlime / jsmad

:saxophone: Javascript MPEG-1 Audio Layer III (mp3) and ID3v2 decoder
http://audiocogs.org/codecs/mp3/
762 stars 41 forks source link

Patch 1 #12

Closed gliese1337 closed 13 years ago

gliese1337 commented 13 years ago

Made some changes to replace strings with more Uint8Arrays. Haven't been able to test it very effectively 'cause I can't get the existing code to run reliably on my computer, but I'm not throwing any errors that weren't already there, at least.

nddrylliog commented 13 years ago

Awesome pull request, you're totally doing what I wish I had the time to do (but exams... grr). We'll have to test this before merging though - just curious, what problems are you encountering when testing locally? The official.fm proxy should deny you the requests but otherwise everything should be fine. You do serve the website from a proper webserver (Apache/nginx) right? Not from file:/// ?

gliese1337 commented 13 years ago

We'll have to test this before merging though - just curious, what problems are you encountering when testing locally?

Well,

The official.fm proxy should deny you the requests but otherwise everything should be fine.

that explains some of it. The index page does not behave very nicely with loading local files; the ajax stream keeps going, which means it keeps throwing errors if it feels like throwing errors, or it just keeps playing and may or may not successfully switch over to playing the local file.

I got around that by just making a new demo page that doesn't have the AjaxStream in it, so I've been testing the string-to-Uint8Array conversion on the FileStream exclusively. Turns out the process of copying a binary string to an array is disgustingly slow (like, freeze-your-browser slow), but new Uint8Array(buffer.split('')); works just fine. Weird. That being seemingly solved, though, there's some problem with reading the file header when using a typed array instead of a string. On my test mp3s, using a string buffer, everything works fine, but with the typed array it determines that it's a free bitrate file and then terminates and doesn't finish the decoding. I'm not sure if it's something to do with reading from a typed array that works differently, or if buffer.split('').map(function(c){return c.charCodeAt(0);}); or Array.prototype.map.call(reader.result,function(c){return c.charCodeAt(0);}); (oddly, this one seems to be slower) somehow produces a different byte stream than successive calls to buffer.charCodeAt().

I thought it could be because JavaScript strings are unicode, and indexing strings is based on variable-length characters and not bytes, but charCodeAt() uses string indexing anyway, and the buffer is a byte string, so I'm totally confused.

If only we could use node memory buffers; that would make the translation from string to byte array really easy.

While waiting for that to get all figured out, I guess I'll just keep working on the commenting & reduction of code duplication.

Also, chrome throws security errors when trying to read local files. I'm pretty sure that's a chrome quirk, though, nothing to do with jsmad in particular. That's entirely expected if you try to do an AJAX request to the filesystem instead of an actual server, but I was surprised to see that it would do that with a FileReader, which after all is supposed to access the local filesystem. To be clear, that problem only occurs when accessing the page from file://, not over a webserver, but I find that odd.