Subject: Re: [linux-audio-dev] audioengine/laaga prototype now operational (audible)
From: Paul Davis (pbd_AT_Op.Net)
Date: Mon Jun 18 2001 - 15:00:18 EEST
Richard -
Thanks very much for your comments. Needless to say, we don't have
quite the same design aesthetic :)
>audioengine_handle_t *
>audioengine_open (const char *client_name,
> AudioengineProcessCallback, void *process_arg,
> AudioengineBufferSizeCallback, void *bufsize_arg,
> AudioengineSampleRateCallback, void *srate_arg);
>
>I dont like having more than one callback at all. Instead the client
its to make things that little bit faster. 99.9% of the time, when we
contact a client, its so they can call process(). there's no reason to
add a "switch (event.type) { case Process: ... }" statement.
>should query the buffer size (whats that at all!?? maximum buffersize?
most clients don't care about the buffer size. and yes, as the
comments say, its the maximum number of frames that the engine will
ever ask the client to process at once. it can change dynamically over
the life of the engine (only in response to user action, however), and
when it does, all clients have their buffer size callback executed (in
a thread-safe fashion - no call to process() will be made during its
execution). again, most clients don't care.
>Not again something like LADSPA delay_10ms delay_500ms delay_1s sort
this is nothing to do with the engine buffer size. its to do with the
size of the client's internal delay buffer, and the possibility of
dynamically resizing it. the fact that we have LADSPA designs like
this is just a reflection of an author's choice; its quite possible to
add a control port in LADSPA (or LAAGA) to set the delay length and
have the plugin/client dynamically resize its internal buffer. it just
has to be very careful about how it does this given the RT
constraints.
>of crap...). Also the sample rate should be per connection and
>constant during its lifetime (just transparently disconnect/reconnect
why? most clients don't care about the SR, and those that do probably
need a particular value.
>Also, is this "client_name" a port? Why do I register the process
No. A client *has* ports. A client is *not* a port.
>callback wrt the registration of my app to the engine? Isnt that
>process a per-port-bundle operation? I.e. more like
No. the process callback is made when all the client's ports are
ready. Thats why we don't support feedback the way GLAME does - we can
only do a weak sort on the graph. When there's no feedback, this sorts
the graph correctly; when there is feedback, as i've noted previously,
there is no correct order anyway, so it doesn't matter.
>Or you planned to support multiple audioengine_open() calls per
>process?
thats certainly possible.
>int audioengine_activate (audioengine_handle_t *handle);
>int audioengine_deactivate (audioengine_handle_t *handle);
>
>I dont see you need those for an always-running-system. Perhaps
>be able to "mute"/"suspend" a connection, but not the app
the system can be stopped at a global level (by something controlling
the engine). but these functions aren't about the global system,
they're about the client that calls them. the name is not clear. they
are need, as commented, so that a client doesn't start running as soon
as it calls audioengine_open(). it will be "waiting" until it calls
audioengine_activate(), and similarly it will return to a "waiting"
state if it calls audioengine_deactivate().
this lets a client register ports before its start running, which
makes life much simpler for simple clients (no thread race
conditions).
>audioengine_port_t *
>audioengine_port_register (audioengine_handle_t *,
> const char *port_name,
> const char *port_type,
> unsigned long flags,
> unsigned long buffer_size);
>
>whats a physical connection!??? whats a multi connection!????
a port with PortIsPhysical set is a port that refers to a real
physical world connector (e.g. an output of an audio interface).
a port with PortIsMulti set is a port that allows multiple connections
to be made to/from it. the need for this flag is unclear as you note -
its either always or never allowed.
>Drop buffer_size, too. In fact - drop support for different typed
>data than audio - you have too much specific stuff in the engine
>based approach anyway. ==>> make it simple.
Support for other port types will be coming soon. Its an important
part of the overall design. I believe it can be done without
complicating things for clients.
>int audioengine_port_unregister (audioengine_handle_t *,
>audioengine_port_t *);
>
>You dont need the engine handle.
Yes you do. Ports don't contain any information on how to contact the
engine. The handle contains all that stuff. Recall that we allow
multiple audioengine_open() calls within one process, and anyway, I
*hate* libraries that use static globals to maintain their "internal"
state. I much prefer what stdio/alsa/many others do: pass around an
opaque handle that the library uses to access its internal state.
>int audioengine_port_lock (audioengine_handle_t *, const char *port_name);
>int audioengine_port_unlock (audioengine_handle_t *, const char
>*port_name);
>
>You dont need those - instead make the ports reference counted by the
>number of open connections and gc them, if they were deleted and no
>connections are there.
You didn't read the comment, though I'll admit that the name is a bad
choice. These functions simply prevent connections from being
made/broken. They are for use by clients which provide their own
connection control UI and don't want 3rd parties interfering with what
they set up.
>GSList *audioengine_get_ports (audioengine_handle_t *,
> const char *port_name_pattern,
> const char *type_name_pattern,
> unsigned long flags);
>
>Dont use GSList's :) List of returned ports are the port identifier
>strings (the spec doesnt tell).
True. I don't want to reimplement Yet Another Generic List Type, so I
used GSList widely. I'm tired of reinventing the wheel, so I should
write some better documentation :)
>void audioengine_listen_for_ports (audioengine_handle_t *,
>AudioenginePortRegistrationCallback, void *);
>
>Uh - if you want to allow highlevel events from the engine, just
>use some more abstract stuff like a SYSV message queue for that
>(not another callback...), then you can add messages without adding
>more calls.
I don't like API's which are intended to be simple and high domain
specific to be weakly typed. I don't want clients doing stuff like:
switch (event.type) {
case Blah:
case Blah:
case Blah:
}
if a client cares about things of type X, the client should register a
specific interest in that. your approach requires potential
recompilation of any client that gets an "event" from the engine when
the event structure changes. mine defines singular event types which
don't (hopefully) change over time. if we add new notifications from
the engine, clients that only registered an interest in the old
notifications will still work without recompilation.
>char *audioengine_port_name (audioengine_port_t *port, char *buf, unsigned
>int bufsize);
>
>you cant get hold of the audioengine_port_t if you;re not the one who
>registered it? So I think this call is pointless.
Yes, I agree with you. Its because I originally started with the idea
that clients would get port_id_t's, then I realized that for
efficiency and speed, the port buffers need to be at the same address
in every process, so then i switched to use port_t's, and then i
realized that only the buffers and ptr-to-buffer need to live in the
same place in each process, and now I'm in the middle of changing
things to be more like this. Once I'm done, I'll try to rationalize
how we use references to ports.
But anyway, the point of this call is also confused. There needs to be
a better definition of a port "name": is it "client:port" or is that
the "fullname", etc.
> Also do not design
>such "compound" I-support-every-kind-of-arg-you-can-think-of calls :)
>Just use
>
>char *audioengine_port_name (audioengine_port_t *port);
>
>its not performace critical at all.
no, its not, but its an idiom i picked up about 10 years ago. to be
fair, it works much nicer in C++, where we have default args:
char *get_some_chars (char *buf = 0, int bufsize = 0);
though you should be using "string" there anyway. i like being able to
do:
char buf[256];
printf ("%s\n", audioengine_get_port_name (port, buf, sizeof(buf));
rather than:
char *ptr = audioengine_get_port_name (port);
printf ("%s\n", ptr);
free (ptr);
>audioengine_port_t *audioengine_port_by_id (audioengine_handle_t *handle,
>audioengine_port_id_t);
>
>Ah - here it is. Whats the port_id ?? You have a unique string identifier,
>so how you need another id?
see above.
>HINT: for internal (inter-process) use just use the virtual address
> (for shm the offset inside the segment) of the struct. Quite a
> useful unique identifier.
thats actually what the ID is :)
--p
This archive was generated by hypermail 2b28 : Mon Jun 18 2001 - 14:58:50 EEST