From c6394fffa80787a318d8c5ae569c27a9f2d5f061 Mon Sep 17 00:00:00 2001 From: Yichi Chen Date: Tue, 12 May 2020 08:58:08 +0800 Subject: [PATCH 1/4] gralloc4-vts: Allow YCRCB_420_SP to be unsupported in Lock_YCRCB_420_SP Some devices may not support the legacy YCRCB_420_SP format. To allow the test can pass through such devices, the patch adds the flexibility when UNSUPPORTED is returned from buffer allocation. Bug: 150461327 Bug: 152510209 Test: VtsHalGraphicsMapperV4_0Target Change-Id: I393fc3c4a7d2421f07eeff88915041c92e8cdf05 --- graphics/mapper/4.0/utils/vts/MapperVts.cpp | 76 +++++++++---------- .../vts/include/mapper-vts/4.0/MapperVts.h | 51 +++++++++++-- .../VtsHalGraphicsMapperV4_0TargetTest.cpp | 24 ++++-- 3 files changed, 98 insertions(+), 53 deletions(-) diff --git a/graphics/mapper/4.0/utils/vts/MapperVts.cpp b/graphics/mapper/4.0/utils/vts/MapperVts.cpp index 9f907e6bbe..5b2a94e65d 100644 --- a/graphics/mapper/4.0/utils/vts/MapperVts.cpp +++ b/graphics/mapper/4.0/utils/vts/MapperVts.cpp @@ -71,7 +71,8 @@ sp Gralloc::getAllocator() const { return mAllocator; } -const native_handle_t* Gralloc::cloneBuffer(const hidl_handle& rawHandle) { +const native_handle_t* Gralloc::cloneBuffer(const hidl_handle& rawHandle, + enum Tolerance /*tolerance*/) { const native_handle_t* bufferHandle = native_handle_clone(rawHandle.getNativeHandle()); EXPECT_NE(nullptr, bufferHandle); @@ -84,42 +85,37 @@ const native_handle_t* Gralloc::cloneBuffer(const hidl_handle& rawHandle) { std::vector Gralloc::allocate(const BufferDescriptor& descriptor, uint32_t count, bool import, - bool allowFailure, uint32_t* outStride) { + enum Tolerance tolerance, + uint32_t* outStride) { std::vector bufferHandles; bufferHandles.reserve(count); - mAllocator->allocate( - descriptor, count, - [&](const auto& tmpError, const auto& tmpStride, const auto& tmpBuffers) { - if (allowFailure && tmpError == Error::UNSUPPORTED) { - return; - } - ASSERT_EQ(Error::NONE, tmpError) << "failed to allocate buffers"; - ASSERT_EQ(count, tmpBuffers.size()) << "invalid buffer array"; + mAllocator->allocate(descriptor, count, + [&](const auto& tmpError, const auto& tmpStride, const auto& tmpBuffers) { + if (canTolerate(tolerance, tmpError)) { + return; + } - for (uint32_t i = 0; i < count; i++) { - const native_handle_t* bufferHandle = nullptr; - if (import) { - if (allowFailure) { - bufferHandle = importBuffer(tmpBuffers[i]); - } else { - ASSERT_NO_FATAL_FAILURE(bufferHandle = importBuffer(tmpBuffers[i])); - } - } else { - if (allowFailure) { - bufferHandle = cloneBuffer(tmpBuffers[i]); - } else { - ASSERT_NO_FATAL_FAILURE(bufferHandle = cloneBuffer(tmpBuffers[i])); - } - } - if (bufferHandle) { - bufferHandles.push_back(bufferHandle); - } - } + ASSERT_EQ(Error::NONE, tmpError) << "failed to allocate buffers"; + ASSERT_EQ(count, tmpBuffers.size()) << "invalid buffer array"; - if (outStride) { - *outStride = tmpStride; - } - }); + for (uint32_t i = 0; i < count; i++) { + const native_handle_t* bufferHandle = nullptr; + if (import) { + ASSERT_NO_FATAL_FAILURE( + bufferHandle = importBuffer(tmpBuffers[i], tolerance)); + } else { + ASSERT_NO_FATAL_FAILURE( + bufferHandle = cloneBuffer(tmpBuffers[i], tolerance)); + } + if (bufferHandle) { + bufferHandles.push_back(bufferHandle); + } + } + + if (outStride) { + *outStride = tmpStride; + } + }); if (::testing::Test::HasFatalFailure()) { bufferHandles.clear(); @@ -129,13 +125,14 @@ std::vector Gralloc::allocate(const BufferDescriptor& de } const native_handle_t* Gralloc::allocate(const IMapper::BufferDescriptorInfo& descriptorInfo, - bool import, bool allowFailure, uint32_t* outStride) { + bool import, enum Tolerance tolerance, + uint32_t* outStride) { BufferDescriptor descriptor = createDescriptor(descriptorInfo); if (::testing::Test::HasFatalFailure()) { return nullptr; } - auto buffers = allocate(descriptor, 1, import, allowFailure, outStride); + auto buffers = allocate(descriptor, 1, import, tolerance, outStride); if (::testing::Test::HasFatalFailure()) { return nullptr; } @@ -160,11 +157,14 @@ BufferDescriptor Gralloc::createDescriptor(const IMapper::BufferDescriptorInfo& return descriptor; } -const native_handle_t* Gralloc::importBuffer(const hidl_handle& rawHandle) { +const native_handle_t* Gralloc::importBuffer(const hidl_handle& rawHandle, + enum Tolerance tolerance) { const native_handle_t* bufferHandle = nullptr; mMapper->importBuffer(rawHandle, [&](const auto& tmpError, const auto& tmpBuffer) { - ASSERT_EQ(Error::NONE, tmpError) - << "failed to import buffer %p" << rawHandle.getNativeHandle(); + if (!canTolerate(tolerance, tmpError)) { + ASSERT_EQ(Error::NONE, tmpError) + << "failed to import buffer %p" << rawHandle.getNativeHandle(); + } bufferHandle = static_cast(tmpBuffer); }); 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 cd40aa4151..22a935f260 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 @@ -37,6 +37,16 @@ using android::hardware::graphics::allocator::V4_0::IAllocator; // A wrapper to IAllocator and IMapper. class Gralloc { public: + enum class Tolerance : uint32_t { + kToleranceStrict = 0x0U, + kToleranceBadDescriptor = 0x1U << std::underlying_type_t(Error::BAD_DESCRIPTOR), + kToleranceBadBuffer = 0x1U << std::underlying_type_t(Error::BAD_BUFFER), + kToleranceBadValue = 0x1U << std::underlying_type_t(Error::BAD_VALUE), + kToleranceNoResource = 0x1U << std::underlying_type_t(Error::NO_RESOURCES), + kToleranceUnSupported = 0x1U << std::underlying_type_t(Error::UNSUPPORTED), + kToleranceAllErrors = ~0x0U, + }; + Gralloc(const std::string& allocatorServiceName = "default", const std::string& mapperServiceName = "default", bool errOnFailure = true); ~Gralloc(); @@ -49,12 +59,27 @@ class Gralloc { // is true, the returned buffers are also imported into the mapper. // // Either case, the returned buffers must be freed with freeBuffer. - std::vector allocate(const BufferDescriptor& descriptor, uint32_t count, - bool import = true, bool allowFailure = false, - uint32_t* outStride = nullptr); + std::vector allocate( + const BufferDescriptor& descriptor, uint32_t count, bool import = true, + enum Tolerance tolerance = Tolerance::kToleranceStrict, uint32_t* outStride = nullptr); + const native_handle_t* allocate(const IMapper::BufferDescriptorInfo& descriptorInfo, - bool import = true, bool allowFailure = false, - uint32_t* outStride = nullptr); + bool import, enum Tolerance tolerance, uint32_t* outStride); + + const native_handle_t* allocate(const IMapper::BufferDescriptorInfo& descriptorInfo, + bool import) { + return allocate(descriptorInfo, import, Tolerance::kToleranceStrict); + } + + const native_handle_t* allocate(const IMapper::BufferDescriptorInfo& descriptorInfo, + bool import, enum Tolerance tolerance) { + return allocate(descriptorInfo, import, tolerance, nullptr); + } + + const native_handle_t* allocate(const IMapper::BufferDescriptorInfo& descriptorInfo, + bool import, uint32_t* outStride) { + return allocate(descriptorInfo, import, Tolerance::kToleranceStrict, outStride); + } // IMapper methods @@ -62,7 +87,11 @@ class Gralloc { BufferDescriptor createDescriptor(const IMapper::BufferDescriptorInfo& descriptorInfo); - const native_handle_t* importBuffer(const hidl_handle& rawHandle); + const native_handle_t* importBuffer(const hidl_handle& rawHandle, enum Tolerance tolerance); + const native_handle_t* importBuffer(const hidl_handle& rawHandle) { + return importBuffer(rawHandle, Tolerance::kToleranceStrict); + } + void freeBuffer(const native_handle_t* bufferHandle); // We use fd instead of hidl_handle in these functions to pass fences @@ -96,11 +125,19 @@ class Gralloc { uint64_t* outReservedSize); private: + bool canTolerate(Tolerance tolerance, Error error) { + return (std::underlying_type_t(tolerance) & + 0x1U << std::underlying_type_t(error)) != 0; + } + void init(const std::string& allocatorServiceName, const std::string& mapperServiceName); // initialize without checking for failure to get service void initNoErr(const std::string& allocatorServiceName, const std::string& mapperServiceName); - const native_handle_t* cloneBuffer(const hidl_handle& rawHandle); + const native_handle_t* cloneBuffer(const hidl_handle& rawHandle, enum Tolerance tolerance); + const native_handle_t* cloneBuffer(const hidl_handle& rawHandle) { + return cloneBuffer(rawHandle, Tolerance::kToleranceStrict); + } sp mAllocator; sp mMapper; diff --git a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp index 5ab647f7b3..f5c4cb9942 100644 --- a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp +++ b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp @@ -42,6 +42,7 @@ namespace { using android::hardware::graphics::common::V1_2::BufferUsage; using android::hardware::graphics::common::V1_2::PixelFormat; +using Tolerance = ::android::hardware::graphics::mapper::V4_0::vts::Gralloc::Tolerance; using MetadataType = android::hardware::graphics::mapper::V4_0::IMapper::MetadataType; using aidl::android::hardware::graphics::common::BlendMode; using aidl::android::hardware::graphics::common::Cta861_3; @@ -331,8 +332,9 @@ TEST_P(GraphicsMapperHidlTest, AllocatorAllocate) { for (uint32_t count = 0; count < 5; count++) { std::vector bufferHandles; uint32_t stride; - ASSERT_NO_FATAL_FAILURE( - bufferHandles = mGralloc->allocate(descriptor, count, false, false, &stride)); + ASSERT_NO_FATAL_FAILURE(bufferHandles = + mGralloc->allocate(descriptor, count, false, + Tolerance::kToleranceStrict, &stride)); if (count >= 1) { EXPECT_LE(mDummyDescriptorInfo.width, stride) << "invalid buffer stride"; @@ -532,7 +534,8 @@ TEST_P(GraphicsMapperHidlTest, LockUnlockBasic) { const native_handle_t* bufferHandle; uint32_t stride; - ASSERT_NO_FATAL_FAILURE(bufferHandle = mGralloc->allocate(info, true, false, &stride)); + ASSERT_NO_FATAL_FAILURE( + bufferHandle = mGralloc->allocate(info, true, Tolerance::kToleranceStrict, &stride)); // lock buffer for writing const IMapper::Rect region{0, 0, static_cast(info.width), @@ -566,7 +569,12 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { const native_handle_t* bufferHandle; uint32_t stride; - ASSERT_NO_FATAL_FAILURE(bufferHandle = mGralloc->allocate(info, true, false, &stride)); + ASSERT_NO_FATAL_FAILURE(bufferHandle = mGralloc->allocate( + info, true, Tolerance::kToleranceUnSupported, &stride)); + if (bufferHandle == nullptr) { + GTEST_SUCCEED() << "YCRCB_420_SP format is unsupported"; + return; + } // lock buffer for writing const IMapper::Rect region{0, 0, static_cast(info.width), @@ -741,8 +749,8 @@ TEST_P(GraphicsMapperHidlTest, FlushRereadBasic) { const native_handle_t* rawHandle; uint32_t stride; - ASSERT_NO_FATAL_FAILURE( - rawHandle = mGralloc->allocate(mDummyDescriptorInfo, false, false, &stride)); + ASSERT_NO_FATAL_FAILURE(rawHandle = mGralloc->allocate(mDummyDescriptorInfo, false, + Tolerance::kToleranceStrict, &stride)); const native_handle_t* writeBufferHandle; const native_handle_t* readBufferHandle; @@ -964,7 +972,7 @@ TEST_P(GraphicsMapperHidlTest, GetProtectedContent) { info.usage = BufferUsage::PROTECTED | BufferUsage::COMPOSER_OVERLAY; const native_handle_t* bufferHandle = nullptr; - bufferHandle = mGralloc->allocate(info, true, true); + bufferHandle = mGralloc->allocate(info, true, Tolerance::kToleranceAllErrors); if (!bufferHandle) { GTEST_SUCCEED() << "unable to allocate protected content"; return; @@ -1267,7 +1275,7 @@ TEST_P(GraphicsMapperHidlTest, SetProtectedContent) { auto info = mDummyDescriptorInfo; info.usage = BufferUsage::PROTECTED | BufferUsage::COMPOSER_OVERLAY; - bufferHandle = mGralloc->allocate(info, true, true); + bufferHandle = mGralloc->allocate(info, true, Tolerance::kToleranceAllErrors); if (!bufferHandle) { GTEST_SUCCEED() << "unable to allocate protected content"; return; From f4d374a7587e0109c8ff541ece13900150a258b3 Mon Sep 17 00:00:00 2001 From: Yichi Chen Date: Tue, 28 Apr 2020 21:52:20 +0800 Subject: [PATCH 2/4] gralloc4-vts: Extract YCbCr888 data operation from Lock_YCRCB_420_SP The patch extracts the common operation on YCbCr888 data to allow a better reuse in tests with other YCbCr color formats Bug: 150461327 Bug: 152510209 Test: VtsHalGraphicsMapperV4_0Target Change-Id: I530f6d895c338fb041f7705aa9a4fd36931a1588 --- .../VtsHalGraphicsMapperV4_0TargetTest.cpp | 99 ++++++++++--------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp index f5c4cb9942..77ed3cb885 100644 --- a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp +++ b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp @@ -277,7 +277,7 @@ class GraphicsMapperHidlTest } } - void verifyRGBA8888(const native_handle_t* bufferHandle, uint8_t* data, uint32_t height, + void verifyRGBA8888(const native_handle_t* bufferHandle, const uint8_t* data, uint32_t height, size_t strideInBytes, size_t widthInBytes, uint32_t seed = 0) { hidl_vec vec; ASSERT_EQ(Error::NONE, @@ -295,6 +295,49 @@ class GraphicsMapperHidlTest } } + void traverseYCbCr888Data(const android_ycbcr& yCbCr, int32_t width, int32_t height, + int64_t hSubsampling, int64_t vSubsampling, + std::function traverseFuncion) { + auto yData = static_cast(yCbCr.y); + auto cbData = static_cast(yCbCr.cb); + auto crData = static_cast(yCbCr.cr); + auto yStride = yCbCr.ystride; + auto cStride = yCbCr.cstride; + auto chromaStep = yCbCr.chroma_step; + + for (uint32_t y = 0; y < height; y++) { + for (uint32_t x = 0; x < width; x++) { + auto val = static_cast(height * y + x); + + traverseFuncion(yData + yStride * y + x, val); + + if (y % vSubsampling == 0 && x % hSubsampling == 0) { + uint32_t subSampleX = x / hSubsampling; + uint32_t subSampleY = y / vSubsampling; + const auto subSampleOffset = cStride * subSampleY + chromaStep * subSampleX; + const auto subSampleVal = + static_cast(height * subSampleY + subSampleX); + + traverseFuncion(cbData + subSampleOffset, subSampleVal); + traverseFuncion(crData + subSampleOffset, subSampleVal + 1); + } + } + } + } + + void fillYCbCr888Data(const android_ycbcr& yCbCr, int32_t width, int32_t height, + int64_t hSubsampling, int64_t vSubsampling) { + traverseYCbCr888Data(yCbCr, width, height, hSubsampling, vSubsampling, + [](auto address, auto fillingData) { *address = fillingData; }); + } + + void verifyYCbCr888Data(const android_ycbcr& yCbCr, int32_t width, int32_t height, + int64_t hSubsampling, int64_t vSubsampling) { + traverseYCbCr888Data( + yCbCr, width, height, hSubsampling, vSubsampling, + [](auto address, auto expectedData) { EXPECT_EQ(*address, expectedData); }); + } + bool isEqual(float a, float b) { return abs(a - b) < 0.0001; } std::unique_ptr mGralloc; @@ -591,37 +634,16 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { ASSERT_NO_FATAL_FAILURE( getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); - auto yData = static_cast(yCbCr.y); - auto cbData = static_cast(yCbCr.cb); - auto crData = static_cast(yCbCr.cr); - auto yStride = yCbCr.ystride; - auto cStride = yCbCr.cstride; - auto chromaStep = yCbCr.chroma_step; - constexpr uint32_t kCbCrSubSampleFactor = 2; - ASSERT_EQ(crData + 1, cbData); - ASSERT_EQ(2, chromaStep); ASSERT_EQ(kCbCrSubSampleFactor, hSubsampling); ASSERT_EQ(kCbCrSubSampleFactor, vSubsampling); - for (uint32_t y = 0; y < info.height; y++) { - for (uint32_t x = 0; x < info.width; x++) { - auto val = static_cast(info.height * y + x); + auto cbData = static_cast(yCbCr.cb); + auto crData = static_cast(yCbCr.cr); + ASSERT_EQ(crData + 1, cbData); + ASSERT_EQ(2, yCbCr.chroma_step); - yData[yStride * y + x] = val; - - if (y % vSubsampling == 0 && x % hSubsampling == 0) { - uint32_t subSampleX = x / hSubsampling; - uint32_t subSampleY = y / vSubsampling; - const auto subSampleOffset = cStride * subSampleY + chromaStep * subSampleX; - const auto subSampleVal = - static_cast(info.height * subSampleY + subSampleX); - - cbData[subSampleOffset] = subSampleVal; - crData[subSampleOffset] = subSampleVal + 1; - } - } - } + fillYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); @@ -632,28 +654,7 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { ASSERT_NO_FATAL_FAILURE( getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); - yData = static_cast(yCbCr.y); - cbData = static_cast(yCbCr.cb); - crData = static_cast(yCbCr.cr); - - for (uint32_t y = 0; y < info.height; y++) { - for (uint32_t x = 0; x < info.width; x++) { - auto val = static_cast(info.height * y + x); - - EXPECT_EQ(val, yData[yStride * y + x]); - - if (y % vSubsampling == 0 && x % hSubsampling == 0) { - uint32_t subSampleX = x / hSubsampling; - uint32_t subSampleY = y / vSubsampling; - const auto subSampleOffset = cStride * subSampleY + chromaStep * subSampleX; - const auto subSampleVal = - static_cast(info.height * subSampleY + subSampleX); - - EXPECT_EQ(subSampleVal, cbData[subSampleOffset]); - EXPECT_EQ(subSampleVal + 1, crData[subSampleOffset]); - } - } - } + verifyYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); if (fence >= 0) { From ad8b9ad68c76cf5675cd6579dc7a5bb0b9c1683f Mon Sep 17 00:00:00 2001 From: Yichi Chen Date: Tue, 28 Apr 2020 21:59:00 +0800 Subject: [PATCH 3/4] gralloc4-vts: Restore Lock_YCBCR_420_888 and create Lock_YV12 The patch adds back Lock_YCBCR_420_888 and also creates Lock_YV12 to verify planner format with the checks on subsampling, chroma step, and CbCr offsets. Bug: 150461327 Bug: 152510209 Test: VtsHalGraphicsMapperV4_0Target Change-Id: I7def937e9b65e99569a3dc2230e7e929477c06eb --- .../VtsHalGraphicsMapperV4_0TargetTest.cpp | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp index 77ed3cb885..891a3dc9ec 100644 --- a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp +++ b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp @@ -662,6 +662,105 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { } } +TEST_P(GraphicsMapperHidlTest, Lock_YV12) { + auto info = mDummyDescriptorInfo; + info.format = PixelFormat::YV12; + + const native_handle_t* bufferHandle; + uint32_t stride; + ASSERT_NO_FATAL_FAILURE( + bufferHandle = mGralloc->allocate(info, true, Tolerance::kToleranceStrict, &stride)); + + // lock buffer for writing + const IMapper::Rect region{0, 0, static_cast(info.width), + static_cast(info.height)}; + int fence = -1; + uint8_t* data; + + ASSERT_NO_FATAL_FAILURE( + data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + + android_ycbcr yCbCr; + int64_t hSubsampling = 0; + int64_t vSubsampling = 0; + ASSERT_NO_FATAL_FAILURE( + getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); + + constexpr uint32_t kCbCrSubSampleFactor = 2; + ASSERT_EQ(kCbCrSubSampleFactor, hSubsampling); + ASSERT_EQ(kCbCrSubSampleFactor, vSubsampling); + + auto cbData = static_cast(yCbCr.cb); + auto crData = static_cast(yCbCr.cr); + ASSERT_EQ(crData + yCbCr.cstride * info.height / vSubsampling, cbData); + ASSERT_EQ(1, yCbCr.chroma_step); + + fillYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); + + ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + + // lock again for reading + ASSERT_NO_FATAL_FAILURE( + data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + + ASSERT_NO_FATAL_FAILURE( + getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); + + verifyYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); + + ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + if (fence >= 0) { + close(fence); + } +} + +TEST_P(GraphicsMapperHidlTest, Lock_YCBCR_420_888) { + auto info = mDummyDescriptorInfo; + info.format = PixelFormat::YCBCR_420_888; + + const native_handle_t* bufferHandle; + uint32_t stride; + ASSERT_NO_FATAL_FAILURE( + bufferHandle = mGralloc->allocate(info, true, Tolerance::kToleranceStrict, &stride)); + + // lock buffer for writing + const IMapper::Rect region{0, 0, static_cast(info.width), + static_cast(info.height)}; + int fence = -1; + uint8_t* data; + + ASSERT_NO_FATAL_FAILURE( + data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + + android_ycbcr yCbCr; + int64_t hSubsampling = 0; + int64_t vSubsampling = 0; + ASSERT_NO_FATAL_FAILURE( + getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); + + constexpr uint32_t kCbCrSubSampleFactor = 2; + ASSERT_EQ(kCbCrSubSampleFactor, hSubsampling); + ASSERT_EQ(kCbCrSubSampleFactor, vSubsampling); + + fillYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); + + ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + + // lock again for reading + ASSERT_NO_FATAL_FAILURE( + data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + + ASSERT_NO_FATAL_FAILURE( + getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); + + verifyYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); + + ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + if (fence >= 0) { + close(fence); + } +} + /** * Test IMapper::unlock with bad access region */ From 5bec8ba8a289c9cc9b8f8dfec55b5be7e7e6a75d Mon Sep 17 00:00:00 2001 From: Yichi Chen Date: Fri, 8 May 2020 22:22:31 +0800 Subject: [PATCH 4/4] gralloc4-vts: Hold fence by unique_fd to avoid leakage There was fd leakage when fence was assigned to other values without releasing previous file descriptor. The patch introduces unique_fd to hold the fence without leaking fd. Bug: 150461327 Bug: 152510209 Test: VtsHalGraphicsMapperV4_0Target Change-Id: I926887ccd2c626da2d2c1a6b9d7cd433f9d5b717 --- .../VtsHalGraphicsMapperV4_0TargetTest.cpp | 86 ++++++++----------- 1 file changed, 35 insertions(+), 51 deletions(-) diff --git a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp index 891a3dc9ec..9e56534f88 100644 --- a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp +++ b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -40,6 +41,7 @@ namespace V4_0 { namespace vts { namespace { +using ::android::base::unique_fd; using android::hardware::graphics::common::V1_2::BufferUsage; using android::hardware::graphics::common::V1_2::PixelFormat; using Tolerance = ::android::hardware::graphics::mapper::V4_0::vts::Gralloc::Tolerance; @@ -583,27 +585,24 @@ TEST_P(GraphicsMapperHidlTest, LockUnlockBasic) { // lock buffer for writing const IMapper::Rect region{0, 0, static_cast(info.width), static_cast(info.height)}; - int fence = -1; + unique_fd fence; uint8_t* data; - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); // RGBA_8888 fillRGBA8888(data, info.height, stride * 4, info.width * 4); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); // lock again for reading - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); ASSERT_NO_FATAL_FAILURE( verifyRGBA8888(bufferHandle, data, info.height, stride * 4, info.width * 4)); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); - if (fence >= 0) { - close(fence); - } + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); } TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { @@ -622,11 +621,11 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { // lock buffer for writing const IMapper::Rect region{0, 0, static_cast(info.width), static_cast(info.height)}; - int fence = -1; + unique_fd fence; uint8_t* data; - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); android_ycbcr yCbCr; int64_t hSubsampling = 0; @@ -645,21 +644,18 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCRCB_420_SP) { fillYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); // lock again for reading - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); ASSERT_NO_FATAL_FAILURE( getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); verifyYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); - if (fence >= 0) { - close(fence); - } + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); } TEST_P(GraphicsMapperHidlTest, Lock_YV12) { @@ -674,11 +670,11 @@ TEST_P(GraphicsMapperHidlTest, Lock_YV12) { // lock buffer for writing const IMapper::Rect region{0, 0, static_cast(info.width), static_cast(info.height)}; - int fence = -1; + unique_fd fence; uint8_t* data; - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); android_ycbcr yCbCr; int64_t hSubsampling = 0; @@ -697,21 +693,18 @@ TEST_P(GraphicsMapperHidlTest, Lock_YV12) { fillYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); // lock again for reading - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); ASSERT_NO_FATAL_FAILURE( getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); verifyYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); - if (fence >= 0) { - close(fence); - } + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); } TEST_P(GraphicsMapperHidlTest, Lock_YCBCR_420_888) { @@ -726,11 +719,11 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCBCR_420_888) { // lock buffer for writing const IMapper::Rect region{0, 0, static_cast(info.width), static_cast(info.height)}; - int fence = -1; + unique_fd fence; uint8_t* data; - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); android_ycbcr yCbCr; int64_t hSubsampling = 0; @@ -744,21 +737,18 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCBCR_420_888) { fillYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); // lock again for reading - ASSERT_NO_FATAL_FAILURE( - data = static_cast(mGralloc->lock(bufferHandle, info.usage, region, fence))); + ASSERT_NO_FATAL_FAILURE(data = static_cast( + mGralloc->lock(bufferHandle, info.usage, region, fence.get()))); ASSERT_NO_FATAL_FAILURE( getAndroidYCbCr(bufferHandle, data, &yCbCr, &hSubsampling, &vSubsampling)); verifyYCbCr888Data(yCbCr, info.width, info.height, hSubsampling, vSubsampling); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(bufferHandle)); - if (fence >= 0) { - close(fence); - } + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(bufferHandle))); } /** @@ -874,11 +864,10 @@ TEST_P(GraphicsMapperHidlTest, FlushRereadBasic) { fillRGBA8888(writeData, info.height, stride * 4, info.width * 4); - int fence; - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->flushLockedBuffer(writeBufferHandle)); + unique_fd fence; + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->flushLockedBuffer(writeBufferHandle))); if (fence >= 0) { ASSERT_EQ(0, sync_wait(fence, 3500)); - close(fence); } ASSERT_NO_FATAL_FAILURE(mGralloc->rereadLockedBuffer(readBufferHandle)); @@ -886,14 +875,9 @@ TEST_P(GraphicsMapperHidlTest, FlushRereadBasic) { ASSERT_NO_FATAL_FAILURE( verifyRGBA8888(readBufferHandle, readData, info.height, stride * 4, info.width * 4)); - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(readBufferHandle)); - if (fence >= 0) { - close(fence); - } - ASSERT_NO_FATAL_FAILURE(fence = mGralloc->unlock(writeBufferHandle)); - if (fence >= 0) { - close(fence); - } + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(readBufferHandle))); + + ASSERT_NO_FATAL_FAILURE(fence.reset(mGralloc->unlock(writeBufferHandle))); } /**