libgui: Add generation numbers to BufferQueue
This change allows producers to set a generation number on a
BufferQueue. This number will be embedded in any new GraphicBuffers
created in that BufferQueue, and attempts to attach buffers which have
a different generation number will fail.
It also plumbs the setGenerationNumber method through Surface, with the
additional effect that any buffers attached to the Surface after
setting a new generation number will automatically be updated with the
new number (as opposed to failing, as would happen on through IGBP).
Bug: 20923096
Change-Id: I32bf726b035f99c3e5834beaf76afb9f01adcbc2
(cherry picked from commit 812ed0644f8f8f71ca403f4e5793f0dbc1fcf9b2)
diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h
index d7686ec..99134ea 100644
--- a/include/gui/BufferQueueCore.h
+++ b/include/gui/BufferQueueCore.h
@@ -275,6 +275,11 @@
// buffer as the number of frames that have elapsed since it was last queued
uint64_t mBufferAge;
+ // mGenerationNumber stores the current generation number of the attached
+ // producer. Any attempt to attach a buffer with a different generation
+ // number will fail.
+ uint32_t mGenerationNumber;
+
}; // class BufferQueueCore
} // namespace android
diff --git a/include/gui/BufferQueueProducer.h b/include/gui/BufferQueueProducer.h
index ed660fb..afa7eb1 100644
--- a/include/gui/BufferQueueProducer.h
+++ b/include/gui/BufferQueueProducer.h
@@ -175,6 +175,9 @@
// See IGraphicBufferProducer::allowAllocation
virtual status_t allowAllocation(bool allow);
+ // See IGraphicBufferProducer::setGenerationNumber
+ virtual status_t setGenerationNumber(uint32_t generationNumber);
+
private:
// This is required by the IBinder::DeathRecipient interface
virtual void binderDied(const wp<IBinder>& who);
diff --git a/include/gui/IGraphicBufferConsumer.h b/include/gui/IGraphicBufferConsumer.h
index 6363a3a..60ec9cc 100644
--- a/include/gui/IGraphicBufferConsumer.h
+++ b/include/gui/IGraphicBufferConsumer.h
@@ -110,7 +110,8 @@
// will be deallocated as stale.
//
// Return of a value other than NO_ERROR means an error has occurred:
- // * BAD_VALUE - outSlot or buffer were NULL
+ // * BAD_VALUE - outSlot or buffer were NULL, or the generation number of
+ // the buffer did not match the buffer queue.
// * INVALID_OPERATION - cannot attach the buffer because it would cause too
// many buffers to be acquired.
// * NO_MEMORY - no free slots available
diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h
index 5c50b2b..4ca4cd5 100644
--- a/include/gui/IGraphicBufferProducer.h
+++ b/include/gui/IGraphicBufferProducer.h
@@ -218,8 +218,9 @@
//
// Return of a negative value means an error has occurred:
// * NO_INIT - the buffer queue has been abandoned.
- // * BAD_VALUE - outSlot or buffer were NULL or invalid combination of
- // async mode and buffer count override.
+ // * BAD_VALUE - outSlot or buffer were NULL, invalid combination of
+ // async mode and buffer count override, or the generation
+ // number of the buffer did not match the buffer queue.
// * INVALID_OPERATION - cannot attach the buffer because it would cause
// too many buffers to be dequeued, either because
// the producer already has a single buffer dequeued
@@ -470,6 +471,15 @@
// eligible slot is available, dequeueBuffer will block or return an error
// as usual.
virtual status_t allowAllocation(bool allow) = 0;
+
+ // Sets the current generation number of the BufferQueue.
+ //
+ // This generation number will be inserted into any buffers allocated by the
+ // BufferQueue, and any attempts to attach a buffer with a different
+ // generation number will fail. Buffers already in the queue are not
+ // affected and will retain their current generation number. The generation
+ // number defaults to 0.
+ virtual status_t setGenerationNumber(uint32_t generationNumber) = 0;
};
// ----------------------------------------------------------------------------
diff --git a/include/gui/Surface.h b/include/gui/Surface.h
index fd6d48c..261b07c 100644
--- a/include/gui/Surface.h
+++ b/include/gui/Surface.h
@@ -101,6 +101,11 @@
*/
void allocateBuffers();
+ /* Sets the generation number on the IGraphicBufferProducer and updates the
+ * generation number on any buffers attached to the Surface after this call.
+ * See IGBP::setGenerationNumber for more information. */
+ status_t setGenerationNumber(uint32_t generationNumber);
+
protected:
virtual ~Surface();
@@ -305,6 +310,10 @@
// When a non-CPU producer is attached, this reflects the surface damage
// (the change since the previous frame) passed in by the producer.
Region mDirtyRegion;
+
+ // Stores the current generation number. See setGenerationNumber and
+ // IGraphicBufferProducer::setGenerationNumber for more information.
+ uint32_t mGenerationNumber;
};
}; // namespace android
diff --git a/include/ui/GraphicBuffer.h b/include/ui/GraphicBuffer.h
index f91d192..3da720f 100644
--- a/include/ui/GraphicBuffer.h
+++ b/include/ui/GraphicBuffer.h
@@ -94,6 +94,11 @@
Rect getBounds() const { return Rect(width, height); }
uint64_t getId() const { return mId; }
+ uint32_t getGenerationNumber() const { return mGenerationNumber; }
+ void setGenerationNumber(uint32_t generation) {
+ mGenerationNumber = generation;
+ }
+
status_t reallocate(uint32_t inWidth, uint32_t inHeight,
PixelFormat inFormat, uint32_t inUsage);
@@ -166,6 +171,11 @@
sp<ANativeWindowBuffer> mWrappedBuffer;
uint64_t mId;
+
+ // Stores the generation number of this buffer. If this number does not
+ // match the BufferQueue's internal generation number (set through
+ // IGBP::setGenerationNumber), attempts to attach the buffer will fail.
+ uint32_t mGenerationNumber;
};
}; // namespace android
diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index 4174676..ae796b1 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/libs/gui/BufferQueueConsumer.cpp
@@ -248,6 +248,13 @@
return INVALID_OPERATION;
}
+ if (buffer->getGenerationNumber() != mCore->mGenerationNumber) {
+ BQ_LOGE("attachBuffer: generation number mismatch [buffer %u] "
+ "[queue %u]", buffer->getGenerationNumber(),
+ mCore->mGenerationNumber);
+ return BAD_VALUE;
+ }
+
// Find a free slot to put the buffer into
int found = BufferQueueCore::INVALID_BUFFER_SLOT;
if (!mCore->mFreeSlots.empty()) {
diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index e784644..851a396 100644
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -71,7 +71,8 @@
mIsAllocating(false),
mIsAllocatingCondition(),
mAllowAllocation(true),
- mBufferAge(0)
+ mBufferAge(0),
+ mGenerationNumber(0)
{
if (allocator == NULL) {
sp<ISurfaceComposer> composer(ComposerService::getComposerService());
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 2cf7433..42e7d3e 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -383,6 +383,7 @@
return NO_INIT;
}
+ graphicBuffer->setGenerationNumber(mCore->mGenerationNumber);
mSlots[*outSlot].mGraphicBuffer = graphicBuffer;
} // Autolock scope
}
@@ -498,6 +499,13 @@
Mutex::Autolock lock(mCore->mMutex);
mCore->waitWhileAllocatingLocked();
+ if (buffer->getGenerationNumber() != mCore->mGenerationNumber) {
+ BQ_LOGE("attachBuffer: generation number mismatch [buffer %u] "
+ "[queue %u]", buffer->getGenerationNumber(),
+ mCore->mGenerationNumber);
+ return BAD_VALUE;
+ }
+
status_t returnFlags = NO_ERROR;
int found;
// TODO: Should we provide an async flag to attachBuffer? It seems
@@ -1072,6 +1080,15 @@
return NO_ERROR;
}
+status_t BufferQueueProducer::setGenerationNumber(uint32_t generationNumber) {
+ ATRACE_CALL();
+ BQ_LOGV("setGenerationNumber: %u", generationNumber);
+
+ Mutex::Autolock lock(mCore->mMutex);
+ mCore->mGenerationNumber = generationNumber;
+ return NO_ERROR;
+}
+
void BufferQueueProducer::binderDied(const wp<android::IBinder>& /* who */) {
// If we're here, it means that a producer we were connected to died.
// We're guaranteed that we are still connected to it because we remove
diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp
index 7093ffa..cfe726b 100644
--- a/libs/gui/IGraphicBufferProducer.cpp
+++ b/libs/gui/IGraphicBufferProducer.cpp
@@ -47,6 +47,7 @@
SET_SIDEBAND_STREAM,
ALLOCATE_BUFFERS,
ALLOW_ALLOCATION,
+ SET_GENERATION_NUMBER,
};
class BpGraphicBufferProducer : public BpInterface<IGraphicBufferProducer>
@@ -284,6 +285,17 @@
result = reply.readInt32();
return result;
}
+
+ virtual status_t setGenerationNumber(uint32_t generationNumber) {
+ Parcel data, reply;
+ data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor());
+ data.writeUint32(generationNumber);
+ status_t result = remote()->transact(SET_GENERATION_NUMBER, data, &reply);
+ if (result == NO_ERROR) {
+ result = reply.readInt32();
+ }
+ return result;
+ }
};
// Out-of-line virtual method definition to trigger vtable emission in this
@@ -448,6 +460,13 @@
reply->writeInt32(result);
return NO_ERROR;
}
+ case SET_GENERATION_NUMBER: {
+ CHECK_INTERFACE(IGraphicBufferProducer, data, reply);
+ uint32_t generationNumber = data.readUint32();
+ status_t result = setGenerationNumber(generationNumber);
+ reply->writeInt32(result);
+ return NO_ERROR;
+ }
}
return BBinder::onTransact(code, data, reply, flags);
}
diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index 04ac0f4..aeb56e0 100644
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -42,7 +42,8 @@
Surface::Surface(
const sp<IGraphicBufferProducer>& bufferProducer,
bool controlledByApp)
- : mGraphicBufferProducer(bufferProducer)
+ : mGraphicBufferProducer(bufferProducer),
+ mGenerationNumber(0)
{
// Initialize the ANativeWindow function pointers.
ANativeWindow::setSwapInterval = hook_setSwapInterval;
@@ -102,6 +103,14 @@
reqHeight, mReqFormat, mReqUsage);
}
+status_t Surface::setGenerationNumber(uint32_t generation) {
+ status_t result = mGraphicBufferProducer->setGenerationNumber(generation);
+ if (result == NO_ERROR) {
+ mGenerationNumber = generation;
+ }
+ return result;
+}
+
int Surface::hook_setSwapInterval(ANativeWindow* window, int interval) {
Surface* c = getSelf(window);
return c->setSwapInterval(interval);
@@ -698,11 +707,14 @@
Mutex::Autolock lock(mMutex);
sp<GraphicBuffer> graphicBuffer(static_cast<GraphicBuffer*>(buffer));
+ uint32_t priorGeneration = graphicBuffer->mGenerationNumber;
+ graphicBuffer->mGenerationNumber = mGenerationNumber;
int32_t attachedSlot = -1;
status_t result = mGraphicBufferProducer->attachBuffer(
&attachedSlot, graphicBuffer);
if (result != NO_ERROR) {
ALOGE("attachBuffer: IGraphicBufferProducer call failed (%d)", result);
+ graphicBuffer->mGenerationNumber = priorGeneration;
return result;
}
mSlots[attachedSlot].buffer = graphicBuffer;
diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index 1584fef..3d1139d 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -402,4 +402,46 @@
WIDTH * 2, HEIGHT * 2, 0, GRALLOC_USAGE_SW_WRITE_OFTEN));
}
+TEST_F(BufferQueueTest, TestGenerationNumbers) {
+ createBufferQueue();
+ sp<DummyConsumer> dc(new DummyConsumer);
+ ASSERT_EQ(OK, mConsumer->consumerConnect(dc, true));
+ IGraphicBufferProducer::QueueBufferOutput output;
+ ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener,
+ NATIVE_WINDOW_API_CPU, true, &output));
+
+ ASSERT_EQ(OK, mProducer->setGenerationNumber(1));
+
+ // Get one buffer to play with
+ int slot;
+ sp<Fence> fence;
+ ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+ mProducer->dequeueBuffer(&slot, &fence, false, 0, 0, 0, 0));
+
+ sp<GraphicBuffer> buffer;
+ ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buffer));
+
+ // Ensure that the generation number we set propagates to allocated buffers
+ ASSERT_EQ(1U, buffer->getGenerationNumber());
+
+ ASSERT_EQ(OK, mProducer->detachBuffer(slot));
+
+ ASSERT_EQ(OK, mProducer->setGenerationNumber(2));
+
+ // These should fail, since we've changed the generation number on the queue
+ int outSlot;
+ ASSERT_EQ(BAD_VALUE, mProducer->attachBuffer(&outSlot, buffer));
+ ASSERT_EQ(BAD_VALUE, mConsumer->attachBuffer(&outSlot, buffer));
+
+ buffer->setGenerationNumber(2);
+
+ // This should succeed now that we've changed the buffer's generation number
+ ASSERT_EQ(OK, mProducer->attachBuffer(&outSlot, buffer));
+
+ ASSERT_EQ(OK, mProducer->detachBuffer(outSlot));
+
+ // This should also succeed with the new generation number
+ ASSERT_EQ(OK, mConsumer->attachBuffer(&outSlot, buffer));
+}
+
} // namespace android
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 4f87824..cf0043d 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -177,4 +177,37 @@
ASSERT_EQ(TEST_DATASPACE, dataSpace);
}
+TEST_F(SurfaceTest, SettingGenerationNumber) {
+ sp<IGraphicBufferProducer> producer;
+ sp<IGraphicBufferConsumer> consumer;
+ BufferQueue::createBufferQueue(&producer, &consumer);
+ sp<CpuConsumer> cpuConsumer = new CpuConsumer(consumer, 1);
+ sp<Surface> surface = new Surface(producer);
+ sp<ANativeWindow> window(surface);
+
+ // Allocate a buffer with a generation number of 0
+ ANativeWindowBuffer* buffer;
+ int fenceFd;
+ ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffer, &fenceFd));
+ ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffer, fenceFd));
+
+ // Detach the buffer and check its generation number
+ sp<GraphicBuffer> graphicBuffer;
+ sp<Fence> fence;
+ ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&graphicBuffer, &fence));
+ ASSERT_EQ(0U, graphicBuffer->getGenerationNumber());
+
+ ASSERT_EQ(NO_ERROR, surface->setGenerationNumber(1));
+ buffer = static_cast<ANativeWindowBuffer*>(graphicBuffer.get());
+
+ // This should change the generation number of the GraphicBuffer
+ ASSERT_EQ(NO_ERROR, surface->attachBuffer(buffer));
+
+ // Check that the new generation number sticks with the buffer
+ ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffer, -1));
+ ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffer, &fenceFd));
+ graphicBuffer = static_cast<GraphicBuffer*>(buffer);
+ ASSERT_EQ(1U, graphicBuffer->getGenerationNumber());
+}
+
}
diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp
index 6a42a22..e55db30 100644
--- a/libs/ui/GraphicBuffer.cpp
+++ b/libs/ui/GraphicBuffer.cpp
@@ -278,7 +278,7 @@
}
size_t GraphicBuffer::getFlattenedSize() const {
- return static_cast<size_t>(10 + (handle ? handle->numInts : 0)) * sizeof(int);
+ return static_cast<size_t>(11 + (handle ? handle->numInts : 0)) * sizeof(int);
}
size_t GraphicBuffer::getFdCount() const {
@@ -301,15 +301,16 @@
buf[5] = usage;
buf[6] = static_cast<int32_t>(mId >> 32);
buf[7] = static_cast<int32_t>(mId & 0xFFFFFFFFull);
- buf[8] = 0;
+ buf[8] = static_cast<int32_t>(mGenerationNumber);
buf[9] = 0;
+ buf[10] = 0;
if (handle) {
- buf[8] = handle->numFds;
- buf[9] = handle->numInts;
+ buf[9] = handle->numFds;
+ buf[10] = handle->numInts;
memcpy(fds, handle->data,
static_cast<size_t>(handle->numFds) * sizeof(int));
- memcpy(&buf[10], handle->data + handle->numFds,
+ memcpy(&buf[11], handle->data + handle->numFds,
static_cast<size_t>(handle->numInts) * sizeof(int));
}
@@ -325,20 +326,20 @@
status_t GraphicBuffer::unflatten(
void const*& buffer, size_t& size, int const*& fds, size_t& count) {
- if (size < 8*sizeof(int)) return NO_MEMORY;
+ if (size < 11 * sizeof(int)) return NO_MEMORY;
int const* buf = static_cast<int const*>(buffer);
if (buf[0] != 'GBFR') return BAD_TYPE;
- const size_t numFds = static_cast<size_t>(buf[8]);
- const size_t numInts = static_cast<size_t>(buf[9]);
+ const size_t numFds = static_cast<size_t>(buf[9]);
+ const size_t numInts = static_cast<size_t>(buf[10]);
// Limit the maxNumber to be relatively small. The number of fds or ints
// should not come close to this number, and the number itself was simply
// chosen to be high enough to not cause issues and low enough to prevent
// overflow problems.
const size_t maxNumber = 4096;
- if (numFds >= maxNumber || numInts >= (maxNumber - 10)) {
+ if (numFds >= maxNumber || numInts >= (maxNumber - 11)) {
width = height = stride = format = usage = 0;
handle = NULL;
ALOGE("unflatten: numFds or numInts is too large: %zd, %zd",
@@ -346,7 +347,7 @@
return BAD_VALUE;
}
- const size_t sizeNeeded = (10 + numInts) * sizeof(int);
+ const size_t sizeNeeded = (11 + numInts) * sizeof(int);
if (size < sizeNeeded) return NO_MEMORY;
size_t fdCountNeeded = numFds;
@@ -372,7 +373,7 @@
return NO_MEMORY;
}
memcpy(h->data, fds, numFds * sizeof(int));
- memcpy(h->data + numFds, &buf[10], numInts * sizeof(int));
+ memcpy(h->data + numFds, &buf[11], numInts * sizeof(int));
handle = h;
} else {
width = height = stride = format = usage = 0;
@@ -382,6 +383,8 @@
mId = static_cast<uint64_t>(buf[6]) << 32;
mId |= static_cast<uint32_t>(buf[7]);
+ mGenerationNumber = static_cast<uint32_t>(buf[8]);
+
mOwner = ownHandle;
if (handle != 0) {
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
index 11cbdc6..ee8d41c 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -530,6 +530,11 @@
return INVALID_OPERATION;
}
+status_t VirtualDisplaySurface::setGenerationNumber(uint32_t /* generation */) {
+ ALOGE("setGenerationNumber not supported on VirtualDisplaySurface");
+ return INVALID_OPERATION;
+}
+
void VirtualDisplaySurface::updateQueueBufferOutput(
const QueueBufferOutput& qbo) {
uint32_t w, h, transformHint, numPendingBuffers;
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
index 97af980..ff02290 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h
@@ -116,6 +116,7 @@
virtual void allocateBuffers(bool async, uint32_t width, uint32_t height,
PixelFormat format, uint32_t usage);
virtual status_t allowAllocation(bool allow);
+ virtual status_t setGenerationNumber(uint32_t generationNumber);
//
// Utility methods
diff --git a/services/surfaceflinger/MonitoredProducer.cpp b/services/surfaceflinger/MonitoredProducer.cpp
index 9fb555b..6f25bf5 100644
--- a/services/surfaceflinger/MonitoredProducer.cpp
+++ b/services/surfaceflinger/MonitoredProducer.cpp
@@ -114,6 +114,10 @@
return mProducer->allowAllocation(allow);
}
+status_t MonitoredProducer::setGenerationNumber(uint32_t generationNumber) {
+ return mProducer->setGenerationNumber(generationNumber);
+}
+
IBinder* MonitoredProducer::onAsBinder() {
return IInterface::asBinder(mProducer).get();
}
diff --git a/services/surfaceflinger/MonitoredProducer.h b/services/surfaceflinger/MonitoredProducer.h
index b2f8293..ff691d8 100644
--- a/services/surfaceflinger/MonitoredProducer.h
+++ b/services/surfaceflinger/MonitoredProducer.h
@@ -54,6 +54,7 @@
virtual void allocateBuffers(bool async, uint32_t width, uint32_t height,
PixelFormat format, uint32_t usage);
virtual status_t allowAllocation(bool allow);
+ virtual status_t setGenerationNumber(uint32_t generationNumber);
virtual IBinder* onAsBinder();
private: