Skip to content

Commit

Permalink
Merge M71: "Ensure we actually toss all MediaCodec indices after flus…
Browse files Browse the repository at this point in the history
…h()."

Depending on which process the MediaCodec is running in, there may
already be a non-default looper and handler which results in stale
input/output indexes being delivered to the bridge after start().

To prevent this, we now call start() immediately after flush(),
but post the clearing of mPendingStart through the handler/looper.

A sequence counter is added so that rapid flush()/stop() calls
don't prematurely use invalid indices.

BUG=892591
TEST=Run chrome with --in-process-gpu and seek a bunch on any video.

Change-Id: I8c60a73b8c00c85a7ab3d253bc8df2397c1fa00d
Reviewed-on: https://chromium-review.googlesource.com/c/1278293
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599283}(cherry picked from commit 1508795)
Reviewed-on: https://chromium-review.googlesource.com/c/1279958
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#13}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
  • Loading branch information
dalecurtis committed Oct 14, 2018
1 parent 4cee725 commit 90e8f56
Showing 1 changed file with 37 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import android.os.Bundle;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.Looper;
import android.view.Surface;

import org.chromium.base.Log;
Expand Down Expand Up @@ -75,6 +76,7 @@ class MediaCodecBridge {
private boolean mPendingError;
private boolean mPendingStart;
private long mNativeMediaCodecBridge;
private int mSequenceCounter;
private Queue<DequeueInputResult> mPendingInputBuffers;
private Queue<DequeueOutputResult> mPendingOutputBuffers;

Expand Down Expand Up @@ -273,6 +275,7 @@ private synchronized void prepareAsyncApiForRestart() {
mPendingOutputBuffers.clear();
mPendingStart = true;
mCurrentFormat = null;
++mSequenceCounter;
}

@CalledByNative
Expand Down Expand Up @@ -319,6 +322,12 @@ public synchronized void onOutputFormatChanged(MediaFormat format) {
notifyBuffersAvailable();
}

public synchronized void onPendingStartComplete(int sequenceCounter) {
// Ignore events from the past.
if (mSequenceCounter != sequenceCounter) return;
mPendingStart = false;
}

void updateLastPresentationTime(MediaCodec.BufferInfo info) {
if (info.presentationTimeUs < mLastPresentationTimeUs) {
// TODO(qinmin): return a special code through DequeueOutputResult
Expand Down Expand Up @@ -361,8 +370,26 @@ boolean start() {
try {
if (mUseAsyncApi) {
synchronized (this) {
mPendingStart = false;
if (mPendingError) return false;

class CompletePendingStartTask implements Runnable {
private int mThisSequence;
CompletePendingStartTask(int sequence) {
mThisSequence = sequence;
}

@Override
public void run() {
onPendingStartComplete(mThisSequence);
}
};

// Ensure any pending indices are ignored until after start
// by trampolining through the handler/looper that the
// notifications are coming from.
Handler h = sCallbackHandler == null ? new Handler(Looper.getMainLooper())
: sCallbackHandler;
h.post(new CompletePendingStartTask(mSequenceCounter));
}
}

Expand All @@ -386,12 +413,7 @@ private DequeueInputResult dequeueInputBuffer(long timeoutUs) {
if (mUseAsyncApi) {
synchronized (this) {
if (mPendingError) return new DequeueInputResult(MediaCodecStatus.ERROR, -1);
if (mPendingStart) {
return new DequeueInputResult(
start() ? MediaCodecStatus.TRY_AGAIN_LATER : MediaCodecStatus.ERROR,
-1);
}
if (mPendingInputBuffers.isEmpty())
if (mPendingStart || mPendingInputBuffers.isEmpty())
return new DequeueInputResult(MediaCodecStatus.TRY_AGAIN_LATER, -1);
return mPendingInputBuffers.remove();
}
Expand Down Expand Up @@ -423,11 +445,14 @@ private int flush() {
mMediaCodec.flush();

// MediaCodec.flush() invalidates all returned indices, but there
// may be some unhandled callbacks when using the async API. So we
// need to wait until the next dequeuInputBuffer() before we start
// accepting callbacks from MediaCodec again. By that point we can
// be sure that the looper / MessageLoop have cycled at least once.
if (mUseAsyncApi) prepareAsyncApiForRestart();
// may be some unhandled callbacks when using the async API. When
// we call prepareAsyncApiForRestart() it will set mPendingStart,
// start() will then post a task through the callback handler which
// clears mPendingStart to start accepting new buffers.
if (mUseAsyncApi) {
prepareAsyncApiForRestart();
if (!start()) return MediaCodecStatus.ERROR;
}
} catch (Exception e) {
Log.e(TAG, "Failed to flush MediaCodec", e);
return MediaCodecStatus.ERROR;
Expand Down

0 comments on commit 90e8f56

Please sign in to comment.