Skip to content

fix: correct buffering unpause condition to check buffer coverage#393

Open
nukes wants to merge 1 commit intoalnitak:mainfrom
nukes:fix-buffering-state
Open

fix: correct buffering unpause condition to check buffer coverage#393
nukes wants to merge 1 commit intoalnitak:mainfrom
nukes:fix-buffering-state

Conversation

@nukes
Copy link

@nukes nukes commented Dec 31, 2025

Description

Changed the unpause condition from checking if new data >= margin
to checking if buffer covers playback position + margin.

This fixes play/pause toggling when seeking beyond buffered data.
The old logic would unpause immediately after receiving enough new
data, even if the playback position was still beyond the buffer.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Changed the unpause condition from checking if new data >= margin
to checking if buffer covers playback position + margin.

This fixes play/pause toggling when seeking beyond buffered data.
The old logic would unpause immediately after receiving enough new
data, even if the playback position was still beyond the buffer.
@alnitak
Copy link
Owner

alnitak commented Jan 2, 2026

Hi @nukes, by modifying the check like that, the bufferingTimeNeeds parameter for the SoLoud.setBufferStream method is not considered. Doing that, it is like setting bufferingTimeNeeds to 0.

If you tell me what your concerns are, we can try to see what can be done for buffering behavior.

I cannot accept this change.

@nukes
Copy link
Author

nukes commented Jan 4, 2026

Hi @alnitak I think my modification does consider the bufferingTimeNeeds. this fix mainly wants to solve the user seek beyond the current play position. which triggers the play&pause state switch incorrectly. Let me explain more carefully:

Condition Before the Fix

  if (currBufferTime + addedDataTime - mParent->handle[i].bufferingTime >= mBufferingTimeNeeds && isPaused)

Calculation:
(current total buffer duration + newly added duration) - buffer duration when paused >= required margin

amount of data added since pause >= required margin

Meaning: Resume playback once the amount of data received since pause ≥ 2 seconds

Problem Scenario:
Timeline: 0s -------- 5s -------- 10s -------- 15s
Buffer: [===buffered===]
----------------------↑ buffered to 5s
Playback: ----------------------------↑ user seeks to 12s(look carefully about this user seek)

When paused: bufferingTime = 5s
After receiving 2s of new data: currBufferTime = 7s
Check: 7 - 5 = 2s >= 2s ✓ → Resume playback!

But the buffer only reaches 7s, while playback position is at 12s, still missing 5s of data!
the play back logic:

if (pos >= currBufferTime + addedDataTime && !isPaused)

→ Causes immediate pause again → Repeated play/pause toggling

Condition After the Fix

  if (currBufferTime + addedDataTime >= pos + mBufferingTimeNeeds && isPaused)

Calculation:
current total buffer duration + newly added duration >= playback position + required margin
-----------------------↓------------------------------
buffer end position >= playback position + 2 seconds

Meaning: Resume playback only when buffer covers playback position + 2 second margin

Same Scenario:
Timeline: 0s -------- 5s -------- 10s -------- 15s
Buffer: [===buffered===]
------------------------↑ buffered to 5s
Playback:-----------------------------↑ user seeks to 12s

Check: 5s < 12s + 2s = 14s ✗ → Continue waiting

Continue buffering... after buffering reaches 14s:
Check: 14s >= 12s + 2s = 14s ✓ → Resume playback!

Now the buffer covers playback position + 2s margin, playback is stable

@nukes
Copy link
Author

nukes commented Jan 6, 2026

Hi @alnitak ,Any feedback ?

@alnitak
Copy link
Owner

alnitak commented Jan 6, 2026

This doesn't work for me. I've tried using the websocket example (read the note at the start of the code). It works using websocketd, which sends data to the example. If you set a bufferingTimeNeeds: 10 in the example, the resulting behavior is not what is expected, and the unpause starts just after receiving new data without waiting for the 10s of bufferingTimeNeeds.

When paused: bufferingTime = 5s
After receiving 2s of new data: currBufferTime = 7s
Check: 7 - 5 = 2s >= 2s ✓ → Resume playback!

But the buffer only reaches 7s, while playback position is at 12s, still missing 5s of data!

this is the correct behavior, I think. When the buffer reaches 7S, it means that the audio length is 7s (SoLoud.getLength returns 7) and you cannot play at 12s! If the buffering starts at 5s and the bufferingTimeNeeds: 2, your problem scenario is incorrect because it couldn't be 15s.

The websocket example doesn't have a seekbar and I couldn't test some usecases, I will (I am far from my PC these days). Please try it to see how this is working and test your suggestions.

@nukes
Copy link
Author

