From eb357654bbd11f6304a6f5495bb209e59307dfcb Mon Sep 17 00:00:00 2001 From: sheenachhabra Date: Mon, 7 Nov 2022 20:38:44 +0000 Subject: [PATCH] Move muxer initialization off application thread Problem: We are initialising muxer as soon as we start the transformation. Now the startTransformation() method can be called from main thread, but muxer creation is an I/O operation and should be not be done on main thread. Solution: Added lazy initialisation of muxer object. The actual transformation happens on background thread so the muxer will be initialised lazily from background thread only. Another way was to provide an initialize() method on MuxerWrapper which will explicitly initialise muxer object but with this approach the caller need to call the initialise method before calling anything else. With current implementation the renderers are calling MuxerWrapper methods on various callbacks (Not sequentially) and also we are sharing same muxer with multiple renderers so It might become confusing for the caller on when to call the initialise() method. Also there are few methods on MuxerWrapper which dont really need muxer object. So in short it might make MuxerWrapper APIs more confusing. Validation: Verified the transformation from demo app. PiperOrigin-RevId: 486735787 --- .../media3/transformer/DefaultMuxer.java | 5 +-- .../media3/transformer/FrameworkMuxer.java | 24 +++++++---- .../androidx/media3/transformer/Muxer.java | 9 ++--- .../media3/transformer/MuxerWrapper.java | 40 +++++++++++++++++-- .../media3/transformer/Transformer.java | 18 +++++---- .../media3/transformer/TestMuxer.java | 3 +- .../transformer/TransformerEndToEndTest.java | 4 +- 7 files changed, 72 insertions(+), 31 deletions(-) diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultMuxer.java b/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultMuxer.java index 16457bbbd6..fd7d2cb53f 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultMuxer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/DefaultMuxer.java @@ -20,7 +20,6 @@ import androidx.media3.common.C; import androidx.media3.common.Format; import androidx.media3.common.util.UnstableApi; import com.google.common.collect.ImmutableList; -import java.io.IOException; import java.nio.ByteBuffer; /** A default {@link Muxer} implementation. */ @@ -53,12 +52,12 @@ public final class DefaultMuxer implements Muxer { } @Override - public Muxer create(String path) throws IOException { + public Muxer create(String path) throws MuxerException { return new DefaultMuxer(muxerFactory.create(path)); } @Override - public Muxer create(ParcelFileDescriptor parcelFileDescriptor) throws IOException { + public Muxer create(ParcelFileDescriptor parcelFileDescriptor) throws MuxerException { return new DefaultMuxer(muxerFactory.create(parcelFileDescriptor)); } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/FrameworkMuxer.java b/libraries/transformer/src/main/java/androidx/media3/transformer/FrameworkMuxer.java index ae710c3346..29a7968611 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/FrameworkMuxer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/FrameworkMuxer.java @@ -63,18 +63,28 @@ import java.nio.ByteBuffer; } @Override - public FrameworkMuxer create(String path) throws IOException { - MediaMuxer mediaMuxer = new MediaMuxer(path, MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4); + public FrameworkMuxer create(String path) throws MuxerException { + MediaMuxer mediaMuxer; + try { + mediaMuxer = new MediaMuxer(path, MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4); + } catch (IOException e) { + throw new MuxerException("Error creating muxer", e); + } return new FrameworkMuxer(mediaMuxer, maxDelayBetweenSamplesMs); } @RequiresApi(26) @Override - public FrameworkMuxer create(ParcelFileDescriptor parcelFileDescriptor) throws IOException { - MediaMuxer mediaMuxer = - new MediaMuxer( - parcelFileDescriptor.getFileDescriptor(), - MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4); + public FrameworkMuxer create(ParcelFileDescriptor parcelFileDescriptor) throws MuxerException { + MediaMuxer mediaMuxer; + try { + mediaMuxer = + new MediaMuxer( + parcelFileDescriptor.getFileDescriptor(), + MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4); + } catch (IOException e) { + throw new MuxerException("Error creating muxer", e); + } return new FrameworkMuxer(mediaMuxer, maxDelayBetweenSamplesMs); } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/Muxer.java b/libraries/transformer/src/main/java/androidx/media3/transformer/Muxer.java index 1bf8c4c77c..be0b0de1ef 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/Muxer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/Muxer.java @@ -21,7 +21,6 @@ import androidx.media3.common.Format; import androidx.media3.common.MimeTypes; import androidx.media3.common.util.UnstableApi; import com.google.common.collect.ImmutableList; -import java.io.IOException; import java.nio.ByteBuffer; /** @@ -57,9 +56,9 @@ public interface Muxer { * * @param path The path to the output file. * @throws IllegalArgumentException If the path is invalid. - * @throws IOException If an error occurs opening the output file for writing. + * @throws MuxerException If an error occurs opening the output file for writing. */ - Muxer create(String path) throws IOException; + Muxer create(String path) throws MuxerException; /** * Returns a new muxer writing to a file descriptor. @@ -69,9 +68,9 @@ public interface Muxer { * muxer is released. It is the responsibility of the caller to close the * ParcelFileDescriptor. This can be done after this method returns. * @throws IllegalArgumentException If the file descriptor is invalid. - * @throws IOException If an error occurs opening the output file descriptor for writing. + * @throws MuxerException If an error occurs opening the output file descriptor for writing. */ - Muxer create(ParcelFileDescriptor parcelFileDescriptor) throws IOException; + Muxer create(ParcelFileDescriptor parcelFileDescriptor) throws MuxerException; /** * Returns the supported sample {@linkplain MimeTypes MIME types} for the given {@link diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/MuxerWrapper.java b/libraries/transformer/src/main/java/androidx/media3/transformer/MuxerWrapper.java index 9f8a854449..f22b8668fa 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/MuxerWrapper.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/MuxerWrapper.java @@ -16,11 +16,13 @@ package androidx.media3.transformer; +import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Util.maxValue; import static androidx.media3.common.util.Util.minValue; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import android.os.ParcelFileDescriptor; import android.util.SparseIntArray; import android.util.SparseLongArray; import androidx.annotation.Nullable; @@ -33,7 +35,9 @@ import java.nio.ByteBuffer; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; +import org.checkerframework.checker.nullness.qual.EnsuresNonNull; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; +import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** * A wrapper around a media muxer. @@ -50,7 +54,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; */ private static final long MAX_TRACK_WRITE_AHEAD_US = Util.msToUs(500); - private final Muxer muxer; + @Nullable private final String outputPath; + @Nullable private final ParcelFileDescriptor outputParcelFileDescriptor; private final Muxer.Factory muxerFactory; private final Transformer.AsyncErrorListener asyncErrorListener; private final SparseIntArray trackTypeToIndex; @@ -66,10 +71,19 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private long minTrackTimeUs; private @MonotonicNonNull ScheduledFuture abortScheduledFuture; private boolean isAborted; + private @MonotonicNonNull Muxer muxer; public MuxerWrapper( - Muxer muxer, Muxer.Factory muxerFactory, Transformer.AsyncErrorListener asyncErrorListener) { - this.muxer = muxer; + @Nullable String outputPath, + @Nullable ParcelFileDescriptor outputParcelFileDescriptor, + Muxer.Factory muxerFactory, + Transformer.AsyncErrorListener asyncErrorListener) { + if (outputPath == null && outputParcelFileDescriptor == null) { + throw new NullPointerException("Both output path and ParcelFileDescriptor are null"); + } + + this.outputPath = outputPath; + this.outputParcelFileDescriptor = outputParcelFileDescriptor; this.muxerFactory = muxerFactory; this.asyncErrorListener = asyncErrorListener; @@ -135,6 +149,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; trackTypeToIndex.get(trackType, /* valueIfKeyNotFound= */ C.INDEX_UNSET) == C.INDEX_UNSET, "There is already a track of type " + trackType); + ensureMuxerInitialized(); + int trackIndex = muxer.addTrack(format); trackTypeToIndex.put(trackType, trackIndex); trackTypeToSampleCount.put(trackType, 0); @@ -181,6 +197,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; trackTypeToTimeUs.put(trackType, presentationTimeUs); } + checkNotNull(muxer); resetAbortTimer(); muxer.writeSampleData(trackIndex, data, isKeyFrame, presentationTimeUs); previousTrackType = trackType; @@ -213,7 +230,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; public void release(boolean forCancellation) throws Muxer.MuxerException { isReady = false; abortScheduledExecutorService.shutdownNow(); - muxer.release(forCancellation); + if (muxer != null) { + muxer.release(forCancellation); + } } /** Returns the number of {@link #registerTrack() registered} tracks. */ @@ -276,6 +295,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; return trackTimeUs - minTrackTimeUs <= MAX_TRACK_WRITE_AHEAD_US; } + @RequiresNonNull("muxer") private void resetAbortTimer() { long maxDelayBetweenSamplesMs = muxer.getMaxDelayBetweenSamplesMs(); if (maxDelayBetweenSamplesMs == C.TIME_UNSET) { @@ -302,4 +322,16 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; maxDelayBetweenSamplesMs, MILLISECONDS); } + + @EnsuresNonNull("muxer") + private void ensureMuxerInitialized() throws Muxer.MuxerException { + if (muxer == null) { + if (outputPath != null) { + muxer = muxerFactory.create(outputPath); + } else { + checkNotNull(outputParcelFileDescriptor); + muxer = muxerFactory.create(outputParcelFileDescriptor); + } + } + } } 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 af2c8a7721..9900bb1f6e 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/Transformer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/Transformer.java @@ -677,7 +677,7 @@ public final class Transformer { * @throws IllegalArgumentException If the path is invalid. * @throws IllegalStateException If this method is called from the wrong thread. * @throws IllegalStateException If a transformation is already in progress. - * @throws IOException If an error occurs opening the output file for writing. + * @throws IOException If {@link MediaItem} is not supported. */ public void startTransformation(MediaItem mediaItem, String path) throws IOException { if (!mediaItem.clippingConfiguration.equals(MediaItem.ClippingConfiguration.UNSET) @@ -688,7 +688,7 @@ public final class Transformer { } this.outputPath = path; this.outputParcelFileDescriptor = null; - startTransformation(mediaItem, muxerFactory.create(path)); + startTransformationInternal(mediaItem); } /** @@ -711,24 +711,26 @@ public final class Transformer { * @throws IllegalArgumentException If the file descriptor is invalid. * @throws IllegalStateException If this method is called from the wrong thread. * @throws IllegalStateException If a transformation is already in progress. - * @throws IOException If an error occurs opening the output file for writing. */ @RequiresApi(26) - public void startTransformation(MediaItem mediaItem, ParcelFileDescriptor parcelFileDescriptor) - throws IOException { + public void startTransformation(MediaItem mediaItem, ParcelFileDescriptor parcelFileDescriptor) { this.outputParcelFileDescriptor = parcelFileDescriptor; this.outputPath = null; - startTransformation(mediaItem, muxerFactory.create(parcelFileDescriptor)); + startTransformationInternal(mediaItem); } - private void startTransformation(MediaItem mediaItem, Muxer muxer) { + private void startTransformationInternal(MediaItem mediaItem) { verifyApplicationThread(); if (player != null) { throw new IllegalStateException("There is already a transformation in progress."); } TransformerPlayerListener playerListener = new TransformerPlayerListener(mediaItem, looper); MuxerWrapper muxerWrapper = - new MuxerWrapper(muxer, muxerFactory, /* asyncErrorListener= */ playerListener); + new MuxerWrapper( + outputPath, + outputParcelFileDescriptor, + muxerFactory, + /* asyncErrorListener= */ playerListener); this.muxerWrapper = muxerWrapper; DefaultTrackSelector trackSelector = new DefaultTrackSelector(context); trackSelector.setParameters( diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/TestMuxer.java b/libraries/transformer/src/test/java/androidx/media3/transformer/TestMuxer.java index 24a59ae3c6..547a619a95 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/TestMuxer.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/TestMuxer.java @@ -18,7 +18,6 @@ package androidx.media3.transformer; import androidx.media3.common.Format; import androidx.media3.test.utils.DumpableFormat; import androidx.media3.test.utils.Dumper; -import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; @@ -35,7 +34,7 @@ public final class TestMuxer implements Muxer, Dumper.Dumpable { private final List dumpables; /** Creates a new test muxer. */ - public TestMuxer(String path, Muxer.Factory muxerFactory) throws IOException { + public TestMuxer(String path, Muxer.Factory muxerFactory) throws MuxerException { muxer = muxerFactory.create(path); dumpables = new ArrayList<>(); } diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/TransformerEndToEndTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/TransformerEndToEndTest.java index 224958ea5f..24f83e22e0 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/TransformerEndToEndTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/TransformerEndToEndTest.java @@ -912,13 +912,13 @@ public final class TransformerEndToEndTest { } @Override - public Muxer create(String path) throws IOException { + public Muxer create(String path) throws Muxer.MuxerException { testMuxer = new TestMuxer(path, defaultMuxerFactory); return testMuxer; } @Override - public Muxer create(ParcelFileDescriptor parcelFileDescriptor) throws IOException { + public Muxer create(ParcelFileDescriptor parcelFileDescriptor) throws Muxer.MuxerException { testMuxer = new TestMuxer("FD:" + parcelFileDescriptor.getFd(), defaultMuxerFactory); return testMuxer; }