From bcfad4b3b4520d3b898e3429d607c481fc282cde Mon Sep 17 00:00:00 2001 From: tofunmi Date: Thu, 29 Feb 2024 07:38:34 -0800 Subject: [PATCH] Move hdrMode from defaultAssetLoaderFactory constructors to create() 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 --- .../transformer/ForceEndOfStreamTest.java | 1 - .../transformer/TransformerEndToEndTest.java | 6 +++- .../media3/transformer/AssetLoader.java | 24 ++++++++++++- .../DefaultAssetLoaderFactory.java | 34 +++++++------------ .../transformer/ExoPlayerAssetLoader.java | 23 ++++--------- .../media3/transformer/ImageAssetLoader.java | 5 ++- .../transformer/SequenceAssetLoader.java | 15 +++++--- .../media3/transformer/Transformer.java | 3 +- .../transformer/TransformerInternal.java | 6 ++-- .../transformer/ExoPlayerAssetLoaderTest.java | 6 ++-- .../transformer/ImageAssetLoaderTest.java | 3 +- .../transformer/MediaItemExportTest.java | 6 ++-- 12 files changed, 74 insertions(+), 58 deletions(-) diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/ForceEndOfStreamTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/ForceEndOfStreamTest.java index 6063548f7d..543fed78ed 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/ForceEndOfStreamTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/ForceEndOfStreamTest.java @@ -127,7 +127,6 @@ public class ForceEndOfStreamTest { new DefaultAssetLoaderFactory( context, new FrameDroppingDecoderFactory(context, MP4_ASSET_FRAME_COUNT, framesToSkip), - Composition.HDR_MODE_KEEP_HDR, Clock.DEFAULT)) .build(); } diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/TransformerEndToEndTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/TransformerEndToEndTest.java index d76d0e41b6..92f7f0b2da 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/TransformerEndToEndTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/TransformerEndToEndTest.java @@ -82,6 +82,7 @@ import androidx.media3.extractor.text.DefaultSubtitleParserFactory; 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; @@ -1340,7 +1341,10 @@ public class TransformerEndToEndTest { @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) -> { diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/AssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/AssetLoader.java index bbc3b8f1a6..751a4d7853 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/AssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/AssetLoader.java @@ -61,10 +61,14 @@ public interface AssetLoader { * 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. */ @@ -133,6 +137,24 @@ public interface AssetLoader { 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}. diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultAssetLoaderFactory.java b/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultAssetLoaderFactory.java index edc458836e..1bc9c9753e 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultAssetLoaderFactory.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultAssetLoaderFactory.java @@ -34,6 +34,7 @@ import androidx.media3.common.util.Util; 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; @@ -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; @@ -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; @@ -109,7 +101,6 @@ public final class DefaultAssetLoaderFactory implements AssetLoader.Factory { 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; @@ -121,9 +112,6 @@ 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. * @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 @@ -133,13 +121,11 @@ public final class DefaultAssetLoaderFactory implements AssetLoader.Factory { 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; @@ -147,22 +133,26 @@ public final class DefaultAssetLoaderFactory implements AssetLoader.Factory { @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) { diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java index 45ba9272b6..dca4c39015 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java @@ -65,7 +65,6 @@ public final class ExoPlayerAssetLoader implements AssetLoader { 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; @@ -75,20 +74,12 @@ public final class ExoPlayerAssetLoader implements AssetLoader { * @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; } @@ -99,9 +90,6 @@ public final class ExoPlayerAssetLoader implements AssetLoader { * @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 @@ -110,19 +98,20 @@ public final class ExoPlayerAssetLoader implements AssetLoader { 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(); @@ -136,7 +125,7 @@ public final class ExoPlayerAssetLoader implements AssetLoader { editedMediaItem, mediaSourceFactory, decoderFactory, - hdrMode, + compositionSettings.hdrMode, looper, listener, clock); diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java index 9b1d446351..4cd3d6b851 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java @@ -72,7 +72,10 @@ public final class ImageAssetLoader implements AssetLoader { @Override public AssetLoader createAssetLoader( - EditedMediaItem editedMediaItem, Looper looper, Listener listener) { + EditedMediaItem editedMediaItem, + Looper looper, + Listener listener, + CompositionSettings compositionSettings) { return new ImageAssetLoader(editedMediaItem, listener, bitmapLoader); } } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java index 7e785fffef..ff70bd5896 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java @@ -60,8 +60,9 @@ import java.util.concurrent.atomic.AtomicInteger; 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. @@ -102,13 +103,15 @@ import java.util.concurrent.atomic.AtomicInteger; 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<>(); @@ -120,7 +123,8 @@ import java.util.concurrent.atomic.AtomicInteger; // 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; } @@ -505,7 +509,8 @@ import java.util.concurrent.atomic.AtomicInteger; assetLoaderFactory.createAssetLoader( editedMediaItem, checkNotNull(Looper.myLooper()), - /* listener= */ SequenceAssetLoader.this); + /* listener= */ SequenceAssetLoader.this, + compositionSettings); currentAssetLoader.start(); } catch (RuntimeException e) { onError( diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/Transformer.java b/libraries/transformer/src/main/java/androidx/media3/transformer/Transformer.java index 91b7fc7ed7..9399b50e7c 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/Transformer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/Transformer.java @@ -1541,8 +1541,7 @@ public final class Transformer { 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 = diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/TransformerInternal.java b/libraries/transformer/src/main/java/androidx/media3/transformer/TransformerInternal.java index 6381bbf2ea..9b5e2b4a04 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/TransformerInternal.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/TransformerInternal.java @@ -60,6 +60,7 @@ import androidx.media3.common.VideoFrameProcessor; 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; @@ -222,9 +223,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; 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. diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/ExoPlayerAssetLoaderTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/ExoPlayerAssetLoaderTest.java index 63b37bf0dc..70aaab9cae 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/ExoPlayerAssetLoaderTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/ExoPlayerAssetLoaderTest.java @@ -27,6 +27,7 @@ import androidx.media3.common.Format; 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; @@ -143,9 +144,8 @@ public class ExoPlayerAssetLoaderTest { 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 { diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/ImageAssetLoaderTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/ImageAssetLoaderTest.java index 93f0d5fd75..87afb4c2a5 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/ImageAssetLoaderTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/ImageAssetLoaderTest.java @@ -24,6 +24,7 @@ import androidx.media3.common.Format; 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; @@ -122,7 +123,7 @@ public class ImageAssetLoaderTest { .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 { diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/MediaItemExportTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/MediaItemExportTest.java index bc8b59daff..d2662f868e 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/MediaItemExportTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/MediaItemExportTest.java @@ -917,7 +917,6 @@ public final class MediaItemExportTest { new ExoPlayerAssetLoader.Factory( context, decoderFactory, - Composition.HDR_MODE_KEEP_HDR, new FakeClock(/* isAutoAdvancing= */ true), mediaSourceFactory); CapturingMuxer.Factory muxerFactory = @@ -1617,7 +1616,10 @@ public final class MediaItemExportTest { @Override public AssetLoader createAssetLoader( - EditedMediaItem editedMediaItem, Looper looper, Listener listener) { + EditedMediaItem editedMediaItem, + Looper looper, + Listener listener, + CompositionSettings compositionSettings) { return new FakeAssetLoader(listener, supportedOutputTypes, sampleConsumerRef); } }