From b10b4e6d4694c7240073b81c3a09227042b58c21 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 --- .../exoplayer2/transformer/DefaultMuxer.java | 5 +-- .../transformer/FrameworkMuxer.java | 24 +++++++---- .../android/exoplayer2/transformer/Muxer.java | 9 ++--- .../exoplayer2/transformer/MuxerWrapper.java | 40 +++++++++++++++++-- .../exoplayer2/transformer/Transformer.java | 18 +++++---- .../exoplayer2/transformer/TestMuxer.java | 3 +- .../transformer/TransformerEndToEndTest.java | 4 +- 7 files changed, 72 insertions(+), 31 deletions(-) diff --git a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/DefaultMuxer.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/DefaultMuxer.java index de4afce318..8e45bcf522 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/DefaultMuxer.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/DefaultMuxer.java @@ -19,7 +19,6 @@ import android.os.ParcelFileDescriptor; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; import com.google.common.collect.ImmutableList; -import java.io.IOException; import java.nio.ByteBuffer; /** A default {@link Muxer} implementation. */ @@ -51,12 +50,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/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FrameworkMuxer.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FrameworkMuxer.java index be82c9536f..bddae7b37d 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FrameworkMuxer.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/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/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Muxer.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Muxer.java index 37e1c8e318..0b6a14439d 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Muxer.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Muxer.java @@ -20,7 +20,6 @@ import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.util.MimeTypes; import com.google.common.collect.ImmutableList; -import java.io.IOException; import java.nio.ByteBuffer; /** @@ -55,9 +54,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. @@ -67,9 +66,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/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/MuxerWrapper.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/MuxerWrapper.java index c6f549a812..9e7aaf31d7 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/MuxerWrapper.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/MuxerWrapper.java @@ -16,11 +16,13 @@ package com.google.android.exoplayer2.transformer; +import static com.google.android.exoplayer2.util.Assertions.checkNotNull; import static com.google.android.exoplayer2.util.Assertions.checkState; import static com.google.android.exoplayer2.util.Util.maxValue; import static com.google.android.exoplayer2.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/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java index 855f624a36..5dc96edd1a 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java @@ -675,7 +675,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) @@ -686,7 +686,7 @@ public final class Transformer { } this.outputPath = path; this.outputParcelFileDescriptor = null; - startTransformation(mediaItem, muxerFactory.create(path)); + startTransformationInternal(mediaItem); } /** @@ -709,24 +709,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/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TestMuxer.java b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TestMuxer.java index 61712e39e7..b642638e2f 100644 --- a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TestMuxer.java +++ b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TestMuxer.java @@ -18,7 +18,6 @@ package com.google.android.exoplayer2.transformer; import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.testutil.DumpableFormat; import com.google.android.exoplayer2.testutil.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/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerEndToEndTest.java b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerEndToEndTest.java index 3a1816439b..60318310be 100644 --- a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/TransformerEndToEndTest.java +++ b/library/transformer/src/test/java/com/google/android/exoplayer2/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; }