Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pipe read/write #68

Merged
merged 1 commit into from
Jan 19, 2015
Merged

Allow pipe read/write #68

merged 1 commit into from
Jan 19, 2015

Conversation

mgeier
Copy link
Contributor

@mgeier mgeier commented Jan 9, 2015

libsndfile can read from/write to a named pipe. We disabled this feature in #60 because we don't pass the file name directly to libsndfile anymore.

To make this possible again, we could check if a given file name is actually a pipe with

import stat
import os
if stat.S_ISFIFO(os.stat(filename).st_mode):
    # call sf_open()
else:
    # call open() and sf_open_virtual(), as before

We'll probably have to catch an OSError if the file doesn't exist.

To try this, you can create a named pipe for writing:

mkfifo writepipe
cat writepipe

... and then use it:

f = sf.open('writepipe', 'w', 44100, 1, format='au')
f.seekable()  # returns False
f.write([0.0, 0.1, 0.2])
f.close()

... or a named pipe for reading:

mkfifo readpipe
cat myfile.au > readpipe

... and read from it:

f = sf.open('readpipe')
f.seekable()  # again False
f.read(10)
f.close()

Someone on StackOverflow claimed that named pipes don't work with sf_open(), but I tried it and it seems to work.

When using file descriptors, all this already works.

@mgeier
Copy link
Contributor Author

mgeier commented Oct 17, 2014

After opening a pipe in SFM_WRITE mode, we'll have to set frames to 0 manually.

This is not done in libsndfile and it doesn't look like this will be fixed, see libsndfile/libsndfile#77.

@mgeier
Copy link
Contributor Author

mgeier commented Oct 17, 2014

Against my expectations, the issue was just fixed.
We should still set frames to 0, because it will be some time until a new version of libsndfile will be released and the current version might be used by many people for a long time.

@bastibe
Copy link
Owner

bastibe commented Oct 17, 2014

Sounds good

@mgeier mgeier modified the milestone: 0.7 Oct 26, 2014
@mgeier
Copy link
Contributor Author

mgeier commented Nov 27, 2014

If #94 gets merged, this should be mostly fixed. The only thing left is to set frames = 0.

@mgeier mgeier modified the milestones: 0.6.0, 0.6.x Jan 9, 2015
@mgeier
Copy link
Contributor Author

mgeier commented Jan 9, 2015

I added a commit that sets frames to 0 (00e3dda).

If I didn't make a mistake, this can be merged.

@nils-werner
Copy link
Contributor

I am trying to use this branch with argparse.FileType

FileType objects understand the pseudo-argument '-' and automatically convert this into sys.stdin for readable FileType objects and sys.stdout for writable FileType objects:

I still cannot get piping to work:

import argparse
import pysoundfile

parser = argparse.ArgumentParser()

parser.add_argument('input', type=argparse.FileType('r'))
parser.add_argument('output', type=argparse.FileType('wb', 0))

args = parser.parse_args()
print args.input, args.output
data, rate = pysoundfile.read(args.input)
pysoundfile.write(
    data,
    args.output,
    rate,
    format='WAV',
)

Passing files works fine:

$ python test.py input.wav output.wav

<open file 'input.wav', mode 'r' at 0x104502660> <open file 'output.wav', mode 'wb' at 0x10503dc90>

Sending the data to stdout does not, it aborts after a few bytes:

$ python test.py input.wav -

<open file 'input.wav', mode 'r' at 0x1031d0660> <open file '<stdout>', mode 'w' at 0x10303f150>

RIFWAVEfmt D??dataRIFWAVEfmt D??dataTraceback (most recent call last):
  File "test.py", line 15, in <module>
    format='WAV',
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 381, in write
    subtype, endian, format, closefd) as f:
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 690, in __init__
    self._handle_error()
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 1088, in _handle_error
    self._handle_error_number(err)
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 1094, in _handle_error_number
    raise RuntimeError(_ffi.string(err_str).decode())
RuntimeError: Unspecified internal error.

When reading from stdin it fails, too

$ cat input.wav | python test.py - output.wav

<open file '<stdin>', mode 'r' at 0x10f9ef0c0> <open file 'output.wav', mode 'wb' at 0x10fb80660>

From callback <function vio_get_filelen at 0x107665050>:
Traceback (most recent call last):
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 1037, in vio_get_filelen
    curr = file.tell()
IOError: (29, 'Illegal seek')
From callback <function vio_tell at 0x107665230>:
Traceback (most recent call last):
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 1073, in vio_tell
    return file.tell()
IOError: (29, 'Illegal seek')
Traceback (most recent call last):
  File "test.py", line 10, in <module>
    data, rate = pysoundfile.read(args.input)
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 326, in read
    subtype, endian, format, closefd) as f:
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 690, in __init__
    self._handle_error()
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 1088, in _handle_error
    self._handle_error_number(err)
  File "/my/venv/lib/python2.7/site-packages/pysoundfile.py", line 1094, in _handle_error_number
    raise RuntimeError(_ffi.string(err_str).decode())
