ImageWriter: create a thread for buffer detaching
Calling detachNextBuffer while producer/consumer sits in the same
process will cause a deadlock. Creating a thread shared by all
alive ImageWriter instances to detachBuffers.
Test: new CTS tests using TextureView + ImageWriter
Bug: 122740799
Change-Id: I6231c75b93aa862449831a050b376f978b459bed
diff --git a/media/jni/android_media_ImageWriter.cpp b/media/jni/android_media_ImageWriter.cpp
index 031e373..cfcba76 100644
--- a/media/jni/android_media_ImageWriter.cpp
+++ b/media/jni/android_media_ImageWriter.cpp
@@ -18,8 +18,11 @@
#define LOG_TAG "ImageWriter_JNI"
#include "android_media_Utils.h"
+#include <utils/Condition.h>
#include <utils/Log.h>
+#include <utils/Mutex.h>
#include <utils/String8.h>
+#include <utils/Thread.h>
#include <gui/IProducerListener.h>
#include <gui/Surface.h>
@@ -34,6 +37,8 @@
#include <inttypes.h>
#include <android/hardware_buffer_jni.h>
+#include <deque>
+
#define IMAGE_BUFFER_JNI_ID "mNativeBuffer"
#define IMAGE_FORMAT_UNKNOWN 0 // This is the same value as ImageFormat#UNKNOWN.
// ----------------------------------------------------------------------------
@@ -90,14 +95,124 @@
int mFormat;
int mWidth;
int mHeight;
+
+ // Class for a shared thread used to detach buffers from buffer queues
+ // to discard buffers after consumers are done using them.
+ // This is needed because detaching buffers in onBufferReleased callback
+ // can lead to deadlock when consumer/producer are on the same process.
+ class BufferDetacher {
+ public:
+ // Called by JNIImageWriterContext ctor. Will start the thread for first ref.
+ void addRef();
+ // Called by JNIImageWriterContext dtor. Will stop the thread after ref goes to 0.
+ void removeRef();
+ // Called by onBufferReleased to signal this thread to detach a buffer
+ void detach(wp<Surface>);
+
+ private:
+
+ class DetachThread : public Thread {
+ public:
+ DetachThread() : Thread(/*canCallJava*/false) {};
+
+ void detach(wp<Surface>);
+
+ virtual void requestExit() override;
+
+ private:
+ virtual bool threadLoop() override;
+
+ Mutex mLock;
+ Condition mCondition;
+ std::deque<wp<Surface>> mQueue;
+
+ static const nsecs_t kWaitDuration = 20000000; // 20 ms
+ };
+ sp<DetachThread> mThread;
+
+ Mutex mLock;
+ int mRefCount = 0;
+ };
+
+ static BufferDetacher sBufferDetacher;
};
+JNIImageWriterContext::BufferDetacher JNIImageWriterContext::sBufferDetacher;
+
+void JNIImageWriterContext::BufferDetacher::addRef() {
+ Mutex::Autolock l(mLock);
+ mRefCount++;
+ if (mRefCount == 1) {
+ mThread = new DetachThread();
+ mThread->run("BufDtchThrd");
+ }
+}
+
+void JNIImageWriterContext::BufferDetacher::removeRef() {
+ Mutex::Autolock l(mLock);
+ mRefCount--;
+ if (mRefCount == 0) {
+ mThread->requestExit();
+ mThread->join();
+ mThread.clear();
+ }
+}
+
+void JNIImageWriterContext::BufferDetacher::detach(wp<Surface> bq) {
+ Mutex::Autolock l(mLock);
+ if (mThread == nullptr) {
+ ALOGE("%s: buffer detach thread is gone!", __FUNCTION__);
+ return;
+ }
+ mThread->detach(bq);
+}
+
+void JNIImageWriterContext::BufferDetacher::DetachThread::detach(wp<Surface> bq) {
+ Mutex::Autolock l(mLock);
+ mQueue.push_back(bq);
+ mCondition.signal();
+}
+
+void JNIImageWriterContext::BufferDetacher::DetachThread::requestExit() {
+ Thread::requestExit();
+ {
+ Mutex::Autolock l(mLock);
+ mQueue.clear();
+ }
+ mCondition.signal();
+}
+
+bool JNIImageWriterContext::BufferDetacher::DetachThread::threadLoop() {
+ Mutex::Autolock l(mLock);
+ mCondition.waitRelative(mLock, kWaitDuration);
+
+ while (!mQueue.empty()) {
+ if (exitPending()) {
+ return false;
+ }
+
+ wp<Surface> wbq = mQueue.front();
+ mQueue.pop_front();
+ sp<Surface> bq = wbq.promote();
+ if (bq != nullptr) {
+ sp<Fence> fence;
+ sp<GraphicBuffer> buffer;
+ ALOGV("%s: One buffer is detached", __FUNCTION__);
+ mLock.unlock();
+ bq->detachNextBuffer(&buffer, &fence);
+ mLock.lock();
+ }
+ }
+ return !exitPending();
+}
+
JNIImageWriterContext::JNIImageWriterContext(JNIEnv* env, jobject weakThiz, jclass clazz) :
- mWeakThiz(env->NewGlobalRef(weakThiz)),
- mClazz((jclass)env->NewGlobalRef(clazz)),
- mFormat(0),
- mWidth(-1),
- mHeight(-1) {
+ mWeakThiz(env->NewGlobalRef(weakThiz)),
+ mClazz((jclass)env->NewGlobalRef(clazz)),
+ mFormat(0),
+ mWidth(-1),
+ mHeight(-1) {
+ sBufferDetacher.addRef();
}
JNIImageWriterContext::~JNIImageWriterContext() {
@@ -115,6 +230,7 @@
}
mProducer.clear();
+ sBufferDetacher.removeRef();
}
JNIEnv* JNIImageWriterContext::getJNIEnv(bool* needsDetach) {
@@ -153,10 +269,7 @@
// need let this callback give a BufferItem, then only detach if it was attached to this
// Writer. Do the detach unconditionally for opaque format now. see b/19977520
if (mFormat == HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED) {
- sp<Fence> fence;
- sp<GraphicBuffer> buffer;
- ALOGV("%s: One buffer is detached", __FUNCTION__);
- mProducer->detachNextBuffer(&buffer, &fence);
+ sBufferDetacher.detach(mProducer);
}
env->CallStaticVoidMethod(mClazz, gImageWriterClassInfo.postEventFromNative, mWeakThiz);