[marrasen]

This change and its description were written by Claude, an AI agent, working with @marrasen on gunim, a GUI framework that plays its sound through oto. Marcus tested it on his phone.

What issue is this addressing?

Updates #311

What type of issue is this addressing?

feature

What this PR does | solves

An application that draws the sound it plays needs to know which part of a player's data is being heard now. BufferedSize counts only what the player still holds, but after the mux the sound takes a while longer to reach the listener: about 250 ms over Bluetooth earbuds. This adds one method:

// UnplayedSize returns the byte size of the data read from the source that is not heard yet: the buffered data, and
// the data sent to the audio hardware that it has not played yet, also after Pause and after the end of the source.
// Where the platform does not report how long the audio hardware takes to play, UnplayedSize returns the same as
// BufferedSize. Seek and Reset discard the data already sent.
func (p *Player) UnplayedSize() int

So the part of the source being heard now is the bytes read from it less UnplayedSize, after Pause, right after Play, and at the end of the source alike.

The mux

• Mux counts the samples ReadFloat32s mixes.
• Each player records the spans of the mix its data went into, while they may be unheard. Spans the player fills back to back merge, so a player that plays on keeps one.
• A driver reports each measurement with Mux.SetHeard(frame, at): frame frame of the mix is heard at time at. The mux works out the frame heard now from the clock, and caps it at the frames mixed. Mux.ClearHeard clears the report.
• A report holds for a second, and one that puts a frame more than 2 seconds after the report, or more than a second before it, is taken as unknown.
• While nothing is reported, UnplayedSize returns what BufferedSize does. The spans stay, and count again once the driver reports, as after Resume.
• The buffer and the spans are read under the player's lock together, so a read in between cannot move data from one to the other.
• Seek and Reset discard the spans, as their data is from the old position.

Android

Every 100 ms, LoopRead measures when the frame written to the fifo next will be heard, from calculateLatencyMillis and the frames queued, and hands that pair to the mux: the fifo's write counter, which counts the frames read from Go, and the time. So the fifo and the stream are sampled together.

• The stream is asked outside mutex_. Its pointer and a generation counter are copied under the lock, and the result is kept only if the generation is unchanged and no callback ran meanwhile.
• The measurement is reset wherever the stream is dropped, paused or replaced, and the mux is told at once. Reports and resets both reach Go under delay_mutex_, so they arrive in order.
• A negative latency, as around an underrun, is clamped to zero.
• OpenSL ES has no timestamps, so there the frames the stream holds are the estimate.

The other drivers report nothing yet. Each has a TODO beside its mux.New, naming what it would use. Context.OutputLatency, from the first version of this PR, is gone.

Testing

internal/mux/unplayed_test.go drives the mux as a driver does, with reports it controls: none, steady play, after Pause, a report forgotten and made again, a report over a second old, one over 2 seconds away, the end of the source, and after Seek. The tests run in testing/synctest bubbles and wait for the mux's loop with synctest.Wait, so their results depend on no machine's scheduler. They pass with -race -cpu=1,4.

On a Pixel 8 Pro (Android 16) with Sony WF-1000XM4 earbuds, in a music player built on gunim, Marcus checked the spectrum and the animations against the sound: in step during play, after pausing and resuming (from the lock screen too), across a change of track, and with the earbuds taken out and put back.

On the Android emulator, a stress app played through gunim and this change for 15 minutes, 2,300 random actions: seeks, mostly near the end of a track, the next track queued, pauses, the speaker suspended and resumed, its buffer resized, bursts of garbage, and the screen and the app's foreground changed from outside. What it computed as heard never fell further behind than the buffer and the device's delay, 610 ms, and no measurement failed. With the driver reporting to the mux, a 10-minute run gave the same: at worst 611 ms behind.

The branch is based on main, and the workflow passed on it in my fork, on all nine jobs: https://github.com/marrasen/oto/actions/runs/41455412409

🤖 Generated with Claude Code

[hajimehoshi]

Resolve the conflicts.

[marrasen]

Rebased onto 91d77a3, now that #306 is in. The conflicts were all in binding_android.cpp, where both changes add code at the same spots: buffers_ready_ is now set at the end of PrepareBuffersLocked, since the read thread starts in EnsureStreamLocked; a reopened stream's latency is reset after ConfigureRefillLocked; and LoopRead measures the latency before its refill logic. The workflow passed on it in my fork, on all nine jobs: https://github.com/marrasen/oto/actions/runs/37735789308

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.

[hajimehoshi]

Cannot we implement the same things for other platforms? It's ok to focus on Android in this PR, but at least we should leave TODO comments

[hajimehoshi]

Apart from the shape of the API, I found some issues in the Android measurement. Most of this code would carry over to the player-level design, so they apply either way.

This review was drafted with Claude (Claude Code).

[hajimehoshi]

This is not needed. Only LoopRead stores a non-negative stream_latency_frames_, and it starts after fifo_ is made. If Latency checks stream_latency_frames_ first, a value of 0 or more already means fifo_ is ready.

[hajimehoshi]

This is the only place the value is reset. After onErrorAfterClose, a failed restart, or Pause, the last value is still reported as valid, plus a fifo that has filled up because nothing drains it. Could it be reset wherever stream_ is dropped or leaves kRunning?

[hajimehoshi]

stream was measured up to 100 ms ago, and the fifo count is from now. Each jumps by a burst at every callback, in opposite directions (Oboe's documentation says an output stream's latency "will increase abruptly when you write data to it"), so the sum is smooth only when both are sampled together. Otherwise it can be off by up to a burst, which is tens of ms over Bluetooth. How about recording, at each measurement, the fifo's write counter and when that frame will be heard, and computing the latency from that pair here?

[hajimehoshi]

This holds mutex_ across calculateLatencyMillis, which queries AAudio. The comment in StartLocked says nothing under mutex_ may wait on a device, as Pause and Resume take it on the UI thread. try_lock keeps this thread from waiting for others, but not others from waiting for it, and the fifo is not refilled meanwhile either. How about copying stream_ and a generation counter under the lock, querying the copy after releasing it, and storing the result only if the generation is unchanged?

[hajimehoshi]

Oboe implements calculateLatencyMillis only for AAudio. OpenSL ES returns ErrorUnimplemented, and has no getTimestamp either. AudioApiForSdk uses OpenSL ES below Android 11, and StartOrDeferLocked falls back to it when AAudio refuses the configuration, so nothing is ever reported on those devices. Could we estimate it there, e.g. from the stream's buffer size plus the fifo? Otherwise the documentation should say so.

[hajimehoshi]

calculateLatencyMillis is the time the next frame will be heard minus now, with no clamping, so it can be negative, e.g. around an underrun. Latency treats any negative value as not reported yet, so the result flips to unavailable for at least 100 ms. Clamping it to 0 here would avoid that.

[hajimehoshi]

Thank you for working on this. Before going into the details, I'd like to settle the shape of the API.

Context or player

The delay after the mux is shared by all the players, so measuring it per context in each driver makes sense. But apps need it per player: which part of this player's data is being heard now. Ebitengine's audio.Player.Position needs that, and so do gunim's visuals. Subtracting Player.BufferedSize and Context.OutputLatency from the bytes read gives the right answer only while the player keeps playing:

• After Pause, the mux stops mixing the player at once, but what is already queued keeps playing for one latency. The computed position stops one latency short of what was heard, and stays off until one latency after the player resumes. Over Bluetooth that is about 250 ms.
• Right after Play, the computed position is negative for one latency.
• At the end of the source, the player is paused as soon as its last bytes are mixed, so IsPlaying becomes false about 250 ms before the sound ends.
• BufferedSize and OutputLatency are read at different times, and a 10 ms read can move data from one to the other between the two calls.

Only the mux can get these right, so how about this?

• Internally, each driver reports how many of the frames the mux has mixed are not heard yet. Most of the Android code in this PR carries over.
• The mux counts the frames it mixes, and each player keeps track of which mixed frames its bytes went into, for as long as they can still be in flight.
• A new Player method returns the bytes read from the source that are not heard yet. Just a suggestion, but it could be UnplayedSize() int. Where the driver reports nothing, it returns the same as BufferedSize, so no bool is needed.

Then Ebitengine only has to call the new method instead of BufferedSize to compute the position.

I'd rather not make Context.OutputLatency public for now. If a use apart from players comes up, it can be added later, but it cannot be removed in v3.

Unit

Bytes, like BufferedSize and SetBufferSize. Callers subtract it from the bytes read from the source, and bytes stay exact whole frames. Between the drivers and the mux, frames rather than time.Duration, which also avoids converting ms to frames to Duration and back.

Name

With this, OutputLatency goes away. Even for a context-level value I'd avoid it: Web Audio's AudioContext.outputLatency and iOS's AVAudioSession.outputLatency leave out the app's own buffering, and this value includes it, so the same name would mean something different.

Just a suggestion, nothing decided: if a context-level value is made public, OutputDelay might fit. ALSA's snd_pcm_delay means the same thing: how long a frame written now takes to be heard.

This comment was drafted with Claude (Claude Code).

[marrasen]

Thanks, this is a better shape. 3d1f768 reworks the PR along your outline, and 8913cd7 runs its tests in virtual time:

• The drivers report, through an internal Mux.SetDelayFunc, how many of the frames the mux has mixed are not heard yet. Only Android does so far; each other driver has a TODO beside its mux.New.
• The mux counts the samples it mixes, and each player records the spans of the mix its data went into, while they may be unheard.
• Player.UnplayedSize() int returns the bytes read from the source that are not heard yet, and what BufferedSize returns where the driver reports nothing. The buffer and the spans are read together under the player's lock. One choice of ours: Seek and Reset forget the spans, as their data is from the old position.
• Context.OutputLatency is gone, and between the driver and the mux everything is in frames.

The six points from your review of the Android code are all in: no buffers_ready_; the measurement is forgotten wherever the stream is dropped, paused or replaced; the fifo and the stream are sampled together, as a pair of the write counter and when that frame will be heard; the stream is asked outside mutex_, with a generation check; OpenSL ES estimates from the stream's buffer; and a negative latency is clamped to zero.

The new tests in internal/mux cover no delay reported, steady play, right after Play, after Pause, the end of the source, and Seek, in testing/synctest bubbles. Marcus checked it on his phone with Bluetooth earbuds, in a music player built on gunim: in step during play, after pause and resume (from the lock screen too), across a track change, and with the earbuds out and back in. On the emulator, a stress app ran 2,300 random seeks, pauses and suspends over 15 minutes without falling behind. The workflow passed on all nine jobs in my fork: https://github.com/marrasen/oto/actions/runs/38035884350

One thing we haven't explained: once, during the phone test, right after a seek near the end of a track, the app's visuals went silent, as if the music had stopped, while the music played on, and they stayed silent across track changes until the app restarted. The app draws from a history of about 5.5 seconds of the mix, at the position it computes as heard, so this fits UnplayedSize staying more than 5.5 seconds too high for good. It never happened again, on the phone or in the stress test, and it might as well be in the app. A build that logs the delay measurements is out with the tester, and if it happens again we'll report what we find. If you know of a way AAudio timestamps can get stuck, that would be a good lead.

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.

[hajimehoshi]

Thank you for the rework. The shape is what I had in mind, and the six points from the Android review are all addressed.

The visuals that went silent

For UnplayedSize to stay more than 5.5 seconds too high, the Android delay has to stay above 5.5 seconds. I see two ways it could get there, and only one fits:

• While measurements succeed, each one starts over from calculateLatencyMillis, so it would have to keep returning more than 5 seconds. Nothing bounds a reading, so such a value goes straight through.
• When measurements stop succeeding (an error other than ErrorUnimplemented, or a callback during all three tries), Delay keeps extrapolating from the last one, with no expiry. After a stall that makes UnplayedSize too low rather than too high, so it does not explain this, but it is never corrected either.

I don't know of a documented case where AAudio timestamps get stuck. To contain it whatever it is, how about:

• treating a reading above a bound, e.g. 2 seconds, as unknown,
• forgetting the measurement after about a second without a successful one,
• and, in the tester's build, logging the raw getTimestamp position and time and getFramesWritten along with the fifo's counters, which is what would show a timestamp that is stuck or skewed.

One more lead, though I don't think it is this one: Seek calls the source's Seek while holding the player's lock, so a slow seek stalls the mix and the stream runs dry. That predates this PR, and the next measurement recovers from it.

Smaller points

• Suspend forgets what is in flight. Pause forgets the measurement, and UnplayedSize then clears sent. The paused stream keeps that data and plays it after Resume, so for one latency after Resume the position runs ahead by up to one latency. Could sent be kept while the delay is unknown, and only left out of the sum? After a device change the spans are dropped anyway, as the flushed frames count as heard.
• How the driver reports the delay. heard reads mixed and then the driver's delay, at two moments. ReadFloat32s counts a read as mixed before LoopRead writes it to the fifo, so a query in between is off by a read, 10 ms. Also, I'd rather not have the mux call into the driver on every query. How about the driver handing each measurement to the mux instead: which frame is heard when, which on Android is the write counter and the time it is heard? The mux can then work out the frame heard now from the clock alone, with the same result as Delay today, and nothing is read at two moments. On Android that is a call from LoopRead every 100 ms, like oto_oboe_read.
• TestUnplayedSize in oto_test.go compares UnplayedSize and BufferedSize from two calls, and the mux can fill the buffer between them right after Play, so it can fail without a bug. Could it drop that comparison? The tests in internal/mux cover the behavior.
• Tests: please use t.Errorf rather than t.Fatalf for got/want checks, and keep t.Fatalf for setup that the rest depends on.
• The TODO in driver_js.go says "scheduled", but oto queues the data in its worklet. getOutputTimestamp might also do there: it gives the context time of the frame being heard now.

This comment was drafted with Claude (Claude Code).

[marrasen]

Thanks. 8655691, 3d5e24e and da8252d take all of these:

• The driver hands the mux each measurement. Mux.SetDelayFunc is gone. The driver calls Mux.SetHeard(frame, at): frame frame of the mix is heard at time at. The mux works out the frame heard now from the clock, and caps it at the frames mixed. On Android, LoopRead reports every 100 ms with the fifo's write counter, which counts the frames read from Go, so the mixed count is never read at a second moment. When the stream is dropped, paused or replaced, ResetDelayLocked tells the mux with Mux.ClearHeard. Both calls reach Go under delay_mutex_, so a report of an old stream can't land after it. This means Pause and onErrorAfterClose call into Go while holding mutex_. The Go side takes only the mux's own lock, which calls nothing while held.
• Bounds. A report holds for a second, and one that puts a frame more than 2 seconds after the report, or more than a second before it, is taken as unknown. Both live in the mux, so they apply to every driver.
• Suspend. While nothing is reported, a player keeps its spans and leaves them out of the sum. They count again once the driver reports, so the position no longer runs ahead after Resume.
• Tests. New ones cover a report forgotten and made again, a report over a second old, and one over 2 seconds away. TestUnplayedSize only checks that the value is never negative. Got/want checks use t.Errorf.
• The TODO in driver_js.go now names getOutputTimestamp and the frames queued in the worklet or the script processor.

On the emulator, the stress app ran 10 minutes on 8655691: at worst 611 ms behind, the same as before, and no measurement failed. The workflow passed on all nine jobs in my fork: https://github.com/marrasen/oto/actions/runs/38528513752

