Fix remote GraphicBuffer allocation in SurfaceFlinger.

This change fixes a horrible hack that I did to allow application
processes to create GraphicBuffer objects by making a binder call to
SurfaceFlinger.  This change introduces a new binder interface
specifically for doing this, and does it in such a way that
SurfaceFlinger will maintain a reference to the buffers until the app is
done with them.

Change-Id: Icb240397c6c206d7f69124c1497a829f051cb49b

include/gui/SurfaceTexture.h

@@ -26,12 +26,15 @@
 #include <ui/GraphicBuffer.h>
 
 #include <utils/threads.h>
+#include <utils/Vector.h>
 
 #define ANDROID_GRAPHICS_SURFACETEXTURE_JNI_ID "mSurfaceTexture"
 
 namespace android {
 // ----------------------------------------------------------------------------
 
+class IGraphicBufferAlloc;
+
 class SurfaceTexture : public BnSurfaceTexture {
 public:
     enum { MIN_BUFFER_SLOTS = 3 };
@@ -140,6 +143,12 @@ private:
     // reset mCurrentTexture to INVALID_BUFFER_SLOT.
     int mCurrentTexture;
 
+    // mCurrentTextureBuf is the graphic buffer of the current texture. It's
+    // possible that this buffer is not associated with any buffer slot, so we
+    // must track it separately in order to properly use
+    // IGraphicBufferAlloc::freeAllGraphicBuffersExcept.
+    sp<GraphicBuffer> mCurrentTextureBuf;
+
     // mCurrentCrop is the crop rectangle that applies to the current texture.
     // It gets set to mLastQueuedCrop each time updateTexImage is called.
     Rect mCurrentCrop;
@@ -176,6 +185,16 @@ private:
     // changed with a call to setTexName.
     const GLuint mTexName;
 
+    // mGraphicBufferAlloc is the connection to SurfaceFlinger that is used to
+    // allocate new GraphicBuffer objects.
+    sp<IGraphicBufferAlloc> mGraphicBufferAlloc;
+
+    // mAllocdBuffers is mirror of the list of buffers that SurfaceFlinger is
+    // referencing. This is kept so that gralloc implementations do not need to
+    // properly handle the case where SurfaceFlinger no longer holds a reference
+    // to a buffer, but other processes do.
+    Vector<sp<GraphicBuffer> > mAllocdBuffers;
+
     // mMutex is the mutex used to prevent concurrent access to the member
     // variables of SurfaceTexture objects. It must be locked whenever the
     // member variables are accessed.

include/surfaceflinger/IGraphicBufferAlloc.h

@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2011 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.
+ */
+
+#ifndef ANDROID_SF_IGRAPHIC_BUFFER_ALLOC_H
+#define ANDROID_SF_IGRAPHIC_BUFFER_ALLOC_H
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include <utils/RefBase.h>
+
+#include <binder/IInterface.h>
+
+namespace android {
+// ----------------------------------------------------------------------------
+
+class IGraphicBufferAlloc : public IInterface
+{
+public:
+    DECLARE_META_INTERFACE(GraphicBufferAlloc);
+
+    /* Create a new GraphicBuffer for the client to use.  The server will
+     * maintain a reference to the newly created GraphicBuffer until
+     * freeAllGraphicBuffers is called.
+     */
+    virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
+            PixelFormat format, uint32_t usage) = 0;
+
+    /* Free all but one of the GraphicBuffer objects that the server is
+     * currently referencing. If bufIndex is not a valid index of the buffers
+     * the server is referencing, then all buffers are freed.
+     */
+    virtual void freeAllGraphicBuffersExcept(int bufIndex) = 0;
+};
+
+// ----------------------------------------------------------------------------
+
+class BnGraphicBufferAlloc : public BnInterface<IGraphicBufferAlloc>
+{
+public:
+    virtual status_t    onTransact( uint32_t code,
+                                    const Parcel& data,
+                                    Parcel* reply,
+                                    uint32_t flags = 0);
+};
+
+// ----------------------------------------------------------------------------
+
+}; // namespace android
+
+#endif // ANDROID_SF_IGRAPHIC_BUFFER_ALLOC_H

include/surfaceflinger/ISurfaceComposer.h

@@ -28,6 +28,7 @@
 #include <ui/PixelFormat.h>
 
 #include <surfaceflinger/ISurfaceComposerClient.h>
+#include <surfaceflinger/IGraphicBufferAlloc.h>
 
 namespace android {
 // ----------------------------------------------------------------------------
@@ -96,6 +97,10 @@ public:
      */
     virtual sp<ISurfaceComposerClient> createClientConnection() = 0;
 
+    /* create a graphic buffer allocator
+     */
+    virtual sp<IGraphicBufferAlloc> createGraphicBufferAlloc() = 0;
+
     /* retrieve the control block */
     virtual sp<IMemoryHeap> getCblk() const = 0;
 
@@ -131,13 +136,6 @@ public:
      * This is an ASYNCHRONOUS call.
      */
     virtual void signal() const = 0;
-
-    /* Create a new GraphicBuffer for the client to use.  SurfaceFlinger will
-     * not maintain a reference to the GraphicBuffer, so the underlying native
-     * handle will be freed once the client references are released.
-     */
-    virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
-            PixelFormat format, uint32_t usage) const = 0;
 };
 
 // ----------------------------------------------------------------------------