nukes commented Jan 8, 2026

This doesn't work for me. I've tried using the websocket example (read the note at the start of the code). It works using websocketd, which sends data to the example. If you set a bufferingTimeNeeds: 10 in the example, the resulting behavior is not what is expected, and the unpause starts just after receiving new data without waiting for the 10s of bufferingTimeNeeds.

When paused: bufferingTime = 5s
After receiving 2s of new data: currBufferTime = 7s
Check: 7 - 5 = 2s >= 2s ✓ → Resume playback!
But the buffer only reaches 7s, while playback position is at 12s, still missing 5s of data!

this is the correct behavior, I think. When the buffer reaches 7S, it means that the audio length is 7s (SoLoud.getLength returns 7) and you cannot play at 12s! If the buffering starts at 5s and the bufferingTimeNeeds: 2, your problem scenario is incorrect because it couldn't be 15s.

The websocket example doesn't have a seekbar and I couldn't test some usecases, I will (I am far from my PC these days). Please try it to see how this is working and test your suggestions.

Got it, i will try to write a demo with seekbar to reproduce this issue. just give me some time.

@nukes
Copy link
Author

nukes commented Mar 4, 2026

Hi @alnitak,
I've created a single-file demo(I use AI to write this demo, hopefully it's good) that reproduces the buffering bug visually. You can run it directly on macOS to see the issue.

Demo file (check the attached demo file, and rename to xxx.dart)

buffering_bug_demo.txt

How to reproduce

cd example
flutter run -d macos -t tests/buffering_bug_demo.dart
  1. Tap "Start Streaming" — this generates a 60s sine wave and feeds PCM data slowly (1 second
    of audio every 200ms) to simulate network streaming.
  2. Drag the playback slider to a position far beyond the buffer (e.g. 30s+). You'll see an orange warning "Seeking beyond buffer!" while dragging.
  3. Release the slider — watch the log panel at the bottom. onBuffering rapidly toggles between
    true and false on every data chunk. The toggle counter will turn red.

Expected: onBuffering(true) then onBuffering(false) once when buffer finally
reaches seek position + margin.

Actual: onBuffering toggles dozens of times until the buffer catches up to the seek position.

Root cause

In src/audiobuffer/audiobuffer.cpp, the checkBuffering() unpause condition is:

if (currBufferTime + addedDataTime - mParent->handle[i].bufferingTime >=
mBufferingTimeNeeds &&
isPaused) {

bufferingTime is recorded when the handle is paused, so this calculates how much new data has
arrived since the pause. The problem is: it unpauses as soon as new data >= margin (e.g. 0.5s),
regardless of whether the buffer actually covers the playback position.

When seeking to 30s with only 10s buffered:

  • Pause triggers correctly (pos 30s > buffer 10s)
  • New 1s chunk arrives → buffer is now 11s, new data (1s) >= margin (0.5s) → unpause
  • But playback position is still at 30s, buffer only covers 11s → immediately pause again
  • Next chunk → unpause → pause → unpause → pause → ... (rapid toggling)

Fix

Change the unpause condition from "new data >= margin" to "buffer covers playback position +
margin":

// Before (buggy):
if (currBufferTime + addedDataTime - mParent->handle[i].bufferingTime >=
mBufferingTimeNeeds &&
isPaused) {

// After (fixed):
if (currBufferTime + addedDataTime >= pos + mBufferingTimeNeeds && isPaused) {

This way, the handle stays paused until the buffer actually reaches pos + margin (e.g. 30.5s),
then unpauses exactly once. The fix is a one-line change with no side effects on normal
(non-seeking) playback, since when the playback position is within the buffer, pos is always <=
currBufferTime, so the condition is equivalent.

@alnitak
Copy link
Owner

alnitak commented Mar 4, 2026

@nukes thank you for taking the time to provide the example. I realize now that I may not have fully understood the problem.

This can be merged, but the root cause of this issue is when you seek beyond the length of the actual stream data. I think this should not be permitted (ie seek at 70s in your example).
It is also illogical to attempt to seek to non-existent audio data.

What are your thoughts?

@nukes
Copy link
Author

nukes commented Mar 5, 2026

@alnitak
I use this feature because my app use server for audio mixing. In my case mix the voice and music with ducking, minimizing the latency between mix parameter change(which triggered by user operation in my app) and the first audio frame the user can hear. The audio stream under this scenario is not something like radio station on web, but is a finite mp3 file with streaming data. so my app UI has seek feature to allow the seek operation. Thus, the playback state should be right with seeking beyond the length of the actual stream data.

In summary: streaming audio for simultaneously minimizing the latency and playback control should be considered in system.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants