gralloc: update lock and lockYCbCr

IMapper 4.0 does support lockYCbCr functionality through lock and
BufferMetadata getters. However, we will wait to add the support in
one central gralloc library. For now just stub out the call so
there aren't any compiler errors.

Bug: 141631415
Test: Compiles

Change-Id: I9d2f70c87412f8ac2114db85eb6dc01539876e2b

7 files changed, 29 insertions, 203 deletions

diff --git a/camera/common/1.0/default/HandleImporter.cpp b/camera/common/1.0/default/HandleImporter.cpp
index 76f97789c0..ac32c954c4 100644
--- a/camera/common/1.0/default/HandleImporter.cpp
+++ b/camera/common/1.0/default/HandleImporter.cpp
@@ -252,8 +252,7 @@ void* HandleImporter::lock(
         // No need to use bytesPerPixel and bytesPerStride because we are using
         // an 1-D buffer and accressRegion.
         mMapperV4->lock(buffer, cpuUsage, accessRegion, acquireFenceHandle,
-                        [&](const auto& tmpError, const auto& tmpPtr, const auto& /*bytesPerPixel*/,
-                            const auto& /*bytesPerStride*/) {
+                        [&](const auto& tmpError, const auto& tmpPtr) {
                             if (tmpError == MapperErrorV4::NONE) {
                                 ret = tmpPtr;
                             } else {
@@ -301,7 +300,13 @@ YCbCrLayout HandleImporter::lockYCbCr(
     }
 
     if (mMapperV4 != nullptr) {
-        return lockYCbCrInternal<IMapperV4, MapperErrorV4>(mMapperV4, buf, cpuUsage, accessRegion);
+        // No device currently supports IMapper 4.0 so it is safe to just return an error code here.
+        //
+        // This will be supported by a combination of lock and BufferMetadata getters. We are going
+        // to refactor all the IAllocator/IMapper versioning code into a shared library. We will
+        // then add the IMapper 4.0 lockYCbCr support then.
+        ALOGE("%s: MapperV4 doesn't support lockYCbCr directly!", __FUNCTION__);
+        return {};
     }
 
     if (mMapperV3 != nullptr) {
diff --git a/graphics/composer/2.1/utils/vts/ComposerVts.cpp b/graphics/composer/2.1/utils/vts/ComposerVts.cpp
index a0745cef60..a8e1480d4f 100644
--- a/graphics/composer/2.1/utils/vts/ComposerVts.cpp
+++ b/graphics/composer/2.1/utils/vts/ComposerVts.cpp
@@ -369,10 +369,7 @@ void* Gralloc::lock(const native_handle_t* bufferHandle, uint64_t cpuUsage,
         accessRegion.top = accessRegionRect.top;
         accessRegion.width = accessRegionRect.width;
         accessRegion.height = accessRegionRect.height;
-        int32_t bytesPerPixel;
-        int32_t bytesPerStride;
-        return mGralloc4->lock(bufferHandle, cpuUsage, accessRegion, acquireFence, &bytesPerPixel,
-                               &bytesPerStride);
+        return mGralloc4->lock(bufferHandle, cpuUsage, accessRegion, acquireFence);
     } else if (mGralloc3) {
         IMapper3::Rect accessRegion;
         accessRegion.left = accessRegionRect.left;
diff --git a/graphics/mapper/4.0/IMapper.hal b/graphics/mapper/4.0/IMapper.hal
index a5413826ce..98ac02c020 100644
--- a/graphics/mapper/4.0/IMapper.hal
+++ b/graphics/mapper/4.0/IMapper.hal
@@ -199,16 +199,20 @@ interface IMapper {
      * outside of @p accessRegion is undefined, except that it must not cause
      * process termination.
      *
+     * This function can lock both single-planar and multi-planar formats. The caller
+     * should use get() to get information about the buffer they are locking.
+     * get() can be used to get information about the planes, offsets, stride,
+     * etc.
+     *
+     * This function must also work on buffers with
+     * `AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_*` if supported by the device, as well
+     * as with any other formats requested by multimedia codecs when they are
+     * configured with a flexible-YUV-compatible color format.
+     *
      * On success, @p data must be filled with a pointer to the locked buffer
      * memory. This address will represent the top-left corner of the entire
      * buffer, even if @p accessRegion does not begin at the top-left corner.
      *
-     * On success, bytesPerPixel must contain the number of bytes per pixel in
-     * the buffer. If the bytesPerPixel is unknown or variable, a value of -1
-     * should be returned. bytesPerStride must contain the bytes per stride of
-     * the buffer. If the bytesPerStride is unknown or variable, a value of -1
-     * should be returned.
-     *
      * The locked buffer must adhere to the format requested at allocation time
      * in the BufferDescriptorInfo.
      *
@@ -231,55 +235,13 @@ interface IMapper {
      *     - `NO_RESOURCES` if the buffer cannot be locked at this time. Note
      *       that locking may succeed at a later time.
      * @return data CPU-accessible pointer to the buffer data.
-     * @return bytesPerPixel the number of bytes per pixel in the buffer
-     * @return bytesPerStride the number of bytes per stride of the buffer
      */
     lock(pointer buffer,
          uint64_t cpuUsage,
          Rect accessRegion,
          handle acquireFence)
             generates (Error error,
-                       pointer data,
-                       int32_t bytesPerPixel,
-                       int32_t bytesPerStride);
-
-    /**
-     * Locks a YCbCr buffer for the specified CPU usage.
-     *
-     * This is largely the same as lock(), except that instead of returning a
-     * pointer directly to the buffer data, it returns a `YCbCrLayout` struct
-     * describing how to access the data planes.
-     *
-     * This function must work on buffers with
-     * `AHARDWAREBUFFER_FORMAT_Y8Cb8Cr8_*` if supported by the device, as well
-     * as with any other formats requested by multimedia codecs when they are
-     * configured with a flexible-YUV-compatible color format.
-     *
-     * @param buffer Buffer to lock.
-     * @param cpuUsage CPU usage flags to request. See +ndk
-     *     libnativewindow#AHardwareBuffer_UsageFlags for possible values.
-     * @param accessRegion Portion of the buffer that the client intends to
-     *     access.
-     * @param acquireFence Handle containing a file descriptor referring to a
-     *     sync fence object, which will be signaled when it is safe for the
-     *     mapper to lock the buffer. @p acquireFence may be empty if it is
-     *     already safe to lock.
-     * @return error Error status of the call, which may be
-     *     - `NONE` upon success.
-     *     - `BAD_BUFFER` if the buffer is invalid or is incompatible with this
-     *       function.
-     *     - `BAD_VALUE` if @p cpuUsage is 0, contains non-CPU usage flags, or
-     *       is incompatible with the buffer.
-     *     - `NO_RESOURCES` if the buffer cannot be locked at this time. Note
-     *       that locking may succeed at a later time.
-     * @return layout Data layout of the locked buffer.
-     */
-    lockYCbCr(pointer buffer,
-              uint64_t cpuUsage,
-              Rect accessRegion,
-              handle acquireFence)
-            generates (Error error,
-                       YCbCrLayout layout);
+                       pointer data);
 
     /**
      * Unlocks a buffer to indicate all CPU accesses to the buffer have
diff --git a/graphics/mapper/4.0/types.hal b/graphics/mapper/4.0/types.hal
index 2fdfa6586b..00b660743a 100644
--- a/graphics/mapper/4.0/types.hal
+++ b/graphics/mapper/4.0/types.hal
@@ -53,32 +53,3 @@ enum Error : int32_t {
  */
 typedef vec<uint8_t> BufferDescriptor;
 
-/**
- * Structure for describing YCbCr formats for consumption by applications.
- * This is used with PixelFormat::YCBCR_*_888.
- *
- * Buffer chroma subsampling is defined in the format.
- * e.g. PixelFormat::YCBCR_420_888 has subsampling 4:2:0.
- *
- * Buffers must have a 8 bit depth.
- *
- * y, cb, and cr point to the first byte of their respective planes.
- *
- * Stride describes the distance in bytes from the first value of one row of
- * the image to the first value of the next row. It includes the width of the
- * image plus padding.
- * yStride is the stride of the luma plane.
- * cStride is the stride of the chroma planes.
- *
- * chromaStep is the distance in bytes from one chroma pixel value to the
- * next. This is 2 bytes for semiplanar (because chroma values are interleaved
- * and each chroma value is one byte) and 1 for planar.
- */
-struct YCbCrLayout {
-    pointer y;
-    pointer cb;
-    pointer cr;
-    uint32_t yStride;
-    uint32_t cStride;
-    uint32_t chromaStep;
-};
diff --git a/graphics/mapper/4.0/utils/vts/MapperVts.cpp b/graphics/mapper/4.0/utils/vts/MapperVts.cpp
index 531f31131c..8073e6924c 100644
--- a/graphics/mapper/4.0/utils/vts/MapperVts.cpp
+++ b/graphics/mapper/4.0/utils/vts/MapperVts.cpp
@@ -199,8 +199,7 @@ void Gralloc::freeBuffer(const native_handle_t* bufferHandle) {
 }
 
 void* Gralloc::lock(const native_handle_t* bufferHandle, uint64_t cpuUsage,
-                    const IMapper::Rect& accessRegion, int acquireFence, int32_t* outBytesPerPixel,
-                    int32_t* outBytesPerStride) {
+                    const IMapper::Rect& accessRegion, int acquireFence) {
     auto buffer = const_cast<native_handle_t*>(bufferHandle);
 
     NATIVE_HANDLE_DECLARE_STORAGE(acquireFenceStorage, 1, 0);
@@ -211,17 +210,11 @@ void* Gralloc::lock(const native_handle_t* bufferHandle, uint64_t cpuUsage,
         acquireFenceHandle = h;
     }
 
-    *outBytesPerPixel = -1;
-    *outBytesPerStride = -1;
-
     void* data = nullptr;
     mMapper->lock(buffer, cpuUsage, accessRegion, acquireFenceHandle,
-                  [&](const auto& tmpError, const auto& tmpData, int32_t tmpBytesPerPixel,
-                      int32_t tmpBytesPerStride) {
+                  [&](const auto& tmpError, const auto& tmpData) {
                       ASSERT_EQ(Error::NONE, tmpError) << "failed to lock buffer " << buffer;
                       data = tmpData;
-                      *outBytesPerPixel = tmpBytesPerPixel;
-                      *outBytesPerStride = tmpBytesPerStride;
                   });
 
     if (acquireFence >= 0) {
@@ -231,33 +224,6 @@ void* Gralloc::lock(const native_handle_t* bufferHandle, uint64_t cpuUsage,
     return data;
 }
 
-YCbCrLayout Gralloc::lockYCbCr(const native_handle_t* bufferHandle, uint64_t cpuUsage,
-                               const IMapper::Rect& accessRegion, int acquireFence) {
-    auto buffer = const_cast<native_handle_t*>(bufferHandle);
-
-    NATIVE_HANDLE_DECLARE_STORAGE(acquireFenceStorage, 1, 0);
-    hidl_handle acquireFenceHandle;
-    if (acquireFence >= 0) {
-        auto h = native_handle_init(acquireFenceStorage, 1, 0);
-        h->data[0] = acquireFence;
-        acquireFenceHandle = h;
-    }
-
-    YCbCrLayout layout = {};
-    mMapper->lockYCbCr(buffer, cpuUsage, accessRegion, acquireFenceHandle,
-                       [&](const auto& tmpError, const auto& tmpLayout) {
-                           ASSERT_EQ(Error::NONE, tmpError)
-                                   << "failed to lockYCbCr buffer " << buffer;
-                           layout = tmpLayout;
-                       });
-
-    if (acquireFence >= 0) {
-        close(acquireFence);
-    }
-
-    return layout;
-}
-
 int Gralloc::unlock(const native_handle_t* bufferHandle) {
     auto buffer = const_cast<native_handle_t*>(bufferHandle);
 
diff --git a/graphics/mapper/4.0/utils/vts/include/mapper-vts/4.0/MapperVts.h b/graphics/mapper/4.0/utils/vts/include/mapper-vts/4.0/MapperVts.h
index 28555fa05d..6251e6649c 100644
--- a/graphics/mapper/4.0/utils/vts/include/mapper-vts/4.0/MapperVts.h
+++ b/graphics/mapper/4.0/utils/vts/include/mapper-vts/4.0/MapperVts.h
@@ -70,10 +70,7 @@ class Gralloc {
     // in and out of the mapper.  The ownership of the fd is always transferred
     // with each of these functions.
     void* lock(const native_handle_t* bufferHandle, uint64_t cpuUsage,
-               const IMapper::Rect& accessRegion, int acquireFence, int32_t* outBytesPerPixel,
-               int32_t* outBytesPerStride);
-    YCbCrLayout lockYCbCr(const native_handle_t* bufferHandle, uint64_t cpuUsage,
-                          const IMapper::Rect& accessRegion, int acquireFence);
+               const IMapper::Rect& accessRegion, int acquireFence);
     int unlock(const native_handle_t* bufferHandle);
 
     bool validateBufferSize(const native_handle_t* bufferHandle,
diff --git a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp
index 1e8ec01e33..781476fba4 100644
--- a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp
+++ b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp
@@ -20,6 +20,9 @@
 #include <thread>
 #include <vector>
 
+//#include <aidl/android/hardware/graphics/common/BlendMode.h>
+//#include <aidl/android/hardware/graphics/common/Compression.h>
+
 #include <VtsHalHidlTargetTestBase.h>
 #include <android-base/logging.h>
 #include <gralloctypes/Gralloc4.h>
@@ -391,15 +394,8 @@ TEST_F(GraphicsMapperHidlTest, LockUnlockBasic) {
                                static_cast<int32_t>(info.height)};
     int fence = -1;
     uint8_t* data;
-    int32_t bytesPerPixel = -1;
-    int32_t bytesPerStride = -1;
     ASSERT_NO_FATAL_FAILURE(
-            data = static_cast<uint8_t*>(mGralloc->lock(bufferHandle, info.usage, region, fence,
-                                                        &bytesPerPixel, &bytesPerStride)));
-
-    // Valid return values are -1 for unsupported or the number bytes for supported which is >=0
-    EXPECT_GT(bytesPerPixel, -1);
-    EXPECT_GT(bytesPerStride, -1);
+            data = static_cast<uint8_t*>(mGralloc->lock(bufferHandle, info.usage, region, fence)));
 
     // RGBA_8888
     size_t strideInBytes = stride * 4;
@@ -412,13 +408,9 @@ TEST_F(GraphicsMapperHidlTest, LockUnlockBasic) {
 
     ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle));
 
-    bytesPerPixel = -1;
-    bytesPerStride = -1;
-
     // lock again for reading
     ASSERT_NO_FATAL_FAILURE(
-            data = static_cast<uint8_t*>(mGralloc->lock(bufferHandle, info.usage, region, fence,
-                                                        &bytesPerPixel, &bytesPerStride)));
+            data = static_cast<uint8_t*>(mGralloc->lock(bufferHandle, info.usage, region, fence)));
     for (uint32_t y = 0; y < info.height; y++) {
         for (size_t i = 0; i < writeInBytes; i++) {
             EXPECT_EQ(static_cast<uint8_t>(y), data[i]);
@@ -426,69 +418,6 @@ TEST_F(GraphicsMapperHidlTest, LockUnlockBasic) {
         data += strideInBytes;
     }
 
-    EXPECT_GT(bytesPerPixel, -1);
-    EXPECT_GT(bytesPerStride, -1);
-
-    ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle));
-    if (fence >= 0) {
-        close(fence);
-    }
-}
-
-/**
- * Test IMapper::lockYCbCr.  This locks a YV12 buffer, and makes sure we can
- * write to and read from it.
- */
-TEST_F(GraphicsMapperHidlTest, LockYCbCrBasic) {
-    auto info = mDummyDescriptorInfo;
-    info.format = PixelFormat::YV12;
-
-    const native_handle_t* bufferHandle;
-    uint32_t stride;
-    ASSERT_NO_FATAL_FAILURE(bufferHandle = mGralloc->allocate(info, true, false, &stride));
-
-    // lock buffer for writing
-    const IMapper::Rect region{0, 0, static_cast<int32_t>(info.width),
-                               static_cast<int32_t>(info.height)};
-    int fence = -1;
-    YCbCrLayout layout;
-    ASSERT_NO_FATAL_FAILURE(layout = mGralloc->lockYCbCr(bufferHandle, info.usage, region, fence));
-
-    auto yData = static_cast<uint8_t*>(layout.y);
-    auto cbData = static_cast<uint8_t*>(layout.cb);
-    auto crData = static_cast<uint8_t*>(layout.cr);
-    for (uint32_t y = 0; y < info.height; y++) {
-        for (uint32_t x = 0; x < info.width; x++) {
-            auto val = static_cast<uint8_t>(info.height * y + x);
-
-            yData[layout.yStride * y + x] = val;
-            if (y % 2 == 0 && x % 2 == 0) {
-                cbData[layout.cStride * y / 2 + x / 2] = val;
-                crData[layout.cStride * y / 2 + x / 2] = val;
-            }
-        }
-    }
-
-    ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle));
-
-    // lock again for reading
-    ASSERT_NO_FATAL_FAILURE(layout = mGralloc->lockYCbCr(bufferHandle, info.usage, region, fence));
-
-    yData = static_cast<uint8_t*>(layout.y);
-    cbData = static_cast<uint8_t*>(layout.cb);
-    crData = static_cast<uint8_t*>(layout.cr);
-    for (uint32_t y = 0; y < info.height; y++) {
-        for (uint32_t x = 0; x < info.width; x++) {
-            auto val = static_cast<uint8_t>(info.height * y + x);
-
-            EXPECT_EQ(val, yData[layout.yStride * y + x]);
-            if (y % 2 == 0 && x % 2 == 0) {
-                EXPECT_EQ(val, cbData[layout.cStride * y / 2 + x / 2]);
-                EXPECT_EQ(val, crData[layout.cStride * y / 2 + x / 2]);
-            }
-        }
-    }
-
     ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle));
     if (fence >= 0) {
         close(fence);
@@ -518,8 +447,7 @@ TEST_F(GraphicsMapperHidlTest, LockBadAccessRegion) {
 
     auto buffer = const_cast<native_handle_t*>(bufferHandle);
     mGralloc->getMapper()->lock(buffer, info.usage, accessRegion, acquireFenceHandle,
-                                [&](const auto& tmpError, const auto& /*tmpData*/,
-                                    int32_t /*tmpBytesPerPixel*/, int32_t /*tmpBytesPerStride*/) {
+                                [&](const auto& tmpError, const auto& /*tmpData*/) {
                                     EXPECT_EQ(Error::BAD_VALUE, tmpError)
                                             << "locking with a bad access region should fail";
                                 });