GNOME.org
 

Re: bsepcmdevice-jack.cc (Re: Beast Jack driver)

On Tue, 6 Jun 2006, Stefan Westerfeld wrote:


On Mon, Jun 05, 2006 at 04:31:23AM +0200, Tim Janik wrote:



   free (jack_ports);
 }

return devices;


ugh, isn't this a full fledged recursive map+strings+DeviceDetails copy?


Yes. So? Its not in any time critical code, so I don't bother to
optimize it.


well, it's not exactly "hard to optimize" here. just return a pointer.


I am trying to avoid manual new/delete in all code I write, because its
a potential source of errors. So returning a pointer is not so good. It
also makes map accesses somewhat strange to read, i.e.

 (*devices)["foo"] = bar;

instead of

 devices["foo"] = bar;

Under some circumstances, the C++ compiler will also construct objects
that are returned from a function within the memory of the caller, so
that no copy will done at all. But I don't know if the device map case
is one of these cases, or not.


well, it'd be good to find out about that then ;)
i.e. does std::map do the kind of internal data refcounting that std::string does? 


So if you insist that it must be "optimized", I would suggest changing
the API to

static void
query_jack_devices (BsePcmDeviceJack           *self,
                   map<string, DeviceDetails> &devices)

then I can still keep my automatic deletion feature and nice to read
map access code.


sure. it'd still be interesting to know how expensive map copies are though ;)


you're not setting error in case you failed to find
the required devices to connect the channels.


I don't want to. If you for instance start beast with

-pjack=jamin

to use jamin for mastering, and later the user chooses to terminate
jamin, then we can still run. Its just that then the ports will be
initially unconnected, which is not a problem, because it can be easily
fixed in qjackctl.


sorry, would you care to enlighten me here please?
what's -pjack=jamin supposed to mean?
should the beast output ports be auto-connected to input ports named
"jamin"?

Yes.


and, if input ports named "jamin" are not present, that'll silently be
ignored?

Yes.


will the jack callback thread still properly run and request BSE engine data
if the beast output port is unconnected?

Yes.


if that's the case, i think at least the behaviour of silently ignoring
unsuccessfull connections has to be changed. we also throw proper errors
in other drivers if unconectable devices are being adressed.
(if unsuccessful connections really are common usage with jack, we can
still add device flags, like we do with OSS, so you could say
-pjack=jamin,auto for automated connections)


I don't know from your example what you envision the syntax to be like.
There are four good cases as I see the situation.

(1) request beast to autoconnect, and request beast to find a suitable
   device to autoconnect to without asking

-> this is what -pjack does right now, and I think it should be kept
  this way


you mean, the shorthand -p jack is equivalent to -p jack=alsa_pcm ?
i'd say that makes sense.


(2) request beast to connect to a specific client if possible, but
   continue if that doesn't work (we could have some [X] report errors
   on jack connection problems message, then the user could disable
   the error, or we could make this configurable)

-> this is what -pjack=jamin does right now


i don't think continuation is a good idea here, as i already outlined.
we don't accept failing destinations in the other drivers either.
in general, we don't by default silently ignore warnings. and shouldn't
do that here either. about making the warning/error dialog optional,
that is always a good idea anyway. the point is about having the warning/
error in the first place though, and yes, that should be the case as it
is with other drivers. however, you said it would make sense in some scenrios to let -p jack=port 
effectively behave like -p jack=unconnected in case "port" can't be found.
that's what i suggested the "auto" flag for. i.e. -p jack=port,auto could
either succeed in connecting to "port" or behave like -p jack=unconnected
if "port" couldn#t be connected to.


(3) request beast to connect to a specific client, and let it fail and
   report an error if it doesn't work


that's exactly like (2), since reporting an error/warning will be a dialog
with a [x] deduping option.


(4) request beast to start without connecting a client

-> this is what -pjack=unconnected does right now, as long as there is
  no jack client called "unconnected", in which case this behaves like
  (2)


is "unconnected" some kind of 'standard' in the jack world?
what do other programs use as identifier for this mode?
i'm asking because i think "unconnected" is maybe a bit more tedious
than just "open". what do you think?


What I wasn't too sure about when designing the syntax is whether the
position of the argument should play a role.


definitely, take a peek at beast --bse-driver-list. note that i've not
seen anything about your "syntax" yet though, you just wrote "jack=PORT"
in the docs. in any case, you should basically pick up what the OSS driver
does, for jack that'd be (mockup):

  jack jack=PORT,MODE                 // or s/MODE/FLAGS/ if you prefer
    JACK sound daemon PCM driver:
      PORT - Jack port name (use "open" for unconnected ports)
      MODE - may contain "rw" or "wo" for read-write or write-only
             access; adding "hs" forces hard sync on underruns.
             Adding "auto" allows failing port connects and behaves
             as if "open" was specified as PORT.
    Devices:
   >        alsa_pcm (read-write)



Suppose we also decided
some day to add ro (readonly) and wo (writeonly) as options. Should then

