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 0c216f3519..69f566a56f 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/TransformerInternal.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/TransformerInternal.java @@ -41,7 +41,6 @@ import android.util.Log; import android.util.SparseArray; import androidx.annotation.GuardedBy; import androidx.annotation.IntDef; -import androidx.annotation.IntRange; import androidx.annotation.Nullable; import androidx.media3.common.C; import androidx.media3.common.DebugViewProvider; @@ -101,6 +100,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private static final int MSG_REGISTER_SAMPLE_EXPORTER = 1; private static final int MSG_DRAIN_EXPORTERS = 2; private static final int MSG_END = 3; + private static final int MSG_UPDATE_PROGRESS = 4; private static final String TAG = "TransformerInternal"; private static final int DRAIN_EXPORTERS_DELAY_MS = 10; @@ -129,9 +129,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private final List sampleExporters; private final MuxerWrapper muxerWrapper; - private final ConditionVariable transformerCancelConditionVariable; + private final ConditionVariable transformerConditionVariable; private final Object setMaxSequenceDurationUsLock; - private final Object progressLock; private boolean isDrainingExporters; @@ -146,21 +145,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * The current {@link Transformer.ProgressState}. * - *

Modified on the internal thread. Accessed on internal thread and application thread (in - * {@link #getProgress}). + *

Modified on the internal thread. Accessed on the application thread (in {@link + * #getProgress}). */ - @GuardedBy("progressLock") - private @Transformer.ProgressState int progressState; - - /** - * The current progress value, from 0 to 100. - * - *

Modified on the internal thread. Accessed on internal thread and application thread (in - * {@link #getProgress}). - */ - @GuardedBy("progressLock") - @IntRange(from = 0, to = 100) - private int progressValue; + private volatile @Transformer.ProgressState int progressState; /** * The boolean tracking if this component has been released. @@ -227,10 +215,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; compositionHasLoopingSequence = nonLoopingSequencesWithNonFinalDuration != composition.sequences.size(); setMaxSequenceDurationUsLock = new Object(); - transformerCancelConditionVariable = new ConditionVariable(); - progressLock = new Object(); sampleExporters = new ArrayList<>(); - + transformerConditionVariable = new ConditionVariable(); // It's safe to use "this" because we don't send a message before exiting the constructor. @SuppressWarnings("nullness:methodref.receiver.bound") HandlerWrapper internalHandler = @@ -247,13 +233,12 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; if (released) { return PROGRESS_STATE_NOT_STARTED; } - - synchronized (progressLock) { - if (progressState == PROGRESS_STATE_AVAILABLE) { - progressHolder.progress = progressValue; - } - return progressState; - } + verifyInternalThreadAlive(); + internalHandler.obtainMessage(MSG_UPDATE_PROGRESS, progressHolder).sendToTarget(); + // TODO: figure out why calling clock.onThreadBlocked() here makes the tests fail. + transformerConditionVariable.blockUninterruptible(); + transformerConditionVariable.close(); + return progressState; } public void cancel() { @@ -265,8 +250,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; .obtainMessage(MSG_END, END_REASON_CANCELLED, /* unused */ 0, /* exportException */ null) .sendToTarget(); clock.onThreadBlocked(); - transformerCancelConditionVariable.blockUninterruptible(); - transformerCancelConditionVariable.close(); + transformerConditionVariable.blockUninterruptible(); + transformerConditionVariable.close(); if (cancelException != null) { throw cancelException; } @@ -297,7 +282,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // handled to report release timeouts and to unblock the transformer condition variable in case // of cancellation. Progress update messages must be handled to unblock the transformer // condition variable. - if (released && msg.what != MSG_END) { + if (released && msg.what != MSG_END && msg.what != MSG_UPDATE_PROGRESS) { return true; } try { @@ -314,6 +299,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; case MSG_END: endInternal(/* endReason= */ msg.arg1, /* exportException= */ (ExportException) msg.obj); break; + case MSG_UPDATE_PROGRESS: + updateProgressInternal(/* progressHolder= */ (ProgressHolder) msg.obj); + break; default: return false; } @@ -344,8 +332,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; while (sampleExporters.get(i).processData()) {} } - updateProgressInternal(); - if (!muxerWrapper.isEnded()) { internalHandler.sendEmptyMessageDelayed(MSG_DRAIN_EXPORTERS, DRAIN_EXPORTERS_DELAY_MS); } @@ -363,11 +349,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; boolean releasedPreviously = released; if (!released) { released = true; - synchronized (progressLock) { - progressState = PROGRESS_STATE_NOT_STARTED; - progressValue = 0; - } - // VideoSampleExporter can hold buffers from the asset loader's decoder in a surface texture, // so we release the VideoSampleExporter first to avoid releasing the codec while its buffers // are pending processing. @@ -410,8 +391,12 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; internalHandler.post(internalHandlerThread::quitSafely); } + // Update progress before opening variable to avoid getProgress returning an invalid combination + // of state and progress. + progressState = PROGRESS_STATE_NOT_STARTED; + transformerConditionVariable.open(); + if (forCancellation) { - transformerCancelConditionVariable.open(); return; } @@ -452,50 +437,25 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } } - /** - * Calculates the current progress of the export, updating the thread-safe class variables. - * - *

Should only be called on the internal thread. - */ - private void updateProgressInternal() { - if (released) { - return; - } - - synchronized (progressLock) { - if (progressState == PROGRESS_STATE_NOT_STARTED) { - return; - } - } - - @Transformer.ProgressState int individualProgressState; - ProgressHolder individualProgressHolder = new ProgressHolder(); - + private void updateProgressInternal(ProgressHolder progressHolder) { int progressSum = 0; int progressCount = 0; - + ProgressHolder individualProgressHolder = new ProgressHolder(); for (int i = 0; i < sequenceAssetLoaders.size(); i++) { if (composition.sequences.get(i).isLooping) { // Looping sequence progress is always unavailable. Skip it. continue; } - - individualProgressHolder.progress = 0; - individualProgressState = sequenceAssetLoaders.get(i).getProgress(individualProgressHolder); - if (individualProgressState != PROGRESS_STATE_AVAILABLE) { - synchronized (progressLock) { - progressState = individualProgressState; - progressValue = 0; - } + progressState = sequenceAssetLoaders.get(i).getProgress(individualProgressHolder); + if (progressState != PROGRESS_STATE_AVAILABLE) { + transformerConditionVariable.open(); return; } progressSum += individualProgressHolder.progress; progressCount++; } - synchronized (progressLock) { - progressState = PROGRESS_STATE_AVAILABLE; - progressValue = progressSum / progressCount; - } + progressHolder.progress = progressSum / progressCount; + transformerConditionVariable.open(); } private final class SequenceAssetLoaderListener implements AssetLoader.Listener {