When using the wave module to output wave files, the output file cannot
be a Unix pipeline.

Example. The following program outputs a (trivial) wave file on stdout:

#!/usr/bin/env python
import sys
import wave
w = wave.open(sys.stdout, 'w')
w.setnchannels(1)
w.setsampwidth(1)
w.setframerate(32000)
w.setnframes(0)
w.close()

It can create a wave file like this:

$ ./bugex > foo.wav

When used in a pipeline we get:

$ ./bugex | wc
Traceback (most recent call last):
  File "./bugex", line 9, in <module>
    w.close()
  File 
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/wave.py
", line 437, in close
    self._ensure_header_written(0)
  File 
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/wave.py
", line 458, in _ensure_header_written
    self._write_header(datasize)
  File 
"/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/wave.py
", line 465, in _write_header
    self._form_length_pos = self._file.tell()
IOError: [Errno 29] Illegal seek
Exception exceptions.IOError: (29, 'Illegal seek') in <bound method 
Wave_write.__del__ of <wave.Wave_write instance at 0x71418>> ignored
       0       1       8

The wave module has almost all it needs to work around this problem.
The wave module will only seek the output if it needs to patch the
header. If you use setnframes to write the correct number of frames
before writing them with writeframesraw then the header will not be
patched upon calling close. However...

The problem is that the "tell" method is invoked on the output stream
(to record where the header is, in the event that we need to patch it);
the "tell" method fails with an exception when the output is a pipeline
(see example, above).

Exceptions from "tell" when writing the header initially (in
_write_header) should be ignored. If _patchheader is later invoked it
will fail due to lack of pos.

Attached is a patch which is a diff from this version of wave.py :

http://svn.python.org/view/*checkout*/python/trunk/Lib/wave.py?rev=54394

Wouldn't it be better if you only ignored the 'illegal seek' error
instead of ignoring any ioerror (should it even be always discarded) ? I
get a 'bad file descriptor' under Windows 7, but, again, can it be
always discarded ?

You can also reproduce the problem without using wave:

>> import sys
>> sys.stdout.tell()

I'm really unsure about the proposed patch.

On 10 Feb 2009, at 12:28, Guilherme Polo wrote:

Guilherme Polo <ggpolo@gmail.com> added the comment:

Wouldn't it be better if you only ignored the 'illegal seek' error
instead of ignoring any ioerror (should it even be always discarded) ?

No.

I
get a 'bad file descriptor' under Windows 7, but, again, can it be
always discarded ?

Yes.

To expand: Observe that the exception is raised when we are writing
the header for the first time. The exception is not raised when we
attempt to seek to patch the header, it is raised when we recording
the file position so that we can seek to it later.

We record the file position even though we might not use it later
(the file position is only needed if we need to patch the header).

So if we don't need to patch the header, we do not need the file
position. So we can clearly ignore any error in attempting to get
the file position.

If we do need to patch the header, then we need the file position.
If we do not have the file position (because the earlier attempt to
get it failed), then patching the header will fail when it attempts a
seek. This seems reasonable to me.

You can also reproduce the problem without using wave:

>>> import sys
>>> sys.stdout.tell()

That does not reproduce the problem. The problem is not that tell
raises an exception, the problem is that tell raises an exception and
it is only being used to get some information that may be not needed
later. Therefore the exception should be ignored, and a problem
should only be raised if it turns out that we did need for
information that we couldn't get.

I'm really unsure about the proposed patch.

Noted.

I also note that my patch can be improved by removing its last 11 lines.

On 10 Feb 2009, at 12:28, Guilherme Polo wrote:

Guilherme Polo <ggpolo@gmail.com> added the comment:

I'm really unsure about the proposed patch.

Perhaps my example was too trivial. The point is that if you call
setnframes then you can get wave.py to avoid patching the header; so
it does not need to seek on the output file. However, that _still_
doesn't let you pipe the output, because of the "tell" problem.
That's what the patch is for.

Here is a (slightly) less trivial example:

#!/usr/bin/env python
import sys
import wave
w = wave.open(sys.stdout, 'w')
w.setnchannels(1)
w.setsampwidth(1)
w.setframerate(2000)
w.setnframes(100)
for _ in range(50): w.writeframesraw('\x00\xff')
w.close()

(The wave file that it outputs is 100ms of 1000 Hz sine wave by the way)

Note the call to setnframes _before_ the data is written. That's
what means the header does not need to be patched. With my patch
applied the output of this program can be fed to a pipe.

If you remove the call to setnframes then the header will need to be
patched, and this still (correctly, usefully) raises an error with my
patch applied.

On 10 Feb 2009, at 13:02, David Jones wrote:

I also note that my patch can be improved by removing its last 11
lines.

Er, no it can't. What was I thinking?

I see what you want to do, but I fell really uncomfortable by totally
ignoring IOError. I could get a bad file descriptor under Linux too, and
I wouldn't like to see it discarded for no reason.

