From 34445d98ede19218aadd01bec0fe9f4c48ad8769 Mon Sep 17 00:00:00 2001 From: Jason Macnak Date: Fri, 13 Mar 2020 12:18:08 -0700 Subject: [PATCH] gralloc4-vts: fix Lock_YCBCR_420_888 test Updates Cb and Cr plane indexing. The existing code seems to use chromaStep as a sub-sampling factor and seems to assume that the underlying format is tri-planar. Updates Cb and Cr value checking. The existing code would write values into the Cb and Cr planes using a value derived from even-x-value and odd-y-value full-image-coordinates (e.g. (0,1)) but then check against the values in the Cb and Cr planes using a value derived from even-x-value and even-y-value coordinates (e.g. (0,0)). Updates y-plane sample increment check to confirm that it is multiple of 8. I don't see any requirements stating this needs to be 32 bits. Bug: b/146515640 Test: VtsHalGraphicsMapperV4_0Target Change-Id: Ia9e496ae43b2d1ac9ea8d57a4d4fc55f0be7b2b6 --- .../VtsHalGraphicsMapperV4_0TargetTest.cpp | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp index 6cc5e34326..82dbeb9c20 100644 --- a/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp +++ b/graphics/mapper/4.0/vts/functional/VtsHalGraphicsMapperV4_0TargetTest.cpp @@ -221,7 +221,7 @@ class GraphicsMapperHidlTest case PlaneLayoutComponentType::Y: ASSERT_EQ(nullptr, outYCbCr->y); ASSERT_EQ(8, planeLayoutComponent.sizeInBits); - ASSERT_EQ(32, planeLayout.sampleIncrementInBits); + ASSERT_EQ(8, planeLayout.sampleIncrementInBits); outYCbCr->y = tmpData; outYCbCr->ystride = planeLayout.strideInBytes; break; @@ -569,15 +569,21 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCBCR_420_888) { auto cStride = yCbCr.cstride; auto chromaStep = yCbCr.chroma_step; + constexpr uint32_t kCbCrSubSampleFactor = 2; + 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); yData[yStride * y + x] = val; - if (y % chromaStep && x % chromaStep == 0) { - cbData[cStride * y / chromaStep + x / chromaStep] = val; - crData[cStride * y / chromaStep + x / chromaStep] = val; + if (y % kCbCrSubSampleFactor && x % kCbCrSubSampleFactor == 0) { + uint32_t subSampleX = x / kCbCrSubSampleFactor; + uint32_t subSampleY = y / kCbCrSubSampleFactor; + auto subSampleVal = static_cast(info.height * subSampleY + subSampleX); + + cbData[cStride * subSampleY + chromaStep * subSampleX] = subSampleVal; + crData[cStride * subSampleY + chromaStep * subSampleX] = subSampleVal; } } } @@ -599,9 +605,13 @@ TEST_P(GraphicsMapperHidlTest, Lock_YCBCR_420_888) { EXPECT_EQ(val, yData[yStride * y + x]); - if (y % chromaStep == 0 && x % chromaStep == 0) { - EXPECT_EQ(val, cbData[cStride * y / chromaStep + x / chromaStep]); - EXPECT_EQ(val, crData[cStride * y / chromaStep + x / chromaStep]); + if (y % kCbCrSubSampleFactor == 0 && x % kCbCrSubSampleFactor == 0) { + uint32_t subSampleX = x / kCbCrSubSampleFactor; + uint32_t subSampleY = y / kCbCrSubSampleFactor; + auto subSampleVal = static_cast(info.height * subSampleY + subSampleX); + + EXPECT_EQ(subSampleVal, cbData[cStride * subSampleY + chromaStep * subSampleX]); + EXPECT_EQ(subSampleVal, crData[cStride * subSampleY + chromaStep * subSampleX]); } } }