Skip to content

added getSamples() to process samples directly#2

Open
rvosa wants to merge 1 commit intoffdead:masterfrom
rvosa:master
Open

added getSamples() to process samples directly#2
rvosa wants to merge 1 commit intoffdead:masterfrom
rvosa:master

Conversation

@rvosa
Copy link

@rvosa rvosa commented Oct 1, 2014

Hi,

I've added a method to access the sample data directly (one of your TODO items). Hope this is useful. Thanks for figuring out the WAV header stuff for me :)

I think parsing the bytes that spell 'data' wasn't quite working correctly, so I changed that also.

Best wishes,

Rutger

@ffdead
Copy link
Owner

ffdead commented Oct 10, 2014

Hey Rutger,

Thanks for this! I haven't used this library in a long time, so to be honest I'm not up to date on all your changes.

I see you no longer read the size of the Data chunk from the wav file (chunkSize) - were you not able to pick this up?

Also a bit strange that you found the beginning of the Data chunk at a different start position. (chunkType = this.readText(38, 4) instead of 36). Do you it would be possible to find this dynamically instead of hardcoding the value?

@rvosa
Copy link
Author

rvosa commented Oct 10, 2014

Hey,

to be honest I barely know what I'm doing - it's the first time I'm dealing with binary data in JavaScript (and I don't do a lot with it in other languages either).

For the size of the data I ended up just believing that the file size in bytes minus the header size would be the data chunk size, but actually that is probably not true if the file has other things (I don't know, loop points and such?), right? What you were working on is probably better, but all I needed was to get it to work on the wav files that are captured on iPhones, which don't have any other fancy chunks, AFAIK.

I think it would certainly be better if the Data chunk could be found dynamically as I was a bit surprised about getting the data bytes at 38, but that's what I got stepping through the code on some test files. So yes, keep doing what you were working on - if you find the time - this pull request was basically just to show you that, yes, getSamples() would indeed be lovely to have ;-)

Rutger

@ffdead
Copy link
Owner

ffdead commented Oct 10, 2014

Gotcha, great to see you were able to modify it to fit your use case :)

I'll leave this pull request open for now. The getSamples() implementation might be useful for someone else coming here.

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