Now, is there some problem if we remove the calls to the "tell" method
in _write_header ? See patch attached (tests are very welcome too).

On 10 Feb 2009, at 16:57, Guilherme Polo wrote:

Guilherme Polo <ggpolo@gmail.com> added the comment:

Now, is there some problem if we remove the calls to the "tell" method
in _write_header ? See patch attached (tests are very welcome too).

Yes

On 10 Feb 2009, at 21:15, David Jones wrote:

David Jones <drj@pobox.com> added the comment:

On 10 Feb 2009, at 16:57, Guilherme Polo wrote:

>
> Guilherme Polo <ggpolo@gmail.com> added the comment:
>
> Now, is there some problem if we remove the calls to the "tell"
> method
> in _write_header ? See patch attached (tests are very welcome too).

Yes

Ahem. Pardon me for answering you without reading your patch. I
have now read your patch and it does more than just remove the calls
to "tell". In fact it looks very fine.

It makes wave.py more like sunau.py in that it "just knows" what the
offsets into the header are. I think I like that (especially the way
you use the struct format string to compute the second offset). It
also removes that nagging question at the back of my mind: "why does
wave.py use tell when it could simply just know the offsets, which
are constant anyway?".

And it works. How cool is that? I had changed my project to use
sunau anyway, because that worked with pipes already.

Tests, you say...

Nice.

I said tests in hope wave gets more tests, since right one there is a
single one. I will see if I can produce something.

The following program does a very basic do-i-get-back-what-i-wrote test.
sunau can't cope; I am investigating.

#!/usr/bin/env python
# $Id$
# Audio File Tests

import aifc
import sunau
import wave

import struct
import sys
from StringIO import StringIO

frames = struct.pack('256B', *range(256))
log = sys.stderr

# Basic test of reproducability.
# We test that a set of frames (an entirely artifical set, see `frames`, 
# above) can be written to an audio file and read back again to get the 
# same set of frames.
# We test mono/stereo, 8-bit/16-bit, and a few framerates.    
# As of 2009-02-12 sunau does not pass these tests, so I recommend that 
# you remove it.
for af in (aifc, sunau, wave):
  for nchannels in (1, 2):
    for sampwidth in (1, 2):
      for framerate in (11000, 44100, 96000):
        print >> log, "%s %d/%d/%d" % (af.__name__,
          nchannels, sampwidth, framerate)
        f = StringIO()
        w = af.open(f, 'w')
        w.setnchannels(nchannels)
        w.setsampwidth(sampwidth)
        w.setframerate(framerate)
        w.writeframesraw(frames)
        w.close()
        s = f.getvalue()
        f = StringIO(s)
        w = af.open(f)
        assert w.getnchannels() == nchannels
        assert w.getsampwidth() == sampwidth
        assert w.getframerate() == framerate
        assert w.readframes(len(frames)//nchannels//sampwidth) == frames
        assert w.readframes(1) == ''

On 12 Feb 2009, at 09:00, David Jones wrote:

David Jones <drj@pobox.com> added the comment:

The following program does a very basic do-i-get-back-what-i-wrote
test.
sunau can't cope; I am investigating.

I see. sunau uses mu-law compression by default which makes it non-
invertable. How stupid. Inserting:

         w.setcomptype('NONE', 'Pointless Argument')

just after setframerate fixes the tests so that all 3 modules pass.

Of course, this is still only the most very basic test that one might
want to do. And it doesn't cover the case mentioned in this bug
report anyway.

(drat, just found this, should've sent it yesterday)

Is this still a problem with 2.7-3.2?
GP, what state do you think either patch is in?

Now, is there some problem if we remove the calls to the "tell" method
in _write_header ? See patch attached (tests are very welcome too).

Yes, there is a problem. User can pass already open file to wave.open() and file position can be not 0 at the start of the WAVE file. But you can do with only one tell(). Note a magic number 36 in many places of the code. This is struct.calcsize(wave_header_format).

Test is needed as well as a documentation change.

I think this is rather a new feature and should be added only in 3.4. Actually the current behavior is documented: "If *file* is a string, open the file by that name, otherwise treat it as a seekable file-like object."

Here is corrected patch (it uses relative seek()) with a lot of tests.

Oh, I forgot attach a patch. In any case it already slightly outdated.

After looking at other audio modules I think David's approach is better. It is also used in the chunk module. Here is updated patch with tests (tests are not final, bpo-18919 provides better tests).

Here is simplified and updated to tip patch.

New changeset 6a599249e8b7 by Serhiy Storchaka in branch 'default':
Issue bpo-5202: Added support for unseekable files in the wave module.
http://hg.python.org/cpython/rev/6a599249e8b7

New changeset b861c7717c79 by R David Murray in branch 'default':
whatsnew: Wave_write handles unseekable files. (bpo-5202)
http://hg.python.org/cpython/rev/b861c7717c79