Bug 1540036 - p2: Check buffer and codec state before processing buffers. r=jya

HandleOutput() runs on Android binder thread pool and could be preempted
by RemoteDateDecoder task queue. That means ProcessOutput() could be scheduled
after ProcessShutdown() or ProcessFlush(). When that happens, aBuffer is no
long valid and should never be processed, and aSample can be
recycled immediately.
Also assert preconditions of buffers received from Java callbacks.

Differential Revision: https://phabricator.services.mozilla.com/D26189

--HG--
extra : moz-landing-system : lando
This commit is contained in:
John Lin 2019-04-05 21:01:56 +00:00
parent fa9a0bc673
commit e86d5d39e0
3 changed files with 28 additions and 0 deletions

View File

@ -94,6 +94,7 @@ class RemoteVideoDecoder : public RemoteDataDecoder {
void HandleOutput(Sample::Param aSample,
java::SampleBuffer::Param aBuffer) override {
MOZ_ASSERT(!aBuffer, "Video sample should be bufferless");
// aSample will be implicitly converted into a GlobalRef.
mDecoder->ProcessOutput(std::move(aSample));
}
@ -241,6 +242,10 @@ class RemoteVideoDecoder : public RemoteDataDecoder {
}
AssertOnTaskQueue();
if (GetState() == State::SHUTDOWN) {
mJavaDecoder->DisposeOutput(aSample);
return;
}
UniquePtr<VideoData::Listener> releaseSample(
new CompositeListener(mJavaDecoder, aSample));
@ -371,6 +376,7 @@ class RemoteAudioDecoder : public RemoteDataDecoder {
void HandleOutput(Sample::Param aSample,
java::SampleBuffer::Param aBuffer) override {
MOZ_ASSERT(aBuffer, "Audio sample should have buffer");
// aSample will be implicitly converted into a GlobalRef.
mDecoder->ProcessOutput(std::move(aSample), std::move(aBuffer));
}
@ -420,6 +426,11 @@ class RemoteAudioDecoder : public RemoteDataDecoder {
AssertOnTaskQueue();
if (GetState() == State::SHUTDOWN || !aBuffer->IsValid()) {
mJavaDecoder->DisposeOutput(aSample);
return;
}
RenderOrReleaseOutput autoRelease(mJavaDecoder, aSample);
BufferInfo::LocalRef info = aSample->Info();

View File

@ -412,6 +412,18 @@ public final class CodecProxy {
return true;
}
// Dispose Sample objects without sending requests to remote codec.
// Native callbacks use this method instead of releaseOutput() to save
// unnecessary IPC calls when recycling samples that have been released by
// release() or flush().
@WrapForJNI
public void disposeOutput(final Sample sample) {
if (mOutputSurface != null) {
mSurfaceOutputs.remove(sample);
}
sample.dispose();
}
/* package */ void reportError(final boolean fatal) {
mCallbacks.reportError(fatal);
}

View File

@ -88,6 +88,11 @@ public final class SampleBuffer implements Parcelable {
}
}
@WrapForJNI
public boolean isValid() {
return mSharedMem != null;
}
@Override public String toString() {
return "Buffer: " + mSharedMem;
}