monksterman / java-audio-utils

Automatically exported from code.google.com/p/java-audio-utils
0 stars 0 forks source link

Map functions for tracking port connections from JACK API #4

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?

What version of the product are you using? On what operating system?

Please provide any additional information below.

This Jack api function is requested in order to enable the surveillance of 
"current connections" of the Jack server, so as to e.g. enable persistency 
features in own application. 

Original issue reported on code.google.com by thefruit...@gmail.com on 17 Jan 2012 at 9:39

GoogleCodeExporter commented 9 years ago

Original comment by neilcsmi...@googlemail.com on 17 Jan 2012 at 9:59

GoogleCodeExporter commented 9 years ago
To track port connections, we need to map jack_port_get_connections(...) *and* 
jack_port_get_all_connections(...)  The latter is required for tracking 
non-local ports (ie. not created through JNAJack)  See 
http://jackaudio.org/files/docs/html/group__PortFunctions.html

jack_port_get_connections(..) should map to JackPort.getConnections()

jack_port_get_all_connections(...) is more problematic as it requires a port 
pointer.  JackPort objects cannot currently be created for ports not on a 
JackClient object, and is probably better kept that way!  This function also 
has restrictions on when it can be called. 

Likely mapping is Jack.getAllConnections(JackClient client, String portName)

To monitor changes should also consider mapping some or all of -
jack_set_client_registration_callback(..)
jack_set_port_registration_callback(..)
jack_set_port_connect_callback(..)
jack_set_graph_order_callback(..)

Original comment by neilcsmi...@googlemail.com on 17 Jan 2012 at 6:22

GoogleCodeExporter commented 9 years ago
Created a clone to work on this issue. Will report back here with update.

Original comment by cjrit...@gmail.com on 22 Jul 2012 at 6:01

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Attached is a .zip file containing modifications to JNAJack version 120123. The 
following classes were modified:

org.jaudiolibs.jnajack.Jack
org.jaudiolibs.jnajack.JackPort
org.jaudiolibs.jnajack.JackClient

The following classes were added:
org.jaudiolibs.jnajack.JackClientRegistrationCallback
org.jaudiolibs.jnajack.JackPortRegistrationCallback
org.jaudiolibs.jnajack.JackPortConnectCallback
org.jaudiolibs.jnajack.JackGraphOrderChangeCallback

These modifications were for, and are limited to, the issue and solutions 
described above. They include JavaDoc comments. Exception throwing and handling 
behavior mirrors pre-existing code.

Strays from original JACK spec:
Port ids are converted to full port names ( client:port ) before being passed 
to JackPort*Callback. New Port objects are not created, as this would require 
some sort of means to avoid multiple JackPort instances to be created for the 
same JACK Port pointer and could break hash-dependent data structures using 
these objects.

Known issues:
Stray "invalid port id: 0" warning on console occasionally occurs without known 
cause. No evidence of improper behavior correlated with this warning.

I, Charles J. Ritola of Atlantic Mine, MI USA hereby certify that the 
modifications described above and applied to the attached files, are my own 
work, and grant license under the Lesser General Public License (LGPL) as 
described at (http://www.gnu.org/licenses/lgpl-3.0.en.html) for their use and 
implementation.

Original comment by cjrit...@gmail.com on 23 Jul 2012 at 7:34

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks Chuck!  Generally looks good in principle, though I haven't tested yet.  
Will aim to integrate with the main code ASAP (may take a few weeks - it's 
holiday season! :) )

Few comments / questions

* Do you have some code you've been using to test these additions?  Be useful 
to have access and use the same so any issues are reproduced.
* Callbacks should not include Pointer. a) We shouldn't leak this type from the 
API. b) AFAIK the Pointer is in all cases data passed in during callback 
registration, which JNAJack doesn't let you do (so it should always be null). 
If this feature is ever required, it should be added completely on the Java 
side.
* Would prefer callback methods didn't include the word "Event" for consistency 
- ie. graphOrderChanged() rather than graphOrderChangeEvent()
* Possibly prefer using two methods rather than the boolean isConnecting, etc.
* The JackGraphOrderChangeCallback should not return int.  Should either be 
void, a boolean, an enum or even an Exception.  Not sure I see a use case for 
it not being void atm.  Haven't found an example in the Jack API of what 
happens on error - presumably it's a way to veto?
* Should the callbacks include the JackClient (others do) for consistency?  
(There is a larger conundrum there in whether to add a JackServer type. There 
can theoretically be more than one JACK server running, and the JackClient 
actually acts as the pointer to which one).
* What code is running when you see the "invalid port id: 0" error?
* JNAJack is LGPL 2.1 (to match JACK) - presume you're OK with that? 

