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

API: Allow caching for output ports #602

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

Conversation

twischer-adit
Copy link
Contributor

This commit reverts 3d97601 ("More documentation in jack.h") partly.

The following call stack is used:
jack_port_get_buffer()
JackGraphManager::GetBuffer(jack_port_id_t, jack_nframes_t)
JackGraphManager::GetBuffer(jack_port_id_t)
JackPort::GetBuffer()

To call JackGraphManager::GetBuffer(jack_port_id_t, jack_nframes_t)
the object JackLibGlobals::fGlobals->fGraphManage is used. It is only
set in JackLibClient::Open(). Therefore it is not changed in the life
time of a JACK client.

In case port->fTied will be modified at runtime the returned audio
buffer would change. This variable can only be modified by the
deprecated jack_port_tie() function. Therefore as long as not calling
this deprecated function the audio buffer would not change.

The previous analyses is only valid for JACK audio output ports
(JackPortIsOutput). The creating JACK client can write to those
ports. For JACK audio input ports (JackPortIsInput) it is still not
allowed to cache the audio buffer address because for those ports the
returned audio buffer is depending on the port connections
(pipelining). The decision for the pipelining is done in
JackGraphManager::GetBuffer(jack_port_id_t, jack_nframes_t).

This commit reverts 3d97601 ("More documentation in jack.h") partly.

The following call stack is used:
jack_port_get_buffer()
JackGraphManager::GetBuffer(jack_port_id_t, jack_nframes_t)
JackGraphManager::GetBuffer(jack_port_id_t)
JackPort::GetBuffer()

To call JackGraphManager::GetBuffer(jack_port_id_t, jack_nframes_t)
the object JackLibGlobals::fGlobals->fGraphManage is used. It is only
set in JackLibClient::Open(). Therefore it is not changed in the life
time of a JACK client.

In case port->fTied will be modified at runtime the returned audio
buffer would change. This variable can only be modified by the
deprecated jack_port_tie() function. Therefore as long as not calling
this deprecated function the audio buffer would not change.

The previous analyses is only valid for JACK audio output ports
(JackPortIsOutput). The creating JACK client can write to those
ports. For JACK audio input ports (JackPortIsInput) it is still not
allowed to cache the audio buffer address because for those ports the
returned audio buffer is depending on the port connections
(pipelining). The decision for the pipelining is done in
JackGraphManager::GetBuffer(jack_port_id_t, jack_nframes_t).

Change-Id: Ief1cd18d018d5e407876630c22254d87f3c55442
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
@twischer-adit
Copy link
Contributor Author

Hello @sletz,
sorry for pinging you directly, but it looks like you are the originator of 3d97601. Therefore I think it would be quite beneficial when you could give your feedback why the following sentences were added.

Caching output ports is DEPRECATED in Jack 2.0, due to some new optimization (like "pipelining").
Port buffers have to be retrieved in each callback for proper functionning.

Is there a misunderstanding in the comment or is it even for JACK2 not required and caching output ports is with the already mentioned limitations still allowed?

@falkTX
Copy link
Member

falkTX commented Oct 11, 2020

The caching is not mentioned in the jack1 headers at https://github.com/jackaudio/headers/blob/master/jack.h#L643
It does seem safe to cache the output buffers (assuming buffer-size callback is in place).
@sletz any comments on this?

@sletz
Copy link
Member

sletz commented Oct 11, 2020

This "pipelining" experimental development was actually never merged, so yes caching for output ports is safe.

@falkTX
Copy link
Member

falkTX commented Oct 11, 2020

Alright, thanks!

@falkTX
Copy link
Member

falkTX commented Oct 11, 2020

@twischer-adit can you add a note about port-tie? It is a deprecated API, but only because it ended up not being used by client applications. as far as I know, ladish is the only one that uses it

also, should be noted that this is a jack2-specific behaviour.
@wtay is it safe to cache output port buffers under pipewire-jack too?

@twischer
Copy link

Hi @falkTX,

can you add a note about port-tie?

Should I add the corresponding commit description as a comment into the source code

In case port->fTied will be modified at runtime the returned audio
buffer would change. This variable can only be modified by the
deprecated jack_port_tie() function. Therefore as long as not calling
this deprecated function the audio buffer would not change.

also, should be noted that this is a jack2-specific behaviour.

As far as I understand it is also allowed to cache the output port with JACK1. Therefore what do you mean special for JACK2

@wtay
Copy link

wtay commented Oct 13, 2020 via email

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.

None yet

5 participants