Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e56086814f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } catch (e) { | ||
| _completeSeekWithError(playerId, e); |
There was a problem hiding this comment.
Propagate native seek failures to the returned future
When the native seek rejects, such as for non-seekable content or an invalid player state, this catch completes an internal completer but then returns normally. The first caller therefore observes a successful seekTo, and queued seeks enter the .then branch and also complete successfully; meanwhile the internal completer's error is unobserved. Rethrow the exception or ensure _startSeek returns the operation's completer future.
Useful? React with 👍 / 👎.
| final _SeekOperation op = _SeekOperation(targetPosition); | ||
| _pendingSeeks.putIfAbsent(playerId, () => <_SeekOperation>[]); | ||
| _pendingSeeks[playerId]!.add(op); | ||
| return op.completer.future; |
There was a problem hiding this comment.
Coalesce pending seeks instead of replaying every position
When several seekTo calls arrive while one is active, every requested position is appended and later issued as a separate native seek. During rapid scrubbing this replays stale intermediate positions before reaching the latest request, causing latency proportional to the number of calls and defeating the intended latest-pending-position behavior. Retain only the newest pending target while preserving completion of all associated futures.
Useful? React with 👍 / 👎.
| final _SeekOperation? op = _activeSeeks.remove(playerId); | ||
| if (op != null && !op.completer.isCompleted) { | ||
| op.completer.completeError('Player was disposed.'); |
There was a problem hiding this comment.
Complete the future actually returned for an active seek
If a player is disposed while its first seek is in flight, this reports cancellation through the active operation's completer, but that future was never returned—the caller received _startSeek's Pigeon future instead. Disposal destroys the native callback, so the caller's future can remain pending indefinitely while this orphaned completeError is emitted as an unhandled asynchronous error.
Useful? React with 👍 / 👎.
Main changes:
Track active and pending
seekoperations in the Dart FFI implementation so only onenative seekis in flight per player. Coalesce consecutiveseekTorequests to the latestpendingposition, complete all waitingfutureswhen theseekfinishes, and failpending seekswhen the player isdisposedor reports an error.Send a
seekCompletedevent from native code so Dart can complete theactive seekand start anypending seek.