Comments (15)
Very good catch! I'll hopefully have time to work on this this weekend.
from python-soundfile.
When you fix #11 and merge #12 there will be only one of the three instances left, therefore a Context Manager won't be necessary.
from python-soundfile.
The broken seek
s should be fixed in 7a27d57 . Or did I miss something?
from python-soundfile.
Thanks. I tried it and it works perfectly as far as I can tell!
from python-soundfile.
I'd like to re-open this issue.
The bug which I described was fixed, but this was just a side note. The original issue was about renaming seek()
.
I strongly dislike the current situation (having seek()
and seek_absolute()
), because it is unnecessarily asymmetric and it is not how it works with normal files in Python. The default for the seek()
method in Python is to seek from the beginning!
I see three possible ways to fix this:
- remove
seek_absolute()
and useseek()
with thewhence
option. To be consistent with Python,whence=0
should be the default (os.SEEK_SET
). This seems not Pythonic to me, but ironically this is used by the normal Python file objects. - remove
seek_absolute()
and useseek()
with another option, e.g.f.seek(123, absolute=False)
. The default should also in this case beabsolute=True
. - provide
seek_absolute()
andseek_relative()
, avoiding any doubts about default settings.
Obviously, I'd prefer the third option, but any of the three is better than the current situation.
from python-soundfile.
If the file is opened with SFM_RDWR
, it get's even more interesting ...
The read and write pointer can be positioned separately by OR-ing the standard whence values with either SFM_READ
or SFM_WRITE
.
http://www.mega-nerd.com/libsndfile/api.html#seek
I think PySoundFile should also support this, but I don't know (yet) how this could be done in a Pythonic way.
from python-soundfile.
I like the whence
idea! The more pythonic the better!
Regarding SFM_RDWR
, this is pretty finicky anyway and does not work reliably with every sound file configuration. How about providing an additional keyword argument to seek
that says seek(256, whence=128, which='rw')
, which defaults to 'r'
for read-only, 'w'
for write-only and 'rw'
for read-write SoundFiles
?
from python-soundfile.
I'm sorry, but I don't get what whence=128
is supposed to mean in your example. Can you please explain?
The which
argument would mean that both pointers cannot be set independently in one call.
What about providing the named arguments read
and write
?
seek_absolute(read=42)
seek_absolute(write=23)
seek_absolute(7) # both read and write are set to 7
seek_absolute(42, 23) # read and write are set individually
from python-soundfile.
Oh my, you are completely right. I misunderstood the meaning of whence
. The example should have been seek(256, whence=os.SEEK_CUR, which='rw')
.
We might be overcomplicating things a bit. I think this feature is pretty obscure and probably of very little importance to most people.
That said, I don't like separate read
and write
arguments too much, since they would make the function signature hard do understand. The meaning of read
and write
in seek(read[, write, whence])
is not obvious if you expect seek(offset[, whence])
. Also, it is confusing that read
has a different meaning depending on whether write
is assigned or not. If anything, I would go for seek(offset[, whence, read_offset, write_offset])
, where *_offset
would overwrite whence
, if present.
How about using a tuple?
seek(7) # both read and write
seek((7,)) # both read and write
seek((7,None)) # read only
seek((None,7)) # write only
seek((7,8)) # read and write separately
from python-soundfile.
I updated #18 (esp. commit 5a4d76e) before reading your response ...
I agree that the feature is a little obscure, but I nevertheless would like to have it in PySoundFile.
Especially the whence
option is confusing, that's why removed it (as you did in your examples).
I also agree that messing around with the arguments in seek()
will confuse users.
That's why I renamed seek()
to seek_relative()
... incidentally that was my main point with this issue.
Now it's officially a new function and doesn't have to have any similarities to the arguments of seek()
.
In my proposal, the function signature is "officially" seek_relative(both)
and seek_relative(read, write)
. Same for seek_absolute()
.
In reality, it's a little more convoluted, it's seek_relative(both, write, read)
, but both
and read
can never be specified at the same time. The first positional argument is actually shared between both
and read
. This is not perfect, but I think the function calls nevertheless look quite reasonable.
Ironically, without having read your response, both
can also be a 2-tuple. This is mainly because the return value is also a tuple (but only if mode=='r+'
).
I agree that separate read
and write
arguments are a little ugly, but I think having to specify None
explicitly is worse (and less explicit).
Here your examples again, augmented with a few more, according to my proposal. The comments specify if something is supposed to work or not.
seek_relative(7) # both read and write if mode=='r+', read only if mode=='r'
seek_relative((7,)) # doesn't work
seek_relative((7,None)) # read only
seek_relative(r=7) # read only
seek_relative((None,7)) # write only
seek_relative(w=7) # write only
seek_relative((7, 8)) # read and write separately
seek_relative(7, 8) # read and write separately
seek_relative(r=7, w=8) # read and write separately
Note that the return value is a tuple if mode=='r+'
and a single int
if mode=='r'
Note also that seeking is not at all possible if mode=='w'
.
from python-soundfile.
And of course there is a bug! I fixed it in b00d052.
This idiom works always, regardless if curr
is a tuple or an int
:
curr = self.seek_relative(0)
# do something (including seeking)
self.seek_absolute(curr)
However, if something else has to be done with curr
, special care has to be taken, e.g.:
curr = self.seek_relative(r=0)
# now curr is an int!
or
r, w = self.seek_relative(0)
# this only works if mode == 'r+'!
from python-soundfile.
I must say, all this seeking business is unexpectedly confusing. I really don't like how seek
now returns a tuple in "r+"
mode.
Honestly, I would prefer a pythonic seek
with whence
that does the standard Python thing to both the read pointer and the write pointer. I think for most use cases, this is the function we need and sticking to the Python default will avoid confusion. Then, we can still provide more fine-grained control with a seek_read
and seek_write
.
from python-soundfile.
I must say, all this seeking business is unexpectedly confusing.
Indeed!
I really don't like how
seek
now returns a tuple in"r+"
mode.
But there are actually two different values to return ...
It seems that in libsndfile, if you do f.seek(n, sf.SEEK_CUR)
on a rw
file, the current position of the read pointer is ignored and both are set relative to the write pointer.
So in this case, the information about the read pointer is lost.
Honestly, I would prefer a pythonic
seek
withwhence
that does the standard Python thing to both the read pointer and the write pointer. I think for most use cases, this is the function we need and sticking to the Python default will avoid confusion.
I think you're right.
I implemented this in a new pull request #21, feel free to merge if you like it.
Then, we can still provide more fine-grained control with a
seek_read
andseek_write
.
I don't like adding new method names if the behavior changes only slightly compared to an already existing method.
I would prefer to switch behavior with arguments in this case.
In #21, the read/write pointer can be set by OR'ing READ/WRITE into the mix, all this is just forwarded to libsndfile. The implementation of seek()
is now just a one-liner, seek_absolute()
is not needed anymore.
I also changed the mode handling: a3295f7. I used READ/WRITE/RDWR because that's closer to the actual constants in libsndfile. And I added the type _ModeType
which causes the actual name of the constant to be displayed when returning it, e.g. from f.mode
.
This is the same behavior as I've planned for the file formats and subtypes (see #18).
I abandoned the idea of using 'r+'
instead of 'rw'
, for an explanation see a3295f7.
from python-soundfile.
I like it! The reasoning for 'rw'
makes sense.
from python-soundfile.
Then merge away!
from python-soundfile.
Related Issues (20)
- [src/libmpg123/layer3.c:INT123_do_layer3():1771] error: part2_3_length (1920) too large for available bit count (1568) HOT 1
- Breadcrumbs from old pysoundfile to python-soundfile are partially broken HOT 1
- Incorrect stereo configuration when writing file causes unhelpful error message (especially if not using NumPy) HOT 1
- Writing integer values through the non-numpy interface results in a wav of all 0s HOT 13
- numpy is a dependency, but this is not declared HOT 3
- Feature request: Support interleaved stereo data HOT 4
- buffer_read doesn't work in basic test HOT 2
- Soundfile read/write wav is not symmetric with default arguments HOT 3
- module 'soundfile' has no attribute 'SoundFileRuntimeError' HOT 5
- Value Error: array is too big when loading .flac file
- soundfile crashes when trying to export long .ogg file HOT 1
- soundfile.LibsndfileError: Error opening <_io.BytesIO object at 0x0000022B81C21850>: Format not recognised. HOT 2
- Cannot suppress warnings when decoding mp3
- Cannot open Ogg FLAC file: unknown error in flac decoder HOT 1
- miniforge3/lib/python3.9/site-packages/soundfile.py", line 1229, in _init_virtual_io def vio_get_filelen(user_data): MemoryError: Cannot allocate write+execute memory for ffi.callback(). You might be running on a system that prevents this. For more information, see https://cffi.readthedocs.io/en/latest/using.html#callbacks
- Cannot write Ogg files over 96 seconds long at 44.1 kHz HOT 4
- soundfile.info does not work on mp3 HOT 2
- soundfile.LibsndfileError: <exception str() failed> HOT 6
- How to save left and right channel in separate files? HOT 1
- soundfile.LibsndfileError: Error : unknown error in flac decoder. HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from python-soundfile.