Original comment by neilcsmi...@googlemail.com on 23 Jul 2012 at 11:16

GoogleCodeExporter commented 9 years ago
In this attachment:
* Revised .java files plus executable test code
[default-package].TestPortConnectionTracking attached to this message
which announces graph transients to the console.
* Pointers removed from callback sig
* Removed 'Event' nomenclature
* Split methods for previously boolean-defined nature.
* JackGraphOrderChangeCallback now returns void. Added 'throws
GraphOrderRejectedException' and is handled by callback wrapper to
return -1 just in case it has use. Extends Exception and not
JackException to avoid confusion that the Exception may be from JACK.
* Added JackClient to callback sigs; would rather weasel the
JackServer out of the JackClient than break existing programs with
signature change or adding method-version clutter.
* The attached test code is running when error prints to console.
* I am fine with 2.x on previous and future LGPL submissions.

There is no rush. It is turning out that the proposed project which
would use this will be pushed back.

Original comment by cjrit...@gmail.com on 23 Jul 2012 at 12:57

GoogleCodeExporter commented 9 years ago
Apparently attaching a file in gmail in response to a bug report convo does not 
actually post it to the bug report. Attached (hopefully for real this time) is 
the revised .zip.

Original comment by cjrit...@gmail.com on 23 Jul 2012 at 1:57

Attachments:

GoogleCodeExporter commented 9 years ago
Got confirmation from the JACK developers mailing list that the return code 
from the graph order callback is actually ignored, and doesn't provide any 
useful purpose.  We can lose the exception and just return void.

Original comment by neilcsmi...@googlemail.com on 23 Jul 2012 at 2:59

GoogleCodeExporter commented 9 years ago
Attached is revision without said Exception.

Original comment by cjrit...@gmail.com on 23 Jul 2012 at 3:15

Attachments:

GoogleCodeExporter commented 9 years ago
Fixed garbage collection bug in the newly-added callbacks, test program 
addresses possible bug cause pointed out by Neil where querying port 
connections from JACK thread might trigger invalid port number warning.

Original comment by cjrit...@gmail.com on 25 Jul 2012 at 4:35

Attachments:

GoogleCodeExporter commented 9 years ago
Committed with some changes to the Praxis repo [1].  Will be marked fixed at 
next release.

Changes made -
* Fixed memory leak - need to call free on Pointer after querying connections.
* Changed some terminology to better match JACK API / docs ( 
JackGraphOrderChangedCallback -> JackGraphOrderCallback, deregistered -> 
unregistered, etc.)
* Changed order of arguments in callbacks - JackClient always first for 
consistency.
* Extended JavaDocs based on JACK code documentation.
* Revised some low-level code to return Pointer rather than PointerByReference 
to simplify some added methods (automatic mappings from JNAerator had already 
been changed elsewhere)
* Code reformatted for consistency.
* Added / changed headers and copyright notices.
* Fixed threading issue in test code - need to use a single-threaded executor.

[1] - 
http://code.google.com/p/praxis/source/browse/#hg%2Faudio.jnajack%2Fsrc%2Forg%2F
jaudiolibs%2Fjnajack

Original comment by neilcsmi...@googlemail.com on 23 Aug 2012 at 5:39

GoogleCodeExporter commented 9 years ago
Sorry about the leak- I will be more careful next time.

Original comment by cjrit...@gmail.com on 23 Aug 2012 at 9:53

GoogleCodeExporter commented 9 years ago

Original comment by neilcsmi...@googlemail.com on 14 Sep 2012 at 11:36