Skip to content

[audioplayers] Fix seek and playback completion handling - #1168

Open
seungsoo47 wants to merge 4 commits into
flutter-tizen:mainfrom
seungsoo47:audioplayers/fix-seek-completion-handling
Open

seungsoo47 wants to merge 4 commits into
flutter-tizen:mainfrom
seungsoo47:audioplayers/fix-seek-completion-handling

Conversation

@seungsoo47

Copy link
Copy Markdown
Contributor
  • Serialize seeks and defer playback controls until seeking completes.
  • Avoid missing network seek completion callbacks after stop by rewinding while paused.
  • Reset the playback position before reporting completion in stop mode.

@JSUYA

JSUYA commented Sep 22, 2026

Copy link
Copy Markdown
Member

@codex review

Comment on lines +143 to +144
ResetPlayer();
PreparePlayer();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PreparePlayer() is called right after ResetPlayer() (which calls player_unprepare()) without re-setting the source. Same at L521-L523.
From the player_unprepare() doc comment in the Tizen SDK player.h:

The most recently used media is reset and no longer associated with the player. Playback is no longer possible. If you want to use the player again, you must set the data URI and call player_prepare() again.

Every existing unprepare→prepare path in this file re-sets the source in between: Play() L71/L80, SetUrl() L188, SetDataSource() L202. Only the two paths added by this PR skip it.

Extract the IDLE branch of Play() (L69-L87) into a PrepareSource() helper and use it in all three places.

Comment on lines +470 to +475
if (player->should_seek_to_ >= 0) {
try {
int position = player->should_seek_to_;
player->should_seek_to_ = -1;
player->Seek(position);
return G_SOURCE_REMOVE;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When Stop() is called mid-seek, L129-L132 only queues pending_action_ = kPause and should_seek_to_ = 0 and returns. On the first seek completion this block (L470-L475) runs the queued Seek(0) first and returns, so the pause is applied only after the second seek completes (L480-L489). In between the player is still PLAYING while rewinding, and Dart stop() has already returned.

Right after seeking_ = false, apply Pause() first when pending_action_ == kPause, then run the queued seek. That makes it the "rewind while paused" the PR description describes.

 player->seeking_ = false;
 if (player->pending_action_ == PendingAction::kPause) {
   player->pending_action_ = PendingAction::kNone;
   try {
     player->Pause();
   } catch (const AudioPlayerError &error) {
     player->OnLog(error.code() + ": " + error.message());
   }
 }
 if (player->should_seek_to_ >= 0) {

Comment on lines +476 to +478
} catch (const AudioPlayerError &error) {
player->OnLog(error.code() + ": " + error.message());
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the queued second Seek() fails, this only logs and then falls through to seek_completed_listener_ at L496. Dart AudioPlayer.seek() awaits onSeekComplete.first (audioplayers 6.8.1 audioplayer.dart L293-L304), so it resolves successfully although the target was never reached.
OnPrepared in this same PR reports the deferred-seek failure via error_listener_ (L433-L435). Please do the same here and suppress the success event.

}

void AudioPlayer::Seek(int32_t position) {
completing_ = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On completion in stop mode, L521-L523 sets completing_ = true and starts an async re-prepare; the completion event is sent only from OnPrepared L411-L414 while completing_ is still set. A seek() arriving before the re-prepare finishes clears it here, so onPlayerComplete is never delivered. A seek does not start new playback and should not cancel a completion that already happened.

Don't clear completing_ in Seek(); only Play()/Stop()/ResetPlayer() genuinely supersede it

@seungsoo47
seungsoo47 force-pushed the audioplayers/fix-seek-completion-handling branch from 42f1ca1 to ca10229 Compare September 28, 2026 05:50
@seungsoo47
seungsoo47 force-pushed the audioplayers/fix-seek-completion-handling branch from ca10229 to 6d7c6d1 Compare September 30, 2026 06:02
}

void AudioPlayer::Pause() {
completing_ = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The same pending-completion issue remains in Pause(). In stop mode, OnPlayCompleted() sets completing_ and starts an async re-prepare. If pause() arrives before OnPrepared() handles it, this line clears the flag and the finished track's onPlayerComplete event is lost. Pausing should not discard a completion that has already occurred. Please remove this line; Stop() already clears the flag before calling Pause().

- Do not discard a pending completion on pause.
- Report a pending completion before play, stop and source reset.
- Report completion even if re-preparing fails after playback ends.
- Run the deferred play when a chained seek fails.
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.

2 participants