Skip to content

new sync engine#1198

Open
frostbyte73 wants to merge 10 commits into
mainfrom
sync-engine
Open

new sync engine#1198
frostbyte73 wants to merge 10 commits into
mainfrom
sync-engine

Conversation

@frostbyte73
Copy link
Copy Markdown
Member

No description provided.


a.remaining = compensationDuration.Abs()
logger.Debugw("starting audio pacer", "remaining", a.remaining, "rate", rate)
a.inputAtStart = a.inputAccum.Load()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data race on inputAtStart? the streaming thread from the pad probe reads it? We can make it atomic as well.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for the following 2

var toStart time.Duration
if tc.current == 0 && tc.pending.Abs() >= DefaultThreshold {
// Only start if applying 'pending' keeps processed within budget.
if (tc.processed + tc.pending).Abs() < MaxDriftBudget {
Copy link
Copy Markdown
Contributor

@milos-lk milos-lk Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max drift budget might still be needed - right? There might be pathological case where we keep getting drift values in the same direction and they add up to pipeline latency - mixer will discard it if it's drifting in the past or they might get overwritten inside leaky queues if it's drifting in the future. Having some logging for that case might be useful.

if conf.EnableSyncEngine {
w.trackSync.OnSenderReport(func(drift time.Duration) {
if w.driftHandler != nil {
w.driftHandler.SetDrift(drift)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm - I am not sure if we are sending the right value as the drift param from the SDK.
GetPTS runs per packet and correctly returns wallPTS for audio when tempo controller is enabled. The onSR callback (syncengine.go:259-269) still computes drift := sessionPTS - expectedElapsed // ≈ 0

This doesn't seem to use wallClockPTS at all. It doesn't know what PTS the audio track is actually using. It just compares the regression output against wall clock — which are nearly equal.

So it looks like GetPTS is doing the right thing (wall clock for audio). But the drift signal fed to the tempo controller doesn't reflect that GetPTS is using wall clock? The drift signal needs to measure the gap between what GetPTS is producing (wall clock) and what it should be producing (NTPregression).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants