Skip to content

Commit

Permalink
Remove @Nullable from MediaSource.Factory setters
Browse files Browse the repository at this point in the history 
The null-behaviour of these methods creates a minimization footgun,
because **any** call to these setters will prevent R8 from removing
the default implementation (even if it's never used by the app) - this
is because R8 can't tell the default implementation is only used if the
parameter is `null`.

PiperOrigin-RevId: 450386627
  • Loading branch information
@icbaker
icbaker committed May 24, 2022
1 parent 48a0301 commit 57182ac
Show file tree
Hide file tree
Showing 14 changed files with 187 additions and 168 deletions.
4 changes: 2 additions & 2 deletions .../main/java/com/google/android/exoplayer2/ext/ima/ImaServerSideAdInsertionMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,14 @@ public Factory(AdsLoader adsLoader, MediaSource.Factory contentMediaSourceFactor

@Override
public MediaSource.Factory setLoadErrorHandlingPolicy(
@Nullable LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
contentMediaSourceFactory.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy);
return this;
}

@Override
public MediaSource.Factory setDrmSessionManagerProvider(
@Nullable DrmSessionManagerProvider drmSessionManagerProvider) {
DrmSessionManagerProvider drmSessionManagerProvider) {
contentMediaSourceFactory.setDrmSessionManagerProvider(drmSessionManagerProvider);
return this;
}
Expand Down
10 changes: 6 additions & 4 deletions library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -949,10 +949,12 @@ private static MediaSource createMediaSourceInternal(
MediaItem mediaItem,
DataSource.Factory dataSourceFactory,
@Nullable DrmSessionManager drmSessionManager) {
return new DefaultMediaSourceFactory(dataSourceFactory, ExtractorsFactory.EMPTY)
.setDrmSessionManagerProvider(
drmSessionManager != null ? unusedMediaItem -> drmSessionManager : null)
.createMediaSource(mediaItem);
DefaultMediaSourceFactory mediaSourceFactory =
new DefaultMediaSourceFactory(dataSourceFactory, ExtractorsFactory.EMPTY);
if (drmSessionManager != null) {
mediaSourceFactory.setDrmSessionManagerProvider(unusedMediaItem -> drmSessionManager);
}
return mediaSourceFactory.createMediaSource(mediaItem);
}

private static boolean isProgressive(MediaItem.LocalConfiguration localConfiguration) {
Expand Down
45 changes: 30 additions & 15 deletions ...ry/core/src/main/java/com/google/android/exoplayer2/source/DefaultMediaSourceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,15 +331,25 @@ public DefaultMediaSourceFactory setLiveMaxSpeed(float maxSpeed) {

@Override
public DefaultMediaSourceFactory setDrmSessionManagerProvider(
@Nullable DrmSessionManagerProvider drmSessionManagerProvider) {
delegateFactoryLoader.setDrmSessionManagerProvider(drmSessionManagerProvider);
DrmSessionManagerProvider drmSessionManagerProvider) {
delegateFactoryLoader.setDrmSessionManagerProvider(
checkNotNull(
drmSessionManagerProvider,
"MediaSource.Factory#setDrmSessionManagerProvider no longer handles null by"
+ " instantiating a new DefaultDrmSessionManagerProvider. Explicitly construct and"
+ " pass an instance in order to retain the old behavior."));
return this;
}

@Override
public DefaultMediaSourceFactory setLoadErrorHandlingPolicy(
@Nullable LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
this.loadErrorHandlingPolicy = loadErrorHandlingPolicy;
LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
this.loadErrorHandlingPolicy =
checkNotNull(
loadErrorHandlingPolicy,
"MediaSource.Factory#setLoadErrorHandlingPolicy no longer handles null by"
+ " instantiating a new DefaultLoadErrorHandlingPolicy. Explicitly construct and"
+ " pass an instance in order to retain the old behavior.");
delegateFactoryLoader.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy);
return this;
}
Expand Down Expand Up @@ -414,16 +424,23 @@ public MediaSource createMediaSource(MediaItem mediaItem) {
SubtitleDecoderFactory.DEFAULT.createDecoder(format), format)
: new UnknownSubtitlesExtractor(format)
};
ProgressiveMediaSource.Factory progressiveMediaSourceFactory =
new ProgressiveMediaSource.Factory(dataSourceFactory, extractorsFactory);
if (loadErrorHandlingPolicy != null) {
progressiveMediaSourceFactory.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy);
}
mediaSources[i + 1] =
new ProgressiveMediaSource.Factory(dataSourceFactory, extractorsFactory)
.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy)
.createMediaSource(
MediaItem.fromUri(subtitleConfigurations.get(i).uri.toString()));
progressiveMediaSourceFactory.createMediaSource(
MediaItem.fromUri(subtitleConfigurations.get(i).uri.toString()));
} else {
SingleSampleMediaSource.Factory singleSampleMediaSourceFactory =
new SingleSampleMediaSource.Factory(dataSourceFactory);
if (loadErrorHandlingPolicy != null) {
singleSampleMediaSourceFactory.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy);
}
mediaSources[i + 1] =
new SingleSampleMediaSource.Factory(dataSourceFactory)
.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy)
.createMediaSource(subtitleConfigurations.get(i), /* durationUs= */ C.TIME_UNSET);
singleSampleMediaSourceFactory.createMediaSource(
subtitleConfigurations.get(i), /* durationUs= */ C.TIME_UNSET);
}
}

Expand Down Expand Up @@ -532,16 +549,14 @@ public MediaSource.Factory getMediaSourceFactory(@C.ContentType int contentType)
return mediaSourceFactory;
}

public void setDrmSessionManagerProvider(
@Nullable DrmSessionManagerProvider drmSessionManagerProvider) {
public void setDrmSessionManagerProvider(DrmSessionManagerProvider drmSessionManagerProvider) {
this.drmSessionManagerProvider = drmSessionManagerProvider;
for (MediaSource.Factory mediaSourceFactory : mediaSourceFactories.values()) {
mediaSourceFactory.setDrmSessionManagerProvider(drmSessionManagerProvider);
}
}

public void setLoadErrorHandlingPolicy(
@Nullable LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
public void setLoadErrorHandlingPolicy(LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
this.loadErrorHandlingPolicy = loadErrorHandlingPolicy;
for (MediaSource.Factory mediaSourceFactory : mediaSourceFactories.values()) {
mediaSourceFactory.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy);
Expand Down
11 changes: 2 additions & 9 deletions library/core/src/main/java/com/google/android/exoplayer2/source/MediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@
import com.google.android.exoplayer2.MediaItem;
import com.google.android.exoplayer2.Timeline;
import com.google.android.exoplayer2.analytics.PlayerId;
import com.google.android.exoplayer2.drm.DefaultDrmSessionManagerProvider;
import com.google.android.exoplayer2.drm.DrmSessionEventListener;
import com.google.android.exoplayer2.drm.DrmSessionManager;
import com.google.android.exoplayer2.drm.DrmSessionManagerProvider;
import com.google.android.exoplayer2.upstream.Allocator;
import com.google.android.exoplayer2.upstream.DefaultLoadErrorHandlingPolicy;
import com.google.android.exoplayer2.upstream.LoadErrorHandlingPolicy;
import com.google.android.exoplayer2.upstream.TransferListener;
import java.io.IOException;
Expand Down Expand Up @@ -67,21 +65,16 @@ interface Factory {
* Sets the {@link DrmSessionManagerProvider} used to obtain a {@link DrmSessionManager} for a
* {@link MediaItem}.
*
* <p>If not set, {@link DefaultDrmSessionManagerProvider} is used.
*
* @return This factory, for convenience.
*/
Factory setDrmSessionManagerProvider(
@Nullable DrmSessionManagerProvider drmSessionManagerProvider);
Factory setDrmSessionManagerProvider(DrmSessionManagerProvider drmSessionManagerProvider);

/**
* Sets an optional {@link LoadErrorHandlingPolicy}.
*
* @param loadErrorHandlingPolicy A {@link LoadErrorHandlingPolicy}, or {@code null} to use the
* {@link DefaultLoadErrorHandlingPolicy}.
* @return This factory, for convenience.
*/
Factory setLoadErrorHandlingPolicy(@Nullable LoadErrorHandlingPolicy loadErrorHandlingPolicy);
Factory setLoadErrorHandlingPolicy(LoadErrorHandlingPolicy loadErrorHandlingPolicy);

/**
* Returns the {@link C.ContentType content types} supported by media sources created by this
Expand Down
70 changes: 48 additions & 22 deletions library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,18 @@ public static final class Factory implements MediaSourceFactory {
@Nullable private Object tag;

/**
* Creates a new factory for {@link ProgressiveMediaSource}s, using the extractors provided by
* {@link DefaultExtractorsFactory}.
* Creates a new factory for {@link ProgressiveMediaSource}s.
*
* <p>The factory will use the following default components:
*
* @param dataSourceFactory A factory for {@link DataSource}s to read the media.
* <ul>
* <li>{@link DefaultExtractorsFactory}
* <li>{@link DefaultDrmSessionManagerProvider}
* <li>{@link DefaultLoadErrorHandlingPolicy}
* </ul>
*
* @param dataSourceFactory A factory for {@linkplain DataSource data sources} to read the
* media.
*/
public Factory(DataSource.Factory dataSourceFactory) {
this(dataSourceFactory, new DefaultExtractorsFactory());
Expand All @@ -75,6 +83,18 @@ public Factory(DataSource.Factory dataSourceFactory) {
/**
* Equivalent to {@link #Factory(DataSource.Factory, ProgressiveMediaExtractor.Factory) new
* Factory(dataSourceFactory, () -> new BundledExtractorsAdapter(extractorsFactory)}.
*
* <p>The factory will use the following default components:
*
* <ul>
* <li>{@link DefaultDrmSessionManagerProvider}
* <li>{@link DefaultLoadErrorHandlingPolicy}
* </ul>
*
* @param dataSourceFactory A factory for {@linkplain DataSource data sources} to read the
* media.
* @param extractorsFactory A factory for the {@linkplain Extractor extractors} used to extract
* the media from its container.
*/
public Factory(DataSource.Factory dataSourceFactory, ExtractorsFactory extractorsFactory) {
this(dataSourceFactory, playerId -> new BundledExtractorsAdapter(extractorsFactory));
Expand All @@ -83,9 +103,17 @@ public Factory(DataSource.Factory dataSourceFactory, ExtractorsFactory extractor
/**
* Creates a new factory for {@link ProgressiveMediaSource}s.
*
* @param dataSourceFactory A factory for {@link DataSource}s to read the media.
* <p>The factory will use the following default components:
*
* <ul>
* <li>{@link DefaultDrmSessionManagerProvider}
* <li>{@link DefaultLoadErrorHandlingPolicy}
* </ul>
*
* @param dataSourceFactory A factory for {@linkplain DataSource data sources} to read the
* media.
* @param progressiveMediaExtractorFactory A factory for the {@link ProgressiveMediaExtractor}
* to extract media from its container.
* to extract the media from its container.
*/
public Factory(
DataSource.Factory dataSourceFactory,
Expand All @@ -101,7 +129,8 @@ public Factory(
/**
* Creates a new factory for {@link ProgressiveMediaSource}s.
*
* @param dataSourceFactory A factory for {@link DataSource}s to read the media.
* @param dataSourceFactory A factory for {@linkplain DataSource data sources} to read the
* media.
* @param progressiveMediaExtractorFactory A factory for the {@link ProgressiveMediaExtractor}
* to extract media from its container.
* @param drmSessionManagerProvider A provider to obtain a {@link DrmSessionManager} for a
Expand All @@ -124,19 +153,14 @@ public Factory(
this.continueLoadingCheckIntervalBytes = continueLoadingCheckIntervalBytes;
}

/**
* Sets the {@link LoadErrorHandlingPolicy}. The default value is created by calling {@link
* DefaultLoadErrorHandlingPolicy#DefaultLoadErrorHandlingPolicy()}.
*
* @param loadErrorHandlingPolicy A {@link LoadErrorHandlingPolicy}.
* @return This factory, for convenience.
*/
public Factory setLoadErrorHandlingPolicy(
@Nullable LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
@Override
public Factory setLoadErrorHandlingPolicy(LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
this.loadErrorHandlingPolicy =
loadErrorHandlingPolicy != null
? loadErrorHandlingPolicy
: new DefaultLoadErrorHandlingPolicy();
checkNotNull(
loadErrorHandlingPolicy,
"MediaSource.Factory#setLoadErrorHandlingPolicy no longer handles null by"
+ " instantiating a new DefaultLoadErrorHandlingPolicy. Explicitly construct and"
+ " pass an instance in order to retain the old behavior.");
return this;
}

Expand All @@ -157,11 +181,13 @@ public Factory setContinueLoadingCheckIntervalBytes(int continueLoadingCheckInte

@Override
public Factory setDrmSessionManagerProvider(
@Nullable DrmSessionManagerProvider drmSessionManagerProvider) {
DrmSessionManagerProvider drmSessionManagerProvider) {
this.drmSessionManagerProvider =
drmSessionManagerProvider != null
? drmSessionManagerProvider
: new DefaultDrmSessionManagerProvider();
checkNotNull(
drmSessionManagerProvider,
"MediaSource.Factory#setDrmSessionManagerProvider no longer handles null by"
+ " instantiating a new DefaultDrmSessionManagerProvider. Explicitly construct"
+ " and pass an instance in order to retain the old behavior.");
return this;
}

Expand Down
15 changes: 0 additions & 15 deletions ...ore/src/test/java/com/google/android/exoplayer2/source/DefaultMediaSourceFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,6 @@ public void createMediaSource_withPath_progressiveSource() {
assertThat(mediaSource).isInstanceOf(ProgressiveMediaSource.class);
}

@Test
public void createMediaSource_withNull_usesNonNullDefaults() {
DefaultMediaSourceFactory defaultMediaSourceFactory =
new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext());
MediaItem mediaItem = new MediaItem.Builder().setUri(URI_MEDIA).build();

MediaSource mediaSource =
defaultMediaSourceFactory
.setDrmSessionManagerProvider(null)
.setLoadErrorHandlingPolicy(null)
.createMediaSource(mediaItem);

assertThat(mediaSource).isNotNull();
}

@Test
public void createMediaSource_withSubtitle_isMergingMediaSource() {
DefaultMediaSourceFactory defaultMediaSourceFactory =
Expand Down
46 changes: 30 additions & 16 deletions library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ public static final class Factory implements MediaSourceFactory {
/**
* Creates a new factory for {@link DashMediaSource}s.
*
* <p>The factory will use the following default components:
*
* <ul>
* <li>{@link DefaultDashChunkSource.Factory}
* <li>{@link DefaultDrmSessionManagerProvider}
* <li>{@link DefaultLoadErrorHandlingPolicy}
* <li>{@link DefaultCompositeSequenceableLoaderFactory}
* </ul>
*
* @param dataSourceFactory A factory for {@link DataSource} instances that will be used to load
* manifest and media data.
*/
Expand All @@ -122,6 +131,14 @@ public Factory(DataSource.Factory dataSourceFactory) {
/**
* Creates a new factory for {@link DashMediaSource}s.
*
* <p>The factory will use the following default components:
*
* <ul>
* <li>{@link DefaultDrmSessionManagerProvider}
* <li>{@link DefaultLoadErrorHandlingPolicy}
* <li>{@link DefaultCompositeSequenceableLoaderFactory}
* </ul>
*
* @param chunkSourceFactory A factory for {@link DashChunkSource} instances.
* @param manifestDataSourceFactory A factory for {@link DataSource} instances that will be used
* to load (and refresh) the manifest. May be {@code null} if the factory will only ever be
Expand All @@ -141,27 +158,24 @@ public Factory(

@Override
public Factory setDrmSessionManagerProvider(
@Nullable DrmSessionManagerProvider drmSessionManagerProvider) {
DrmSessionManagerProvider drmSessionManagerProvider) {
this.drmSessionManagerProvider =
drmSessionManagerProvider != null
? drmSessionManagerProvider
: new DefaultDrmSessionManagerProvider();
checkNotNull(
drmSessionManagerProvider,
"MediaSource.Factory#setDrmSessionManagerProvider no longer handles null by"
+ " instantiating a new DefaultDrmSessionManagerProvider. Explicitly construct"
+ " and pass an instance in order to retain the old behavior.");
return this;
}

/**
* Sets the {@link LoadErrorHandlingPolicy}. The default value is created by calling {@link
* DefaultLoadErrorHandlingPolicy#DefaultLoadErrorHandlingPolicy()}.
*
* @param loadErrorHandlingPolicy A {@link LoadErrorHandlingPolicy}.
* @return This factory, for convenience.
*/
public Factory setLoadErrorHandlingPolicy(
@Nullable LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
@Override
public Factory setLoadErrorHandlingPolicy(LoadErrorHandlingPolicy loadErrorHandlingPolicy) {
this.loadErrorHandlingPolicy =
loadErrorHandlingPolicy != null
? loadErrorHandlingPolicy
: new DefaultLoadErrorHandlingPolicy();
checkNotNull(
loadErrorHandlingPolicy,
"MediaSource.Factory#setLoadErrorHandlingPolicy no longer handles null by"
+ " instantiating a new DefaultLoadErrorHandlingPolicy. Explicitly construct and"
+ " pass an instance in order to retain the old behavior.");
return this;
}

Expand Down
15 changes: 0 additions & 15 deletions ...rc/test/java/com/google/android/exoplayer2/source/dash/DefaultMediaSourceFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,6 @@ public void createMediaSource_withPath_dashSource() {
assertThat(mediaSource).isInstanceOf(DashMediaSource.class);
}

@Test
public void createMediaSource_withNull_usesNonNullDefaults() {
DefaultMediaSourceFactory defaultMediaSourceFactory =
new DefaultMediaSourceFactory((Context) ApplicationProvider.getApplicationContext());
MediaItem mediaItem = new MediaItem.Builder().setUri(URI_MEDIA + "/file.mpd").build();

MediaSource mediaSource =
defaultMediaSourceFactory
.setDrmSessionManagerProvider(null)
.setLoadErrorHandlingPolicy(null)
.createMediaSource(mediaItem);

assertThat(mediaSource).isNotNull();
}

@Test
public void getSupportedTypes_dashModule_containsTypeDash() {
int[] supportedTypes =
Expand Down
Loading

0 comments on commit 57182ac

Please sign in to comment.