Merge "Move PlaneLayout lookup to GraphicBuffer" into main

Author: jreck

libs/ui/Gralloc4.cpp

@@ -262,37 +262,8 @@ void Gralloc4Mapper::getTransportSize(buffer_handle_t bufferHandle, uint32_t* ou
 status_t Gralloc4Mapper::lock(buffer_handle_t bufferHandle, uint64_t usage, const Rect& bounds,
                               int acquireFence, void** outData, int32_t* outBytesPerPixel,
                               int32_t* outBytesPerStride) const {
-    std::vector<ui::PlaneLayout> planeLayouts;
-    status_t err = getPlaneLayouts(bufferHandle, &planeLayouts);
-
-    if (err == NO_ERROR && !planeLayouts.empty()) {
-        if (outBytesPerPixel) {
-            int32_t bitsPerPixel = planeLayouts.front().sampleIncrementInBits;
-            for (const auto& planeLayout : planeLayouts) {
-                if (bitsPerPixel != planeLayout.sampleIncrementInBits) {
-                    bitsPerPixel = -1;
-                }
-            }
-            if (bitsPerPixel >= 0 && bitsPerPixel % 8 == 0) {
-                *outBytesPerPixel = bitsPerPixel / 8;
-            } else {
-                *outBytesPerPixel = -1;
-            }
-        }
-        if (outBytesPerStride) {
-            int32_t bytesPerStride = planeLayouts.front().strideInBytes;
-            for (const auto& planeLayout : planeLayouts) {
-                if (bytesPerStride != planeLayout.strideInBytes) {
-                    bytesPerStride = -1;
-                }
-            }
-            if (bytesPerStride >= 0) {
-                *outBytesPerStride = bytesPerStride;
-            } else {
-                *outBytesPerStride = -1;
-            }
-        }
-    }
+    if (outBytesPerPixel) *outBytesPerPixel = -1;
+    if (outBytesPerStride) *outBytesPerStride = -1;
 
     auto buffer = const_cast<native_handle_t*>(bufferHandle);
 

libs/ui/Gralloc5.cpp

@@ -597,37 +597,8 @@ void Gralloc5Mapper::getTransportSize(buffer_handle_t bufferHandle, uint32_t *ou
 status_t Gralloc5Mapper::lock(buffer_handle_t bufferHandle, uint64_t usage, const Rect &bounds,
                               int acquireFence, void **outData, int32_t *outBytesPerPixel,
                               int32_t *outBytesPerStride) const {
-    std::vector<ui::PlaneLayout> planeLayouts;
-    status_t err = getPlaneLayouts(bufferHandle, &planeLayouts);
-
-    if (err == NO_ERROR && !planeLayouts.empty()) {
-        if (outBytesPerPixel) {
-            int32_t bitsPerPixel = planeLayouts.front().sampleIncrementInBits;
-            for (const auto &planeLayout : planeLayouts) {
-                if (bitsPerPixel != planeLayout.sampleIncrementInBits) {
-                    bitsPerPixel = -1;
-                }
-            }
-            if (bitsPerPixel >= 0 && bitsPerPixel % 8 == 0) {
-                *outBytesPerPixel = bitsPerPixel / 8;
-            } else {
-                *outBytesPerPixel = -1;
-            }
-        }
-        if (outBytesPerStride) {
-            int32_t bytesPerStride = planeLayouts.front().strideInBytes;
-            for (const auto &planeLayout : planeLayouts) {
-                if (bytesPerStride != planeLayout.strideInBytes) {
-                    bytesPerStride = -1;
-                }
-            }
-            if (bytesPerStride >= 0) {
-                *outBytesPerStride = bytesPerStride;
-            } else {
-                *outBytesPerStride = -1;
-            }
-        }
-    }
+    if (outBytesPerPixel) *outBytesPerPixel = -1;
+    if (outBytesPerStride) *outBytesPerStride = -1;
 
     auto status = mMapper->v5.lock(bufferHandle, usage, bounds, acquireFence, outData);
 

libs/ui/GraphicBuffer.cpp

@@ -22,7 +22,7 @@
 #include <cutils/atomic.h>
 
 #include <grallocusage/GrallocUsageConversion.h>
-
+#include <sync/sync.h>
 #include <ui/GraphicBufferAllocator.h>
 #include <ui/GraphicBufferMapper.h>
 #include <utils/Trace.h>
@@ -40,6 +40,38 @@ static uint64_t getUniqueId() {
     return id;
 }
 
+static void resolveLegacyByteLayoutFromPlaneLayout(const std::vector<ui::PlaneLayout>& planeLayouts,
+                                                   int32_t* outBytesPerPixel,
+                                                   int32_t* outBytesPerStride) {
+    if (planeLayouts.empty()) return;
+    if (outBytesPerPixel) {
+        int32_t bitsPerPixel = planeLayouts.front().sampleIncrementInBits;
+        for (const auto& planeLayout : planeLayouts) {
+            if (bitsPerPixel != planeLayout.sampleIncrementInBits) {
+                bitsPerPixel = -1;
+            }
+        }
+        if (bitsPerPixel >= 0 && bitsPerPixel % 8 == 0) {
+            *outBytesPerPixel = bitsPerPixel / 8;
+        } else {
+            *outBytesPerPixel = -1;
+        }
+    }
+    if (outBytesPerStride) {
+        int32_t bytesPerStride = planeLayouts.front().strideInBytes;
+        for (const auto& planeLayout : planeLayouts) {
+            if (bytesPerStride != planeLayout.strideInBytes) {
+                bytesPerStride = -1;
+            }
+        }
+        if (bytesPerStride >= 0) {
+            *outBytesPerStride = bytesPerStride;
+        } else {
+            *outBytesPerStride = -1;
+        }
+    }
+}
+
 sp<GraphicBuffer> GraphicBuffer::from(ANativeWindowBuffer* anwb) {
     return static_cast<GraphicBuffer *>(anwb);
 }
@@ -279,10 +311,7 @@ status_t GraphicBuffer::lock(uint32_t inUsage, const Rect& rect, void** vaddr,
         return BAD_VALUE;
     }
 
-    status_t res = getBufferMapper().lock(handle, inUsage, rect, vaddr, outBytesPerPixel,
-                                          outBytesPerStride);
-
-    return res;
+    return lockAsync(inUsage, rect, vaddr, -1, outBytesPerPixel, outBytesPerStride);
 }
 
 status_t GraphicBuffer::lockYCbCr(uint32_t inUsage, android_ycbcr* ycbcr)
@@ -302,14 +331,12 @@ status_t GraphicBuffer::lockYCbCr(uint32_t inUsage, const Rect& rect,
                 width, height);
         return BAD_VALUE;
     }
-    status_t res = getBufferMapper().lockYCbCr(handle, inUsage, rect, ycbcr);
-    return res;
+    return lockAsyncYCbCr(inUsage, rect, ycbcr, -1);
 }
 
 status_t GraphicBuffer::unlock()
 {
-    status_t res = getBufferMapper().unlock(handle);
-    return res;
+    return unlockAsync(nullptr);
 }
 
 status_t GraphicBuffer::lockAsync(uint32_t inUsage, void** vaddr, int fenceFd,
@@ -336,10 +363,49 @@ status_t GraphicBuffer::lockAsync(uint64_t inProducerUsage, uint64_t inConsumerU
         return BAD_VALUE;
     }
 
-    status_t res = getBufferMapper().lockAsync(handle, inProducerUsage, inConsumerUsage, rect,
-                                               vaddr, fenceFd, outBytesPerPixel, outBytesPerStride);
+    // Resolve the bpp & bps before doing a lock in case this fails we don't have to worry about
+    // doing an unlock
+    int32_t legacyBpp = -1, legacyBps = -1;
+    if (outBytesPerPixel || outBytesPerStride) {
+        const auto mapperVersion = getBufferMapperVersion();
+        // For gralloc2 we need to guess at the bpp & bps
+        // For gralloc3 the lock() call will return it
+        // For gralloc4 & later the PlaneLayout metadata query is vastly superior and we
+        // resolve bpp & bps just for compatibility
+
+        // TODO: See if we can't just remove gralloc2 support.
+        if (mapperVersion == GraphicBufferMapper::GRALLOC_2) {
+            legacyBpp = bytesPerPixel(format);
+            if (legacyBpp > 0) {
+                legacyBps = stride * legacyBpp;
+            } else {
+                legacyBpp = -1;
+            }
+        } else if (mapperVersion >= GraphicBufferMapper::GRALLOC_4) {
+            auto planeLayout = getBufferMapper().getPlaneLayouts(handle);
+            if (!planeLayout.has_value()) return planeLayout.asStatus();
+            resolveLegacyByteLayoutFromPlaneLayout(planeLayout.value(), &legacyBpp, &legacyBps);
+        }
+    }
 
-    return res;
+    const uint64_t usage = static_cast<uint64_t>(
+            android_convertGralloc1To0Usage(inProducerUsage, inConsumerUsage));
+
+    auto result = getBufferMapper().lock(handle, usage, rect, base::unique_fd{fenceFd});
+
+    if (!result.has_value()) {
+        return result.error().asStatus();
+    }
+    auto value = result.value();
+    *vaddr = value.address;
+
+    if (outBytesPerPixel) {
+        *outBytesPerPixel = legacyBpp != -1 ? legacyBpp : value.bytesPerPixel;
+    }
+    if (outBytesPerStride) {
+        *outBytesPerStride = legacyBps != -1 ? legacyBps : value.bytesPerStride;
+    }
+    return OK;
 }
 
 status_t GraphicBuffer::lockAsyncYCbCr(uint32_t inUsage, android_ycbcr* ycbcr,
@@ -360,14 +426,18 @@ status_t GraphicBuffer::lockAsyncYCbCr(uint32_t inUsage, const Rect& rect,
                 width, height);
         return BAD_VALUE;
     }
-    status_t res = getBufferMapper().lockAsyncYCbCr(handle, inUsage, rect, ycbcr, fenceFd);
-    return res;
+    auto result = getBufferMapper().lockYCbCr(handle, static_cast<int64_t>(inUsage), rect,
+                                              base::unique_fd{fenceFd});
+    if (!result.has_value()) {
+        return result.error().asStatus();
+    }
+    *ycbcr = result.value();
+    return OK;
 }
 
 status_t GraphicBuffer::unlockAsync(int *fenceFd)
 {
-    status_t res = getBufferMapper().unlockAsync(handle, fenceFd);
-    return res;
+    return getBufferMapper().unlockAsync(handle, fenceFd);
 }
 
 status_t GraphicBuffer::isSupported(uint32_t inWidth, uint32_t inHeight, PixelFormat inFormat,

libs/ui/GraphicBufferMapper.cpp

@@ -41,9 +41,13 @@
 
 #include <system/graphics.h>
 
+using unique_fd = ::android::base::unique_fd;
+
 namespace android {
 // ---------------------------------------------------------------------------
 
+using LockResult = GraphicBufferMapper::LockResult;
+
 ANDROID_SINGLETON_STATIC_INSTANCE( GraphicBufferMapper )
 
 void GraphicBufferMapper::preloadHal() {
@@ -135,63 +139,86 @@ status_t GraphicBufferMapper::freeBuffer(buffer_handle_t handle)
     return NO_ERROR;
 }
 
-status_t GraphicBufferMapper::lock(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
-                                   void** vaddr, int32_t* outBytesPerPixel,
-                                   int32_t* outBytesPerStride) {
-    return lockAsync(handle, usage, bounds, vaddr, -1, outBytesPerPixel, outBytesPerStride);
-}
-
-status_t GraphicBufferMapper::lockYCbCr(buffer_handle_t handle, uint32_t usage,
-        const Rect& bounds, android_ycbcr *ycbcr)
-{
-    return lockAsyncYCbCr(handle, usage, bounds, ycbcr, -1);
-}
+ui::Result<LockResult> GraphicBufferMapper::lock(buffer_handle_t handle, int64_t usage,
+                                                 const Rect& bounds, unique_fd&& acquireFence) {
+    ATRACE_CALL();
 
-status_t GraphicBufferMapper::unlock(buffer_handle_t handle)
-{
-    int32_t fenceFd = -1;
-    status_t error = unlockAsync(handle, &fenceFd);
-    if (error == NO_ERROR && fenceFd >= 0) {
-        sync_wait(fenceFd, -1);
-        close(fenceFd);
+    LockResult result;
+    status_t status = mMapper->lock(handle, usage, bounds, acquireFence.release(), &result.address,
+                                    &result.bytesPerPixel, &result.bytesPerStride);
+    if (status != OK) {
+        return base::unexpected(ui::Error::statusToCode(status));
+    } else {
+        return result;
     }
-    return error;
-}
-
-status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
-                                        void** vaddr, int fenceFd, int32_t* outBytesPerPixel,
-                                        int32_t* outBytesPerStride) {
-    return lockAsync(handle, usage, usage, bounds, vaddr, fenceFd, outBytesPerPixel,
-                     outBytesPerStride);
 }
 
-status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint64_t producerUsage,
-                                        uint64_t consumerUsage, const Rect& bounds, void** vaddr,
-                                        int fenceFd, int32_t* outBytesPerPixel,
-                                        int32_t* outBytesPerStride) {
+ui::Result<android_ycbcr> GraphicBufferMapper::lockYCbCr(buffer_handle_t handle, int64_t usage,
+                                                         const Rect& bounds,
+                                                         base::unique_fd&& acquireFence) {
     ATRACE_CALL();
 
-    const uint64_t usage = static_cast<uint64_t>(
-            android_convertGralloc1To0Usage(producerUsage, consumerUsage));
-    return mMapper->lock(handle, usage, bounds, fenceFd, vaddr, outBytesPerPixel,
-                         outBytesPerStride);
+    android_ycbcr result = {};
+    status_t status = mMapper->lock(handle, usage, bounds, acquireFence.release(), &result);
+    if (status != OK) {
+        return base::unexpected(ui::Error::statusToCode(status));
+    } else {
+        return result;
+    }
 }
 
-status_t GraphicBufferMapper::lockAsyncYCbCr(buffer_handle_t handle,
-        uint32_t usage, const Rect& bounds, android_ycbcr *ycbcr, int fenceFd)
-{
+status_t GraphicBufferMapper::unlock(buffer_handle_t handle, base::unique_fd* outFence) {
     ATRACE_CALL();
+    int fence = mMapper->unlock(handle);
+    if (outFence) {
+        *outFence = unique_fd{fence};
+    } else {
+        sync_wait(fence, -1);
+        close(fence);
+    }
+    return OK;
+}
 
-    return mMapper->lock(handle, usage, bounds, fenceFd, ycbcr);
+status_t GraphicBufferMapper::lock(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
+                                   void** vaddr) {
+    auto result = lock(handle, static_cast<int64_t>(usage), bounds);
+    if (!result.has_value()) return result.asStatus();
+    auto val = result.value();
+    *vaddr = val.address;
+    return OK;
 }
 
-status_t GraphicBufferMapper::unlockAsync(buffer_handle_t handle, int *fenceFd)
-{
-    ATRACE_CALL();
+status_t GraphicBufferMapper::lockYCbCr(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
+                                        android_ycbcr* ycbcr) {
+    auto result = lockYCbCr(handle, static_cast<int64_t>(usage), bounds);
+    if (!result.has_value()) return result.asStatus();
+    *ycbcr = result.value();
+    return OK;
+}
+
+status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
+                                        void** vaddr, int fenceFd) {
+    auto result = lock(handle, static_cast<int64_t>(usage), bounds, unique_fd{fenceFd});
+    if (!result.has_value()) return result.asStatus();
+    auto val = result.value();
+    *vaddr = val.address;
+    return OK;
+}
 
-    *fenceFd = mMapper->unlock(handle);
+status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint64_t producerUsage,
+                                        uint64_t consumerUsage, const Rect& bounds, void** vaddr,
+                                        int fenceFd) {
+    return lockAsync(handle, android_convertGralloc1To0Usage(producerUsage, consumerUsage), bounds,
+                     vaddr, fenceFd);
+}
 
-    return NO_ERROR;
+status_t GraphicBufferMapper::lockAsyncYCbCr(buffer_handle_t handle, uint32_t usage,
+                                             const Rect& bounds, android_ycbcr* ycbcr,
+                                             int fenceFd) {
+    auto result = lockYCbCr(handle, static_cast<int64_t>(usage), bounds, unique_fd{fenceFd});
+    if (!result.has_value()) return result.asStatus();
+    *ycbcr = result.value();
+    return OK;
 }
 
 status_t GraphicBufferMapper::isSupported(uint32_t width, uint32_t height,
@@ -287,6 +314,17 @@ status_t GraphicBufferMapper::getPlaneLayouts(buffer_handle_t bufferHandle,
     return mMapper->getPlaneLayouts(bufferHandle, outPlaneLayouts);
 }
 
+ui::Result<std::vector<ui::PlaneLayout>> GraphicBufferMapper::getPlaneLayouts(
+        buffer_handle_t bufferHandle) {
+    std::vector<ui::PlaneLayout> temp;
+    status_t status = mMapper->getPlaneLayouts(bufferHandle, &temp);
+    if (status == OK) {
+        return std::move(temp);
+    } else {
+        return base::unexpected(ui::Error::statusToCode(status));
+    }
+}
+
 status_t GraphicBufferMapper::getDataspace(buffer_handle_t bufferHandle,
                                            ui::Dataspace* outDataspace) {
     return mMapper->getDataspace(bufferHandle, outDataspace);