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

pretext to start a discussion ;) #15

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

Conversation

Houston4444
Copy link

As the title say, I don't expect this PR to be merged as it is. I made some changes in many parts for many reasons, I could propose things one by one of course.
I am motivated to make some work on this lib, I use it myself 3 times in my softs (Patchance + RaySession x 2).

I would be happy to have your opinion about the following points:

  1. typing everywhere, I find it really nicer to know the types in the IDE. It includes typing on Structure classes. It allows easier auto-completion.
  2. I made something on callbacks declarations because I saw a big redundancy of code with many try/except structs, this way it works the same, API functions for setting callbacks are now decorated, and all is made in the decorator.
    I was wondering why there were callbacks CFUNCTYPE saved as global (_thread_init_callback, _shutdown_callback ... example line 699/700) in the API functions. I discovered that it was just a way to prevent python cleaner to remove these objects. In addition to the fact that I find it quite dirty, I see the impossibility to get 2 jack clients with same callback working correctly (the 2nd callback will erase the 1st, not tested but I am pretty sure). That's why I simply add theses objects to a list (_used_callbacks).
  3. I find the file too big, hard to parse. It would be nice to split it, having the API functions in the same file.
  4. I find strange to have a c_char_p pointer as output from get_all_ports, I think it should deliver a list[str] directly.
  5. enums could use enum lib (with IntEnum or IntFlag), I have been using it for some time, I find it nice for many reasons (debug, easier import, parsing all values, readability...).

@SpotlightKid SpotlightKid added enhancement New feature or request question Further information is requested labels Oct 3, 2022
@SpotlightKid
Copy link
Collaborator

Hi @Houston4444, yes all these points are worth discussing, but I'm currently a bit busy with other things. But I'll write a longer answer soon (next week).

@Houston4444
Copy link
Author

@SpotlightKid , we both seem to have forgotten this thread.

As I am reworking on a 6 months old RaySession branch, I wonder if I keep the pyjacklib submodule linked to my fork, or if work could be done in this one.

@SpotlightKid
Copy link
Collaborator

@Houston4444 To be honest, I haven't done any serious FLOSS development in the past months (or almost a year). I'm just lacking motivation at the moment, and I am more interested in making music currently.

So I suggest you either maintain your own branch or talk to @falkTX to give you access to this repo.

Concerning your changes, I think most things are good, but I would like this lib to maintain a strong separation between pure wrapping of the C libjack and functionality added by Python. But since I don't see myself doing any serious work on it soon, I'm ok with someone else taking these decisions.

@Houston4444
Copy link
Author

Ok, I've done some code split and enums here : https://github.com/Houston4444/pyjacklib/tree/orga/new_split

I'll use my own version now, but I am still open to discussion of course about changes in jackaudio version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants