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

ABI mode to API mode transition on cffi #338

Open
wants to merge 44 commits into
base: master
Choose a base branch
from

Conversation

ayghri
Copy link

@ayghri ayghri commented Jun 8, 2022

Build error on win32 due to the declaration:
"""
SNDFILE* sf_wchar_open (const wchar_t *wpath, int mode, SF_INFO *sfinfo);
"""
Perhaps it should be removed.

Tested on Macbook Pro M1 and works with no issues.

Find wheel here: https://github.com/aghriss/python-soundfile/releases/tag/0.11.1

Build error on win32 due to the declaration:
"""
    SNDFILE* sf_wchar_open (const wchar_t *wpath, int mode, SF_INFO *sfinfo);
    """
Perhaps it should be removed.

Tested on Macbook Pro M1
@ayghri
Copy link
Author

ayghri commented Jun 8, 2022

I've accidentally removed _SoundFileInfo

@bastibe
Copy link
Owner

bastibe commented Jun 21, 2022

Could you give a summary of what you are doing here?

@ayghri
Copy link
Author

ayghri commented Jun 26, 2022

  1. In soundfile.py
  • Added library import:
from _soundfile import lib
  • Replaced decorator :
 @_ffi.callback("?")

with:

 @_ffi.def_extern()
  • added the decorated methods as part of "lib" (`lib.decorared_method)
    self._virtual_io = {'get_filelen': lib.vio_get_filelen,
                        'seek': lib.vio_seek,
                        'read': lib.vio_read,
                        'write': lib.vio_write,
                        'tell': lib.vio_tell}
  1. In soundfile_build.py,
  • Replaced:
ffibuilder.set_source("_soundfile", None)

with :

ffibuilder.set_source('_soundfile', '''
#include <sndfile.h>
''', libraries=['sndfile'])
  • Added the extern declarations to ffibuilder.cdef
'''
extern "Python" sf_count_t  vio_get_filelen (void *user_data) ;
extern "Python" sf_count_t  vio_seek        (sf_count_t offset, int whence, void *user_data) ;
extern "Python" sf_count_t  vio_read        (void *ptr, sf_count_t count, void *user_data) ;
extern "Python" sf_count_t  vio_write       (const void *ptr, sf_count_t count, void *user_data) ;
extern "Python" sf_count_t  vio_tell        (void *user_data) ;
'''

Any other changes can be ignored. There was a problem compiling for win32 (see my first comment)

@bastibe
Copy link
Owner

bastibe commented Jun 28, 2022

Thank you for providing a prototype implementation of the ABI mode. It seems that there is some more work to do to get the test suite to run correctly with this change. To be frank, I currently have very little time to invest into this. I'll try to come back to it once 0.11 has been released.

@ayghri
Copy link
Author

ayghri commented Jul 3, 2022

it seems the issue comes from the sndfile.h import. I'm guessing it's a path issue or sndfile library is not installed.

@bastibe
Copy link
Owner

bastibe commented Jul 4, 2022

Probably. Would you like to try to fix it?

@ayghri
Copy link
Author

ayghri commented Jul 5, 2022

I'll work on it this weekend, god willing.

@bastibe
Copy link
Owner

bastibe commented Jul 6, 2022

Great! Please don't feel pressured to rush. It's completely acceptable for things to take time.

@hiccup7
Copy link

hiccup7 commented Jul 12, 2022

Is there a way to solve the "ffi.callback() cause MemoryError on macOS M1" and allow soundfile to continue to use the ABI mode of CFFI? The CPython and PyPy ecosystems are rapidly evolving. The ABI mode allows a soundfile release to work with new CPython and PyPy versions released later. A large community of users depend on soundfile, and the owner of the soundfile project (@bastibe) may not always have time to release new builds quickly.

Ayoub Ghriss added 3 commits July 30, 2022 12:58
Added cffi requirement
libsndfile1 -> libsndfile1-dev
@ayghri
Copy link
Author

ayghri commented Jul 31, 2022

I got it to work on Ubuntu and macOS. https://github.com/aghriss/python-soundfile/actions/runs/2770259758
I'm having difficulty editing Windows includes to add sndfile.h

@bastibe
Copy link
Owner

bastibe commented Aug 2, 2022

Thank you for working on this! Compiling on Windows is always a headache.

@ayghri
Copy link
Author

ayghri commented Aug 7, 2022

The thing I'm stuck with is how to add include paths for a Windows action.
I was able to install libsndfile using choco. I guess I'll try to do it on a local machine and see how to tweak things.

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