On the silent visuals: it hasn't happened again, on the phone or in the stress runs, and we have no lead on why it happened. If it comes back, we'll log the raw getTimestamp position and time, getFramesWritten and the fifo's counters, as you suggest, and report what they show. One correction to what we said before: the build on the phone was an earlier revision, which didn't discard the spans on Seek. That puts the position off by at most one latency, so it doesn't explain more than 5.5 seconds either.

We'll leave the Seek lock out of this PR, as it predates it.

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.

[hajimehoshi]

Thank you for the update. I found a crash: Suspend before playing starts panics on Android.

Stream::Pause now calls ResetDelayLocked unconditionally, which calls oto_oboe_clearHeard, which calls theClearFunc. theClearFunc is set only in oboe.Play, so before that it is nil.

This can happen in an app. NewContext returns before its goroutine calls oboe.Play, and Context.Suspend does not wait for it: whichever takes the Android context's lock first runs first. Ebitengine calls Suspend on lifecycle events as soon as NewContext returns, without waiting for the context to be ready, so an app sent to the background right at launch can crash.

I reproduced it with this test in internal/oboe, built for android/arm64 and run on an emulator (Pixel 3a, Android 14):

func TestSuspendBeforePlay(t *testing.T) {
	if err := oboe.Suspend(); err != nil {
		t.Errorf("Suspend before Play: %v", err)
	}
}

It passes on main, and on this PR it panics:

panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
github.com/ebitengine/oto/v3/internal/oboe.oto_oboe_clearHeard(...)
	internal/oboe/binding_android.go:85
github.com/ebitengine/oto/v3/internal/oboe._Cfunc_oto_oboe_Suspend()
github.com/ebitengine/oto/v3/internal/oboe.Suspend()
	internal/oboe/binding_android.go:58

Could ResetDelayLocked skip telling Go until Play has been called? play_called_ already tracks that, and nothing has been reported before it.

This comment was drafted with Claude (Claude Code).

[marrasen]

Thanks for catching that. 6692dc6 does as you suggest: ResetDelayLocked calls into Go only once play_called_ is set. The only other call into Go that this PR adds, oto_oboe_setHeard, comes from LoopRead, which starts only after Play.

