Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CacheWriter doesn't correctly implement allowShortContent case #7326

Closed
temq09 opened this issue May 4, 2020 · 4 comments
Closed

CacheWriter doesn't correctly implement allowShortContent case #7326

temq09 opened this issue May 4, 2020 · 4 comments
Assignees
@ojw28
Labels
bug

Comments

@temq09
Copy link

temq09 commented May 4, 2020

[REQUIRED] Searched documentation and issues

Found the article about video preloading here - https://medium.com/google-exoplayer/pre-caching-downloading-progressive-streams-in-exoplayer-3a816c75e8f6

[REQUIRED] Question

Hi! I have an issue when trying to preload the first part of a video. I don't know the exact size of the video so just trying to preload the first 800kb. In case the video is less than 800kb I get com.google.android.exoplayer2.upstream.HttpDataSource$HttpDataSourceException: java.io.EOFException even when enableEOFException==false. Is it intentional and how can I solve the issue in the application?

A full bug report captured from the device

Not a bug report, just stacktrace

     Caused by: com.google.android.exoplayer2.upstream.HttpDataSource$HttpDataSourceException: java.io.EOFException
        at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.read(DefaultHttpDataSource.java:359)
        at com.google.android.exoplayer2.upstream.DefaultDataSource.read(DefaultDataSource.java:182)
        at com.google.android.exoplayer2.upstream.cache.CacheDataSource.read(CacheDataSource.java:308)
        at com.google.android.exoplayer2.upstream.TeeDataSource.read(TeeDataSource.java:71)
        at com.google.android.exoplayer2.upstream.cache.CacheDataSource.read(CacheDataSource.java:308)
        at com.google.android.exoplayer2.upstream.cache.CacheUtil.readAndDiscard(CacheUtil.java:314)
        at com.google.android.exoplayer2.upstream.cache.CacheUtil.cache(CacheUtil.java:210)
        at com.google.android.exoplayer2.upstream.cache.CacheUtil.cache(CacheUtil.java:128)
        at dev.temq.testexoplayercacheutil.MainActivity$onCreate$1.run(MainActivity.kt:55)
        at io.reactivex.rxjava3.internal.operators.completable.CompletableFromAction.subscribeActual(CompletableFromAction.java:36)
        at io.reactivex.rxjava3.core.Completable.subscribe(Completable.java:2850)
        at io.reactivex.rxjava3.internal.operators.completable.CompletableSubscribeOn$SubscribeOnObserver.run(CompletableSubscribeOn.java:64)
        at io.reactivex.rxjava3.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:41)
        at io.reactivex.rxjava3.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:28)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:919)
     Caused by: java.io.EOFException
        at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.readInternal(DefaultHttpDataSource.java:700)
        at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.read(DefaultHttpDataSource.java:357)
        at com.google.android.exoplayer2.upstream.DefaultDataSource.read(DefaultDataSource.java:182) 
        at com.google.android.exoplayer2.upstream.cache.CacheDataSource.read(CacheDataSource.java:308) 
        at com.google.android.exoplayer2.upstream.TeeDataSource.read(TeeDataSource.java:71) 
        at com.google.android.exoplayer2.upstream.cache.CacheDataSource.read(CacheDataSource.java:308) 
        at com.google.android.exoplayer2.upstream.cache.CacheUtil.readAndDiscard(CacheUtil.java:314) 
        at com.google.android.exoplayer2.upstream.cache.CacheUtil.cache(CacheUtil.java:210) 
        at com.google.android.exoplayer2.upstream.cache.CacheUtil.cache(CacheUtil.java:128) 
        at dev.temq.testexoplayercacheutil.MainActivity$onCreate$1.run(MainActivity.kt:55) 
        at io.reactivex.rxjava3.internal.operators.completable.CompletableFromAction.subscribeActual(CompletableFromAction.java:36) 
        at io.reactivex.rxjava3.core.Completable.subscribe(Completable.java:2850) 
        at io.reactivex.rxjava3.internal.operators.completable.CompletableSubscribeOn$SubscribeOnObserver.run(CompletableSubscribeOn.java:64) 
        at io.reactivex.rxjava3.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:41) 
        at io.reactivex.rxjava3.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:28) 
        at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301) 
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) 
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) 
        at java.lang.Thread.run(Thread.java:919) 

Link to test content

Reproducible sample - https://github.com/temq09/TestExoPlayerCacheUtil
@ojw28 ojw28 self-assigned this May 4, 2020
@otopba
Copy link

otopba commented Feb 18, 2021

Same issue =/

@ojw28
Copy link
Contributor

ojw28 commented Feb 22, 2021

There are a bunch of things that need fixing here:

  1. The case where the entire stream is cached already but SimpleCache doesn't know the stream's length. This can happen in the edge case that a previous write attempt fails right at the very end, between having written the last byte to the cache and having established that there are no subsequent bytes. To correctly handle this case, CacheDataSource.open should handle POSITION_OUT_OF_RANGE errors from upstream by trying to read the single previous byte, again from upstream, to confirm the stream length. There's also a point in CacheDataSource.read that will handle POSITION_OUT_OF_RANGE only if currentDataSpecLengthUnset. I think the handling there should probably be used if the length is set as well.
  2. When CacheWriter attempts to read a fixed length that extends beyond the end of the stream, we need to make sure that when the end of the stream is encountered, both CacheDataSource and CacheWriter do the correct thing. CacheDataSource should fill in the content length if it's currently missing. CacheWriter should fail or not, depending on whether allowShortContent is true. One thing making this harder currently, is that different HttpDataSource implementations are not all throwing a consistent failure in this case. So a first step will be to clearly define the expected failure, and enforce that this is what happens across all of the HttpDataSource implementations (ideally all DataSource implementations).

@ojw28 ojw28 changed the title EOFException when preaload file with CacheUtil.cache() CacheWriter doesn't correctly implement allowShortContent case Feb 23, 2021
@ojw28 ojw28 mentioned this issue Feb 23, 2021
Closed
marcbaechinger pushed a commit that referenced this issue Feb 24, 2021
@ojw28 @marcbaechinger
Contract test for end-position-out-of-range case
d8b5a1d 
Assert that an exception is not thrown from DataSource.open
if the DataSpec's start position is valid but its end position
extends beyond the end of the data.

HTTP based DataSource implementations have no good way of
knowing when this is the case, so it makes sense to make this
the required behaviour, rather than requiring an exception to
be thrown or allowing both.

There are also use cases where the caller may want to use the
end position as an upper bound, without knowing for sure how
long the content is. An example of this use case is wanting to
pre-cache the first N bytes of a stream. This implies that any
exception should be thrown after reading to the end of the
data, rather than preemptively in open.

Issue: #7326
PiperOrigin-RevId: 359063721
marcbaechinger pushed a commit that referenced this issue Feb 24, 2021
@ojw28 @marcbaechinger
Contract test for start-position-out-of-range case
19adc43 
Currently, this only asserts that *if* an exception is thrown, it
must be a position-out-of-range exception as determined by
DataSourceException.isCausedByPositionOutOfRange.

Issue: #7326
PiperOrigin-RevId: 359092124
@jonasN5
Copy link

jonasN5 commented Mar 24, 2021

This is still an issue.

@ojw28
Copy link
Contributor

ojw28 commented Mar 24, 2021

This is fixed in the dev-v2 branch, and will be included in the 2.14.0 release which is tentatively scheduled for the beginning of May.

@ojw28 ojw28 closed this as completed Mar 24, 2021
@google google locked and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ojw28 ojw28

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@otopba @temq09 @ojw28 @jonasN5