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.

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

Failure to play some ogg files after updating to 2.12 #7992

Closed
theboubougne opened this issue Sep 26, 2020 · 2 comments
Closed

Failure to play some ogg files after updating to 2.12 #7992

theboubougne opened this issue Sep 26, 2020 · 2 comments
Assignees
Labels

Comments

@theboubougne
Copy link

theboubougne commented Sep 26, 2020

The following OGG file plays properly with earlier version of EXOPlayer (v2.11.x), but now it fails with the following error when using v2.12

File: https://files.iis.aksw.org/s/QH3bzmeHRcL4XzE/download
Another file: https://media.april.org/audio/radio-cause-commune/libre-a-vous/emissions/20200915/libre-a-vous-20200915.ogg

Android version and device don't change anything here (tested on Android 9 & 11, Samsung and Pixel 3)

09-26 06:22:22.590 13536  7239 E LoadTask: Unexpected exception loading stream
09-26 06:22:22.590 13536  7239 E LoadTask:   java.lang.ArrayIndexOutOfBoundsException: size=89639 offset=89639 byteCount=24614
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.android.okhttp.okio.Util.checkOffsetAndCount(Util.java:31)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.android.okhttp.okio.RealBufferedSource$1.read(RealBufferedSource.java:369)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.readInternal(DefaultHttpDataSource.java:735)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.read(DefaultHttpDataSource.java:395)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.upstream.StatsDataSource.read(StatsDataSource.java:92)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.DefaultExtractorInput.readFromUpstream(DefaultExtractorInput.java:283)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.DefaultExtractorInput.readFully(DefaultExtractorInput.java:75)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.DefaultExtractorInput.readFully(DefaultExtractorInput.java:83)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.ogg.OggPacket.populate(OggPacket.java:93)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.ogg.StreamReader.readHeaders(StreamReader.java:124)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.ogg.StreamReader.read(StreamReader.java:108)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.extractor.ogg.OggExtractor.read(OggExtractor.java:89)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.source.BundledExtractorsAdapter.read(BundledExtractorsAdapter.java:127)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.source.ProgressiveMediaPeriod$ExtractingLoadable.load(ProgressiveMediaPeriod.java:1046)
09-26 06:22:22.590 13536  7239 E LoadTask:       at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:415)
09-26 06:22:22.590 13536  7239 E LoadTask:       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
09-26 06:22:22.590 13536  7239 E LoadTask:       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
09-26 06:22:22.590 13536  7239 E LoadTask:       at java.lang.Thread.run(Thread.java:923)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal: Playback error
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:   com.google.android.exoplayer2.ExoPlaybackException: Source error
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:554)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at android.os.Handler.dispatchMessage(Handler.java:102)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at android.os.Looper.loop(Looper.java:223)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at android.os.HandlerThread.run(HandlerThread.java:67)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:   Caused by: com.google.android.exoplayer2.upstream.Loader$UnexpectedLoaderException: Unexpected ArrayIndexOutOfBoundsException: size=89639 offset=89639 byteCount=24614
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:436)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at java.lang.Thread.run(Thread.java:923)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:   Caused by: java.lang.ArrayIndexOutOfBoundsException: size=89639 offset=89639 byteCount=24614
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.android.okhttp.okio.Util.checkOffsetAndCount(Util.java:31)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.android.okhttp.okio.RealBufferedSource$1.read(RealBufferedSource.java:369)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.readInternal(DefaultHttpDataSource.java:735)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.upstream.DefaultHttpDataSource.read(DefaultHttpDataSource.java:395)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.upstream.StatsDataSource.read(StatsDataSource.java:92)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.DefaultExtractorInput.readFromUpstream(DefaultExtractorInput.java:283)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.DefaultExtractorInput.readFully(DefaultExtractorInput.java:75)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.DefaultExtractorInput.readFully(DefaultExtractorInput.java:83)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.ogg.OggPacket.populate(OggPacket.java:93)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.ogg.StreamReader.readHeaders(StreamReader.java:124)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.ogg.StreamReader.read(StreamReader.java:108)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.extractor.ogg.OggExtractor.read(OggExtractor.java:89)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.source.BundledExtractorsAdapter.read(BundledExtractorsAdapter.java:127)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.source.ProgressiveMediaPeriod$ExtractingLoadable.load(ProgressiveMediaPeriod.java:1046)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:415)
09-26 06:22:22.592 13536  7234 E ExoPlayerImplInternal:       ... 3 more
@icbaker
Copy link
Collaborator

icbaker commented Sep 28, 2020

Thanks for the report and the file links, I can reproduce the problem in the demo app using https://media.april.org/audio/radio-cause-commune/libre-a-vous/emissions/20200915/libre-a-vous-20200915.ogg on 2.12.0 and it works on 2.11.8.

I'll take a deeper look to understand what's going on here.

@icbaker
Copy link
Collaborator

icbaker commented Sep 30, 2020

I've worked out this is a regression I introduced in ce2e6e2

In OggPacket where I switched to using reset(), I should have preserved limit (instead of allowing it to be set to data.length), because limit is used later in the same function to decide where in the array to copy data to. By letting limit be set to data.length, the copy fails because it tries to copy X bytes into data starting at data.length.

This seems to only impact Ogg files that have a segment that spans across multiple pages. This doesn't generally happen with normal Vorbis or FLAC audio data (because the packets are usually pretty small), but it can easily happen with metadata if it includes album art (which the files you've provided seem to).

I've sent a change to fix this - sorry for the trouble and thanks for reporting!

kim-vde pushed a commit that referenced this issue Oct 6, 2020
When I moved ParsableByteArray#data behind a getter I replaced some
assignments with calls to reset(byte[]):
ce2e6e2

reset(byte[]) deliberately sets `limit` to `data.length`, in order to
handle cases that were reassigning `data` but not updating `limit`.
However OggPacket was already using `limit` to track where to write
'new' data into the array, so changing `limit` to `data.length` caused
us to try and write new data beyond the end of the array.

I looked at other uses of reset(byte[]) in ce2e6e2
and condluded the only other usage in MatroskaExtractor is legit and
shouldn't be updated like this (because MatroskaExtractor previously
*wasn't* correctly updating/maintaining `limit`).

Issue: #7992
PiperOrigin-RevId: 334601586
@icbaker icbaker closed this as completed Oct 13, 2020
ojw28 pushed a commit that referenced this issue Oct 21, 2020
When I moved ParsableByteArray#data behind a getter I replaced some
assignments with calls to reset(byte[]):
ce2e6e2

reset(byte[]) deliberately sets `limit` to `data.length`, in order to
handle cases that were reassigning `data` but not updating `limit`.
However OggPacket was already using `limit` to track where to write
'new' data into the array, so changing `limit` to `data.length` caused
us to try and write new data beyond the end of the array.

I looked at other uses of reset(byte[]) in ce2e6e2
and condluded the only other usage in MatroskaExtractor is legit and
shouldn't be updated like this (because MatroskaExtractor previously
*wasn't* correctly updating/maintaining `limit`).

Issue: #7992
PiperOrigin-RevId: 334601586
@google google locked and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants