update video player to media3-exoplayer and improve caching #403
Conversation
michaelknoch
changed the title
| @Suppress("unused") | ||
| class AVPlayer(parent: SDLActivity, asset: AVURLAsset) { | ||
| internal val exoPlayer: SimpleExoPlayer | ||
| private var listener: Player.EventListener | ||
| internal val exoPlayer: ExoPlayer |
There was a problem hiding this comment.
ok I'm confused. there's media3 which contains ExoPlayer now? I thought they just renamed it 😵
There was a problem hiding this comment.
Jetpack Media3’s ExoPlayer module is still ExoPlayer, it's just repackaged under the AndroidX Media3 umbrella (whatever androidx is, lol)
src/main/java/org/uikit/VideoJNI.kt
Outdated
| SeekParameters.CLOSEST_SYNC | ||
| } | ||
|
|
||
| val seekParameters = if (delta < 250) SeekParameters.EXACT else SeekParameters.CLOSEST_SYNC |
There was a problem hiding this comment.
I think this should probably be for delta < 100 only
There was a problem hiding this comment.
You think I should just change it, or do we need to test it manually?
There was a problem hiding this comment.
If you want to be more conservative I'd say delta < 150 and not test it. if you want to be sure we could test it at delta < 100. Either way I don't think it'll cause UX issues
| init { | ||
| Media3Singleton.init( | ||
| context = context, | ||
| httpCacheSize = 20L * 1024 * 1024, |
There was a problem hiding this comment.
do you know what the difference is here? why are they different sizes?
There was a problem hiding this comment.
-
httpCacheSize (20 MiB) is for OkHttp’s disk cache of HTTP responses/validators (ETags, headers, small bodies), so we get fast 304s and header compression—but we don’t store full video ranges there.
-
mediaCacheSize (512 MiB) is ExoPlayer’s SimpleCache, which actually holds the raw MP4 bytes
I just restructured the initialization and size definitions a bit to make it easier to follow
There was a problem hiding this comment.
ok sounds good to me! thanks. might be worth leaving a code comment with some of that info
src/main/java/org/uikit/VideoJNI.kt
Outdated
| val cacheFactory = CacheDataSourceFactory( | ||
| context = context, | ||
| maxCacheSize = 512L * 1024 * 1024, | ||
| maxFileSize = 64L * 1024 * 1024 |
There was a problem hiding this comment.
do we really want to limit the max file size to 64MB? I would just put these two values the same – if we cache one massive video or lots of little ones doesn't really matter i think?
There was a problem hiding this comment.
I think it should not make a huge difference, since most of our videos are way smaller than < 64mb. But theoretically I agree, I'm happy to increase the number
There was a problem hiding this comment.
Yeah, most of them are. But if there is a video larger than 64MB, would we really not want to cache it?
Type of change: update dependency
Motivation (current vs expected behavior)
This change replaces our ExoPlayer v2 integration with the new Media3 APIs for improved compatibility and future support, swaps in an OkHttp-backed
OkHttpDataSource.Factoryto gain HTTP/2 multiplexing, header compression and on-disk HTTP caching, and centralizes both theOkHttpClientand ExoPlayer’sSimpleCacheinto a singleMedia3Singleton—ensuring one LRU index and one connection pool across all videos, for faster startup, fewer stalls, and reduced resource usage.Disclaimer: I just updated the library, but chatGPT wanted to improve the caching. I'm still trying to measure the differences, but I can confirm the video cache still works. Also scrolling through the video feels smooth on my pixel device.
Please check if the PR fulfills these requirements