Skip to content

Commit 5210808

Browse files
author
Jim Shargo
committed
bufferqueues: Simplify calls that don't use GL fences
We wanna get rid of this API, and we can simplify all these calls to just avoid the argument. Bug: 339705065 Flag: EXEMPT refactor Test: old tests Change-Id: I4e94e66003cdcdc197254435e5a815dd53e67a20
1 parent 2e3bb37 commit 5210808

12 files changed

Lines changed: 50 additions & 78 deletions

File tree

‎libs/gui/BufferItemConsumer.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ status_t BufferItemConsumer::releaseBufferSlotLocked(int slotIndex, const sp<Gra
140140
BI_LOGE("Failed to addReleaseFenceLocked");
141141
}
142142

143-
err = releaseBufferLocked(slotIndex, buffer, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR);
143+
err = releaseBufferLocked(slotIndex, buffer);
144144
if (err != OK && err != IGraphicBufferConsumer::STALE_BUFFER_SLOT) {
145145
BI_LOGE("Failed to release buffer: %s (%d)",
146146
strerror(-err), err);

‎libs/gui/GLConsumer.cpp‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ status_t GLConsumer::releaseTexImage() {
314314
// so... basically, nothing more to do here.
315315
}
316316

317-
err = releaseBufferLocked(buf, mSlots[buf].mGraphicBuffer, mEglDisplay, EGL_NO_SYNC_KHR);
317+
err = releaseBufferLocked(buf, mSlots[buf].mGraphicBuffer);
318318
if (err < NO_ERROR) {
319319
GLC_LOGE("releaseTexImage: failed to release buffer: %s (%d)",
320320
strerror(-err), err);
@@ -418,16 +418,14 @@ status_t GLConsumer::updateAndReleaseLocked(const BufferItem& item,
418418
if (!mAttached) {
419419
GLC_LOGE("updateAndRelease: GLConsumer is not attached to an OpenGL "
420420
"ES context");
421-
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer,
422-
mEglDisplay, EGL_NO_SYNC_KHR);
421+
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer);
423422
return INVALID_OPERATION;
424423
}
425424

426425
// Confirm state.
427426
err = checkAndUpdateEglStateLocked();
428427
if (err != NO_ERROR) {
429-
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer,
430-
mEglDisplay, EGL_NO_SYNC_KHR);
428+
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer);
431429
return err;
432430
}
433431

@@ -440,8 +438,7 @@ status_t GLConsumer::updateAndReleaseLocked(const BufferItem& item,
440438
if (err != NO_ERROR) {
441439
GLC_LOGW("updateAndRelease: unable to createImage on display=%p slot=%d",
442440
mEglDisplay, slot);
443-
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer,
444-
mEglDisplay, EGL_NO_SYNC_KHR);
441+
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer);
445442
return UNKNOWN_ERROR;
446443
}
447444

@@ -453,8 +450,7 @@ status_t GLConsumer::updateAndReleaseLocked(const BufferItem& item,
453450
// release the old buffer, so instead we just drop the new frame.
454451
// As we are still under lock since acquireBuffer, it is safe to
455452
// release by slot.
456-
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer,
457-
mEglDisplay, EGL_NO_SYNC_KHR);
453+
releaseBufferLocked(slot, mSlots[slot].mGraphicBuffer);
458454
return err;
459455
}
460456
}

‎libs/gui/IGraphicBufferConsumer.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include <utils/NativeHandle.h>
2828
#include <utils/String8.h>
29+
#include <cstdint>
2930

3031
namespace android {
3132

@@ -84,7 +85,8 @@ class BpGraphicBufferConsumer : public SafeBpInterface<IGraphicBufferConsumer> {
8485
EGLDisplay display __attribute__((unused)),
8586
EGLSyncKHR fence __attribute__((unused)),
8687
const sp<Fence>& releaseFence) override {
87-
return callRemote<ReleaseBuffer>(Tag::RELEASE_BUFFER, buf, frameNumber, releaseFence);
88+
using Signature = status_t (IGraphicBufferConsumer::*)(int, uint64_t, const sp<Fence>&);
89+
return callRemote<Signature>(Tag::RELEASE_BUFFER, buf, frameNumber, releaseFence);
8890
}
8991

9092
status_t consumerConnect(const sp<IConsumerListener>& consumer, bool controlledByApp) override {
@@ -188,8 +190,10 @@ status_t BnGraphicBufferConsumer::onTransact(uint32_t code, const Parcel& data,
188190
return callLocal(data, reply, &IGraphicBufferConsumer::detachBuffer);
189191
case Tag::ATTACH_BUFFER:
190192
return callLocal(data, reply, &IGraphicBufferConsumer::attachBuffer);
191-
case Tag::RELEASE_BUFFER:
192-
return callLocal(data, reply, &IGraphicBufferConsumer::releaseHelper);
193+
case Tag::RELEASE_BUFFER: {
194+
using Signature = status_t (IGraphicBufferConsumer::*)(int, uint64_t, const sp<Fence>&);
195+
return callLocal<Signature>(data, reply, &IGraphicBufferConsumer::releaseBuffer);
196+
}
193197
case Tag::CONSUMER_CONNECT:
194198
return callLocal(data, reply, &IGraphicBufferConsumer::consumerConnect);
195199
case Tag::CONSUMER_DISCONNECT:

‎libs/gui/StreamSplitter.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,7 @@ void StreamSplitter::onBufferReleasedByOutput(
234234
LOG_ALWAYS_FATAL_IF(status != NO_ERROR,
235235
"attaching buffer to input failed (%d)", status);
236236

237-
status = mInput->releaseBuffer(consumerSlot, /* frameNumber */ 0,
238-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, tracker->getMergedFence());
237+
status = mInput->releaseBuffer(consumerSlot, /* frameNumber */ 0, tracker->getMergedFence());
239238
LOG_ALWAYS_FATAL_IF(status != NO_ERROR,
240239
"releasing buffer to input failed (%d)", status);
241240

‎libs/gui/include/gui/GLConsumer.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,9 @@ class GLConsumer : public ConsumerBase {
268268

269269
// releaseBufferLocked overrides the ConsumerBase method to update the
270270
// mEglSlots array in addition to the ConsumerBase.
271-
virtual status_t releaseBufferLocked(int slot,
272-
const sp<GraphicBuffer> graphicBuffer,
273-
EGLDisplay display, EGLSyncKHR eglFence) override;
271+
virtual status_t releaseBufferLocked(int slot, const sp<GraphicBuffer> graphicBuffer,
272+
EGLDisplay display = EGL_NO_DISPLAY,
273+
EGLSyncKHR eglFence = EGL_NO_SYNC_KHR) override;
274274

275275
status_t releaseBufferLocked(int slot,
276276
const sp<GraphicBuffer> graphicBuffer, EGLSyncKHR eglFence) {

‎libs/gui/include/gui/IGraphicBufferConsumer.h‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,9 @@ class IGraphicBufferConsumer : public RefBase {
137137
virtual status_t releaseBuffer(int buf, uint64_t frameNumber, EGLDisplay display,
138138
EGLSyncKHR fence, const sp<Fence>& releaseFence) = 0;
139139

140-
status_t releaseHelper(int buf, uint64_t frameNumber, const sp<Fence>& releaseFence) {
140+
status_t releaseBuffer(int buf, uint64_t frameNumber, const sp<Fence>& releaseFence) {
141141
return releaseBuffer(buf, frameNumber, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, releaseFence);
142142
}
143-
// This is explicitly *not* the actual signature of IGBC::releaseBuffer, but:
144-
// 1) We have no easy way to send the EGL objects across Binder
145-
// 2) This has always been broken, probably because
146-
// 3) IGBC is rarely remoted
147-
// For now, we will choose to bury our heads in the sand and ignore this problem until such time
148-
// as we can finally finish converting away from EGL sync to native Android sync
149-
using ReleaseBuffer = decltype(&IGraphicBufferConsumer::releaseHelper);
150143

151144
// consumerConnect connects a consumer to the BufferQueue. Only one consumer may be connected,
152145
// and when that consumer disconnects the BufferQueue is placed into the "abandoned" state,

‎libs/gui/tests/BufferQueue_test.cpp‎

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -425,8 +425,7 @@ TEST_F(BufferQueueTest, DetachAndReattachOnConsumerSide) {
425425
ASSERT_EQ(BAD_VALUE, mConsumer->attachBuffer(&newSlot, nullptr));
426426
ASSERT_EQ(OK, mConsumer->attachBuffer(&newSlot, item.mGraphicBuffer));
427427

428-
ASSERT_EQ(OK, mConsumer->releaseBuffer(newSlot, 0, EGL_NO_DISPLAY,
429-
EGL_NO_SYNC_KHR, Fence::NO_FENCE));
428+
ASSERT_EQ(OK, mConsumer->releaseBuffer(newSlot, 0, Fence::NO_FENCE));
430429

431430
ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
432431
mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, GRALLOC_USAGE_SW_WRITE_OFTEN,
@@ -609,8 +608,7 @@ TEST_F(BufferQueueTest, TestSharedBufferModeWithoutAutoRefresh) {
609608
ASSERT_EQ(true, item.mQueuedBuffer);
610609
ASSERT_EQ(false, item.mAutoRefresh);
611610

612-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
613-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
611+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
614612

615613
// attempt to acquire a second time should return no buffer available
616614
ASSERT_EQ(IGraphicBufferConsumer::NO_BUFFER_AVAILABLE,
@@ -653,8 +651,7 @@ TEST_F(BufferQueueTest, TestSharedBufferModeWithAutoRefresh) {
653651
ASSERT_EQ(i == 0, item.mQueuedBuffer);
654652
ASSERT_EQ(true, item.mAutoRefresh);
655653

656-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
657-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
654+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
658655
}
659656

660657
// Repeatedly queue and dequeue a buffer from the producer side, it should
@@ -684,8 +681,7 @@ TEST_F(BufferQueueTest, TestSharedBufferModeWithAutoRefresh) {
684681
ASSERT_EQ(i == 0, item.mQueuedBuffer);
685682
ASSERT_EQ(true, item.mAutoRefresh);
686683

687-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
688-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
684+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
689685
}
690686
}
691687

@@ -735,8 +731,7 @@ TEST_F(BufferQueueTest, TestSharedBufferModeUsingAlreadyDequeuedBuffer) {
735731
ASSERT_EQ(true, item.mQueuedBuffer);
736732
ASSERT_EQ(false, item.mAutoRefresh);
737733

738-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
739-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
734+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
740735

741736
// attempt to acquire a second time should return no buffer available
742737
ASSERT_EQ(IGraphicBufferConsumer::NO_BUFFER_AVAILABLE,
@@ -874,8 +869,7 @@ TEST_F(BufferQueueTest, CanRetrieveLastQueuedBuffer) {
874869
for (size_t i = 0; i < 2; ++i) {
875870
BufferItem item;
876871
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
877-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
878-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
872+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
879873
}
880874

881875
// Make sure we got the second buffer back
@@ -929,8 +923,7 @@ TEST_F(BufferQueueTest, TestOccupancyHistory) {
929923
nullptr, nullptr));
930924
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
931925
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
932-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
933-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
926+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
934927
std::this_thread::sleep_for(16ms);
935928
}
936929

@@ -946,8 +939,7 @@ TEST_F(BufferQueueTest, TestOccupancyHistory) {
946939
nullptr, nullptr));
947940
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
948941
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
949-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
950-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
942+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
951943
std::this_thread::sleep_for(16ms);
952944
}
953945
ASSERT_EQ(OK,
@@ -959,12 +951,10 @@ TEST_F(BufferQueueTest, TestOccupancyHistory) {
959951
nullptr));
960952
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
961953
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
962-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
963-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
954+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
964955
std::this_thread::sleep_for(16ms);
965956
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
966-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
967-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
957+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
968958

969959
// Sleep between segments
970960
std::this_thread::sleep_for(500ms);
@@ -981,13 +971,11 @@ TEST_F(BufferQueueTest, TestOccupancyHistory) {
981971
nullptr, nullptr));
982972
ASSERT_EQ(OK, mProducer->queueBuffer(slot, input, &output));
983973
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
984-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
985-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
974+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
986975
std::this_thread::sleep_for(16ms);
987976
}
988977
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
989-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
990-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
978+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
991979

992980
// Now we read the segments
993981
std::vector<OccupancyTracker::Segment> history;
@@ -1108,8 +1096,7 @@ TEST_F(BufferQueueTest, TestDiscardFreeBuffers) {
11081096

11091097
// Acquire and free 1 buffer
11101098
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
1111-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
1112-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
1099+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
11131100
int releasedSlot = item.mSlot;
11141101

11151102
// Acquire 1 buffer, leaving 1 filled buffer in queue
@@ -1376,8 +1363,7 @@ TEST_F(BufferQueueTest, TestStaleBufferHandleSentAfterDisconnect) {
13761363
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
13771364
ASSERT_EQ(slot, item.mSlot);
13781365
ASSERT_NE(nullptr, item.mGraphicBuffer.get());
1379-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
1380-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
1366+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
13811367

13821368
// Dequeue and queue the buffer again
13831369
ASSERT_EQ(OK,
@@ -1390,8 +1376,7 @@ TEST_F(BufferQueueTest, TestStaleBufferHandleSentAfterDisconnect) {
13901376
ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0));
13911377
ASSERT_EQ(slot, item.mSlot);
13921378
ASSERT_EQ(nullptr, item.mGraphicBuffer.get());
1393-
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
1394-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
1379+
ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
13951380

13961381
// Dequeue and queue the buffer again
13971382
ASSERT_EQ(OK,

‎libs/gui/tests/StreamSplitter_test.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,7 @@ TEST_F(StreamSplitterTest, OneInputOneOutput) {
9595
ASSERT_EQ(*dataOut, TEST_DATA);
9696
ASSERT_EQ(OK, item.mGraphicBuffer->unlock());
9797

98-
ASSERT_EQ(OK, outputConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
99-
EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, Fence::NO_FENCE));
98+
ASSERT_EQ(OK, outputConsumer->releaseBuffer(item.mSlot, item.mFrameNumber, Fence::NO_FENCE));
10099

101100
// This should succeed even with allocation disabled since it will have
102101
// received the buffer back from the output BufferQueue
@@ -168,9 +167,9 @@ TEST_F(StreamSplitterTest, OneInputMultipleOutputs) {
168167
ASSERT_EQ(*dataOut, TEST_DATA);
169168
ASSERT_EQ(OK, item.mGraphicBuffer->unlock());
170169

171-
ASSERT_EQ(OK, outputConsumers[output]->releaseBuffer(item.mSlot,
172-
item.mFrameNumber, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR,
173-
Fence::NO_FENCE));
170+
ASSERT_EQ(OK,
171+
outputConsumers[output]->releaseBuffer(item.mSlot, item.mFrameNumber,
172+
Fence::NO_FENCE));
174173
}
175174

176175
// This should succeed even with allocation disabled since it will have

‎libs/gui/tests/Surface_test.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,9 @@ class SurfaceTest : public ::testing::Test {
201201
releasedItems.resize(1+extraDiscardedBuffers);
202202
for (size_t i = 0; i < releasedItems.size(); i++) {
203203
ASSERT_EQ(NO_ERROR, consumer->acquireBuffer(&releasedItems[i], 0));
204-
ASSERT_EQ(NO_ERROR, consumer->releaseBuffer(releasedItems[i].mSlot,
205-
releasedItems[i].mFrameNumber, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR,
206-
Fence::NO_FENCE));
204+
ASSERT_EQ(NO_ERROR,
205+
consumer->releaseBuffer(releasedItems[i].mSlot, releasedItems[i].mFrameNumber,
206+
Fence::NO_FENCE));
207207
}
208208
int32_t expectedReleaseCb = (enableReleasedCb ? releasedItems.size() : 0);
209209
if (hasSurfaceListener) {

‎libs/nativedisplay/include/surfacetexture/SurfaceTexture.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,8 @@ class ANDROID_API SurfaceTexture : public ConsumerBase {
344344
* mEglSlots array in addition to the ConsumerBase.
345345
*/
346346
virtual status_t releaseBufferLocked(int slot, const sp<GraphicBuffer> graphicBuffer,
347-
EGLDisplay display, EGLSyncKHR eglFence) override;
347+
EGLDisplay display = EGL_NO_DISPLAY,
348+
EGLSyncKHR eglFence = EGL_NO_SYNC_KHR) override;
348349

349350
/**
350351
* freeBufferLocked frees up the given buffer slot. If the slot has been

0 commit comments

Comments
 (0)