Your test is in internal/oboe/[suspend_android_test.go](https://github.com/internal/oboe/suspend_android_test.go). On an Android 16 emulator (x86_64), it panics without the fix and passes with it. The workflow passed on all nine jobs in my fork: https://github.com/marrasen/oto/actions/runs/38800479250

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.

[hajimehoshi]

Sorry, I'm the one who suggested UnplayedSize, and now I'm having second thoughts about whether it is enough on its own. I'd like to hear your view before you change anything more, so I have a couple of questions about the API.

1. How do you use UnplayedSize in gunim? Do you take the bytes read from the source less UnplayedSize as the position being heard? And how would you expect an app to use it for calibration, like a rhythm game's audio offset? UnplayedSize keeps moving with the player's buffer, so the latency itself cannot be read from it directly.

2. Would you prefer a separate API for the output latency? For example, a Context method returning (time.Duration, bool): how long a frame takes from being mixed to being heard, without the players' buffers. The mux already has what it needs for it. One option is to have both: UnplayedSize for a player's position, as it handles Pause, Play and the end of the source, and the latency for calibration or for showing it to the user. Or do you think UnplayedSize alone is enough? The name is open, if we add it.

This comment was drafted with Claude (Claude Code).

[marrasen]

Thanks for asking first.

1. Yes. gunim plays its whole mix through one player, so the frame of the mix being heard is the frames read from the source less UnplayedSize. gunim draws its visuals at that frame, and stamps recorded input with it. For a rhythm game, that position is enough on its own: the app times its notes and the player's taps against the part being heard. The output latency then needs no calibration, and a calibration step only covers what the system doesn't report.

2. Yes, we would use a latency too, so I'd prefer both. Before this PR, gunim's copy of oto had Context.OutputLatency() (time.Duration, bool), the first version of this PR. gunim used it for three things UnplayedSize can't give:

   • sizing the player's buffer, so the buffer and the device's delay together come to the latency the app asked for;
   • telling the app how long a sound takes to be heard, which gunim's round-trip measurement through a microphone adds in;
   • waiting until a fade-out has been heard before it stops the stream on suspend.

   With UnplayedSize alone, gunim uses a fixed guess for these, so over Bluetooth earbuds they come out about 250 ms short. UnplayedSize less BufferedSize gives the latency only while the player plays.

OutputLatency works as a name for me. Would you like it in this PR, or in a separate one after it? I'd lean to a separate one, so this one stays as you've reviewed it.

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.

[hajimehoshi]

I'll reply this tomorrow. I'm sorry for keeping you waiting

[marrasen]

No rush! I'm using a fork for my stuff and I totally understand that you've got a lot on your table. Thanks for looking in to this!

[hajimehoshi]

Sorry for the late reply, and thank you for the detailed answers.

I'm not sure oto needs OutputLatency yet, so let's focus on UnplayedSize in this PR and leave OutputLatency for now. Of the three uses you listed, waiting until a fade-out has been heard can already be done with UnplayedSize: wait until the bytes read from the source less UnplayedSize reach the end of the fade-out. We can come back to the latency itself if more uses come up.

Two small things on UnplayedSize:

• Could the godoc drop its last sentence, "So the part of the source being heard now is the bytes read from it less UnplayedSize."? It is about how callers use the value rather than what it is.
• TestUnplayedSize in oto_test.go only checks that the value is not negative, and the tests in internal/mux cover the behavior. Could it be removed?

This comment was drafted with Claude (Claude Code).

[marrasen]

Thanks, that works for us. 64c8b2f drops the last sentence of the godoc and removes TestUnplayedSize. The workflow passed on all nine jobs in my fork: https://github.com/marrasen/oto/actions/runs/41455412409

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.

[hajimehoshi]

A few fixes to the docs.

This review was drafted with Claude (Claude Code).

[hajimehoshi]

Two fixes here:

• "Seek and Reset discard the data already sent" reads as if they stop that data from playing, but it still plays out. Only UnplayedSize stops counting it.
• "Where" covers only platforms that never report. On Android the report is also cleared while suspended, or after a second without a measurement, and UnplayedSize returns the same as BufferedSize then too. "While" covers both.

Suggested change

	// Where the platform does not report how long the audio hardware takes to play, UnplayedSize returns the same as
	// BufferedSize. Seek and Reset discard the data already sent.
	// While the platform does not report how long the audio hardware takes to play, UnplayedSize returns the same as
	// BufferedSize. After Seek or Reset, the data sent before it is not counted.

[hajimehoshi]

"About every 100 ms" is an implementation detail (kDelayEvery), and it isn't guaranteed: a measurement is skipped when mutex_ is busy, when a callback interrupts all three tries, or on an error. What the mux relies on is already in SetHeard's doc. Also, "with when a frame is heard" doesn't parse.

Suggested change

	// which can be after Play returns. heardFunc is called about every 100 ms while
	// playing with when a frame is heard, where frame 0 is the first frame read with
	// readFunc. clearFunc is called when the last of those no longer holds.
	// which can be after Play returns. heardFunc is called while playing with a
	// frame and when it is heard, where frame 0 is the first frame read with
	// readFunc. clearFunc is called when the last of those no longer holds.

[hajimehoshi]

Nit: "put a frame's being heard after the report" is hard to parse. How about:

Suggested change

	// maxReportedDelay is the longest a report can put a frame's being heard after
	// the report.
	// maxReportedDelay is the furthest after a report that the frame it reports
	// can be heard.

[marrasen]

Thanks! 7c0329b takes all three suggestions as written.

Reply written by Claude (Anthropic), an AI agent, and posted at Marcus's request.