feat(android): cache #3514
feat(android): cache #3514
Conversation
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
Outdated
Show resolved
Hide resolved
This reverts commit 09637b1.
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
Outdated
Show resolved
Hide resolved
|
nope i did not test that. and im assuming each video component creates a
new exoplayerView, thus simpleCache is not shared?
if cache objects cant share folders i can add like an instance cache id for
different cache folders, but devs should remember to clean them afterwards.
…On Fri, Feb 16, 2024, 12:35 AM Adam ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
<#3514 (comment)>
:
> minBufferMs = newMinBufferMs;
maxBufferMs = newMaxBufferMs;
bufferForPlaybackMs = newBufferForPlaybackMs;
bufferForPlaybackAfterRebufferMs = newBufferForPlaybackAfterRebufferMs;
maxHeapAllocationPercent = newMaxHeapAllocationPercent;
minBackBufferMemoryReservePercent = newMinBackBufferMemoryReservePercent;
minBufferMemoryReservePercent = newMinBufferMemoryReservePercent;
+ if (cacheSize == 0) {
+ cacheDataSourceFactory = null;
+ } else {
+ SimpleCache simpleCache = new SimpleCache(new File(this.getContext().getCacheDir(), "RNVCache"), new LeastRecentlyUsedCacheEvictor((long) cacheSize*1024*1024), new StandaloneDatabaseProvider(this.getContext()));
Have you tested what happens when you have multiple videos on the same
screen? I was playing around with this and I think that they both try and
create a folder/cache at the same time and you get an error but it might
have been something weird with my setup.
—
Reply to this email directly, view it on GitHub
<#3514 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZMOVVT2323Y3CSUHKE2QYDYT4K4BAVCNFSM6AAAAABCVWEF6KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOBUGU3TAOJTGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***
com>
|
Yeah the issue is the fact multiple instances of I believe the typical approach of using a cache with I don't know if it's possible but it would be ideal I think if the cache could be instantiated once per app and then used by every Video instace |
heres a singleton flavor if it works |
|
found a bug: simpleCache should not be used when using file:/// URIs |
|
@lovegaoshi The singleton solution seems to do the trick with multiple video instances! Will keep giving it a poke and a prod but looks good so far! |
|
@lovegaoshi can you please also fix conflict and check linter issue ? |
…ideo into dev-android-cache
|
@freeboub lint is clear |
android/src/main/java/com/brentvatne/common/toolbox/RNVSimpleCache.kt
Outdated
Show resolved
Hide resolved
|
@lovegaoshi Addtionnally if you can add a button in the sample to enable caching, it would be wonderful ! |
…ideo into dev-android-cache
|
@freeboub tbh I dont know how it behaves changing cache on/off on the fly, but its added. |
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerSimpleCache.kt
Show resolved
Hide resolved
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerSimpleCache.kt
Show resolved
Hide resolved
|
One additional comment added, can you please double check it ? |
|
instead of cleaning this singleton cache, i put a cache pointer within the
view; if buffer is set to 0 that pointer is set to null so that view wont
use simpleCache, while previous views can still do so
i dont feel comfortable disposing cache or change its size onthefly bc the
previous video views using a now disposed cache could break. but i dont
know for sure as its out of my use scope
i agree if rnv has a setup function like rntp's setipPlayer, then this
cache param should reside in there as within props we are giving false
promises that different views can use different caches. is there one? i
didnt look very deeply
im also not confident to hard code a cache size as my use case is to cache
the entire video so that when using repeat, exoplayer wont send new network
requests. The videos may be small or large i cant tell
…On Thu, Apr 25, 2024, 9:59 AM Olivier Bouillet ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerSimpleCache.kt
<#3514 (comment)>
:
> +import android.content.Context
+import androidx.media3.database.StandaloneDatabaseProvider
+import androidx.media3.datasource.DataSource
+import androidx.media3.datasource.HttpDataSource
+import androidx.media3.datasource.cache.CacheDataSource
+import androidx.media3.datasource.cache.LeastRecentlyUsedCacheEvictor
+import androidx.media3.datasource.cache.SimpleCache
+import java.io.File
+
+object RNVSimpleCache {
+ // TODO: when to release? how to check if cache is released?
+ var simpleCache: SimpleCache? = null
+ var cacheDataSourceFactory: DataSource.Factory? = null
+
+ fun setSimpleCache(context: Context, cacheSize: Int, factory: HttpDataSource.Factory) {
+ if (cacheDataSourceFactory != null || cacheSize == 0) return
@lovegaoshi <https://github.com/lovegaoshi> reminder on this point !
But I thought a little bit more:
- cache shouldn't be removed (ie: I don't think cache size = 0 should
clean it)
- Maybe the cache size should be an external property (out of buffer
props) ...
On iOS cache size is hard coded:
options.sizeConstraintBytes = 1024 * 1024 * 100;
Let me know your point of view please !
—
Reply to this email directly, view it on GitHub
<#3514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZMOVVR7CLGWR2KRTAXCKD3Y7EY73AVCNFSM6AAAAABCVWEF6KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRTGEYDEMRQGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
|
@lovegaoshi I don't ask to hardcode the cache size ! :) (would be better to be able to configure it on iOS :D (I don't ask for it ! )) I understand that you are unmounting the <Video object when you change video content, but this should not be necessary... I am ok just to add notes in documentation, and we can go back on the feature latter ! |
|
yeah if possible i don't want to deal with multiple caches each for
different views. it wasnt my intent for the pr/issue linked above and
cleaning up various caches is tedious, messy and really unnecessary. i dont
use multiple <video> myself
right now the cache is a singleton that a video component can use it or not
by a ref within it; i have doubts changing this singleton size bc that
involves disposing the cache and recreating it, which probably will cause
other components fail. if multiple sizes were set only the first size will
be used.
it sounds good if ud like to make ios configurable! not my use case since
ios doesn't have that repeat request bug i linked initially
appreciate the work as always btw!
…On Thu, Apr 25, 2024, 1:24 PM Olivier Bouillet ***@***.***> wrote:
@lovegaoshi <https://github.com/lovegaoshi> I don't ask to hardcode the
cache size ! :) (would be better to be able to configure it on iOS :D (I
don't ask for it ! ))
I understand that you are unmounting the <Video object when you change
video content, but this should not be necessary...
Then between 2 playbacks on the same <Video maybe users would like to
change the cache size, I think It would be your case if you don't unmount
the <Video Object ?! BTW, I am not sure this is really important.
The use case which makes me threaten a little bit more is users with
multiple <Video Views. if user specify multiple different cache size, what
would happen ?
I am ok just to add notes in documentation, and we can go back on the
feature latter !
—
Reply to this email directly, view it on GitHub
<#3514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZMOVVSVIXO3FEMX2ZRNZZ3Y7FQ65AVCNFSM6AAAAABCVWEF6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZYGEYTCNRSHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
lemme correct myself that the cache singleton can be shared within multiple
<video> but they cant use different sizes of caches. the size is whatever
the first size set. component can also opt out of caching behavior, but not
use different caches, only possivle on or off
thx! ill fry yo update thd docs.
…On Thu, Apr 25, 2024, 2:19 PM Olivier Bouillet ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/pages/component/props.mdx
<#3514 (comment)>
:
> @@ -63,7 +63,7 @@ Adjust the buffer settings. This prop takes an object with one or more of the pr
| maxHeapAllocationPercent | number | The percentage of available heap that the video can use to buffer, between 0 and 1 |
| minBackBufferMemoryReservePercent | number | The percentage of available app memory at which during startup the back buffer will be disabled, between 0 and 1 |
| minBufferMemoryReservePercent | number | The percentage of available app memory to keep in reserve that prevents buffer from using it, between 0 and 1 |
-
+| cacheSizeMB | number | Cache size in MB, it will allow applications to store video data for a while in the cache folder, it is useful to decrease bandwidth usage when repeating small videos. Android only. |
@lovegaoshi <https://github.com/lovegaoshi> here is a proposal for doc
update !
—
Reply to this email directly, view it on GitHub
<#3514 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZMOVVQ3ETXKRWKR7A745PDY7FXPDAVCNFSM6AAAAABCVWEF6KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRTGYYTIMZXHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
|
Thanks for the update @lovegaoshi ! |
Describe the changes
closes #3428
this PR adds a simpleCache for android that the progressiveMedia sources will be cached to device. Via this when repeating a video, exoplayer no longer repeatably requests the source URI but resorts to the cached source instead.
bufferConfig now has a new property, bufferSize that denotes the size of this simpleCache in MB.
The basic example has been set up to show this problem; first remove the bufferConfig prop and watch the network request (with HTTP Toolkit) while bigbunny is playing; then add bufferConfig and watch the network request again.