Re: Beast Jack driver (Re: Lockfree ringbuffer review)

[Stefan Westerfeld <stefan space twc de>]

   Hi!

I am still working on a new version of the code, where I merge your
comments, so I'll just reply to points I can immediately reply, and post
new code when done.

On Thu, Jun 01, 2006 at 10:53:31PM +0200, Tim Janik wrote:
> >>>Finally, if we were to optimize the JACK driver for maximum performance,
> >>>a more important factor would probably be that the ringbuffer even has
> >>>this API.
> >>
> >>sorry? what API are you talking about?
> >>void    bse_block_copy_float    (guint          n_values,
> >>                                 gfloat        *values,
> >>                                 const gfloat  *ivalues);
> >>copying?
> >>
> >>>It means that we need to deinterleave and reinterleave the
> >>>data JACK supplies on the stack, and then read/write it to the
> >>>ringbuffer. If the ringbuffer would offer an API to access its memory
> >>>directly, the extra copy would disappear. But it makes the code harder
> >>>to maintain and for now my first priority has been getting things
> >>>implemented correctly.
> >>
> >>i don't quite understand this point.
> >>are you saying that replacing memcpy() by bse_block_copy_float() can't
> >>be done due to interleaving? :-O
> >
> >No. It can be replaced, and the new version has it. However, our data
> >flow looks like this:
> >
> > input
> >
> >jack thread process() callback supplies N separate buffers
> >  |
> >  V
> >interleave on the stack (manually implemented, in process())       
> >*jack-thread*
> >  |
> >  V
> >copy into the input ringbuffer (input ringbuffer write)            
> >*jack-thread*
> >  |
> >  V
> >copy in bse engine supplied buffer (input ringbuffer read)         
> >*engine-thread*
> >  |
> >  V
> 
> >[ beast processing ]                                               
> >*engine-thread*
> 
> note that this is processing in BSE, not beast (the GUI).
> and BSE does deinterleave its data, process it and then reinterleave
> because standard audio drivers want that. for jack this doesn't
> make sense though, so the data flow should instead be:
>   n-jack-buffers -> ringbuffer-write -> ||
>   || -> ringbuffer-read -> n-bse-channel-blocks

Ok, the new version gets rid of the interleaving/deinterleaving. That
requires changes in the driver API of BSE, though.

> >>>void
> >>>clear()
> >>>{
> >>>  Atomic::int_set (&m_read_frame_pos, 0);
> >>>  Atomic::int_set (&m_write_frame_pos, 0);
> >>>}
> >>
> >>is this really needed outside of resize() though?
> >
> >The jack driver needs it (to get into a sane state after a dropout).
> 
> how are you avoiding concurrent access when clearing due to a drop
> out then?

The process callback checks an atomic integer before accessing any data
structures (its called callback_state). Before resetting the ringbuffer,
this callback state will be set to "inactive" (in the engine thread),
and one condition wait will be done to ensure that if a process()
callback was still running (and thus accessing data structures), it
terminated.

Thus, when the actual clear() is executed, it is guaranteed that the
jack process callback can't interfere, and - on the other hand - the
engine thread can also not interfere, because it is the thread calling
the clear() method.

Afterwards, the atomic integer is set back to "active" to reactivate the
jack thread callback audio processing.

   Cu... Stefan
-- 
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan