Re: [liblo] bundle timestamps

Hi Stephen,

Thanks for your considered comments...

On Tue, 2008-12-30 at 13:08 -0500, Stephen Sinclair wrote:
> It looks like it's going in the right direction, but I have some comments.
> First though, what would you change in the function names?  

Hm, tricky...  Maybe lo_stamped_method_handler?

This all seems a little messy though.  The way I see it,
lo_method_handler should be passed the timestamp, but that would need an
API change.  Another idea is to intead add lo_method_handler_p (or
similar) that gets parameters in a struct.  The timestamp could go in
there, as could any future info without breaking existing code.  

Something like: 

typedef struct {
        const char *path;
        const char *types, lo_arg **argv;
        int argc;
        struct _lo_message *msg;
        void *user_data;
        lo_timetag *tt;
} lo_message_p;

typedef int (*lo_method_handler_p)(lo_message_p *p);

Does that make sense to you?

> One thing
> I might have done differently is add the timetag parameter last,
> instead of first.

I'd be happy to change that if we stick with this patch.  My reasoning
for adding it first is because it comes first in the bundle.

> Also, could you add some documentation, clearly describing the
> difference between a normal method and a timetag method?

Yes for sure.  I was going to wait for your comments but now realise not
documenting makes it harder to review...

> This patch seems to add a new method type that is called exactly the
> same way as normal methods, but includes timetag information.  

That's right.

> Correct
> me if I'm wrong, but I thought the idea was to avoid queuing timetag
> method calls, so that they can be for example used in message
> forwarding.

Well there's two problems.  One is as you describe, that in some cases
you might want to handle queueing elsewhere.  Another is that bundled
messages don't have access to the timetag.  That's what I'm fixing here.

I see these as separate problems, because you might want to use liblo's
message queue but still know the timestamps.  In fact, that's what I now
want to do.  I'd like to sync with supercollider, which substracts a
fixed global latency (default 0.2 seconds) from all timestamps, giving
time for calculation.  That is, the timestamps reflect when calculation
should begin, and the calculation code adds the global latency to the
timestamp to find when the samples should actually start being written
out.  

Well that's how I understand it -- I'm not an sclang programmer, but am
working with a friend who is.  We're trying to get a simple way of
syncing software together.  So far we have sclang, perl and haskell
syncing nicely, and I'd love to add C to the list via liblo.  A preview
of our workings is here: http://netclock.slab.org/

> I realize this means actually doing method matching on receipt of a
> message, instead of just comparing timetags simply, so maybe it's not
> a very efficient thing to do.
> 
> It may also mean separating out the method matching code in
> dispatch_method() from the actual handler call, which might be a good
> idea anyway.  Perhaps if timetag handlers were kept in a separate
> list, so that it's not necessary to search through all registered
> handlers to find whether a timetag method is matched?

Well now I've better understood things, I'm happy with the queuing in
liblo the way it is (with Dominic's patch).  Until there's a clear use
case for turning off the queueing perhaps there's no point in
complicating things.

> I'm not sure this if statement really adds anything.

Oops!  I confess to being inexperienced with unions.

Cheers,

alex