[issue4913] wave.py: add writesamples() and readsamples()
Serhiy Storchaka added the comment: I hope all mentioned bugs were already fixed in the wave module. As for new writesamples() and readsamples() methods, perhaps it would be better to add utility functions in the audioop module for packing/unpacking integers. In any case a user can use array.array. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Joe Button added the comment: Forgive my unfamiliarity with python's development process, but, what is happening with this? Is there any chance of this enhancement making it into the python libs? What would need to happen? Thanks. -- nosy: +Joeboy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
R. David Murray added the comment: Someone has to find the time to do a commit review on the patch. As Guilherme said, there's no specific maintainer for wave, so I'm afraid it just got forgotten about. On the other hand, as a new feature it would now go in 3.5, and we're at the start of the approximately one year window for new features, so if you ping this issue (as you just did) periodically, someone will get to it ;) What you could do to help move it along is to do your own review of the patch, including making sure it still applies to default...which it may not, since there have in fact been some changes in wave.py. If that's the case you can also help by updating the patch. -- nosy: +r.david.murray stage: test needed - patch review versions: +Python 3.5 -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Terry J. Reedy added the comment: Serhiy, is this something you can review? -- nosy: +serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Joe Button added the comment: On quickly looking at this, the immediate issue seems to me to be that there is no patch, as I understand the term. If it would be helpful I can look at turning the code in the attached files into a patch against default and ensure the tests pass (but not right now as it's ~1am here). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Terry J. Reedy added the comment: A patch against default, including a test, would be helpful. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Alex Robinson alex_python_...@tranzoa.com added the comment: Here go, Terry. Copies of the two files in the latest ZIP file. Hmmm. Well. Maybe just one of 'em. Looks like the only way to upload files is to add a msg, so I'll upload the other file in another msg. -- Added file: http://bugs.python.org/file18451/wave_futz.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Alex Robinson alex_python_...@tranzoa.com added the comment: OK, here's the other. -- Added file: http://bugs.python.org/file18452/test_wave.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Terry J. Reedy tjre...@udel.edu added the comment: Please upload plain-text files with unique names for each file uploaded. -- nosy: +terry.reedy stage: - unit test needed versions: +Python 3.2 -Python 2.7, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Alex Robinson alex_python_...@tranzoa.com added the comment: I'll upload the latest monkey-patch file, wave_futz.py, and test_wave.py, which has a gob of tests added to it. I found a 64-bit bug in the wave.py formats for 32-bit sample wave files. The pcm files read in to CoolEdit ok, including the 32-bit sample files. Added file: http://bugs.python.org/file12985/wave_futz.zip ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Guilherme Polo ggp...@gmail.com added the comment: Documentation, tests and patch against trunk are needed to get this into Python, but to me the request is fine. -- title: wave.py writes 16 bit sample files of half the correct duration - wave.py: add writesamples() and readsamples() versions: -Python 2.4, Python 2.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Changes by Guilherme Polo ggp...@gmail.com: -- versions: -Python 2.6, Python 3.0 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Alex Robinson alex_python_...@tranzoa.com added the comment: Polo: I could do it, but I'm in disagreement with big part of your patch. Why surely you can't mean the bug. :) (The test program has it fixed.) What is the disagreement? Apparently this bug system allows file attachments, so I will upload a test program and wave file. The program is hard coded to read the wave file and write a bunch of wave files, the names of which describe what they sound like. Added file: http://bugs.python.org/file12703/wave_futz.zip ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Guilherme Polo ggp...@gmail.com added the comment: Aren't 8 bit samples stored as unsigned bytes ? If yes, they don't range between -128 and 127 (first disagreement). So this line: wav = [ s - 128 for s in wav ] and the respective one (that adds +128 in writesamples) should go. Why is this check: if len(wavs) not in [ 1, 2, 4 ] needed ? Calling setnchannels inside writesamples looks very wrong to me, weren't you going to writesamples ? Then why is it modified the number of channels ? The caller should be responsible for calling setnchannels, besides, what is the use of calling setnchannels here ? I see writesamples is expecting wavs to be a list of lists containing integers, is that the best format to expect ? writeframes works with strings (which are actually byte strings). The code layout didn't help me to get in agreement with it either. The above paragraphs are the things I disagree with the patch, hopefully you can help on those questions. Also, it would be better to hand write the wave file for testing so we can be sure about its content without needing much analysis. ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Alex Robinson alex_python_...@tranzoa.com added the comment: 8 bit samples stored as unsigned bytes? 8 bit samples are 0..255 in the file. But to work with them, you'll want them -128..127. The code assumes DC==0 sample values for simplicity. if len(wavs) not in [ 1, 2, 4 ] ? That way if you're working with mono, you can simply pass your samples down to writesamples() without having to remember to [ samples ] them. If you forget, no big deal. It's too bad that readsamples() can't know that you want only mono samples. That would make mono work simpler. Anyway, I don't argue very strongly for this spiff. In some ways it's worse to be there. After all, the caller may be writing 1 frame at a time, though I don't think that such logic would work. And would be pretty slow, too, probably. Calling setnchannels? Since the number of channels *must* be the number of sample arrays passed to writesamples(), either writesamples() must rely on the caller already having gratuitously set the number of channels (correctly), or writesamples() can simply force the issue. If the caller set the number of channels wrongly, then the output file will be corrupt or writesamples() would need to raise an exception. Both just make work for the caller. get/set_nchannels() are not particularly useful if you are using the read/write_sample API. If there were no ..frame() API, getnchannels() might still be handy to use to find out how many channels a wave file being read has before any samples have been read. But that's about it. integers, is that the best format? Far bettern than the byte stream form which is useless, confusing, error prone, and exposes the internal wave file format to the caller who could generally care less how a wave file stores the samples. But, you bring up a very good point, I think. I forgot to int(s) the samples when they are put in to the arrays. The reason for an int() call on all the samples is so that the caller can deal with samples as floats. (Which is how he will want to deal with them if he's doing anything interesting.) So, this: ws = array.array('l', [ int(s) for s in wav ]) ws = array.array('h', [ int(s) for s in wav ]) ws = array.array('B', [ int(s + 128) for s in wav ]) And, for testing, in normalize_samples(): samps = [ s * mxm for s in samps ] So normalize_samples() always sends floats to writesamples(). The code layout? :) Well. Whatever. I know that the official python thing is to push colons left. I don't like that. I've experimented in the last few years with doing a lot of vertical alignment. Over time, I've found that it is a great way to do things. It's pretty amazing how much easier it is to scan and read code that has, as a start, the ='s vertically aligned. And, over the last few years, I've put more and more blank lines in. Vertically compressed code tends to look like assembler language. I edit with 200+ character wide screen so, since I stopped forcing everything to fit on TTY, text mode CGA, or punch cards, I've lost a taste for narrow code. In fact, I personally have a real hard time reading line-broken code. That said, multi-lining if statements in ways that allow delete/insert of lines, 1 for each operator () expression, can be very nice. Calls to routines with a gob of parameters, each on a separate line, can sure be a good way to deal with a bad thing (routines that take a lot of params are bad, that is). Etc. Anyway, I assumed that the code would be reformatted by whomever maintains wave.py. No biggy. hand write the wave file for testing? Good point! Allows a test to do things like odd numbers of frames, 1 frame, max'ed out sample values, long runs of silence, DC offsets, etc. Do you know whether there are already test files for wave.py? They'd have those sorts of things in them. Hmmm. It's odd that wave.py doesn't run from the command line and dump the header or something, at least. Maybe do some simple conversions (8/16, mono/stereo switches, reverses ... that sort of thing). Would be handy. This code is not tested on a big-endian machine. I ran it under XP (py2.4) and Ubuntu64 (py2.5) and all the output files CRC the same on both PCs. ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Guilherme Polo ggp...@gmail.com added the comment: 1) wave.py doesn't do assumptions about what the user wants, so I don't think it is the place to put the DC (0 hz) assumption. 3) writesamples would raise an exception in the case of the current number of channels set being wrong. 4) Well, lets fix a format then. I said list of lists of integers, or it could use generators, and you didn't disagree here so it seems to be fine. The problem in the current code is that you are making mono channels special by being the one where a list of lists of integers is not returned, but instead a single is returned. This is troublesome for the caller to set the number of channels then, it is also a different format then when something with 2 channels or other configuration is used. With that in mind I have simplified some of your code as this: def readsamples(self, nframes) : Return a list of lists of integers. The number of these inner lists is given by the number of channels in the wave file. Each list contains the channel samples formatted as integers. wav = self.readframes(nframes) sampwidth = self.getsampwidth() wav = struct.unpack( '%d%s' % (len(wav) / sampwidth, wave._array_fmts[sampwidth]), wav) nc = self.getnchannels() if nc 1: wavs = [] for c in xrange(nc): wavs.append([wav[si] for si in xrange(c, len(wav), nc)]) else: wavs = [[wav]] return wavs def writesamples(self, *wavs) : Write samples to the wave file. wavs must follow the structure returned by readsamples. if self.getnchannels() != len(wavs): raise wave.Error(# of channels != # of samples) wav = [] for w in zip(*wavs): wav.extend(w) ws = array.array(wave._array_fmts[self.getsampwidth()], wav) ws = ws.tostring() # we want all the samples in writeframes() format so that _convert # can be called on them self.writeframes(ws) You can monkey patch wave then by doing: wave.Wave_write.writesamples = writesamples wave.Wave_read.readsamples = readsamples And then change some other parts of your code. 5) There is a very small test for wave in Python's source, Lib/test/test_wave.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Guilherme Polo ggp...@gmail.com added the comment: I was going to reply about your code layout answer but forgot. Well, each one has their preferences so I'm not going to question yours. The only problem is that there is no maintainer for wave.py, so, the more you follow the rules for Python code (or at least code that gets included in Python) the greater are the chances for them getting included. ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4913] wave.py: add writesamples() and readsamples()
Alex Robinson alex_python_...@tranzoa.com added the comment: DC (0 hz) assumption? wave.py makes the assumption that what the user wants is whatever happens to be in the file, however arbitrary. (That 8 bit samples are unsigned bytes is probably an artifact of early ADC logic. Typically you got an absolute, n-bit value from an old ADC. Newer chips often return signed values.) It's very unlikely that anything but a copy program would try to work with unsigned char samples. Too many things to go wrong. Too much confusion. Zero means zero in most of the world, in and out of audio processing. :) That said, not having to offset the 8 bit samples sparsifies the read/write_sample code. But, I'm thinking that that's at the expense of every program that uses it. When in doubt, I figure, do what is more useful. Don't force the caller to write a wrapper if he'll need to do it 99% of the time. But this is not a religious thing with me. A wrapper can be written. And, in fact, I'd sure think it would be nice to include wrappers like auto-scaling and auto-zeroing in wave.py. But maybe not, as these ops probably belong in some array.py type module. Anyway, a non-audio guy who just wants to read a wave file, diddle with it, and write it out. Or who just wants to generate some sound and write it out. Or who just wants to read a wave file and graph it or something. All of these guys will be stunned when they find out to their hours-of-work chagrin that wave files' 8 bit samples are not signed chars. And, if I were one of them, I'd be plenty peeved after having to spend all that time learning about some historical artifact just to read an danged audio file, for gosh sakes. But not putting the 8-bit offset in the read/write_samples logic does eliminate 2 lines of code in each routine. writesamples would raise an exception Yep, taste. I'm inclined to find this irritating and I don't like being irritated by packages I use. Makes for a poor out-of-box experience. But, taste. :) 4) Well, lets fix a format then. else: wavs = [[wav]] ? That's an extra [] I think. [[samples]] would be an array of array of an array of samples. s = [1,2]; print [ [ a ] ];[[[1,2]]] On reflection, I'd say I agree with you more than I do with me on the ability of writesamples() to take a simple array of mono samples. Not a good thing to do. wavs.extend(wav) ? I had to look up extend() and try it in the Python shell! :) To each his own. But when I found out that one could do list+=added_list in Python I never looked back. Intuitive. I special-cased mono for speed purposes. No reason to do the +=/extend for mono samples. But, maybe the interpretor handles all that. Don't know. Didn't measure it. monkey patch?' Wonderful! This makes your rewrite of the code *so* much cleaner. Thanks for the tip! code layout? Har, har. Yep, no one in software has ever spent any time discussing code layout before. Let's do it for the first time in history. test_wave.py? Oooo. Bit minimal, that. Yeah, I think a couple of things could be fleshed out there. Gotta run now. But will try to update the code in wave_futz later. Other things on plate, though. Guilherme, I really appreciate your handling this and your guidance. Thanks! ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4913 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com