-p jack=auto,ro,alsa_pcm

be the same like

-p jack=alsa_pcm,ro,auto


nope, like OSS, you're supposed to use jack=RHS with:
  RHS          := DEVICEorPORT [ ',' FLAG ]*
  DEVICEorPORT := 'alsa_pcm' | 'open'
  FLAG         := 'rw' | 'wo' | 'auto' | 'hs'


This rules out the possibility of connecting clients called "auto" or
"ro", which is maybe a bad decision. So maybe we should start with
something like

-p jack=connect:alsa_pcm
-p jack=connect:alsa_pcm,wo

or so? I am really not sure what would be a good and clean syntax here,
that handles all cases that are needed.


i don't think we have to do that just yet without any use cases.
using the port name as first arg is good and easy enough and allowes
for arbitrary (albeit not comma-comprised) port names.



It could even happen that we are not connected initially (latency = 0),
and only get connected afterwards by a patchbay, where the user is
loading a session. Thus in such a case we would always misbehave.


what's that about being non-connected.
does jack still request audio from beast when unconnected?


Yes.


ok, that's where you'd then usually want to use -p jack=PATCHBAY-INPUT,auto
if i understand correctly. right?


and, you said jack clients would always auto connect to alsa_pcm
and that alsa_pcm should always be present. so why would we start
up disconnected?


Thats not what I wanted to say. I just pointed out that lots of people
hardcode alsa_pcm somewhere in their code. However, whether a jack
application should startup connected or unconnected, or how defaulting
or auto connecting should work for "normal" jack applications is an open
issue as far as I understand from reading the mailing list. There have
been some threads suggesting this or that (like api extensions, extra
default flags on ports and the like), and some people have been pointing
out that they like this or that behaviour better.

So I think until people have figured out what they want, I'd like to see
beast to support both: connect on open and open unconnected (so that
some external program can do the work).


ok, i see.
well, ... and default to "alsa_pcm" i presume?


guint user_buffer_frames = BSE_PCM_DEVICE (device)->req_latency_ms *
handle->mix_freq / 1000;
guint buffer_frames = max (min_buffer_frames, user_buffer_frames);

jack->input_ringbuffer.resize (buffer_frames, handle->n_channels);
jack->output_ringbuffer.resize (buffer_frames, handle->n_channels);


the way this is written, you're doubling the latency because you apply
it to the input and the output buffer. you should just apply it to the
output buffer (and also subtract the input buffer latency before doing
that)


The input buffer should not produce latency. Ideally, the filled frames
of the input buffer and the filled frames of the output buffer should
add up to exactly buffer_frames. Note that the input_buffer is *empty*
initially while the output_buffer is *full* initially, and the jack
callback methodically only adds as many frames to the input_buffer as it
takes from the output_buffer, whereas the engine process cycle does the
inverse.


that means input latency increases with every output underrun, because
in that case you're faking extra output values. basically, in duplex
scenrios under latency constraints you have to properly resync input
and output and can't just pad output values.


I don't think it can happen. After an output underrun, the
jack_device_retrigger code clears both ringbuffers (input and output),
and then refills the output buffer with zeros.


well, note that for the sake of making resyncronization/retriggering
less nasty when you encounter underruns, we might want to do less
than fully retriggerring the device.
e.g. we could "soft-sync" by writing an extra 0-buffer into jack and
by throwing away a single input buffer (to keep the input latency).
doing that will need the jack thread to write-access the read-pointer
of the ring buffer though, which you code currently doesn't account for.
or, it might also be possible to have an atomic throw-away-input-blocks
counter that the BSE driver trhead could decrement and ignore incoming
buffers.... that's a bit hackish in the presence of a limited ring buffer
size though, so adjusting the read-pointer from the jack thread would be
much better.
reducing underrun impact by using soft-sync vs. hard-sync is also
done by the OSS driver ("hs" flag).


Timeouts are a bad solution because they either occur too soon (in which
case the engine thread may wake up, but can't do the computation), or
too late, because there is always jitter. In any case you loose precious
time or waste precious CPU cycles or both.


note that the BSE audio driver model *and* the BSE engine model are based
on timeouts. that's what check_io() provides and what the engine needs.
you won't get any other programming model with BSE, regardless of whether
you like it or not ;)


Well, pipes are another programming model. I don't think we'll need
timeouts when we can use pipes.


it's not a question of "need". we *have* timeout behaviour already.


But right now I can only make
theoretical claims about it: I will have to work on the code to see
whether the code can be written in a way so that check_io will rely on
the notifications of pipes (and the lockless ringbuffers) only.


fine. if we switched to pipe based notification, we should do that for
*all* drivers. since the changes related to this are not going to be
trivial, i really want to seperate this from the general jack driver
development. so please lets concentrate on finishing up bse-jack with
the current timing model and after that look into timing alternatives.


Then it
will be able to return an arbitary timeout, but the pipe will still
ensure that everything goes well.

  Cu... Stefan


---
ciaoTJ
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]