-
Notifications
You must be signed in to change notification settings - Fork 361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Asynchronous onAddMediaItems immediately get skipped when seeking to them #85
Comments
Thanks for reporting! We are actually aware that this may an issue that needs to be fixed. The reason this is happening is the asynchronous nature of the method call. After calling The solution will be to better enforce the ordering of commands even if some of them need asynchronous resolution (this applies to other asynchronous commands as well, e.g. |
Some commands may be asynchronous and subsequent commands need to wait for them to complete before running. This change updates the queue to use (and listen to) Futures instead of calling Runnables directly. The commands are currently still added as Runanbles though, so this change is a no-op. Also moves the permission check in MediaSessionImpl to before queueing the command because the permission should be check at the time of calling the method. When executing the comamnds in the queue, we need to be careful to avoid recursion in the same thread (which happens when both the Future is immediate and running on the correct thread already). To avoid recursion, we detect this case and loop the commands instead. Issue: #85 PiperOrigin-RevId: 461827264
The commands currently use a task and a postTask that are chained together manually. In some cases, e.g. when adding MediaItems, the postTask is already a chain of commands in itself. To allow using the entire command handling as a single task (for simplified queueing), we can change the implementation to always create a single task. If multiple subtasks need to be chained together, we can do that by wrapping the method calls. In case a task is asynchronous, we can also use Futures to chain them together. Overall, this is just a refactoring and changes no logic. Issue: #85 PiperOrigin-RevId: 462085724
Some commands are run asynchronously and subsequent commands need to wait until the previous one finished. This can be supported by returning a Future for each command and using the existing command execution logic to wait for each Future to complete. As some MediaSessionStub code is now executed delayed to when it was originally created, we also need to check if the session is not released before triggering any actions or sending result codes. Issue: #85 PiperOrigin-RevId: 462101136
Some commands may be asynchronous and subsequent commands need to wait for them to complete before running. This change updates the queue to use (and listen to) Futures instead of calling Runnables directly. The commands are currently still added as Runanbles though, so this change is a no-op. Also moves the permission check in MediaSessionImpl to before queueing the command because the permission should be check at the time of calling the method. When executing the comamnds in the queue, we need to be careful to avoid recursion in the same thread (which happens when both the Future is immediate and running on the correct thread already). To avoid recursion, we detect this case and loop the commands instead. Issue: #85 PiperOrigin-RevId: 461827264 (cherry picked from commit dee8078)
The commands currently use a task and a postTask that are chained together manually. In some cases, e.g. when adding MediaItems, the postTask is already a chain of commands in itself. To allow using the entire command handling as a single task (for simplified queueing), we can change the implementation to always create a single task. If multiple subtasks need to be chained together, we can do that by wrapping the method calls. In case a task is asynchronous, we can also use Futures to chain them together. Overall, this is just a refactoring and changes no logic. Issue: #85 PiperOrigin-RevId: 462085724 (cherry picked from commit 45f1f5b)
Some commands are run asynchronously and subsequent commands need to wait until the previous one finished. This can be supported by returning a Future for each command and using the existing command execution logic to wait for each Future to complete. As some MediaSessionStub code is now executed delayed to when it was originally created, we also need to check if the session is not released before triggering any actions or sending result codes. Issue: #85 PiperOrigin-RevId: 462101136 (cherry picked from commit 7cb7636)
@tonihei could you please confirm in which release version this issue was fixed? |
If you use the latest version that is 1.0.0-beta03 then this is included. |
Hi there, we’ve been trying out the main branch for the
onAddMediaItems
callback which is exactly what we require for our use case:Our app adds a media item to the playlist via the controller but it only knows the media id of the item, the session itself then resolves the URL and metadata by communicating to an external server.
It’s been working great so far but the one edge case we’ve found so far is that when seeking to a media item that was just added, but not yet resolved, the player seems to immediately skip over it.
For example, if your playlist looks like
where A is the currently playing media item, and then you add an “unresolved" media item
B
so it then becomesIf you seek to the next media item via
seekToNextMediaItem
, it immediately skips pastB
toC
:I’ve tested that this doesn’t happen if
onAddMediaItems
resolves immediately with a URI (viaFutures.immediateFuture
).Is this by design? Or are we maybe misusing
onAddMediaItems
here?The revision this is happening on is a105d03. (We’ve built snapshot artifacts here if anyone else is interested in riding the bleeding edge)
The text was updated successfully, but these errors were encountered: