media icon indicating copy to clipboard operation
media copied to clipboard

ExoPlayer still wrongly decode some MP3 file

Open Tolriq opened this issue 1 year ago • 3 comments

Version

Media3 pre-release (alpha, beta or RC not in this list)

More version details

1.4.0 Beta 1

Devices that reproduce the issue

All

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

Play the attached files, they should support gapless but it's not working and logs shows :

internalError [eventTime=4684.06, mediaPos=129.00, window=0, period=0, loadError
                                                                               androidx.media3.common.ParserException: Searched too many bytes.{contentIsMalformed=true, dataType=1}
                                                                                   at androidx.media3.extractor.mp3.Mp3Extractor.synchronize(Mp3Extractor.java:412)
                                                                                   at androidx.media3.extractor.mp3.Mp3Extractor.readInternal(Mp3Extractor.java:281)
                                                                                   at androidx.media3.extractor.mp3.Mp3Extractor.read(Mp3Extractor.java:254)
                                                                                   at androidx.media3.exoplayer.source.BundledExtractorsAdapter.read(BundledExtractorsAdapter.java:147)
                                                                                   at androidx.media3.exoplayer.source.ProgressiveMediaPeriod$ExtractingLoadable.load(ProgressiveMediaPeriod.java:1074)
                                                                                   at androidx.media3.exoplayer.upstream.Loader$LoadTask.run(Loader.java:421)
                                                                                   at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
                                                                                   at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
                                                                                   at java.lang.Thread.run(Thread.java:1012)
                                                                             ]

This is probably a follow up to https://github.com/androidx/media/issues/1376 as the user report that in the previous version (so before the fix from that issue) the duration was wrongly reported. Now the duration is good but there's still some decoding issue somewhere.

Expected result

No error in logs and gapless working

Actual result

Does not gapless and error in logs:

https://github.com/androidx/media/issues/1376

Media

Downloads.zip

Bug Report

  • [ ] You will email the zip file produced by adb bugreport to [email protected] after filing this issue.

Tolriq avatar Jun 21 '24 12:06 Tolriq

@icbaker This is most probably a follow up to your fixes from https://github.com/androidx/media/issues/1376.

Maybe different root issue, but they did report wrong duration before and now report correct duration, but gapless still does not work and there's error logged.

Tolriq avatar Jun 21 '24 12:06 Tolriq

Thanks for the report. I can reproduce the issue by playing the 01 - Since I Left You.mp3 file in the demo app built from the tip of main and waiting for the loading position to reach the end of the file (this is when the stack trace is first logged). Playback then fails when the playback position reaches the end of the file.

I tried reverting https://github.com/androidx/media/commit/5b3066f380a433ccfaa3afbbf7c1e1e283bf4b7a and I see the same behaviour.

I built the demo app at 1.3.1 and I see the same behaviour there (though with the expected incorrect duration), so I don't think this is a regression. Did the user report this file working correctly previously? On what media3 version?

icbaker avatar Jun 28 '24 17:06 icbaker

Did not say it's a regression, just that on those files the duration was also wrong before that patch by a few seconds and it's now the correct one.

So it's possible the issue is tied like you fixed most of the stuff and it shows proper durations but there's still a wrong calculation leading to the app trying to read too much at the end by not using the updated calculation and so error and so break Gapless.

Tolriq avatar Jun 28 '24 17:06 Tolriq

https://github.com/androidx/media/issues/1563 points out that this is a regression since 1.2.1, and it is (like https://github.com/androidx/media/issues/1376) due to using "constant bitrate seeking" for files with an Info frame, instead of the (less accurate) table-of-contents in the Info frame. The ConstantBitrateSeeker currently always indicates the "last data position" is unknown:

https://github.com/androidx/media/blob/40de898b226ac1873dce9a577be5614b8ef1f933/libraries/extractor/src/main/java/androidx/media3/extractor/mp3/ConstantBitrateSeeker.java#L71-L73

I've fixed this by changing this method to return inputLength if it's known, instead of always indicating that the end position is unknown. This ensures that we stop searching for MP3 sync bytes once we get past the length indicated by the Info header, and resolves the error.

I've sent this change for internal review.

icbaker avatar Jul 31 '24 15:07 icbaker

Should be fixed by the commit linked above.

icbaker avatar Aug 02 '24 10:08 icbaker

Thanks, is 1.5 alpha 1 planned relatively soon or should I just backport on my copy ?

Tolriq avatar Aug 03 '24 16:08 Tolriq

I plan to include https://github.com/androidx/media/commit/b09cea9e3a3f9a039b77c6e6db3ee5f450becc50 in 1.4.1 which should be released this month (August). It will also be included in 1.5.0-alpha01, which atm is due to be released towards the beginning of next month (September).

icbaker avatar Aug 05 '24 14:08 icbaker

Perfect thanks.

Tolriq avatar Aug 05 '24 16:08 Tolriq

Hello Ian,

I still receive the error, while using 1.4.1 version, please check the MP3 Extractor again!

image

parkbrakereminder avatar Aug 27 '24 20:08 parkbrakereminder

@parkbrakereminder then open a new issue and provide a media that reproduce it. They can't do anything without a repro.

Tolriq avatar Aug 28 '24 08:08 Tolriq