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

Expose jack_client_t* #232

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Expose jack_client_t* #232

wants to merge 1 commit into from

Conversation

monocasual
Copy link
Contributor

Warning: this is more of a proof of concept than an actual PR.

In Giada we need to take a pointer to the jack_client_t struct in order to provide JACK transport capabilities (start, stop, relocate playhead, change tempo, ...). Unfortunately there's no way to access such structure in "vanilla" RtAudio.

This PR shows you what we are currently doing: a terrible hack that returns a void pointer, just to avoid to pollute RtAudio.h with the jack header (the caller has to cast it back to jack_client_t*).

I'm pretty sure there is a cleaner way to do this, yet getting the jack_client_t handle is useful when you want to do more than playing audio with JACK.

Thoughts?

@radarsat1
Copy link
Contributor

I think RtAudio has always been quite hesitant to include access to pointers of backend objects. Not sure it's "right", but it's hard to come up with a good general way to accomplish this for different platforms. In theory programs that need this may be better off just using the backend API directly (ie. just use JACK itself), but I also understand that sometimes the generalization afforded by RtAudio with only some small tweaks through the pointer may be very convenient.

But I have to ask, (without digging into your source code myself sorry for the laziness), what do you use the jack_client_t for, ultimately? Maybe it is a function that makes sense to expose in a more abstract way.

@monocasual
Copy link
Contributor Author

monocasual commented Feb 19, 2020

The jack_client_t is required by the JACK transport and timebase functions, which enable synchronization between multiple JACK clients. For example, the function below starts the transport for all connected clients:

void jack_transport_start (jack_client_t * client);

Now, I'm aware that providing transport capability is way outside the scope of RtAudio. Also, it's extremely API-specific: there's no such thing on Windows with WASAPI for example. That's why the void pointer trick was the "best solution" I could come up with back then.

@garyscavone
Copy link
Contributor

Would it work to add a member to the StreamOptions structure (perhaps something like "clientPointer") and return the value that way (during the probeDeviceOpen() call)? This would be a bit of a hack but avoid any API changes. There are already a few members of that structure that are only used by a few APIs.

@monocasual
Copy link
Contributor Author

Would it work to add a member to the StreamOptions structure (perhaps something like "clientPointer") and return the value that way (during the probeDeviceOpen() call)? This would be a bit of a hack but avoid any API changes.

I vote for no API changes, so adding a member to the StreamOptions structure sounds perfect.

@radarsat1
Copy link
Contributor

Well, changing StreamOptions is also an API change ;)

But in any case perhaps it's a good idea to have a void* pointer in there that may be reserved for particular APIs.

This was also handled a bit more eloquently with the "port descriptor" API proposal from a couple of years ago, that is, replacing the ordered-integer port numbers with descriptor classes that could be specialized for each API. But in that case the user was forced to deal with some big changes. Instead we could point to an API-specific object from SteamOptions perhaps which could contain whatever custom objects are useful, but otherwise be completely ignored. It would be nice maybe to have a small inheritence structure though so that user code could use dynamic casting:

api_specific = dynamic_cast<JackAPISpecific*>(options->APISpecific)
if (api_specific != nullptr) ...

Then api_specific would be nullptr both in the case that there is no info, and in the case that it doesn't match the expected API.

@garyscavone
Copy link
Contributor

garyscavone commented Feb 24, 2020 via email

@radarsat1
Copy link
Contributor

Hm that's a good point and a bit messy.. will think..

@garyscavone
Copy link
Contributor

I don't have a good working Jack environment to test but I think my idea above would work. Create a new void *clientPointer member to the StreamOptions structure with a default value of NULL or 0. If the structure is passed to the probeDeviceOpen() function, then copy the Jack client pointer to it. The Jack client is closed in the closeStream() function, so there shouldn't be any memory issues. I suppose a user could really mess things up by using the jack_client pointer in nefarious ways but that would their problem.

@monocasual
Copy link
Contributor Author

I'm available for any help with testing on a Jack environment, of course ✋

@ntonnaett
Copy link
Contributor

I don't like this idea of exposing specific backend api. That's contrary to why people use RtAudio in the first place. If the playback functionality would be encapsulated by RtAudio on the other hand, it could be used by other APIs like Ableton Link for example. So it wouldn't be limited to JACK. Maybe you would want to sync JACK and Ableton Link then…

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.

5 participants