Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aa5a0b26 authored by John Reck's avatar John Reck
Browse files

Add a better getLastQueuedBuffer

Avoid obfuscation via a matrix that's not necessarily useful or in
the desired origin of the caller. Instead return the source data,
which is also a lot smaller than the matrix is...

Bug: 183553027
Test: atest android.view.cts.PixelCopyTest (+new testBufferQueueCrop)
Change-Id: I1f7b5981405b2f20293bce9119414fc7780b8eb6
parent 18c79705
Loading
Loading
Loading
Loading
+20 −0
Original line number Diff line number Diff line
@@ -1621,6 +1621,26 @@ status_t BufferQueueProducer::getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer,
    return NO_ERROR;
}

status_t BufferQueueProducer::getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence,
                                                  Rect* outRect, uint32_t* outTransform) {
    ATRACE_CALL();
    BQ_LOGV("getLastQueuedBuffer");

    std::lock_guard<std::mutex> lock(mCore->mMutex);
    if (mCore->mLastQueuedSlot == BufferItem::INVALID_BUFFER_SLOT) {
        *outBuffer = nullptr;
        *outFence = Fence::NO_FENCE;
        return NO_ERROR;
    }

    *outBuffer = mSlots[mCore->mLastQueuedSlot].mGraphicBuffer;
    *outFence = mLastQueueBufferFence;
    *outRect = mLastQueuedCrop;
    *outTransform = mLastQueuedTransform;

    return NO_ERROR;
}

void BufferQueueProducer::getFrameTimestamps(FrameEventHistoryDelta* outDelta) {
    addAndGetFrameTimestamps(nullptr, outDelta);
}
+95 −0
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ enum {
    QUEUE_BUFFERS,
    CANCEL_BUFFERS,
    QUERY_MULTIPLE,
    GET_LAST_QUEUED_BUFFER2,
};

class BpGraphicBufferProducer : public BpInterface<IGraphicBufferProducer>
@@ -646,6 +647,56 @@ public:
        return result;
    }

    virtual status_t getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence,
                                         Rect* outRect, uint32_t* outTransform) override {
        Parcel data, reply;
        data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor());
        status_t result = remote()->transact(GET_LAST_QUEUED_BUFFER2, data, &reply);
        if (result != NO_ERROR) {
            ALOGE("getLastQueuedBuffer failed to transact: %d", result);
            return result;
        }
        status_t remoteError = NO_ERROR;
        result = reply.readInt32(&remoteError);
        if (result != NO_ERROR) {
            ALOGE("getLastQueuedBuffer failed to read status: %d", result);
            return result;
        }
        if (remoteError != NO_ERROR) {
            return remoteError;
        }
        bool hasBuffer = false;
        result = reply.readBool(&hasBuffer);
        if (result != NO_ERROR) {
            ALOGE("getLastQueuedBuffer failed to read buffer: %d", result);
            return result;
        }
        sp<GraphicBuffer> buffer;
        if (hasBuffer) {
            buffer = new GraphicBuffer();
            result = reply.read(*buffer);
            if (result == NO_ERROR) {
                result = reply.read(*outRect);
            }
            if (result == NO_ERROR) {
                result = reply.readUint32(outTransform);
            }
        }
        if (result != NO_ERROR) {
            ALOGE("getLastQueuedBuffer failed to read buffer: %d", result);
            return result;
        }
        sp<Fence> fence(new Fence);
        result = reply.read(*fence);
        if (result != NO_ERROR) {
            ALOGE("getLastQueuedBuffer failed to read fence: %d", result);
            return result;
        }
        *outBuffer = buffer;
        *outFence = fence;
        return result;
    }

    virtual void getFrameTimestamps(FrameEventHistoryDelta* outDelta) {
        Parcel data, reply;
        status_t result = data.writeInterfaceToken(
@@ -870,6 +921,11 @@ public:
                outBuffer, outFence, outTransformMatrix);
    }

    status_t getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence, Rect* outRect,
                                 uint32_t* outTransform) override {
        return mBase->getLastQueuedBuffer(outBuffer, outFence, outRect, outTransform);
    }

    void getFrameTimestamps(FrameEventHistoryDelta* outDelta) override {
        return mBase->getFrameTimestamps(outDelta);
    }
