2.12.0 change in extractor handling

[Tolriq]

I'm heavily optimizing my application size via R8 and selection of the supported renderers and extractors to allow R8 to remove unused ones from final dex.

Since 2.12 despite still using :

extractorsFactory = ExtractorsFactory { arrayOf(Mp3Extractor(), FlacExtractor(), OggExtractor(), WavExtractor(), MatroskaExtractor(), Mp4Extractor()) }

and

ProgressiveMediaSource.Factory(dataSourceFactory, extractorsFactory).createMediaSource(MediaItem.Builder().setUri(finalUri).build())

All extractors are kept in final APK.

R8 gives the following reason:

com.google.android.exoplayer2.extractor.ts.TsExtractor
|- is referenced from:
|  int com.google.android.exoplayer2.extractor.ts.TsExtractor.read(com.google.android.exoplayer2.extractor.DefaultExtractorInput,com.google.android.exoplayer2.extractor.PositionHolder)
|- is overriding method:
|  int com.google.android.exoplayer2.extractor.Extractor.read(com.google.android.exoplayer2.extractor.DefaultExtractorInput,com.google.android.exoplayer2.extractor.PositionHolder)
|- is invoked from:
|  void com.google.android.exoplayer2.source.ProgressiveMediaPeriod$ExtractingLoadable.load()
|- is invoked from:
|  void com.google.android.exoplayer2.upstream.Loader$LoadTask.run()
|- is defined in library method overridden by:
|  com.google.android.exoplayer2.upstream.Loader$LoadTask
|- is instantiated in:
|  void com.google.android.exoplayer2.source.ProgressiveMediaPeriod.startLoading()
|- is invoked from:
|  void com.google.android.exoplayer2.source.ProgressiveMediaPeriod.prepare(com.google.android.exoplayer2.source.MediaPeriod$Callback,long)
|- is overriding method:
|  void com.google.android.exoplayer2.source.MediaPeriod.prepare(com.google.android.exoplayer2.source.MediaPeriod$Callback,long)
|- is invoked from:
|  void com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork()
|- is invoked from:
|  boolean com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(android.os.Message)
|- is defined in library method overridden by:
|  com.google.android.exoplayer2.ExoPlayerImplInternal
|- is referenced from:
|  void com.google.android.exoplayer2.SimpleExoPlayer.updatePlayWhenReady(boolean,int,int)

Since APK size impact is huge it would be nice to see if this can be fixed.

[icbaker]

Can you provide instructions for me to reproduce what you're seeing?

A minimal reproducible example that demonstrates the problem in a way that we can build locally. This could be an Android Studio project on GitHub, or zipped up and sent to dev.exoplayer@gmail.com (with subject "Issue #7937").

[icbaker]

You may also want to take a look at https://exoplayer.dev/shrinking.html which was updated for 2.12 in 314758c.

Specifically you can now pass the custom ExtractorsFactory to SimpleExoPlayer rather than ProgressiveMediaSource.

[Tolriq]

When passing no extractorFactory to ProgressiveMediaSource it use DefaultExtractorsFactory that does refer all extractors and will keep them.

I tried passing the extractors to SimplExoPlayer but the result is the same:

com.google.android.exoplayer2.extractor.ts.TsExtractor
|- is referenced from:
|  int com.google.android.exoplayer2.extractor.ts.TsExtractor.read(com.google.android.exoplayer2.extractor.DefaultExtractorInput,com.google.android.exoplayer2.extractor.PositionHolder)
|- is overriding method:
|  int com.google.android.exoplayer2.extractor.Extractor.read(com.google.android.exoplayer2.extractor.DefaultExtractorInput,com.google.android.exoplayer2.extractor.PositionHolder)
|- is invoked from:
|  void com.google.android.exoplayer2.source.ProgressiveMediaPeriod$ExtractingLoadable.load()
|- is invoked from:
|  void com.google.android.exoplayer2.upstream.Loader$LoadTask.run()
|- is defined in library method overridden by:
|  com.google.android.exoplayer2.upstream.Loader$LoadTask
|- is instantiated in:
|  void com.google.android.exoplayer2.source.ProgressiveMediaPeriod.startLoading()
|- is invoked from:
|  void com.google.android.exoplayer2.source.ProgressiveMediaPeriod.prepare(com.google.android.exoplayer2.source.MediaPeriod$Callback,long)
|- is overriding method:
|  void com.google.android.exoplayer2.source.MediaPeriod.prepare(com.google.android.exoplayer2.source.MediaPeriod$Callback,long)
|- is invoked from:
|  void com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork()
|- is invoked from:
|  boolean com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(android.os.Message)
|- is defined in library method overridden by:
|  com.google.android.exoplayer2.ExoPlayerImplInternal
|- is referenced from:
|  void com.google.android.exoplayer2.SimpleExoPlayer.updatePlayWhenReady(boolean,int,int)

I'm using exactly what is described in the shrinking page and it worked until 2.12.
It this part of test suite?
R8 extract I give from -whyareyoukeeping class com.google.android.exoplayer2.extractor.ts.TsExtractor is supposed to give you the exact reason of why the class is kept so have the answer in it.

[Tolriq]

bfe17ae