@@ -151,7 +149,7 @@ public:
         BOOT_FINISHED = IBinder::FIRST_CALL_TRANSACTION,
         CREATE_CONNECTION,
         CREATE_CLIENT_CONNECTION,
-        CREATE_GRAPHIC_BUFFER,
+        CREATE_GRAPHIC_BUFFER_ALLOC,
         GET_CBLK,
         OPEN_GLOBAL_TRANSACTION,
         CLOSE_GLOBAL_TRANSACTION,

libs/gui/SurfaceTexture.cpp

@@ -29,6 +29,7 @@
 
 #include <surfaceflinger/ISurfaceComposer.h>
 #include <surfaceflinger/SurfaceComposerClient.h>
+#include <surfaceflinger/IGraphicBufferAlloc.h>
 
 #include <utils/Log.h>
 
@@ -83,6 +84,8 @@ SurfaceTexture::SurfaceTexture(GLuint tex) :
         mSlots[i].mEglDisplay = EGL_NO_DISPLAY;
         mSlots[i].mOwnedByClient = false;
     }
+    sp<ISurfaceComposer> composer(ComposerService::getComposerService());
+    mGraphicBufferAlloc = composer->createGraphicBufferAlloc();
 }
 
 SurfaceTexture::~SurfaceTexture() {
@@ -110,9 +113,8 @@ sp<GraphicBuffer> SurfaceTexture::requestBuffer(int buf,
         return 0;
     }
     usage |= GraphicBuffer::USAGE_HW_TEXTURE;
-    sp<ISurfaceComposer> composer(ComposerService::getComposerService());
+    sp<GraphicBuffer> graphicBuffer(
-    sp<GraphicBuffer> graphicBuffer(composer->createGraphicBuffer(w, h,
+            mGraphicBufferAlloc->createGraphicBuffer(w, h, format, usage));
-            format, usage));
     if (graphicBuffer == 0) {
         LOGE("requestBuffer: SurfaceComposer::createGraphicBuffer failed");
     } else {
@@ -122,6 +124,7 @@ sp<GraphicBuffer> SurfaceTexture::requestBuffer(int buf,
             mSlots[buf].mEglImage = EGL_NO_IMAGE_KHR;
             mSlots[buf].mEglDisplay = EGL_NO_DISPLAY;
         }
+        mAllocdBuffers.add(graphicBuffer);
     }
     return graphicBuffer;
 }
@@ -204,27 +207,28 @@ status_t SurfaceTexture::updateTexImage() {
     // Initially both mCurrentTexture and mLastQueued are INVALID_BUFFER_SLOT,
     // so this check will fail until a buffer gets queued.
     if (mCurrentTexture != mLastQueued) {
-        // Update the SurfaceTexture state.
-        mCurrentTexture = mLastQueued;
-        mCurrentCrop = mLastQueuedCrop;
-        mCurrentTransform = mLastQueuedTransform;
-
         // Update the GL texture object.
-        EGLImageKHR image = mSlots[mCurrentTexture].mEglImage;
+        EGLImageKHR image = mSlots[mLastQueued].mEglImage;
         if (image == EGL_NO_IMAGE_KHR) {
             EGLDisplay dpy = eglGetCurrentDisplay();
-            sp<GraphicBuffer> graphicBuffer = mSlots[mCurrentTexture].mGraphicBuffer;
+            sp<GraphicBuffer> graphicBuffer = mSlots[mLastQueued].mGraphicBuffer;
             image = createImage(dpy, graphicBuffer);
-            mSlots[mCurrentTexture].mEglImage = image;
+            mSlots[mLastQueued].mEglImage = image;
-            mSlots[mCurrentTexture].mEglDisplay = dpy;
+            mSlots[mLastQueued].mEglDisplay = dpy;
         }
         glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES, (GLeglImageOES)image);
         GLint error = glGetError();
         if (error != GL_NO_ERROR) {
             LOGE("error binding external texture image %p (slot %d): %#04x",
-                    image, mCurrentTexture, error);
+                    image, mLastQueued, error);
             return -EINVAL;
         }
