Re: [Jack-Devel] jack1 version jack2 public headers comparison

PrevNext  Index
DateFri, 24 Feb 2012 18:44:50 +0100
From Stéphane Letz <[hidden] at grame dot fr>
ToPaul Davis <[hidden] at linuxaudiosystems dot com>
CcJACK <[hidden] at lists dot jackaudio dot org>
In-Reply-ToPaul Davis Re: [Jack-Devel] jack1 version jack2 public headers comparison
Follow-UpDevin Anderson Re: [Jack-Devel] jack1 version jack2 public headers comparison
Le 24 févr. 2012 à 17:41, Paul Davis a écrit :

> We're starting the merge of the header files used by all
> implementations of JACK, following setting up several git repositories
> that make this possible.
> 
> This is a review of the differences between the header files, initally
> done by Stephane, with new comments by me ...
> 
> 
>> control.h  :
>> 
>> New :
>> =====
>> 
>> /** Driver types */
>> typedef enum
>> {
>>    JackMaster = 1,     /**< @brief master driver */
>>    JackSlave               /**< @brief slave driver */
>> } jackctl_driver_type_t;
>> 
>> jackctl_driver_type_t
>> jackctl_driver_get_type(
>>        jackctl_driver_t * driver);
> 
> Seems like a reasonable addition, especially since its specifically
> only for the control API.

OK.


> 
>> Reworked :
>> ==========
>> 
>> /jackctl_sigmask_t *
>> jackctl_setup_signals(
>>    unsigned int flags);
>> 
>> void
>> jackctl_wait_signals(
>>    jackctl_sigmask_t * signals);
> 
> can you or nedko explain the need for this change? again, since its
> for the control API the issue is not that significant but i recall
> that this part of the API attracted some significant criticism from
> torben.

Well at the beginning this jackctl_setup_signals/jackctl_wait_signals was an abstraction of the "setup signal"/"wait for quit" (Ctrl-c..) that was implemented in jackd. The same kind of abstraction was also needed on Windows for jackd.

> 
>> 
>> Reworked :
>> ==========
>> 
>> jackctl_server_start/jackctl_server_close reworked in a jackctl_server_open/jackctl_server_start/jackctl_server_stop/jackctl_server_close
> 
> seems reasonable, though what's the justification?


(Not completely sure anymore), but I think it was related to slave backend management when Devin Anderson did the MIDI backend rewrite in JACK2. This open/start/stop/close scheme was a better way to separated the steps.

> 
>> New :
>> =====
>> 
>> int
>> jackctl_driver_params_parse(
>>    jackctl_driver_t * driver,
>>    int argc,
>>    char* argv[]);
> 
> seems reasonable and sensible.
> 
>> =========================================================================================
>> 
>> jack.h :
>> 
>> New :
>> =====
>> 
>> void
>> jack_get_version(
>>    int *major_ptr,
>>    int *minor_ptr,
>>    int *micro_ptr,
>>    int *proto_ptr) JACK_OPTIONAL_WEAK_EXPORT;
>> 
>> 
>> const char *
>> jack_get_version_string() JACK_OPTIONAL_WEAK_EXPORT;
>> 
>> ==> not really used? Should they be removed?
> 
> i am opposed to any such API in JACK. providing version information
> like this encourages bad/stupid/unnecessary behaviour by developers.
> as the docs for automake describe, if you're interested in whether or
> not a particular feature exists in something you want to use, test for
> the *feature* not some arbitrary version number.

OK, so can be safely removed (not used anyway...)


> 
>> int jack_get_client_pid (const char *name) JACK_OPTIONAL_WEAK_EXPORT;
> 
> needed for?

I think session management.

> 
>> jack_port_type_id_t jack_port_type_id (const jack_port_t *port) JACK_OPTIONAL_WEAK_EXPORT;
> 
> needed for?


I see it used in jackdbus code. I guess Nedko can comment more.


> 
>> =========================================================================================
>> 
>> jslist.h: same API, code reworked a bit to be compilable without too much warning in  a C++ context.
> 
> ok, we should merge the jack2 version.

OK.

> 
>> =========================================================================================
>> 
>> ringbuffer.h
>> 
>> New :
>> =====
>> 
>> void jack_ringbuffer_reset_size (jack_ringbuffer_t * rb, size_t sz);
> 
> what's the need or use case for this? when can the ringbuffer be reset safely?

Well I added that when developing this "adapter" mechanism. Not sure of the real reason anymore. I'll need to look again.

> 
>> =========================================================================================
>> 
>> in thread.h
>> 
>> 
>> New :
>> =====
>> 
>> int jack_client_stop_thread(jack_client_t* client, jack_native_thread_t thread) JACK_OPTIONAL_WEAK_EXPORT;
>> 
>> int jack_client_kill_thread(jack_client_t* client, jack_native_thread_t thread) JACK_OPTIONAL_WEAK_EXPORT;
> 
> what's the use case for these? AFAIK, its not possible to do what the
> first one suggests in the general case (it requires that the thread
> hits a cancellation point). similar objections for the second one,
> though i think that maybe that works more reliably.

Well I see that jack_client_stop_thread is not used at all...but jack_client_kill_thread is use in NetJack2 netmanager, which use jack_client_create_thread API. jack_client_kill_thread does basically pthread_cancel/pthread_join on POSIX thread.

So jack_client_stop_thread could be removed and jack_client_kill_thread possibly kept.


> 
>> =========================================================================================
>> 
>> transport.h and types.h
>> 
>> ==> jack2 version moved some type definition from transport.h to types.h (don't remember the reason.... maybe not a good idea..)
> 
> so this won't matter, probably.
> 
>> New :
>> =====
>> 
>> typedef uint32_t jack_port_type_id_t;
> 
> use?

Needed for jack_port_type_id API.

> 
>> typedef pthread_t            jack_native_thread_t;  ==> moved in  systemdeps.h file
> 
> agreed.
> 
>> POST_PACKED_STRUCTURE is uses in jack1 transport, and missing in jack2 version.
>> 
>> This POST_PACKED_STRUCTURE should probably go in systemdeps.h
> 
> agreed.
> 
>> =========================================================================================
>> 
>> New systemdeps.h that abstract systems dependencies
> 
> agreed.
> 
>> =========================================================================================
>> 
>> weakmacro.h ==> some WIN32 specific
> 
> agreed,  i think.
> 
>> When files are the same, jack2 version something have some more documentation. It should be better to take them.
> 
> seems OK except that i think i've done updates and improvements to
> jack.h in the last several months that should not get lost.

I hope I have reported them. But a check is needed.

> 
>> 2) How to we proceed now?
> 
> another round of email, then i'll start merging it all into the
> "headers" repo. once that's done someone can try a build of jack2
> against the new headers.

Stéphane
PrevNext  Index

1330105507.3609_0.ltw:2,a <A5301633-C8C7-4B6B-92C1-C6747BE0A3B0 at grame dot fr>