libpd / abl_link

Ableton Link integration for Pure Data on desktop and Android.
Other
80 stars 15 forks source link

Using phase output to sync, timing seems unstable #20

Closed jamshark70 closed 3 years ago

jamshark70 commented 5 years ago

I'm running abl_link~ against Ableton's sample QLinkHut app.

  1. Run QLinkHut. Start playing the metronome.
  2. Open PD and turn on DSP.
  3. Open the abl-link-2 patch, click [connect 1( at the top.

Sync starts off reasonably well, but within a minute or two, Pd will be lagging behind QLinkHut noticeably. (Same when SuperCollider and Pd are the Link peers: Pd lags behind.)

Is there something terribly wrong with the approach? Or an abl_link~ issue?

abl-link-test.zip

jamshark70 commented 5 years ago

To make it clearer, I recorded two tests: QLinkHut playing alongside SuperCollider, and QLinkHut with Pd.

SuperCollider is slightly early, but I think this is because of https://github.com/Ableton/link/issues/64 (QLinkHut with the JACK backend doesn't account for audio hardware latency) -- so it's very likely that SC is on time and QLinkHut is late. (EDIT: Confirmed later in my office, running SC vs Ableton Live, sync is very good. So: SC = OK, QLinkHut = a little late.)

But then we come to Pd... if QLinkHut is late, then Pd is very, very late. If I skip QLinkHut and run SC vs Pd, the result is consistent with what you see here (Pd is audibly late).

So there are two possible conclusions. I don't know which one is correct.

qlinkhut-sc-pd

jamshark70 commented 4 years ago

I have also tried https://github.com/libpd/abl_link/blob/master/external/metronome.pd against SuperCollider. abl_link~ correctly picks up the tempo from SC (I had started the LinkClock first) but sounding events in Pd are late.

I believe abl_link~ is not accounting for audio driver latency, as the Ableton spec requires.

So currently this external is not capable of syncing with non-Pd peers.

umlaeute commented 4 years ago

I believe abl_link~ is not accounting for audio driver latency, as the Ableton spec requires.

this is probably true (as Pd doesn't expose this information)

So currently this external is not capable of syncing with non-Pd peers.

but this is plainly wrong. as i see it, both applications are synched (a constant delay doesn't matter in that respect).

jamshark70 commented 4 years ago

(a constant delay doesn't matter in that respect)

Go join a symphony to play, oh, crash cymbals or timpani or snare drum. Play everything 200 ms late. See how long you keep your job :rofl:

You have a point that, in SuperCollider, I could manually shift the time later. But you can't do that with Ableton or, I'd wager, most other Link-enabled software.

So Pd I suppose could manually shift time earlier -- this needs to be included in this package, not something for users to hack up by themselves.

planethcom commented 4 years ago

How about to measure the latency and then set the offset in ms. Or if you can't measure it for whatever technical or other reason, let the user choose it with a slider. https://github.com/libpd/abl_link/blob/master/external/abl_link~.cpp#L105 I'd say that's exactly what it is for, isn't it?

jamshark70 commented 4 years ago

How about to measure the latency and then set the offset in ms. Or if you can't measure it for whatever technical or other reason, let the user choose it with a slider.

That would be fine -- if it can't be set automatically, let the user configure it.

But, does abl-link~ have an inlet for the offset? If it does, it's not mentioned in the help patch.

danomatika commented 4 years ago

It may not be in the patch but https://github.com/libpd/abl_link/blob/master/external/abl_link~.cpp#L184 seems to indicate it will respond to an "offset" message.

jamshark70 commented 4 years ago

It may not be in the patch but https://github.com/libpd/abl_link/blob/master/external/abl_link~.cpp#L184 seems to indicate it will respond to an "offset" message.

Ah ok, chalk that up to me being not quite fluent in the pd way. That indeed looks promising (I did see where the offset is added to some time value). I'm not at my computer today but I'll try it when I can.

If that works, and if (as umläute says) externals don't have access to the audio latency value, then it's a matter of improving the documentation.

danomatika commented 4 years ago

Well, the “Pd way” is to have messages documented in the help patch. I only pointed out the function in the source code since the code was referenced previously.

It was probably added and the help patch was not subsequently updated. If you have time and interest, you could update the patch with the missing messages and post it here.

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On Dec 15, 2019, at 4:45 AM, H. James Harkins notifications@github.com wrote:

It may not be in the patch but https://github.com/libpd/abl_link/blob/master/external/abl_link~.cpp#L184 seems to indicate it will respond to an "offset" message.

Ah ok, chalk that up to me being not quite fluent in the pd way. That indeed looks promising (I did see where the offset is added to some time value). I'm not at my computer today but I'll try it when I can.

If that works, and if (as umläute says) externals don't have access to the audio latency value, then it's a matter of improving the documentation.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

Ant1r commented 4 years ago

I'm late in the thread, but I'm the one who proposed the "offset" message; Peter choose to

deliberately leave it undocumented

(see https://github.com/libpd/abl_link/pull/3#issuecomment-260160764) because he didn't like the idea that the user was responsible for the correct synchronization.

But the fact is that:

abl_link~ is not accounting for audio driver latency, as the Ableton spec requires

for the simple reason that the audio driver doesn't always give a way to get the current audio latency. For e.g I'm not sure that the linkaudio demo compensates the latency in Jack mode.

To be complete: I'm not even sure that the audio driver latency is your real problem; as you said:

Sync starts off reasonably well, but within a minute or two, Pd will be lagging behind QLinkHut

I also regularly have sync issues, but I never succeeded to know if abl_link was actually responsible for that, as it could have many other reasons in my case, like network congestion or flaws in Raspberry Pi WiFi module, and I couldn't find enough time to figure that out...

jamshark70 commented 4 years ago

OK, I tested "offset $1" and, when the offset is about twice SuperCollider's reported driver latency, sync is very good.

To be complete: I'm not even sure that the audio driver latency is your real problem; as you said: "Sync starts off reasonably well, but within a minute or two, Pd will be lagging behind QLinkHut."

I'm running it right now (for several minutes), with an offset in Pd, and actually it's quite stable. I think when I opened the issue, I was confused about what I was hearing. If the two are in sync, that's easy to hear and understand. If they aren't, it's hard to tell if it's "unstable timing" or "stable but delayed."

I also opened this issue about the phase outlet. I've tested both the step and phase outlets, and they are both behaving well.

So I'm pretty confident in it. I can make a PR later.

The reason for not documenting the option is IMO not quite sensible. If everything is going as it should, Pd provides the audio latency and Link uses it to ensure that audio hits the speakers on time. We are already outside of that territory because of Pd's limitation.

So then the question is, do you make it easy for the user to figure out how to get the correct result, or do you lead users to the incorrect conclusion that the external is "fundamentally broken" (as I said on pdpatchrepo) by not explaining upfront how to get a correct result?

danomatika commented 4 years ago

So then the question is, do you make it easy for the user to figure out how to get the correct result, or do you lead users to the incorrect conclusion that the external is "fundamentally broken" (as I said on pdpatchrepo) by not explaining upfront how to get a correct result?

I agree with you. While I understand Peter's initial hesitation, it also seems clear there is a direct need for this as with other applications using Link, ie. Supercollider, have a similar setting.

EDIT: I think it makes sense to document the additional messages with the context that they are "interim solutions" which may change or be removed in the future. We can at least admit the deficiencies of the current design and provide a workaround which, may or may not, actually be replaced later on.

danomatika commented 4 years ago

Also, maybe a subpath with some sort of "tap tempo latency" example usage would help alleviate Peter's concern: ie. the external doesn't calculate latency automatically but we can at least show people how they might be able to work with the offset in some sort of semi-automatic way.

jamshark70 commented 4 years ago

Also, maybe a subpath with some sort of "tap tempo latency" example usage would help alleviate Peter's concern.

If that's meant to be by hand, it would be difficult as latency is often on the order of 10-50 ms.

Might be interesting to get a pulse in through adc~ and measure the difference between that time and the time abl_link thinks it is.

nettoyeurny commented 4 years ago

Late to the party, but here's my take. The offset feature goes against the Link development guidelines (Link is supposed to just work, without requiring user setup), but it was clearly needed. It seemed like a reasonable compromise to implement it without documenting it, to be quietly removed when we think of something better.

It's three years later now and we still haven't thought of a better solution, so we might as well make it official and document the offset message.

danomatika commented 4 years ago

@nettoyeurny Perhaps there is something we could add to the Pd API in the current release cycle, such as estimated buffer size which could be used to compute the offset.

nettoyeurny commented 4 years ago

The buffer size would be useful first approximation, but we'd probably still need the offset message because there might be additional latency, right?

Ant1r commented 4 years ago

there might be additional latency

I'd say yes... For e.g on Android the latency is generally WAY bigger than what asked by the application; it seems there is no other way than letting the user to adjust it manually.

patricksebastien commented 4 years ago

Stupid question, let's say my setup is pd with jack. Jack is reporting 8.71ms latency (samplerate: 44100, Frames/Period: 128, Periods/Buffer: 3) is that the number I should use for the [offset< message?

Or maybe I should do a round-trip latency check for the real value (which is more like 23ms).

What about a round-trip latency divided by 2 (since I am only interested in the output (pd to speaker - not including cable length)).

jamshark70 commented 4 years ago

Late to the party, but here's my take. The offset feature goes against the Link development guidelines (Link is supposed to just work, without requiring user setup), but it was clearly needed.

SuperCollider's LinkClock does have a user-adjustable latency parameter, but that's because of a somewhat exotic but supported use case: In SC, you can have multiple audio engines running on NNTP-synced machines on a LAN, and the control layer can trigger events on any machine using timestamped OSC messages using different Server objects. The timestamp is calculated as "now" + a latency factor, which may (in theory) be different for every Server. LinkClock doesn't know which Server is being addressed, so you have to tell it the server's "latency" (really the OSC timestamp offset). (Of course, if you have different latencies on different servers, you would need multiple LinkClocks with different offsets too... which is a weird use case and probably no one would really do that, but it's not forbidden.) That is, messaging latency is user configurable, so the clock offset must be configurable too.

I don't think that's a serious violation of Ableton's intention.

Stupid question, let's say my setup is pd with jack. Jack is reporting 8.71ms latency (samplerate: 44100, Frames/Period: 128, Periods/Buffer: 3) is that the number I should use for the [offset< message?

Yesterday when I tried it, I had 11.something ms latency reported in Jack but "offset 23" is what finally worked. I don't know if that's "round trip" or "buffer duration * number of periods."

nettoyeurny commented 4 years ago

@patricksebastien Not a stupid question at all. In fact, it's a really tricky one.

The round trip latency is input latency plus output latency. I suppose you could divide that by two as a rough approximation, but input and output latencies can differ a lot, depending on the hardware and the driver.

For a more exact measurement, here's what I did in the past: Have Pd generate a click in response to a MIDI event, then point a mic at your MIDI controller and trigger it with your fingernail, so that adc~ will pick up two clicks, one from your fingernail and one from Pd. The time between the two is your output latency (plus MIDI latency, but that should be small).

danomatika commented 3 years ago

Closing for now as #21 was merged. Reopen if this is incorrect.