Skip to content

Commit

Permalink
Move hdrMode from defaultAssetLoaderFactory constructors to create()
Browse files Browse the repository at this point in the history
Plumbing hdrMode through the default asset loader factory via the constructor is problematic because it breaks API boundaries. It means there is another way to set hdrMode outside of Composition.java and TransformationRequest.java, which is error prone and cause problems if someone an app starts customizing the assetloaderfactory. It also means custom asset loaders can't receive this information without hacking around.

The introduction of the composition-level settings class makes this approach easily extensible for other settings applied on the composition level but use in an individual asset level basis (e.g. ultraHDR support).

PiperOrigin-RevId: 611466920
  • Loading branch information
tof-tof authored and copybara-github committed Feb 29, 2024
1 parent 7395c9a commit bcfad4b
Show file tree
Hide file tree
Showing 12 changed files with 74 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ private static Transformer buildTransformer(Context context, int framesToSkip) {
new DefaultAssetLoaderFactory(
context,
new FrameDroppingDecoderFactory(context, MP4_ASSET_FRAME_COUNT, framesToSkip),
Composition.HDR_MODE_KEEP_HDR,
Clock.DEFAULT))
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import androidx.media3.test.utils.FakeExtractorOutput;
import androidx.media3.test.utils.FakeTrackOutput;
import androidx.media3.test.utils.TestUtil;
import androidx.media3.transformer.AssetLoader.CompositionSettings;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -1340,7 +1341,10 @@ private final class TestTextureAssetLoaderFactory implements AssetLoader.Factory

@Override
public TextureAssetLoader createAssetLoader(
EditedMediaItem editedMediaItem, Looper looper, AssetLoader.Listener listener) {
EditedMediaItem editedMediaItem,
Looper looper,
AssetLoader.Listener listener,
CompositionSettings compositionSettings) {
Format format = new Format.Builder().setWidth(width).setHeight(height).build();
OnInputFrameProcessedListener frameProcessedListener =
(texId, syncObject) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,14 @@ interface Factory {
* been created.
* @param listener The {@link Listener} on which the {@link AssetLoader} should notify of
* events.
* @param compositionSettings The {@link CompositionSettings}.
* @return An {@link AssetLoader}.
*/
AssetLoader createAssetLoader(
EditedMediaItem editedMediaItem, Looper looper, Listener listener);
EditedMediaItem editedMediaItem,
Looper looper,
Listener listener,
CompositionSettings compositionSettings);
}

/** A listener of {@link AssetLoader} events. */
Expand Down Expand Up @@ -133,6 +137,24 @@ interface Listener {
void onError(ExportException exportException);
}

/**
* Customizations set on the {@linkplain Composition composition-level} that are needed when
* {@linkplain AssetLoader loading} each of the {@linkplain EditedMediaItem individual assets} in
* the {@link Composition}.
*/
/* package */ class CompositionSettings {

public final @Composition.HdrMode int hdrMode;

public CompositionSettings() {
this.hdrMode = Composition.HDR_MODE_KEEP_HDR;
}

public CompositionSettings(@Composition.HdrMode int hdrMode) {
this.hdrMode = hdrMode;
}
}