So I guess it's known and fixed in dev. Should have a note in changelog for impacted people to wait for 12.1.

[ghost]

I have also noticed that moving from 2.11.x into 2.12 caused my APK size to increase by around 220KB even though the app code remains the same. I am trying to provide a small sized player and this is not helping. I hope this issue gets solved. I am using R8.

[icbaker]

That commit you referenced is included in 2.12.0: 6290d09 There's no need to wait for 2.12.1 to pick up that code.

When you pass your customised ExtractorsFactory to ProgressiveMediaSource.Factory, are you also passing ExtractorsFactory.EMPTY to SimpleExoPlayer.Builder? As described here:

Note that if you provide a customized DefaultMediaSourceFactory, the ExtractorsFactory passed to the constructor of DefaultMediaSourceFactory will overwrite the passed to SimpleExoPlayer.Builder. To ensure code shrinking works as intended, you should provide ExtractorsFactory.EMPTY to the SimpleExoPlayer.Builder and pass your customized ExtractorsFactory to the DefaultMediaSourceFactory constructor:

[Tolriq]

Ok so yes both changes are needed, should have a note in changelog as it's nowhere noticed there's this impacting change.

My second test was based on your previous comment

Specifically you can now pass the custom ExtractorsFactory to SimpleExoPlayer rather than ProgressiveMediaSource`

Due to #7525 I recreate the factory and the media source manually, hence the need to still pass the extractors there too. Doc talking just about DefaultMediaSourceFactory and only in SimpleExoPlayer builder is too precise to cover the other cases that can be missed.

Wonder if #7525 have changed due to new player playlist handling and tags maybe there's better ways?

[Tolriq]

Posting here to avoid opening another issue for the moment.

But in 2.12.0 there's still a huge size increase due to keeping tons of DRM stuff that I do not use at all. Probably because of e55b345

Is there a way to prevent that?

[tonihei]

We've updated our guide page for shrinking (https://exoplayer.dev/shrinking.html) to explain that you also have to pass ExtractorsFactory.EMPTY to SimpleExoPlayer.Builder if you use ProgressiveMediaSource directly.

@Tolriq What specifically causes a size increase of the "DRM stuff" and by how much? So far DRM configuration wasn't a cause for concern with regards to APK size.

[Tolriq]

Thanks for the doc update and changelog note.

@tonihei the increase is about 80K and 500 methods between 2.11.8 and 2.12.0, the drm package goes from 2k to 24k and 27 methods to 191. I have not checked everything to find where all the diff but this one was very visible and I don't know if that part does not spread to more things outside of this package.

I've added .setDrmSessionManager(DrmSessionManager.getDummyDrmSessionManager()) when creating the media sources and drm part is down to 4k and while the drm package only reduce by 137 methods, the exoplayer2 packages is down by 165 so there's impacts on other packages too.

But If I move to the new playlist and default factory I don't know how I'll be able to still pass that one to keep the reduction.

Anyway the increase is way lower than the extractors, but still every easy gains is nice to have.

[tonihei]

Thanks for the explanations. If you want to have full control of what gets referenced by the player, you can also use the very long constructor of SimpleExoPlayer.Builder to pass in all the components you need (or the respective default). I guess passing in the dummy DRM session manager helps to prevent the issue? (Untested)

[Tolriq]

For the moment I can't really test due to #7525 I have yet to check the new playlist API and see if I can get rid of most of my hacks.

And I'm not sure the dummyDRM allows removal of everything.

Anyway this issue can be closed. If DRM is still an issue when / if I can migrate to the new playlist API I'll open another one.

[tonihei]

Thanks, I'll close this issue then.

[Tolriq]

@tonihei So tested the playlist API it works great but I can't find the drm parameter to SimpleExoPlayer Builder.

If I use following code the DEX increase by 115K so even more than previously probably because of using the player playlist api.

SimpleExoPlayer.Builder(applicationContext, AudioOnlyRendererFactory(applicationContext),
                { arrayOf(Mp3Extractor(), FlacExtractor(), OggExtractor(), WavExtractor(), MatroskaExtractor(), Mp4Extractor()) })
                .setMediaSourceFactory(
                    DefaultMediaSourceFactory(DefaultDataSourceFactory(applicationContext, null, RemoteMediaItemDataSourceFactory(applicationContext, this)))
                        .setDrmSessionManager(DrmSessionManager.getDummyDrmSessionManager())
                )
                .setTrackSelector(trackSelector)
                .build()

Do you have more idea / insight or should I open a new issue?

Edit: Actually not setting setMediaSourceFactory to SimpleExoPlayer and using addMediaSource on Player with

ProgressiveMediaSource.Factory(dataSourceFactory, extractorsFactory)
            .setDrmSessionManager(DrmSessionManager.getDummyDrmSessionManager())
            .createMediaSource(MediaItem.Builder().setUri(finalUri).build())

does gain back those 115K dex.

So DRM change is actually 115K lost and no way to directly use MediaItem :(

[tonihei]

That's interesting. Can you file a new issue with that (just copying what you said here)? We should check where the size increase is coming from and whether it's coming from the DRM config.

[Tolriq]

@tonihei #8012 tell me there if you need more info.

[tonihei]

Thanks! That should be enough for now. Need to have a look first to see what's happening