feat(sdk): add configurable media3 player builder - #127
Conversation
|
Excited to take a look at this! Hopefully will get a chance by EOW |
|
I did a quick pass and I know @brunoro is doing the same since he's kinda owning the audio stuff. I've just got a few little API suggestions that I put up here: 5d073d2 Basically just made a base |
|
Hi @jonathancaudill, thanks for the contribution! I'm glad that you already did some work on Phono, since it's a great real life tool use case that will allow us to properly tune the I think that the approach your proposed is a good compromise between providing a full Light SDK-wrapped layer for general use cases and leaking Android media objects for advanced usage. Me and @dupontgu both agree it flows well with our thoughts of building an straightforward and opinionated audio library (vs building a even more opinionated audio engine). I'll expand a bit on the next comments on what I see as the two open questions we need cover before merging the PR. |
|
What it would take to support the proposed API changes on detached audio mode? This is the trickier part since it involves many aspects of the API. The architecture we're working on for detached (background) audio is to have an SDK-owned foreground service per tool that would own the player instance. That raises some issues:
|
|
Are the proposed changes enough for a Phono Tidal MVP? Now I'm getting more on speculative territory because I only scratched the surface on the Phono codebase, so please help me expand the list. I noticed that the Phono Tidal backend uses some features that aren't supported by LightAudio atm:
|
Yes. will definitely need to dedupe this when phono migrates to SDK.
Yeah, this one is tricky. My plan was to just run in foreground when tool is open, then ship off as
I would actually push back on this a little bit--Guy had mentioned that it would be good to put these in the SDK, and I definitely agree. And in the short term if we migrate to the SDK before those are implemented, your idea could definitely work. The main issue is stream quality, since phono will bump stream quality up/down to speed up the caching process per-song. This could probably be replicated with a pretty simple "speed test" of sorts where if caching is moving unusually slow we bump stream quality down. I do need to work on making this more stable for phono as well, so I'm sure the caching will improve and change over time. I do think that the best solution will probably involve some kind of access to these events, though. This also applies to automatically-detected offline mode. |
Yeah, I think this would be a good solution. |
|
@dupontgu is there anything else you guys need from me at this time for this pr? |
@jonathancaudill not atm! I'll move forward with getting the audio SDK what it needs to build the Phono Tidal MVP. From our discussion:
I'll be putting up a PR shortly for LightAudio detached tool support (background audio) and after that's merged I'll circle back here. There shouldn't be any incompatibilities and I might even add some of the features I discussed above included in there. |
|
Hey hey - Happy to merge this after a rebase! I'm happy to do it for ya if that's helpful |
|
@dupontgu have been slammed at work lately!! will try to rebase this weekend when i get a chance :) |
|
You're welcome to do it yourself too! Main concern is that the LightAudioPlayer/LightAudioPlayerCore split and the live control of the media3 pipeline survive the recent PR even for detached instances! Was spending some time parsing it late last night but ran out of steam, haha. |
|
@jonathancaudill hi again! Sorry for stalling this for so long. I was working on getting LightOS to behave nicely with the audio SDK by making the media session playback controls work, focus arbitration, metadata store, etc. As I expected, detached mode changed a bunch of things so we might need to rethink the approach here. In detached mode the player lives in the SDK-owned foreground service that the system constructs and that outlives the handle owned by the tool, so several hooks in this PR no longer have anywhere to attach. About the four main aspects of this PR, here's a suggestion on how to deal with them:
|
|
@brunoro all of those are great points, and I like the direction you're suggesting. I'll spend some time sketching out a plan tonight or later this weekend, and hopefully we can get something mergeable. I'll also probably need to rewrite phono's Spotify backend to patch and use librespot-java instead, since Rust seems to be a no-go in the SDK. |
…layer Adds add/remove/move/replace and a mediaItemCount flow, so a tool can edit a queue it has already handed over. MediaController forwards all of these, so attached and detached behave identically. Queue state now refreshes from onTimelineChanged as well as item transitions: removing an item ahead of the current one shifts the current index without any transition, and a detached session can be edited by another controller. LightAudioItem gains an optional id, so an item whose location changes still names the same audio. Queue position no longer becomes MediaMetadata track number, since that number cannot survive the queue being reordered.
LightMediaCache names an on-disk store and says whether it evicts; newPlayer takes an ordered list of them. The SDK opens each store, shares one per directory across the process, and wires them between the network and the player it builds. Bytes are filed under the item's identity, so a re-resolved URL still finds what the old one cached. The caches wrap HTTP alone and DefaultDataSource wraps them, so local files and assets are never copied into a cache to sit beside themselves. A detached session takes its caches from process-local state rather than connection hints, because the system constructs the service and onCreate builds the player there before any controller connects. Where a player reads from is a constructor argument, so unlike usage it cannot be adopted later: reconnecting with different caches is refused instead of quietly ignored.
media3 plays DASH by finding a source for it on the classpath, so support is a dependency rather than a switch. Shipping it to every tool would cost size for a format almost none of them play, so a tool asks by declaring the capability and the plugin adds the dependency, the same way a capability already owns the permissions it needs. The version has to be named where the catalog is not visible, so a test fails if the two drift: a tool resolving one media3 version for the SDK and another for its DASH source is a failure nothing else in the build would catch.
Also records why construction-time configuration reaches the detached service through process-local state rather than connection hints, since the reason is not visible from either side on its own: onCreate has already built the player by the time the first controller connects.
1623bf1 to
6cc1c3d
Compare
Caches describe where bytes are kept, which is enough for audio media3 can fetch itself. Audio it cannot — decrypt, unwrap, a session the tool owns — cannot be described as data, so newPlayer now takes an optional LightMediaSourceFactory. It replaces the SDK's HTTP pipeline rather than layering onto it, receives the opened caches, and cannot reach the output half the SDK still owns. Detached construction is the same as caches: staged in process-local state before onCreate. Two factories cannot be compared, so a reconnecting handle must pass none at all. The one-size-limited-cache rule only applies when the SDK is the writer. media3 common, exoplayer, and datasource move to api because the factory hands those types to tools. Tools that only play URLs never need it.
1e3a93c to
4c303c0
Compare
|
Okay, I got some time to sit down with this one. Basically the configure approach we were using wouldn't work with detached audio, as Bruno already mentioned. @brunoro I went with your suggestion and ended up describing most of it as data - Also added the optional |
|
@jonathancaudill amazing, I'm excited to take a closer look at it. I'll take some time this week to review these new commits.
Agreed!
Not exactly a strict no-go, just a bigger step than we're willing to do at this specific point in time. JNI or NDK are deep rabbit holes. |
Yes, I absolutely agree! It's also a use case that would most likely not find much adoption, given that MVVM all-Kotlin is by far the standard these days. |
|
@jonathancaudill i haven't got a chance to review this properly yet but it's in the pipeline |
Okie-dokie!
First draft of some more advanced audio capabilities. I can edit the description later as needed before merge.
So as it turns out, I forgot a crucial detail in #119 . The SDK sandbox blocks most stuff regarding Context. So a straight-up handoff isn't really feasible. Below is what I think will work pretty well, and it still honors the split.
SDK's default audio behavior and simple building of LightAudioPlayer instances is unchanged.
SDK will allow more advanced specs to be given when building an ExoPlayer instance via the configure argument. SDK will also allow tools to change aspects of the media3 pipeline live--queueing, etc. Which is good, because streaming requires a lot of small changes to be made as cell service comes and goes, as the user queues up tracks, etc.
SDK will also enforce one LightMediaEnv per tool & one SimpleCache object per directory.