/**
* Supported output types of an asset loader. Possible flag values are {@link
* #SUPPORTED_OUTPUT_TYPE_ENCODED} and {@link #SUPPORTED_OUTPUT_TYPE_DECODED}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import androidx.media3.datasource.DataSourceBitmapLoader;
import androidx.media3.datasource.DefaultDataSource;
import androidx.media3.exoplayer.source.MediaSource;
import androidx.media3.transformer.AssetLoader.CompositionSettings;
import com.google.common.base.Ascii;
import com.google.common.util.concurrent.MoreExecutors;
import java.util.Objects;
Expand All @@ -46,7 +47,6 @@ public final class DefaultAssetLoaderFactory implements AssetLoader.Factory {

private final Context context;
private final Codec.DecoderFactory decoderFactory;
private final @Composition.HdrMode int hdrMode;
private final Clock clock;
private final MediaSource.@MonotonicNonNull Factory mediaSourceFactory;
private final BitmapLoader bitmapLoader;
Expand All @@ -64,21 +64,13 @@ public final class DefaultAssetLoaderFactory implements AssetLoader.Factory {
* @param context The {@link Context}.
* @param decoderFactory The {@link Codec.DecoderFactory} to use to decode the samples (if
* necessary).
* @param hdrMode The {@link Composition.HdrMode} to apply. Only {@link
* Composition#HDR_MODE_TONE_MAP_HDR_TO_SDR_USING_MEDIACODEC} and {@link
* Composition#HDR_MODE_EXPERIMENTAL_FORCE_INTERPRET_HDR_AS_SDR} are applied in this
* component.
* @param clock The {@link Clock} to use. It should always be {@link Clock#DEFAULT}, except for
* testing.
*/
public DefaultAssetLoaderFactory(
Context context,
Codec.DecoderFactory decoderFactory,
@Composition.HdrMode int hdrMode,
Clock clock) {
Context context, Codec.DecoderFactory decoderFactory, Clock clock) {
this.context = context.getApplicationContext();
this.decoderFactory = decoderFactory;
this.hdrMode = hdrMode;
this.clock = clock;
this.mediaSourceFactory = null;
@Nullable BitmapFactory.Options options = null;
Expand Down Expand Up @@ -109,7 +101,6 @@ public DefaultAssetLoaderFactory(
Context context, @Composition.HdrMode int hdrMode, BitmapLoader bitmapLoader) {
this.context = context.getApplicationContext();
this.decoderFactory = new DefaultDecoderFactory(context);
this.hdrMode = hdrMode;
this.clock = Clock.DEFAULT;
this.mediaSourceFactory = null;
this.bitmapLoader = bitmapLoader;
Expand All @@ -121,9 +112,6 @@ public DefaultAssetLoaderFactory(
* @param context The {@link Context}.
* @param decoderFactory The {@link Codec.DecoderFactory} to use to decode the samples (if
* necessary).
* @param hdrMode The {@link Composition.HdrMode} to apply. Only {@link
* Composition#HDR_MODE_TONE_MAP_HDR_TO_SDR_USING_MEDIACODEC} and {@link
* Composition#HDR_MODE_EXPERIMENTAL_FORCE_INTERPRET_HDR_AS_SDR} are applied.
* @param clock The {@link Clock} to use. It should always be {@link Clock#DEFAULT}, except for
* testing.
* @param mediaSourceFactory The {@link MediaSource.Factory} to use to retrieve the samples to
Expand All @@ -133,36 +121,38 @@ public DefaultAssetLoaderFactory(
public DefaultAssetLoaderFactory(
Context context,
Codec.DecoderFactory decoderFactory,
@Composition.HdrMode int hdrMode,
Clock clock,
MediaSource.Factory mediaSourceFactory,
BitmapLoader bitmapLoader) {
this.context = context.getApplicationContext();
this.decoderFactory = decoderFactory;
this.hdrMode = hdrMode;
this.clock = clock;
this.mediaSourceFactory = mediaSourceFactory;
this.bitmapLoader = bitmapLoader;
}

@Override
public AssetLoader createAssetLoader(
EditedMediaItem editedMediaItem, Looper looper, AssetLoader.Listener listener) {
EditedMediaItem editedMediaItem,
Looper looper,
AssetLoader.Listener listener,
CompositionSettings compositionSettings) {
MediaItem mediaItem = editedMediaItem.mediaItem;
if (isImage(mediaItem.localConfiguration)) {
if (imageAssetLoaderFactory == null) {
imageAssetLoaderFactory = new ImageAssetLoader.Factory(bitmapLoader);
}
return imageAssetLoaderFactory.createAssetLoader(editedMediaItem, looper, listener);
return imageAssetLoaderFactory.createAssetLoader(
editedMediaItem, looper, listener, compositionSettings);
}
if (exoPlayerAssetLoaderFactory == null) {
exoPlayerAssetLoaderFactory =
mediaSourceFactory != null
? new ExoPlayerAssetLoader.Factory(
context, decoderFactory, hdrMode, clock, mediaSourceFactory)
: new ExoPlayerAssetLoader.Factory(context, decoderFactory, hdrMode, clock);
? new ExoPlayerAssetLoader.Factory(context, decoderFactory, clock, mediaSourceFactory)
: new ExoPlayerAssetLoader.Factory(context, decoderFactory, clock);
}
return exoPlayerAssetLoaderFactory.createAssetLoader(editedMediaItem, looper, listener);
return exoPlayerAssetLoaderFactory.createAssetLoader(
editedMediaItem, looper, listener, compositionSettings);
}

private boolean isImage(@Nullable MediaItem.LocalConfiguration localConfiguration) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ public static final class Factory implements AssetLoader.Factory {

private final Context context;
private final Codec.DecoderFactory decoderFactory;
private final @Composition.HdrMode int hdrMode;
private final Clock clock;
@Nullable private final MediaSource.Factory mediaSourceFactory;

Expand All @@ -75,20 +74,12 @@ public static final class Factory implements AssetLoader.Factory {
* @param context The {@link Context}.
* @param decoderFactory The {@link Codec.DecoderFactory} to use to decode the samples (if
* necessary).
* @param hdrMode The {@link Composition.HdrMode} to apply. Only {@link
* Composition#HDR_MODE_TONE_MAP_HDR_TO_SDR_USING_MEDIACODEC} and {@link
* Composition#HDR_MODE_EXPERIMENTAL_FORCE_INTERPRET_HDR_AS_SDR} are applied.
* @param clock The {@link Clock} to use. It should always be {@link Clock#DEFAULT}, except for
* testing.
*/
public Factory(
Context context,
Codec.DecoderFactory decoderFactory,
@Composition.HdrMode int hdrMode,
Clock clock) {
public Factory(Context context, Codec.DecoderFactory decoderFactory, Clock clock) {
this.context = context;
this.decoderFactory = decoderFactory;
this.hdrMode = hdrMode;
this.clock = clock;
this.mediaSourceFactory = null;
}
Expand All @@ -99,9 +90,6 @@ public Factory(
* @param context The {@link Context}.
* @param decoderFactory The {@link Codec.DecoderFactory} to use to decode the samples (if
* necessary).
* @param hdrMode The {@link Composition.HdrMode} to apply. Only {@link
* Composition#HDR_MODE_TONE_MAP_HDR_TO_SDR_USING_MEDIACODEC} and {@link
* Composition#HDR_MODE_EXPERIMENTAL_FORCE_INTERPRET_HDR_AS_SDR} are applied.
* @param clock The {@link Clock} to use. It should always be {@link Clock#DEFAULT}, except for
* testing.
* @param mediaSourceFactory The {@link MediaSource.Factory} to use to retrieve the samples to
Expand All @@ -110,19 +98,20 @@ public Factory(
public Factory(
Context context,
Codec.DecoderFactory decoderFactory,
@Composition.HdrMode int hdrMode,
Clock clock,
MediaSource.Factory mediaSourceFactory) {
this.context = context;
this.decoderFactory = decoderFactory;
this.hdrMode = hdrMode;
this.clock = clock;
this.mediaSourceFactory = mediaSourceFactory;
}

@Override
public AssetLoader createAssetLoader(
EditedMediaItem editedMediaItem, Looper looper, Listener listener) {
EditedMediaItem editedMediaItem,
Looper looper,
Listener listener,
CompositionSettings compositionSettings) {
MediaSource.Factory mediaSourceFactory = this.mediaSourceFactory;
if (mediaSourceFactory == null) {
DefaultExtractorsFactory defaultExtractorsFactory = new DefaultExtractorsFactory();
Expand All @@ -136,7 +125,7 @@ public AssetLoader createAssetLoader(
editedMediaItem,
mediaSourceFactory,
decoderFactory,
hdrMode,
compositionSettings.hdrMode,
looper,
listener,
clock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ public Factory(BitmapLoader bitmapLoader) {

@Override
public AssetLoader createAssetLoader(
EditedMediaItem editedMediaItem, Looper looper, Listener listener) {
EditedMediaItem editedMediaItem,
Looper looper,
Listener listener,
CompositionSettings compositionSettings) {
return new ImageAssetLoader(editedMediaItem, listener, bitmapLoader);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@
private final boolean isLooping;
private final boolean forceAudioTrack;
private final AssetLoader.Factory assetLoaderFactory;
private final HandlerWrapper handler;
private final CompositionSettings compositionSettings;
private final Listener sequenceAssetLoaderListener;
private final HandlerWrapper handler;

/**
* A mapping from track types to {@link SampleConsumer} instances.
Expand Down Expand Up @@ -102,13 +103,15 @@ public SequenceAssetLoader(
EditedMediaItemSequence sequence,
boolean forceAudioTrack,
AssetLoader.Factory assetLoaderFactory,
Looper looper,
CompositionSettings compositionSettings,
Listener listener,
Clock clock) {
Clock clock,
Looper looper) {
editedMediaItems = sequence.editedMediaItems;
isLooping = sequence.isLooping;
this.forceAudioTrack = forceAudioTrack;
this.assetLoaderFactory = assetLoaderFactory;
this.compositionSettings = compositionSettings;
sequenceAssetLoaderListener = listener;
handler = clock.createHandler(looper, /* callback= */ null);
sampleConsumersByTrackType = new HashMap<>();
Expand All @@ -120,7 +123,8 @@ public SequenceAssetLoader(
// constructor.
@SuppressWarnings("nullness:argument.type.incompatible")
AssetLoader currentAssetLoader =
assetLoaderFactory.createAssetLoader(editedMediaItems.get(0), looper, /* listener= */ this);
assetLoaderFactory.createAssetLoader(
editedMediaItems.get(0), looper, /* listener= */ this, compositionSettings);
this.currentAssetLoader = currentAssetLoader;
}

Expand Down Expand Up @@ -505,7 +509,8 @@ private void switchAssetLoader() {
assetLoaderFactory.createAssetLoader(
editedMediaItem,
checkNotNull(Looper.myLooper()),
/* listener= */ SequenceAssetLoader.this);
/* listener= */ SequenceAssetLoader.this,
compositionSettings);
currentAssetLoader.start();
} catch (RuntimeException e) {
onError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1541,8 +1541,7 @@ private void startInternal(
AssetLoader.Factory assetLoaderFactory = this.assetLoaderFactory;
if (useDefaultAssetLoaderFactory || assetLoaderFactory == null) {
assetLoaderFactory =
new DefaultAssetLoaderFactory(
context, new DefaultDecoderFactory(context), transformationRequest.hdrMode, clock);
new DefaultAssetLoaderFactory(context, new DefaultDecoderFactory(context), clock);
}
DebugTraceUtil.reset();
transformerInternal =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import androidx.media3.common.util.Clock;
import androidx.media3.common.util.ConditionVariable;
import androidx.media3.common.util.HandlerWrapper;
import androidx.media3.transformer.AssetLoader.CompositionSettings;
import com.google.common.collect.ImmutableList;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
Expand Down Expand Up @@ -222,9 +223,10 @@ public TransformerInternal(
sequence,
composition.forceAudioTrack,
assetLoaderFactory,
internalLooper,
new CompositionSettings(transformationRequest.hdrMode),
sequenceAssetLoaderListener,
clock));
clock,
internalLooper));
if (!sequence.isLooping) {
// All sequences have a non-final duration at this point, as the AssetLoaders haven't
// started loading yet.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import androidx.media3.common.MediaItem;
import androidx.media3.common.util.Clock;
import androidx.media3.decoder.DecoderInputBuffer;
import androidx.media3.transformer.AssetLoader.CompositionSettings;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import java.time.Duration;
Expand Down Expand Up @@ -143,9 +144,8 @@ private static AssetLoader getAssetLoader(AssetLoader.Listener listener, Clock c
Codec.DecoderFactory decoderFactory = new DefaultDecoderFactory(context);
EditedMediaItem editedMediaItem =
new EditedMediaItem.Builder(MediaItem.fromUri("asset:///media/mp4/sample.mp4")).build();
return new ExoPlayerAssetLoader.Factory(
context, decoderFactory, Composition.HDR_MODE_KEEP_HDR, clock)
.createAssetLoader(editedMediaItem, Looper.myLooper(), listener);
return new ExoPlayerAssetLoader.Factory(context, decoderFactory, clock)
.createAssetLoader(editedMediaItem, Looper.myLooper(), listener, new CompositionSettings());
}

private static final class FakeSampleConsumer implements SampleConsumer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import androidx.media3.common.MediaItem;
import androidx.media3.common.util.TimestampIterator;
import androidx.media3.datasource.DataSourceBitmapLoader;
import androidx.media3.transformer.AssetLoader.CompositionSettings;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import java.time.Duration;
Expand Down Expand Up @@ -122,7 +123,7 @@ private static AssetLoader getAssetLoader(AssetLoader.Listener listener) {
.build();
return new ImageAssetLoader.Factory(
new DataSourceBitmapLoader(ApplicationProvider.getApplicationContext()))
.createAssetLoader(editedMediaItem, Looper.myLooper(), listener);
.createAssetLoader(editedMediaItem, Looper.myLooper(), listener, new CompositionSettings());
}

private static final class FakeSampleConsumer implements SampleConsumer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,6 @@ public void start_withSlowOutputSampleRate_completesWithError() throws Exception
new ExoPlayerAssetLoader.Factory(
context,
decoderFactory,
Composition.HDR_MODE_KEEP_HDR,
new FakeClock(/* isAutoAdvancing= */ true),
mediaSourceFactory);
CapturingMuxer.Factory muxerFactory =
Expand Down Expand Up @@ -1617,7 +1616,10 @@ public Factory(

@Override
public AssetLoader createAssetLoader(
EditedMediaItem editedMediaItem, Looper looper, Listener listener) {
EditedMediaItem editedMediaItem,
Looper looper,
Listener listener,
CompositionSettings compositionSettings) {
return new FakeAssetLoader(listener, supportedOutputTypes, sampleConsumerRef);
}
}
Expand Down

0 comments on commit bcfad4b

Please sign in to comment.