@@ -1362,6 +1418,45 @@ status_t BnGraphicBufferProducer::onTransact(
            }
            return NO_ERROR;
        }
        case GET_LAST_QUEUED_BUFFER2: {
            CHECK_INTERFACE(IGraphicBufferProducer, data, reply);
            sp<GraphicBuffer> buffer(nullptr);
            sp<Fence> fence(Fence::NO_FENCE);
            Rect crop;
            uint32_t transform;
            status_t result = getLastQueuedBuffer(&buffer, &fence, &crop, &transform);
            reply->writeInt32(result);
            if (result != NO_ERROR) {
                return result;
            }
            if (!buffer.get()) {
                reply->writeBool(false);
            } else {
                reply->writeBool(true);
                result = reply->write(*buffer);
                if (result == NO_ERROR) {
                    result = reply->write(crop);
                }
                if (result == NO_ERROR) {
                    result = reply->writeUint32(transform);
                }
            }
            if (result != NO_ERROR) {
                ALOGE("getLastQueuedBuffer failed to write buffer: %d", result);
                return result;
            }
            if (fence == nullptr) {
                ALOGE("getLastQueuedBuffer returned a NULL fence, setting to Fence::NO_FENCE");
                fence = Fence::NO_FENCE;
            }
            result = reply->write(*fence);
            if (result != NO_ERROR) {
                ALOGE("getLastQueuedBuffer failed to write fence: %d", result);
                return result;
            }
            return NO_ERROR;
        }

        case GET_FRAME_TIMESTAMPS: {
            CHECK_INTERFACE(IGraphicBufferProducer, data, reply);
            FrameEventHistoryDelta frameTimestamps;
+36 −0
Original line number Diff line number Diff line
@@ -1492,6 +1492,9 @@ int Surface::perform(int operation, va_list args)
    case NATIVE_WINDOW_GET_LAST_QUEUED_BUFFER:
        res = dispatchGetLastQueuedBuffer(args);
        break;
    case NATIVE_WINDOW_GET_LAST_QUEUED_BUFFER2:
        res = dispatchGetLastQueuedBuffer2(args);
        break;
    case NATIVE_WINDOW_SET_FRAME_TIMELINE_INFO:
        res = dispatchSetFrameTimelineInfo(args);
        break;
@@ -1805,6 +1808,39 @@ int Surface::dispatchGetLastQueuedBuffer(va_list args) {
    return result;
}

int Surface::dispatchGetLastQueuedBuffer2(va_list args) {
    AHardwareBuffer** buffer = va_arg(args, AHardwareBuffer**);
    int* fence = va_arg(args, int*);
    ARect* crop = va_arg(args, ARect*);
    uint32_t* transform = va_arg(args, uint32_t*);
    sp<GraphicBuffer> graphicBuffer;
    sp<Fence> spFence;

    Rect r;
    int result =
            mGraphicBufferProducer->getLastQueuedBuffer(&graphicBuffer, &spFence, &r, transform);

    if (graphicBuffer != nullptr) {
        *buffer = graphicBuffer->toAHardwareBuffer();
        AHardwareBuffer_acquire(*buffer);

        // Avoid setting crop* unless buffer is valid (matches IGBP behavior)
        crop->left = r.left;
        crop->top = r.top;
        crop->right = r.right;
        crop->bottom = r.bottom;
    } else {
        *buffer = nullptr;
    }

    if (spFence != nullptr) {
        *fence = spFence->dup();
    } else {
        *fence = -1;
    }
    return result;
}

int Surface::dispatchSetFrameTimelineInfo(va_list args) {
    ATRACE_CALL();
    auto frameTimelineVsyncId = static_cast<int64_t>(va_arg(args, int64_t));
+4 −0
Original line number Diff line number Diff line
@@ -186,6 +186,10 @@ public:
    virtual status_t getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer,
            sp<Fence>* outFence, float outTransformMatrix[16]) override;

    // See IGraphicBufferProducer::getLastQueuedBuffer
    virtual status_t getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence,
                                         Rect* outRect, uint32_t* outTransform) override;

    // See IGraphicBufferProducer::getFrameTimestamps
    virtual void getFrameTimestamps(FrameEventHistoryDelta* outDelta) override;

+16 −0
Original line number Diff line number Diff line
@@ -640,6 +640,22 @@ public:
    virtual status_t getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer,
            sp<Fence>* outFence, float outTransformMatrix[16]) = 0;

    // Returns the last queued buffer along with a fence which must signal
    // before the contents of the buffer are read. If there are no buffers in
    // the queue, outBuffer will be populated with nullptr and outFence will be
    // populated with Fence::NO_FENCE
    //
    // outRect & outTransform are not modified if outBuffer is null.
    //
    // Returns NO_ERROR or the status of the Binder transaction
    virtual status_t getLastQueuedBuffer([[maybe_unused]] sp<GraphicBuffer>* outBuffer,
                                         [[maybe_unused]] sp<Fence>* outFence,
                                         [[maybe_unused]] Rect* outRect,
                                         [[maybe_unused]] uint32_t* outTransform) {
        // Too many things implement IGraphicBufferProducer...
        return UNKNOWN_TRANSACTION;
    }

    // Gets the frame events that haven't already been retrieved.
    virtual void getFrameTimestamps(FrameEventHistoryDelta* /*outDelta*/) {}

Loading