Avoid buffers used by OpenGL getting GC'ed

The `GlUtil` wrappers for attributes/uniforms allocate buffers that are passed into OpenGL, and it seems that the distorted output was caused by these buffers being garbage collected.

The issue was difficult to reproduce manually. Add a test that does repeated transcodes in a loop (marked `@Ignore` for now because it runs for a long time and requires network access so may be flaky) to make it easier to run many transcodes and look for inconsistent output. For now the consistency check is just based on the file size which is very likely to change if the actual video frames change.

Verified that the test fails before, and passes once the attributes and uniforms are stored in fields as in this CL.

PiperOrigin-RevId: 410581706
This commit is contained in:
andrewlewis 2021-11-17 19:23:20 +00:00 committed by Ian Baker
parent 35c891a7bf
commit e1e54b3e15
4 changed files with 153 additions and 5 deletions

View File

@ -47,6 +47,7 @@ dependencies {
testImplementation 'com.google.truth:truth:' + truthVersion
androidTestImplementation 'junit:junit:' + junitVersion
androidTestImplementation 'androidx.test:runner:' + androidxTestRunnerVersion
androidTestImplementation 'com.google.truth:truth:' + truthVersion
androidTestImplementation project(modulePrefix + 'test-utils')
}

View File

@ -27,14 +27,25 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;
import org.checkerframework.checker.nullness.compatqual.NullableType;
/** Utility methods for instrumentation tests. */
/** Utilities for instrumentation tests. */
/* package */ final class AndroidTestUtil {
public static final String MP4_ASSET_URI = "asset:///media/mp4/sample.mp4";
public static final String SEF_ASSET_URI = "asset:///media/mp4/sample_sef_slow_motion.mp4";
public static final String REMOTE_MP4_10_SECONDS_URI_STRING =
"https://storage.googleapis.com/exoplayer-test-media-1/mp4/android-screens-10s.mp4";
/** Information about the result of successfully running a transformer. */
public static final class TransformationResult {
public long outputSizeBytes;
private TransformationResult(long outputSizeBytes) {
this.outputSizeBytes = outputSizeBytes;
}
}
/** Transforms the {@code uriString} with the {@link Transformer}. */
public static void runTransformer(Context context, Transformer transformer, String uriString)
throws Exception {
public static TransformationResult runTransformer(
Context context, Transformer transformer, String uriString) throws Exception {
AtomicReference<@NullableType Exception> exceptionReference = new AtomicReference<>();
CountDownLatch countDownLatch = new CountDownLatch(1);
@ -74,6 +85,59 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
if (exception != null) {
throw exception;
}
long outputSizeBytes = externalCacheFile.length();
return new TransformationResult(outputSizeBytes);
} finally {
externalCacheFile.delete();
}
}
// TODO(internal b/202131097): Deduplicate with the other overload when TranscodingTransformer is
// merged into Transformer.
/** Transforms the {@code uriString} with the {@link TranscodingTransformer}. */
public static TransformationResult runTransformer(
Context context, TranscodingTransformer transformer, String uriString) throws Exception {
AtomicReference<@NullableType Exception> exceptionReference = new AtomicReference<>();
CountDownLatch countDownLatch = new CountDownLatch(1);
TranscodingTransformer testTransformer =
transformer
.buildUpon()
.setListener(
new TranscodingTransformer.Listener() {
@Override
public void onTransformationCompleted(MediaItem inputMediaItem) {
countDownLatch.countDown();
}
@Override
public void onTransformationError(MediaItem inputMediaItem, Exception exception) {
exceptionReference.set(exception);
countDownLatch.countDown();
}
})
.build();
Uri uri = Uri.parse(uriString);
File externalCacheFile = createExternalCacheFile(uri, context);
try {
InstrumentationRegistry.getInstrumentation()
.runOnMainSync(
() -> {
try {
testTransformer.startTransformation(
MediaItem.fromUri(uri), externalCacheFile.getAbsolutePath());
} catch (IOException e) {
exceptionReference.set(e);
}
});
countDownLatch.await();
@Nullable Exception exception = exceptionReference.get();
if (exception != null) {
throw exception;
}
long outputSizeBytes = externalCacheFile.length();
return new TransformationResult(outputSizeBytes);
} finally {
externalCacheFile.delete();
}

View File

@ -0,0 +1,59 @@
/*
* Copyright 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package androidx.media3.transformer;
import static androidx.media3.transformer.AndroidTestUtil.runTransformer;
import static com.google.common.truth.Truth.assertWithMessage;
import android.content.Context;
import androidx.media3.common.C;
import androidx.media3.common.MimeTypes;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import org.junit.Test;
import org.junit.runner.RunWith;
/** Tests repeated transcoding operations (as a stress test and to help reproduce flakiness). */
@RunWith(AndroidJUnit4.class)
public final class RepeatedTranscodeTransformationTest {
private static final int TRANSCODE_COUNT = 10;
@Test
public void repeatedTranscode_givesConsistentLengthOutput() throws Exception {
Context context = ApplicationProvider.getApplicationContext();
TranscodingTransformer transcodingTransformer =
new TranscodingTransformer.Builder()
.setVideoMimeType(MimeTypes.VIDEO_H265)
.setContext(context)
.build();
long previousOutputSizeBytes = C.LENGTH_UNSET;
for (int i = 0; i < TRANSCODE_COUNT; i++) {
// Use a long video in case an error occurs a while after the start of the video.
long outputSizeBytes =
runTransformer(
context, transcodingTransformer, AndroidTestUtil.REMOTE_MP4_10_SECONDS_URI_STRING)
.outputSizeBytes;
if (previousOutputSizeBytes != C.LENGTH_UNSET) {
assertWithMessage("Unexpected output size on transcode " + i + " out of " + TRANSCODE_COUNT)
.that(outputSizeBytes)
.isEqualTo(previousOutputSizeBytes);
}
previousOutputSizeBytes = outputSizeBytes;
}
}
}

View File

@ -118,7 +118,14 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
}
return new OpenGlFrameEditor(
eglDisplay, eglContext, eglSurface, textureId, checkNotNull(textureTransformUniform));
eglDisplay,
eglContext,
eglSurface,
textureId,
checkNotNull(textureTransformUniform),
copyProgram,
copyAttributes,
copyUniforms);
}
// Predefined shader values.
@ -137,6 +144,17 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
private final Surface inputSurface;
private final GlUtil.Uniform textureTransformUniform;
// TODO(internal: b/206631334): These fields ensure buffers passed to GL are not GC'ed. Implement
// a better way of doing this so they aren't just unused fields.
@SuppressWarnings("unused")
private final GlUtil.Program copyProgram;
@SuppressWarnings("unused")
private final GlUtil.Attribute[] copyAttributes;
@SuppressWarnings("unused")
private final GlUtil.Uniform[] copyUniforms;
private volatile boolean hasInputData;
private OpenGlFrameEditor(
@ -144,12 +162,18 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
EGLContext eglContext,
EGLSurface eglSurface,
int textureId,
GlUtil.Uniform textureTransformUniform) {
GlUtil.Uniform textureTransformUniform,
GlUtil.Program copyProgram,
GlUtil.Attribute[] copyAttributes,
GlUtil.Uniform[] copyUniforms) {
this.eglDisplay = eglDisplay;
this.eglContext = eglContext;
this.eglSurface = eglSurface;
this.textureId = textureId;
this.textureTransformUniform = textureTransformUniform;
this.copyProgram = copyProgram;
this.copyAttributes = copyAttributes;
this.copyUniforms = copyUniforms;
textureTransformMatrix = new float[16];
inputSurfaceTexture = new SurfaceTexture(textureId);
inputSurfaceTexture.setOnFrameAvailableListener(surfaceTexture -> hasInputData = true);