| msg60930 - (view) |
Author: Greg Kochanski (gpk) |
Date: 2006-06-26 15:16 |
| In wave.py, a floating point frame rate is truncated (rather than rounded) in function _write_header(). Fractional frame rates are perfectly reasonable, even if not normally seen in audio practice, and the software ought to represent them as accurately as possible. Moreover, it's sometimes reasonable to store the inverse of the frame rate, which is the time interval between frames. That's important when you're doing signal processing, and some data formats store 1.0/framerate rather than framerate. Anyhow, if you give setframerate() a float, it truncates rather than rounding, so dt = 1.0/44100 setframerate(1.0/dt) can end up setting the frame rate to 44099 Hz in the resulting data file. The fix is to change the last line of setframerate() to self._framerate = int(round(framerate)) . That also has the beneficial side-effect of giving an earlier error report in case someone gives the wrong type of object to setframerate() (such as None, or "44100", or some class). |
|
|
| msg91026 - (view) |
Author: Neil Tallim (Red HamsterX) |
Date: 2009-07-29 00:23 |
| Attached a patch with a unit test and solution for this problem. Patch built against Python 2.7, r74246. Note: It may make more sense to just have wave.setframerate() assert that it is being fed an integer, and that's a simple fix that won't require a test. However, that's not what was approved for testing, so that's not what I wrote. |
|
|
| msg91027 - (view) |
Author: Neil Tallim (Red HamsterX) |
Date: 2009-07-29 00:27 |
| Attached a patch with a unit test and solution for this problem. Patch built against Python 3.2, r74246. Note: with Python 3.2, an error was thrown if the input value wasn't an integer because a deprecated struct-related cast was removed. It's debatable whether we want to actually apply this patch, since the error couldn't manifest in this version of Python. However, if we patch 2.7, we might as well patch this to maintain consistency. (Patching 3.2 will not break anything or make previously broken logic work differently, so it is safe) |
|
|
| msg114681 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2010-08-22 16:35 |
| I'll take a look at this. |
|
|
| msg114907 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-08-25 13:48 |
| Thanks for the patches. The general idea looks fine to me. Comments: - I'd like to see a doc entry indicating that float inputs to setframerate get rounded to the nearest integer. - In 3.x, the extra int() isn't necessary: round() returns an integer anyway. Neil, if you're still following, and are interested in updating the patches, please go ahead. Otherwise I'll fix things up and then commit in a week or two. |
|
|
| msg114911 - (view) |
Author: Neil Tallim (Red HamsterX) |
Date: 2010-08-25 14:22 |
| The differences are small enough that it probably doesn't matter which one of us makes the extra changes. It'll take one step less for you to implement them while applying the fixes, so it's probably easiest if I leave that in your court. Thanks for asking if I had anything to add, though, Mark. :) The docs-update could probably be something as simple as... Wave_write.setframerate(n) Set the frame rate to n. Fractional inputs are rounded to the nearest integer. That's pretty consistent with the way the rest of the documentation for this module has been written. |
|
|
| msg115158 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-08-28 17:24 |
| Thanks, Neil. Committed to py3k in r84341. I'm a little reluctant to backport this to the maintenance branches, though, since it is a behaviour change, and it doesn't seem to me to be a critical bug fix. Any thoughts? |
|
|
| msg115159 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-08-28 17:27 |
| I also meant to add that I left the int() call in, for slightly obscure reasons: round doesn't currently return an int for all numeric types---it does for ints, floats and Decimal instances, but not for Fraction instances (arguably a defect of the fractions module, but that's another issue). The 'int' doesn't seem like a huge extra cost, and it's conceivable that people might use Fraction for framerates, so it seemed safer to leave it in. |
|
|
| msg115162 - (view) |
Author: Neil Tallim (Red HamsterX) |
Date: 2010-08-28 17:41 |
| I use the wave module a fair bit and never once encountered this issue, so, yeah, it's probably a pretty low priority for backporting. It's definitely an edge-case sort of thing, the likes of which anyone affected has already handled in their application-level code. This just makes expected behaviour default behaviour -- if an int is already going in, as would be the case if someone is pre-processing their inputs as a workaround in existing code, it shouldn't change anything. |
|
|
| msg115164 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2010-08-28 17:57 |
| Thanks for the feedback. I'm going to make an executive decision not to backport. Closing. |
|
|