Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indeterminate z-order of EditedMediaItemSequences #1055

Merged
merged 8 commits into from
May 8, 2024
Merged
Next Next commit
Fix indeterminate z-order of EditedMediaItemSequences by passing sequ…
…enceIndex when registeringInput
  • Loading branch information
AradiPatrik authored and claincly committed May 8, 2024
commit 177f1f33d0dc67c2a7b65466a8d16cf926006113
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,16 @@ interface Listener {
*
* <p>If the method throws, the caller must call {@link #release}.
*
* @param sequenceIndex The sequence index of the input which can aid ordering of the inputs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it's easier if we require sequence indices start from 0, so please add something like "The index must start from zero"

*
* @return The id of the registered input, which can be used to get the underlying {@link
* VideoFrameProcessor} via {@link #getProcessor(int)}.
*/
int registerInput() throws VideoFrameProcessingException;
int registerInput(int sequenceIndex) throws VideoFrameProcessingException;

/**
* Returns the {@link VideoFrameProcessor} that handles the processing for an input registered via
* {@link #registerInput()}. If the {@code inputId} is not {@linkplain #registerInput()
* {@link #registerInput(int)}. If the {@code inputId} is not {@linkplain #registerInput(int)
* registered} before, this method will throw an {@link IllegalStateException}.
*/
VideoFrameProcessor getProcessor(int inputId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import android.opengl.EGLDisplay;
import android.opengl.EGLSurface;
import android.opengl.GLES20;
import android.util.SparseArray;

import androidx.annotation.GuardedBy;
import androidx.annotation.IntRange;
import androidx.annotation.Nullable;
Expand All @@ -41,16 +43,18 @@
import androidx.media3.common.util.Size;
import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;

import org.checkerframework.checker.nullness.qual.MonotonicNonNull;

import java.io.IOException;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.ExecutorService;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;

/**
* A basic {@link VideoCompositor} implementation that takes in frames from input sources' streams
Expand Down Expand Up @@ -88,7 +92,7 @@ public final class DefaultVideoCompositor implements VideoCompositor {
private final VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor;

@GuardedBy("this")
private final List<InputSource> inputSources;
private final SparseArray<InputSource> inputSources;

@GuardedBy("this")
private boolean allInputsEnded; // Whether all inputSources have signaled end of input.
Expand Down Expand Up @@ -124,7 +128,7 @@ public DefaultVideoCompositor(
this.settings = settings;
this.compositorGlProgram = new CompositorGlProgram(context);

inputSources = new ArrayList<>();
inputSources = new SparseArray<>();
outputTexturePool =
new TexturePool(/* useHighPrecisionColorComponents= */ false, textureOutputCapacity);
outputTextureTimestamps = new LongArrayQueue(textureOutputCapacity);
Expand All @@ -142,17 +146,17 @@ public DefaultVideoCompositor(
}

@Override
public synchronized int registerInputSource() {
inputSources.add(new InputSource());
return inputSources.size() - 1;
public synchronized int registerInputSource(int sequenceId) {
inputSources.put(sequenceId, new InputSource());
return sequenceId;
}

@Override
public synchronized void signalEndOfInputSource(int inputId) {
inputSources.get(inputId).isInputEnded = true;
boolean allInputsEnded = true;
for (int i = 0; i < inputSources.size(); i++) {
if (!inputSources.get(i).isInputEnded) {
if (!inputSources.get(inputSources.keyAt(i)).isInputEnded) {
allInputsEnded = false;
break;
}
Expand Down Expand Up @@ -229,7 +233,7 @@ private synchronized void releaseExcessFramesInAllSecondaryStreams() {
if (i == PRIMARY_INPUT_ID) {
continue;
}
releaseExcessFramesInSecondaryStream(inputSources.get(i));
releaseExcessFramesInSecondaryStream(inputSources.get(inputSources.keyAt(i)));
}
}

Expand Down Expand Up @@ -334,7 +338,7 @@ private synchronized ImmutableList<InputFrameInfo> getFramesToComposite() {
return ImmutableList.of();
}
for (int inputId = 0; inputId < inputSources.size(); inputId++) {
if (inputSources.get(inputId).frameInfos.isEmpty()) {
if (inputSources.get(inputSources.keyAt(inputId)).frameInfos.isEmpty()) {
return ImmutableList.of();
}
}
Expand All @@ -353,7 +357,7 @@ private synchronized ImmutableList<InputFrameInfo> getFramesToComposite() {
// 2. Two or more frames, and at least one frame has timestamp greater than the target
// timestamp.
// The smaller timestamp is taken if two timestamps have the same distance from the primary.
InputSource secondaryInputSource = inputSources.get(inputId);
InputSource secondaryInputSource = inputSources.get(inputSources.keyAt(inputId));
if (secondaryInputSource.frameInfos.size() == 1 && !secondaryInputSource.isInputEnded) {
return ImmutableList.of();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import androidx.media3.common.VideoFrameProcessor;
import androidx.media3.common.VideoGraph;
import androidx.media3.common.util.GlUtil;
import androidx.media3.common.util.NullableType;
import androidx.media3.common.util.UnstableApi;
import com.google.common.util.concurrent.MoreExecutors;
import java.util.ArrayDeque;
Expand Down Expand Up @@ -75,7 +76,7 @@ public abstract class MultipleInputVideoGraph implements VideoGraph {
private final Executor listenerExecutor;
private final VideoCompositorSettings videoCompositorSettings;
private final List<Effect> compositionEffects;
private final List<VideoFrameProcessor> preProcessors;
private final List<@NullableType VideoFrameProcessor> preProcessors;

private final ExecutorService sharedExecutorService;

Expand Down Expand Up @@ -211,10 +212,11 @@ public void onEnded() {
}

@Override
public int registerInput() throws VideoFrameProcessingException {
public int registerInput(int forceId) throws VideoFrameProcessingException {
checkStateNotNull(videoCompositor);

int videoCompositorInputId = videoCompositor.registerInputSource();
int videoCompositorInputId;
videoCompositorInputId = videoCompositor.registerInputSource(forceId);
// Creating a new VideoFrameProcessor for the input.
VideoFrameProcessor preProcessor =
videoFrameProcessorFactory
Expand Down Expand Up @@ -257,7 +259,12 @@ public void onEnded() {
onPreProcessingVideoFrameProcessorEnded(videoCompositorInputId);
}
});
preProcessors.add(preProcessor);

while (preProcessors.size() <= videoCompositorInputId) {
//noinspection DataFlowIssue
preProcessors.add(null);
}
preProcessors.set(videoCompositorInputId, preProcessor);
return videoCompositorInputId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
@UnstableApi
public abstract class SingleInputVideoGraph implements VideoGraph {

/** The ID {@link #registerInput()} returns. */
/** The ID {@link #registerInput(int)} returns. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The index of the only {@linkplain #registerInput(int) registered} input.

public static final int SINGLE_INPUT_INDEX = 0;

private final Context context;
Expand Down Expand Up @@ -99,7 +99,7 @@ public void initialize() {
}

@Override
public int registerInput() throws VideoFrameProcessingException {
public int registerInput(int sequenceIndex) throws VideoFrameProcessingException {
checkStateNotNull(videoFrameProcessor == null && !released);

videoFrameProcessor =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ interface Listener {
/**
* Registers a new input source, and returns a unique {@code inputId} corresponding to this
* source, to be used in {@link #queueInputTexture}.
*
* @param sequenceId The sequence ID of the input source, which is can be used to determine the
* order of the input sources.
*/
int registerInputSource();

int registerInputSource(int sequenceId);
/**
* Signals that no more frames will come from the upstream {@link GlTextureProducer.Listener}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,8 @@ public VideoSinkImpl(Context context) {
// reduces decoder timeouts, and consider restoring.
videoFrameProcessorMaxPendingFrameCount =
Util.getMaxPendingFramesCountForMediaCodecDecoders(context);
int videoGraphInputId = videoGraph.registerInput(0);
videoFrameProcessor = videoGraph.getProcessor(videoGraphInputId);

videoEffects = new ArrayList<>();
finalBufferPresentationTimeUs = C.TIME_UNSET;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ private static VideoFrameProcessorTestRunner.Builder createVideoFrameProcessorTe
VideoCompositor videoCompositor,
@Nullable ExecutorService executorService,
GlObjectsProvider glObjectsProvider) {
int inputId = videoCompositor.registerInputSource();
int inputId = videoCompositor.registerInputSource(0);
DefaultVideoFrameProcessor.Factory.Builder defaultVideoFrameProcessorFactoryBuilder =
new DefaultVideoFrameProcessor.Factory.Builder()
.setGlObjectsProvider(glObjectsProvider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public AudioSampleExporter(
}

@Override
public AudioGraphInput getInput(EditedMediaItem editedMediaItem, Format format)
public AudioGraphInput getInput(EditedMediaItem editedMediaItem, Format format, int sequenceIndex)
AradiPatrik marked this conversation as resolved.
Show resolved Hide resolved
throws ExportException {
if (!returnedFirstInput) {
// First input initialized in constructor because output AudioFormat is needed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public boolean queueInputBuffer() {
}

@Override
public GraphInput getInput(EditedMediaItem item, Format format) {
public GraphInput getInput(EditedMediaItem item, Format format, int sequenceIndex) {
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ public SampleExporter(Format firstInputFormat, MuxerWrapper muxerWrapper) {
*
* @param editedMediaItem The initial {@link EditedMediaItem} of the input.
* @param format The initial {@link Format} of the input.
* @param sequenceIndex The sequence index of the input.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supernit: The index of the input sequence?

* @throws ExportException If an error occurs getting the input.
*/
public abstract GraphInput getInput(EditedMediaItem editedMediaItem, Format format)
throws ExportException;
public abstract GraphInput getInput(EditedMediaItem editedMediaItem, Format format, int sequenceIndex) throws ExportException;

/**
* Processes the input data and returns whether it may be possible to process more data by calling
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ public SampleConsumer onOutputFormat(Format assetLoaderOutputFormat) throws Expo
}

GraphInput sampleExporterInput =
sampleExporter.getInput(firstEditedMediaItem, assetLoaderOutputFormat);
sampleExporter.getInput(firstEditedMediaItem, assetLoaderOutputFormat, sequenceIndex);
OnMediaItemChangedListener onMediaItemChangedListener =
(editedMediaItem, durationUs, decodedFormat, isLast) -> {
onMediaItemChanged(trackType, durationUs, isLast);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import androidx.media3.common.Effect;
import androidx.media3.common.VideoFrameProcessingException;
import androidx.media3.common.VideoGraph;
import androidx.media3.common.util.Log;
import androidx.media3.effect.MultipleInputVideoGraph;
import androidx.media3.effect.VideoCompositorSettings;
import java.util.List;
Expand Down Expand Up @@ -79,8 +80,8 @@ private TransformerMultipleInputVideoGraph(
}

@Override
public GraphInput createInput() throws VideoFrameProcessingException {
int inputId = registerInput();
public GraphInput createInput(int sequenceIndex) throws VideoFrameProcessingException {
int inputId = registerInput(sequenceIndex);
return new VideoFrameProcessingWrapper(
getProcessor(inputId), /* presentation= */ null, getInitialTimestampOffsetUs());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,15 @@ private TransformerSingleInputVideoGraph(
}

@Override
public GraphInput createInput() throws VideoFrameProcessingException {
public GraphInput createInput(int sequenceIndex) throws VideoFrameProcessingException {
checkState(videoFrameProcessingWrapper == null);
int inputId = registerInput();
int inputId = registerInput(sequenceIndex);
videoFrameProcessingWrapper =
new VideoFrameProcessingWrapper(
getProcessor(inputId), getPresentation(), getInitialTimestampOffsetUs());
new VideoFrameProcessingWrapper(
getProcessor(inputId),
getInputColorInfo(),
getPresentation(),
getInitialTimestampOffsetUs());
return videoFrameProcessingWrapper;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ TransformerVideoGraph create(
* <p>This method must called exactly once for every input stream.
*
* <p>If the method throws any {@link Exception}, the caller must call {@link #release}.
*
* @param sequenceIndex The sequence index of the input, which can aid ordering of the inputs.
*/
GraphInput createInput() throws VideoFrameProcessingException;
GraphInput createInput(int sequenceIndex) throws VideoFrameProcessingException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import androidx.media3.decoder.DecoderInputBuffer;
import androidx.media3.effect.DebugTraceUtil;
import androidx.media3.effect.VideoCompositorSettings;
import androidx.media3.exoplayer.mediacodec.MediaCodecUtil;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.MoreExecutors;
import java.nio.ByteBuffer;
Expand All @@ -58,6 +59,8 @@
import org.checkerframework.checker.initialization.qual.Initialized;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.dataflow.qual.Pure;
import java.nio.ByteBuffer;
import java.util.List;

/** Processes, encodes and muxes raw video frames. */
/* package */ final class VideoSampleExporter extends SampleExporter {
Expand Down Expand Up @@ -153,10 +156,9 @@ public VideoSampleExporter(
}

@Override
public GraphInput getInput(EditedMediaItem editedMediaItem, Format format)
throws ExportException {
public GraphInput getInput(EditedMediaItem editedMediaItem, Format format, int sequenceIndex) throws ExportException {
try {
return videoGraph.createInput();
return videoGraph.createInput(sequenceIndex);
} catch (VideoFrameProcessingException e) {
throw ExportException.createForVideoFrameProcessingException(e);
}
Expand Down Expand Up @@ -540,8 +542,8 @@ public void initialize() throws VideoFrameProcessingException {
}

@Override
public int registerInput() throws VideoFrameProcessingException {
return videoGraph.registerInput();
public int registerInput(int sequenceIndex) throws VideoFrameProcessingException {
return videoGraph.registerInput(sequenceIndex);
}

@Override
Expand All @@ -550,8 +552,8 @@ public VideoFrameProcessor getProcessor(int inputId) {
}

@Override
public GraphInput createInput() throws VideoFrameProcessingException {
return videoGraph.createInput();
public GraphInput createInput(int sequenceIndex) throws VideoFrameProcessingException {
return videoGraph.createInput(sequenceIndex);
}

@Override
Expand Down