+
+        // Update the SurfaceTexture state.
+        mCurrentTexture = mLastQueued;
+        mCurrentTextureBuf = mSlots[mCurrentTexture].mGraphicBuffer;
+        mCurrentCrop = mLastQueuedCrop;
+        mCurrentTransform = mLastQueuedTransform;
     }
     return OK;
 }
@@ -282,6 +286,19 @@ void SurfaceTexture::freeAllBuffers() {
             mSlots[i].mEglDisplay = EGL_NO_DISPLAY;
         }
     }
+
+    int exceptBuf = -1;
+    for (size_t i = 0; i < mAllocdBuffers.size(); i++) {
+        if (mAllocdBuffers[i] == mCurrentTextureBuf) {
+            exceptBuf = i;
+            break;
+        }
+    }
+    mAllocdBuffers.clear();
+    if (exceptBuf >= 0) {
+        mAllocdBuffers.add(mCurrentTextureBuf);
+    }
+    mGraphicBufferAlloc->freeAllGraphicBuffersExcept(exceptBuf);
 }
 
 EGLImageKHR SurfaceTexture::createImage(EGLDisplay dpy,

libs/surfaceflinger_client/Android.mk

@@ -5,6 +5,7 @@ LOCAL_SRC_FILES:= \
 	ISurfaceComposer.cpp \
 	ISurface.cpp \
 	ISurfaceComposerClient.cpp \
+	IGraphicBufferAlloc.cpp \
 	LayerState.cpp \
 	SharedBufferStack.cpp \
 	Surface.cpp \

libs/surfaceflinger_client/IGraphicBufferAlloc.cpp

@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2011 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.
+ */
+
+// tag as surfaceflinger
+#define LOG_TAG "SurfaceFlinger"
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include <binder/Parcel.h>
+
+#include <ui/GraphicBuffer.h>
+
+#include <surfaceflinger/IGraphicBufferAlloc.h>
+
+// ---------------------------------------------------------------------------
+
+namespace android {
+
+enum {
+    CREATE_GRAPHIC_BUFFER = IBinder::FIRST_CALL_TRANSACTION,
+    FREE_ALL_GRAPHIC_BUFFERS_EXCEPT,
+};
+
+class BpGraphicBufferAlloc : public BpInterface<IGraphicBufferAlloc>
+{
+public:
+    BpGraphicBufferAlloc(const sp<IBinder>& impl)
+        : BpInterface<IGraphicBufferAlloc>(impl)
+    {
+    }
+
+    virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
+            PixelFormat format, uint32_t usage) {
+        Parcel data, reply;
+        data.writeInterfaceToken(
+                IGraphicBufferAlloc::getInterfaceDescriptor());
+        data.writeInt32(w);
+        data.writeInt32(h);
+        data.writeInt32(format);
+        data.writeInt32(usage);
+        remote()->transact(CREATE_GRAPHIC_BUFFER, data, &reply);
+        sp<GraphicBuffer> graphicBuffer;
+        bool nonNull = (bool)reply.readInt32();
+        if (nonNull) {
+            graphicBuffer = new GraphicBuffer();
+            reply.read(*graphicBuffer);
+        }
+        return graphicBuffer;
+    }
+
+    virtual void freeAllGraphicBuffersExcept(int bufIdx) {
+        Parcel data, reply;
+        data.writeInterfaceToken(
+                IGraphicBufferAlloc::getInterfaceDescriptor());
+        data.writeInt32(bufIdx);
+        remote()->transact(FREE_ALL_GRAPHIC_BUFFERS_EXCEPT, data, &reply);
+    }
+};
+
+IMPLEMENT_META_INTERFACE(GraphicBufferAlloc, "android.ui.IGraphicBufferAlloc");
+
+// ----------------------------------------------------------------------
+
+status_t BnGraphicBufferAlloc::onTransact(
+    uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
+{
+    // codes that don't require permission check
+
+    switch(code) {
+        case CREATE_GRAPHIC_BUFFER: {
+            CHECK_INTERFACE(IGraphicBufferAlloc, data, reply);
+            uint32_t w = data.readInt32();
+            uint32_t h = data.readInt32();
+            PixelFormat format = data.readInt32();
+            uint32_t usage = data.readInt32();
+            sp<GraphicBuffer> result(createGraphicBuffer(w, h, format, usage));
+            reply->writeInt32(result != 0);
+            if (result != 0) {
+                reply->write(*result);
+            }
+            return NO_ERROR;
+        } break;
+        case FREE_ALL_GRAPHIC_BUFFERS_EXCEPT: {
+            CHECK_INTERFACE(IGraphicBufferAlloc, data, reply);
+            int bufIdx = data.readInt32();
+            freeAllGraphicBuffersExcept(bufIdx);
+            return NO_ERROR;
+        } break;
+        default:
+            return BBinder::onTransact(code, data, reply, flags);
+    }
+}
+
+}; // namespace android

libs/surfaceflinger_client/ISurfaceComposer.cpp

@@ -26,7 +26,6 @@
 #include <binder/IServiceManager.h>
 
 #include <ui/DisplayInfo.h>
-#include <ui/GraphicBuffer.h>
 
 #include <surfaceflinger/ISurfaceComposer.h>
 
@@ -65,6 +64,15 @@ public:
         return interface_cast<ISurfaceComposerClient>(reply.readStrongBinder());
     }
 
