sfstoolbox / sfs-matlab

SFS Toolbox for Matlab/Octave
https://sfs-matlab.readthedocs.io
MIT License
98 stars 39 forks source link

Remove wavread and wavwrite #81

Closed hagenw closed 8 years ago

hagenw commented 8 years ago

As wavread() and wavwrite() will be removed in future Matlab versions I think it is ok to remove them from the Toolbox as well.

wavread() was easy as it is just replaced by audioread(). For wavwrite() we have the problems of multichannel wav-files as we use it to save the BRS files. I copied the savewav file from the Binaural simulator and stored it as save_ssr_brs() for the moment.

Maybe it is a better idea to store it also as savewav under SFS_general?

I also switched the order of input arguments to be identical to audiowrite().

At the moment we can only save to 32bit. In prior experiments I saved the BRS files as 16bit, was this a bad idea or should we include that option as well to save_ssr_brs() in order to save space as BRS files are normally quite large?

At the moment I have the copyright statement from the original savewav file. Can we simply replace it by the SFS copyright statement or would you prefer to stay with the original one?

After we agree on this questions I will clean up the code of the function.

fietew commented 8 years ago

I think, we should leave it at savewav because that's what it does. Actually I wanted keep the arguments just as for wavwrite, but this is not that important.

Regarding the bit quantization: So you saved the BRIRs in 16 bit integer? Because 32 bit is the only supported floating point format *.wav supports, I think. I don't think, that the 16bit quantization had audible effects on the BRIRs. I dont' think, that is necessary to add 16bit integer to the function, either.

I think we should keep the current copyright message. Strictly speaking, we would violate the MIT License, if we would just remove the copyright statement. Of course, I'm the copyright owner and I can do whatever I want with it, but maybe it's a good example how code from other sources can be integrated into the toolbox. In the end, if you're not very comfortable with it, you can also replace it by the copyright message of the toolbox.

hagenw commented 8 years ago

I agree and have updated the code accordingly.