Add support for enqueuing buffers in arbitrary order

Also added a very simple SharedBufferStack unit test.

Change-Id: I253dbbe98a53c966b78d22d4d6dd59f8aefc8c40

5 files changed, 293 insertions, 22 deletions

diff --git a/include/private/surfaceflinger/SharedBufferStack.h b/include/private/surfaceflinger/SharedBufferStack.h
index dc53d6a..2504d39 100644
--- a/include/private/surfaceflinger/SharedBufferStack.h
+++ b/include/private/surfaceflinger/SharedBufferStack.h
@@ -118,9 +118,10 @@ public:
 
     // not part of the conditions
     volatile int32_t reallocMask;
+    volatile int8_t index[NUM_BUFFER_MAX];
 
     int32_t     identity;       // surface's identity (const)
-    int32_t     reserved32[6];
+    int32_t     reserved32[2];
     Statistics  stats;
     int32_t     reserved;
     BufferData  buffers[NUM_BUFFER_MAX];     // 960 bytes
@@ -249,6 +250,7 @@ private:
 
     int32_t tail;
     int32_t undoDequeueTail;
+    int32_t queued_head;
     // statistics...
     nsecs_t mDequeueTime[NUM_BUFFER_MAX];
 };
diff --git a/libs/surfaceflinger_client/SharedBufferStack.cpp b/libs/surfaceflinger_client/SharedBufferStack.cpp
index 21a40db..cea8bdc 100644
--- a/libs/surfaceflinger_client/SharedBufferStack.cpp
+++ b/libs/surfaceflinger_client/SharedBufferStack.cpp
@@ -246,7 +246,7 @@ SharedBufferClient::LockCondition::LockCondition(
         SharedBufferClient* sbc, int buf) : ConditionBase(sbc), buf(buf) { 
 }
 bool SharedBufferClient::LockCondition::operator()() const {
-    return (buf != stack.head || 
+    return (buf != stack.index[stack.head] ||
             (stack.queued > 0 && stack.inUse != buf));
 }
 
@@ -255,7 +255,7 @@ SharedBufferServer::ReallocateCondition::ReallocateCondition(
 }
 bool SharedBufferServer::ReallocateCondition::operator()() const {
     // TODO: we should also check that buf has been dequeued
-    return (buf != stack.head);
+    return (buf != stack.index[stack.head]);
 }
 
 // ----------------------------------------------------------------------------
@@ -299,7 +299,7 @@ ssize_t SharedBufferServer::RetireUpdate::operator()() {
     int32_t head = stack.head;
 
     // Preventively lock the current buffer before updating queued.
-    android_atomic_write(head, &stack.inUse);
+    android_atomic_write(stack.index[head], &stack.inUse);
 
     // Decrement the number of queued buffers 
     int32_t queued;
@@ -315,7 +315,7 @@ ssize_t SharedBufferServer::RetireUpdate::operator()() {
 
     // lock the buffer before advancing head, which automatically unlocks
     // the buffer we preventively locked upon entering this function
-    android_atomic_write(head, &stack.inUse);
+    android_atomic_write(stack.index[head], &stack.inUse);
 
     // advance head
     android_atomic_write(head, &stack.head);
@@ -342,7 +342,9 @@ SharedBufferClient::SharedBufferClient(SharedClient* sharedClient,
     : SharedBufferBase(sharedClient, surface, num, identity),
       tail(0), undoDequeueTail(0)
 {
+    SharedBufferStack& stack( *mSharedStack );
     tail = computeTail();
+    queued_head = stack.head;
 }
 
 ssize_t SharedBufferClient::dequeue()
@@ -370,10 +372,10 @@ ssize_t SharedBufferClient::dequeue()
         LOGW("dequeue probably called from multiple threads!");
     }
 
-    int dequeued = tail;
+    undoDequeueTail = tail;
+    int dequeued = stack.index[tail];
     tail = ((tail+1 >= mNumBuffers) ? 0 : tail+1);
-    undoDequeueTail = dequeued;
-    LOGD_IF(DEBUG_ATOMICS, "dequeued=%d, tail=%d, %s",
+    LOGD_IF(DEBUG_ATOMICS, "dequeued=%d, tail++=%d, %s",
             dequeued, tail, dump("").string());
 
     mDequeueTime[dequeued] = dequeueTime; 
@@ -383,6 +385,8 @@ ssize_t SharedBufferClient::dequeue()
 
 status_t SharedBufferClient::undoDequeue(int buf)
 {
+    // TODO: we can only undo the previous dequeue, we should
+    // enforce that in the api
     UndoDequeueUpdate update(this);
     status_t err = updateCondition( update );
     if (err == NO_ERROR) {
@@ -393,6 +397,7 @@ status_t SharedBufferClient::undoDequeue(int buf)
 
 status_t SharedBufferClient::lock(int buf)
 {
+    SharedBufferStack& stack( *mSharedStack );
     LockCondition condition(this, buf);
     status_t err = waitForCondition(condition);
     return err;
@@ -400,32 +405,37 @@ status_t SharedBufferClient::lock(int buf)
 
 status_t SharedBufferClient::queue(int buf)
 {
+    SharedBufferStack& stack( *mSharedStack );
+
+    queued_head = ((queued_head+1 >= mNumBuffers) ? 0 : queued_head+1);
+    stack.index[queued_head] = buf;
+
     QueueUpdate update(this);
     status_t err = updateCondition( update );
     LOGD_IF(DEBUG_ATOMICS, "queued=%d, %s", buf, dump("").string());
-    SharedBufferStack& stack( *mSharedStack );
+
     const nsecs_t now = systemTime(SYSTEM_TIME_THREAD);
     stack.stats.totalTime = ns2us(now - mDequeueTime[buf]);
     return err;
 }
 
-bool SharedBufferClient::needNewBuffer(int buffer) const
+bool SharedBufferClient::needNewBuffer(int buf) const
 {
     SharedBufferStack& stack( *mSharedStack );
-    const uint32_t mask = 1<<buffer;
+    const uint32_t mask = 1<<buf;
     return (android_atomic_and(~mask, &stack.reallocMask) & mask) != 0;
 }
 
-status_t SharedBufferClient::setCrop(int buffer, const Rect& crop)
+status_t SharedBufferClient::setCrop(int buf, const Rect& crop)
 {
     SharedBufferStack& stack( *mSharedStack );
-    return stack.setCrop(buffer, crop);
+    return stack.setCrop(buf, crop);
 }
 
-status_t SharedBufferClient::setDirtyRegion(int buffer, const Region& reg)
+status_t SharedBufferClient::setDirtyRegion(int buf, const Region& reg)
 {
     SharedBufferStack& stack( *mSharedStack );
-    return stack.setDirtyRegion(buffer, reg);
+    return stack.setDirtyRegion(buf, reg);
 }
 
 // ----------------------------------------------------------------------------
@@ -440,19 +450,27 @@ SharedBufferServer::SharedBufferServer(SharedClient* sharedClient,
     mSharedStack->queued = 0;
     mSharedStack->reallocMask = 0;
     memset(mSharedStack->buffers, 0, sizeof(mSharedStack->buffers));
+    for (int i=0 ; i<num ; i++) {
+        mSharedStack->index[i] = i;
+    }
 }
 
 ssize_t SharedBufferServer::retireAndLock()
 {
     RetireUpdate update(this, mNumBuffers);
     ssize_t buf = updateCondition( update );
-    LOGD_IF(DEBUG_ATOMICS && buf>=0, "retire=%d, %s", int(buf), dump("").string());
+    if (buf >= 0) {
+        SharedBufferStack& stack( *mSharedStack );
+        buf = stack.index[buf];
+        LOGD_IF(DEBUG_ATOMICS && buf>=0, "retire=%d, %s",
+                int(buf), dump("").string());
+    }
     return buf;
 }
 
-status_t SharedBufferServer::unlock(int buffer)
+status_t SharedBufferServer::unlock(int buf)
 {
-    UnlockUpdate update(this, buffer);
+    UnlockUpdate update(this, buf);
     status_t err = updateCondition( update );
     return err;
 }
@@ -479,17 +497,17 @@ int32_t SharedBufferServer::getQueuedCount() const
     return stack.queued;
 }
 
-status_t SharedBufferServer::assertReallocate(int buffer)
+status_t SharedBufferServer::assertReallocate(int buf)
 {
-    ReallocateCondition condition(this, buffer);
+    ReallocateCondition condition(this, buf);
     status_t err = waitForCondition(condition);
     return err;
 }
 
-Region SharedBufferServer::getDirtyRegion(int buffer) const
+Region SharedBufferServer::getDirtyRegion(int buf) const
 {
     SharedBufferStack& stack( *mSharedStack );
-    return stack.getDirtyRegion(buffer);
+    return stack.getDirtyRegion(buf);
 }
 
 SharedBufferStack::Statistics SharedBufferServer::getStats() const
diff --git a/libs/surfaceflinger_client/tests/Android.mk b/libs/surfaceflinger_client/tests/Android.mk
new file mode 100644
index 0000000..5053e7d
--- /dev/null
+++ b/libs/surfaceflinger_client/tests/Android.mk
@@ -0,0 +1 @@
+include $(call all-subdir-makefiles)
diff --git a/libs/surfaceflinger_client/tests/SharedBufferStack/Android.mk b/libs/surfaceflinger_client/tests/SharedBufferStack/Android.mk
new file mode 100644
index 0000000..d3dfe04
--- /dev/null
+++ b/libs/surfaceflinger_client/tests/SharedBufferStack/Android.mk
@@ -0,0 +1,17 @@
+LOCAL_PATH:= $(call my-dir)
+include $(CLEAR_VARS)
+
+LOCAL_SRC_FILES:= \
+	SharedBufferStackTest.cpp
+
+LOCAL_SHARED_LIBRARIES := \
+	libcutils \
+	libutils \
+    libui \
+    libsurfaceflinger_client
+
+LOCAL_MODULE:= test-sharedbufferstack
+
+LOCAL_MODULE_TAGS := tests
+
+include $(BUILD_EXECUTABLE)
diff --git a/libs/surfaceflinger_client/tests/SharedBufferStack/SharedBufferStackTest.cpp b/libs/surfaceflinger_client/tests/SharedBufferStack/SharedBufferStackTest.cpp
new file mode 100644
index 0000000..6732580
--- /dev/null
+++ b/libs/surfaceflinger_client/tests/SharedBufferStack/SharedBufferStackTest.cpp
@@ -0,0 +1,233 @@
+/*
+ * Copyright (C) 2007 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.
+ */
+
+#undef NDEBUG
+
+#include <assert.h>
+#include <cutils/memory.h>
+#include <cutils/log.h>
+#include <utils/Errors.h>
+#include <private/surfaceflinger/SharedBufferStack.h>
+
+using namespace android;
+
+
+void log(const char* prefix, int *b, size_t num)
+{
+    printf("%s: ", prefix);
+    for (size_t i=0 ; i<num ; i++) {
+        printf("%d ", b[i]);
+    }
+    printf("\n");
+}
+
+int main(int argc, char** argv)
+{
+    status_t err;
+    const size_t num = 4;
+    SharedClient client;
+    SharedBufferServer s(&client, 0, num, 0);
+    SharedBufferClient c(&client, 0, num, 0);
+    int b[num], u[num], r[num];
+
+    for (size_t i=0 ; i<num ; i++) {
+        b[i] = c.dequeue();
+        assert(b[i]==i);
+    }
+    log("DQ", b, num);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.lock(b[i]);
+        assert(err==0);
+    }
+    log("LK", b, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.queue(b[i]);
+        assert(err==0);
+    }
+    log(" Q", b, num-1);
+
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        r[i] = s.retireAndLock();
+        assert(r[i]==i);
+        err = s.unlock(r[i]);
+        assert(err == 0);
+    }
+    log("RT", r, num-1);
+
+    err = c.lock(b[num-1]);
+    assert(err == 0);
+    log("LK", b+num-1, 1);
+
+    err = c.queue(b[num-1]);
+    assert(err == 0);
+    log(" Q", b+num-1, 1);
+
+    r[num-1] = s.retireAndLock();
+    assert(r[num-1]==num-1);
+    err = s.unlock(r[num-1]);
+    assert(err == 0);
+    log("RT", r+num-1, 1);
+
+    // ------------------------------------
+    printf("\n");
+
+    for (size_t i=0 ; i<num ; i++) {
+        b[i] = c.dequeue();
+        assert(b[i]==i);
+    }
+    log("DQ", b, num);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.lock(b[i]);
+        assert(err==0);
+    }
+    log("LK", b, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        u[i] = b[num-2-i];
+    }
+    u[num-1] = num-1;
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.queue(u[i]);
+        assert(err==0);
+    }
+    log(" Q", u, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        r[i] = s.retireAndLock();
+        assert(r[i]==u[i]);
+        err = s.unlock(r[i]);
+        assert(err == 0);
+    }
+    log("RT", r, num-1);
+
+    err = c.lock(b[num-1]);
+    assert(err == 0);
+    log("LK", b+num-1, 1);
+
+    err = c.queue(b[num-1]);
+    assert(err == 0);
+    log(" Q", b+num-1, 1);
+
+    r[num-1] = s.retireAndLock();
+    assert(r[num-1]==num-1);
+    err = s.unlock(r[num-1]);
+    assert(err == 0);
+    log("RT", r+num-1, 1);
+
+    // ------------------------------------
+    printf("\n");
+
+    for (size_t i=0 ; i<num ; i++) {
+        b[i] = c.dequeue();
+        assert(b[i]==u[i]);
+    }
+    log("DQ", b, num);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.lock(b[i]);
+        assert(err==0);
+    }
+    log("LK", b, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.queue(b[i]);
+        assert(err==0);
+    }
+    log(" Q", b, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        r[i] = s.retireAndLock();
+        assert(r[i]==u[i]);
+        err = s.unlock(r[i]);
+        assert(err == 0);
+    }
+    log("RT", r, num-1);
+
+    err = c.lock(u[num-1]);
+    assert(err == 0);
+    log("LK", u+num-1, 1);
+
+    err = c.queue(u[num-1]);
+    assert(err == 0);
+    log(" Q", u+num-1, 1);
+
+    r[num-1] = s.retireAndLock();
+    assert(r[num-1]==num-1);
+    err = s.unlock(r[num-1]);
+    assert(err == 0);
+    log("RT", r+num-1, 1);
+
+    // ------------------------------------
+    printf("\n");
+
+    b[0] = c.dequeue();
+    assert(b[0]==u[0]);
+    log("DQ", b, 1);
+
+    c.undoDequeue(b[0]);
+    assert(err == 0);
+    log("UDQ", b, 1);
+
+    // ------------------------------------
+    printf("\n");
+
+    for (size_t i=0 ; i<num ; i++) {
+        b[i] = c.dequeue();
+        assert(b[i]==u[i]);
+    }
+    log("DQ", b, num);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.lock(b[i]);
+        assert(err==0);
+    }
+    log("LK", b, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        err = c.queue(b[i]);
+        assert(err==0);
+    }
+    log(" Q", b, num-1);
+
+    for (size_t i=0 ; i<num-1 ; i++) {
+        r[i] = s.retireAndLock();
+        assert(r[i]==u[i]);
+        err = s.unlock(r[i]);
+        assert(err == 0);
+    }
+    log("RT", r, num-1);
+
+    err = c.lock(u[num-1]);
+    assert(err == 0);
+    log("LK", u+num-1, 1);
+
+    err = c.queue(u[num-1]);
+    assert(err == 0);
+    log(" Q", u+num-1, 1);
+
+    r[num-1] = s.retireAndLock();
+    assert(r[num-1]==num-1);
+    err = s.unlock(r[num-1]);
+    assert(err == 0);
+    log("RT", r+num-1, 1);
+
+    return 0;
+}