+    virtual sp<IGraphicBufferAlloc> createGraphicBufferAlloc()
+    {
+        uint32_t n;
+        Parcel data, reply;
+        data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
+        remote()->transact(BnSurfaceComposer::CREATE_GRAPHIC_BUFFER_ALLOC, data, &reply);
+        return interface_cast<IGraphicBufferAlloc>(reply.readStrongBinder());
+    }
+
     virtual sp<IMemoryHeap> getCblk() const
     {
         Parcel data, reply;
@@ -170,25 +178,6 @@ public:
         data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
         remote()->transact(BnSurfaceComposer::SIGNAL, data, &reply, IBinder::FLAG_ONEWAY);
     }
-
-    virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
-            PixelFormat format, uint32_t usage) const {
-        Parcel data, reply;
-        data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
-        data.writeInt32(w);
-        data.writeInt32(h);
-        data.writeInt32(format);
-        data.writeInt32(usage);
-        remote()->transact(BnSurfaceComposer::CREATE_GRAPHIC_BUFFER, data,
-                &reply);
-        sp<GraphicBuffer> graphicBuffer;
-        bool nonNull = (bool)reply.readInt32();
-        if (nonNull) {
-            graphicBuffer = new GraphicBuffer();
-            reply.read(*graphicBuffer);
-        }
-        return graphicBuffer;
-    }
 };
 
 IMPLEMENT_META_INTERFACE(SurfaceComposer, "android.ui.ISurfaceComposer");
@@ -209,6 +198,11 @@ status_t BnSurfaceComposer::onTransact(
             sp<IBinder> b = createClientConnection()->asBinder();
             reply->writeStrongBinder(b);
         } break;
+        case CREATE_GRAPHIC_BUFFER_ALLOC: {
+            CHECK_INTERFACE(ISurfaceComposer, data, reply);
+            sp<IBinder> b = createGraphicBufferAlloc()->asBinder();
+            reply->writeStrongBinder(b);
+        } break;
         case OPEN_GLOBAL_TRANSACTION: {
             CHECK_INTERFACE(ISurfaceComposer, data, reply);
             openGlobalTransaction();
@@ -267,18 +261,6 @@ status_t BnSurfaceComposer::onTransact(
             reply->writeInt32(f);
             reply->writeInt32(res);
         } break;
-        case CREATE_GRAPHIC_BUFFER: {
-            CHECK_INTERFACE(ISurfaceComposer, data, reply);
-            uint32_t w = data.readInt32();
-            uint32_t h = data.readInt32();
-            PixelFormat format = data.readInt32();
-            uint32_t usage = data.readInt32();
-            sp<GraphicBuffer> result(createGraphicBuffer(w, h, format, usage));
-            reply->writeInt32(result != 0);
-            if (result != 0) {
-                reply->write(*result);
-            }
-        } break;
         case TURN_ELECTRON_BEAM_OFF: {
             CHECK_INTERFACE(ISurfaceComposer, data, reply);
             int32_t mode = data.readInt32();

services/surfaceflinger/SurfaceFlinger.cpp

@@ -144,6 +144,11 @@ sp<ISurfaceComposerClient> SurfaceFlinger::createClientConnection()
     return bclient;
 }
 
+sp<IGraphicBufferAlloc> SurfaceFlinger::createGraphicBufferAlloc()
+{
+    sp<GraphicBufferAlloc> gba(new GraphicBufferAlloc());
+    return gba;
+}
 
 const GraphicPlane& SurfaceFlinger::graphicPlane(int dpy) const
 {
@@ -2267,25 +2272,6 @@ sp<Layer> SurfaceFlinger::getLayer(const sp<ISurface>& sur) const
 
 // ---------------------------------------------------------------------------
 
-sp<GraphicBuffer> SurfaceFlinger::createGraphicBuffer(uint32_t w, uint32_t h,
-        PixelFormat format, uint32_t usage) const {
-    // XXX: HACK HACK HACK!!!  This should NOT be static, but it is to fix a
-    // race between SurfaceFlinger unref'ing the buffer and the client ref'ing
-    // it.
-    static sp<GraphicBuffer> graphicBuffer(new GraphicBuffer(w, h, format, usage));
-    status_t err = graphicBuffer->initCheck();
-    if (err != 0) {
-        LOGE("createGraphicBuffer: init check failed: %d", err);
-        return 0;
-    } else if (graphicBuffer->handle == 0) {
-        LOGE("createGraphicBuffer: unable to create GraphicBuffer");
-        return 0;
-    }
-    return graphicBuffer;
-}
-
-// ---------------------------------------------------------------------------
-
 Client::Client(const sp<SurfaceFlinger>& flinger)
     : mFlinger(flinger), mNameGenerator(1)
 {
@@ -2465,6 +2451,39 @@ status_t UserClient::setState(int32_t count, const layer_state_t* states) {
 
 // ---------------------------------------------------------------------------
 
+GraphicBufferAlloc::GraphicBufferAlloc() {}
+
+GraphicBufferAlloc::~GraphicBufferAlloc() {}
+
+sp<GraphicBuffer> GraphicBufferAlloc::createGraphicBuffer(uint32_t w, uint32_t h,
+        PixelFormat format, uint32_t usage) {
+    sp<GraphicBuffer> graphicBuffer(new GraphicBuffer(w, h, format, usage));
+    status_t err = graphicBuffer->initCheck();
+    if (err != 0) {
+        LOGE("createGraphicBuffer: init check failed: %d", err);
+        return 0;
+    } else if (graphicBuffer->handle == 0) {
+        LOGE("createGraphicBuffer: unable to create GraphicBuffer");
+        return 0;
+    }
+    Mutex::Autolock _l(mLock);
+    mBuffers.add(graphicBuffer);
+    return graphicBuffer;
+}
+
+void GraphicBufferAlloc::freeAllGraphicBuffersExcept(int bufIdx) {
+    Mutex::Autolock _l(mLock);
+    if (0 <= bufIdx && bufIdx < mBuffers.size()) {
+        sp<GraphicBuffer> b(mBuffers[bufIdx]);
+        mBuffers.clear();
+        mBuffers.add(b);
+    } else {
+        mBuffers.clear();
+    }
+}
+
+// ---------------------------------------------------------------------------
+
 GraphicPlane::GraphicPlane()
     : mHw(0)
 {

services/surfaceflinger/SurfaceFlinger.h

@@ -34,6 +34,7 @@
 #include <ui/PixelFormat.h>
 #include <surfaceflinger/ISurfaceComposer.h>
 #include <surfaceflinger/ISurfaceComposerClient.h>
+#include <surfaceflinger/IGraphicBufferAlloc.h>
 
 #include "Barrier.h"
 #include "Layer.h"
@@ -119,6 +120,21 @@ private:
     sp<SurfaceFlinger> mFlinger;
 };
 
+class GraphicBufferAlloc : public BnGraphicBufferAlloc
+{
+public:
+    GraphicBufferAlloc();
+    virtual ~GraphicBufferAlloc();
+
+    virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
+        PixelFormat format, uint32_t usage);
+    virtual void freeAllGraphicBuffersExcept(int bufIdx);
+
+private:
+    Vector<sp<GraphicBuffer> > mBuffers;
+    Mutex mLock;
+};
+
 // ---------------------------------------------------------------------------
 
 class GraphicPlane
@@ -184,6 +200,7 @@ public:
     // ISurfaceComposer interface
     virtual sp<ISurfaceComposerClient>  createConnection();
     virtual sp<ISurfaceComposerClient>  createClientConnection();
+    virtual sp<IGraphicBufferAlloc>     createGraphicBufferAlloc();
     virtual sp<IMemoryHeap>             getCblk() const;
     virtual void                        bootFinished();
     virtual void                        openGlobalTransaction();
@@ -322,8 +339,6 @@ private:
             status_t electronBeamOnAnimationImplLocked();
             status_t renderScreenToTextureLocked(DisplayID dpy,
                     GLuint* textureName, GLfloat* uOut, GLfloat* vOut);
-            sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
-                    PixelFormat format, uint32_t usage) const;
 
             friend class FreezeLock;
             sp<FreezeLock> getFreezeLock() const;