RuntimeError: Error in WAV file. No 'data' chunk marker.

@mgeier
Copy link
Contributor Author

mgeier commented Jan 9, 2015

WAV doesn't work for pipes, see the StackOverflow link in the issue description at the top of this page.
You should try AU or RAW.
Or you first create a virtual file in memory and then write it to stdout as a whole.

And I think 'r' should be 'rb', right?

It still doesn't work, because what you are using aren't pipes as far as libsndfile is concerned.
When you are passing Python file objects, PySoundFile uses libsndfile's "virtual IO", which doesn't support pipes.

The "pipe mode" is only used when you pass an actual file name or a file descriptor.

If you want to use file descriptors, you can do something like this:

import argparse
import pysoundfile

parser = argparse.ArgumentParser()

parser.add_argument('input')
parser.add_argument('output')

args = parser.parse_args()

infile = 0 if args.input == "-" else args.input
outfile = 1 if args.output == "-" else args.output

data, rate = pysoundfile.read(infile)
pysoundfile.write(
    data,
    outfile,
    rate,
    format='AU',
)

Note that file descriptors may be problematic on Windows.
If you want to avoid copying the whole file to a huge NumPy array (assuming you have a huge file), you can use pysoundfile.blocks().

If you have suggestions how we can improve the error messages and/or the documentation, speak up!

@nils-werner
Copy link
Contributor

OK, sending data into stdout works when using AU, however piping into stdin is still broken. To fix this, the following patch can be used:

diff --git a/pysoundfile.py b/pysoundfile.py
index b2fd47a..f0fb505 100644
--- a/pysoundfile.py
+++ b/pysoundfile.py
@@ -10,6 +10,7 @@ For further information, see http://pysoundfile.readthedocs.org/.
 """
 __version__ = "0.5.0"

+import sys
 import numpy as _np
 import os as _os
 from cffi import FFI as _FFI
@@ -665,6 +666,9 @@ class SoundFile(object):
         if not closefd and not isinstance(file, int):
             raise ValueError("closefd=False only allowed for file descriptors") 
+        if file is sys.stdin:
+            file = "-"
+
         if isinstance(file, str):
             if _os.path.isfile(file):
                 if 'x' in modes:

This sends a filename - to libsndfile which make it pick up data from stdin itself. As a nice sideffect this also circumvents problems with pythons stdin buffering.

@mgeier
Copy link
Contributor Author

mgeier commented Jan 12, 2015

I don't like this, because it violates the principle of least surprise.

When I pass a file object, I expect the "virtual IO" call to be used.
Also, a user might have redirected sys.stdin to something completely different and might not even want the actual standard input to be used.

If you want to pass "-" to libsndfile, you can just do that explicitly.

BTW, is this behavior documented somewhere?
I didn't know about this, that's an interesting feature!

This works for me (the "-" case already works, no need to use the type argument in the first place):

import argparse
import pysoundfile

parser = argparse.ArgumentParser()

parser.add_argument('input')
parser.add_argument('output')

args = parser.parse_args()

data, rate = pysoundfile.read(args.input)
pysoundfile.write(
    data,
    args.output,
    rate,
    format='AU',
)

Example usage:

cat myfile.au | python test.py - - | hd

Does this work for you, too?

@nils-werner
Copy link
Contributor

Yes, I expect leaving out the type to work as ArgumentParser would then just assume it to be a string. And passing "-" to libsndfile just works.

When setting the argument to be a file ArgumentParser either opens the file, throws an error or, in case of "-", returns sys.stdin or sys.stdout.

@mgeier
Copy link
Contributor Author

mgeier commented Jan 13, 2015

I still don't see what the advantages of using the type argument are.
If you use strings, you get exactly the behavior you're describing, only the exception class is different (in case of a non-existing input file):

RuntimeError: System error.

If you absolutely have to use the type argument, you should put the if file is sys.stdin: file = "-" in your code.
I still think it would be a bad idea to add this to PySoundFile.

@bastibe
Copy link
Owner

bastibe commented Jan 19, 2015

I would very much like to push this issue to after 0.6.0, since it is the last blocking issue. Would you be OK with that?

@mgeier
Copy link
Contributor Author

mgeier commented Jan 19, 2015

It's not blocking.
As far as I'm concerned, it can be merged (see https://github.com/bastibe/PySoundFile/pull/68#issuecomment-69329767).
I consider @nils-werner's suggestions as "wontfix" for now, but feel free to create a separate issue for that!

bastibe added a commit that referenced this pull request Jan 19, 2015
@bastibe bastibe merged commit e97dfca into master Jan 19, 2015
@mgeier mgeier deleted the issue-68 branch January 19, 2015 15:48
mgeier added a commit that referenced this pull request Apr 4, 2015
Before, this was only done for file names (#68).
There are no tests yet, see #95.
mgeier added a commit that referenced this pull request Apr 9, 2015
Before, this was only done for file names (#68).
There are no tests yet, see #95.
@mgeier mgeier mentioned this pull request Jul 7, 2015
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.

3 participants