From 749f5af09229e5a358880817ac5ef33a14253be6 Mon Sep 17 00:00:00 2001 From: John Reck Date: Mon, 28 Nov 2022 19:53:12 -0500 Subject: [PATCH] Add some more tests & tweak spec around SMPTE2094-40 Test: this Change-Id: If7c549b8efcf490859f10c225a700188b76a54fa --- .../graphics/common/StandardMetadataType.aidl | 6 +- .../hardware/graphics/mapper/IMapper.h | 22 +- ...VtsHalGraphicsMapperStableC_TargetTest.cpp | 202 +++++++++++++++++- 3 files changed, 215 insertions(+), 15 deletions(-) diff --git a/graphics/common/aidl/android/hardware/graphics/common/StandardMetadataType.aidl b/graphics/common/aidl/android/hardware/graphics/common/StandardMetadataType.aidl index 8126143dd0..4bca795602 100644 --- a/graphics/common/aidl/android/hardware/graphics/common/StandardMetadataType.aidl +++ b/graphics/common/aidl/android/hardware/graphics/common/StandardMetadataType.aidl @@ -22,9 +22,9 @@ package android.hardware.graphics.common; * This is an enum that defines the common types of gralloc 4 buffer metadata. The comments for * each enum include a description of the metadata that is associated with the type. * - * IMapper@4.x must support getting the following standard buffer metadata types, with the exception - * of SMPTE 2094-10 metadata. IMapper@4.x may support setting these standard buffer metadata types - * as well. + * IMapper@4.x & later must support getting the following standard buffer metadata types, with the + * exception of SMPTE 2094-10 and SMPTE 2094-40 metadata. IMapper@4.x & later may support setting + * these standard buffer metadata types as well. * * When encoding these StandardMetadataTypes into a byte stream, the associated MetadataType is * is first encoded followed by the StandardMetadataType value. The MetadataType is encoded by diff --git a/graphics/mapper/stable-c/include/android/hardware/graphics/mapper/IMapper.h b/graphics/mapper/stable-c/include/android/hardware/graphics/mapper/IMapper.h index f27b0f4ce7..0f6d146fb8 100644 --- a/graphics/mapper/stable-c/include/android/hardware/graphics/mapper/IMapper.h +++ b/graphics/mapper/stable-c/include/android/hardware/graphics/mapper/IMapper.h @@ -509,11 +509,12 @@ typedef struct AIMapperV5 { * particular Metadata field. * * The framework will attempt to set the following StandardMetadataType - * values: DATASPACE, SMPTE2086, CTA861_3, SMPTE2094_40 and BLEND_MODE. - * We require everyone to support setting those fields. If a device's Composer - * implementation supports a field, it should be supported here. Over time these - * metadata fields will be moved out of Composer/BufferQueue/etc. and into the - * buffer's Metadata fields. + * values: DATASPACE, SMPTE2086, CTA861_3, and BLEND_MODE. + * We require everyone to support setting those fields. Framework will also attempt to set + * SMPTE2094_40 and SMPTE2094_10 if available, and it is required to support setting those + * if it is possible to get them. If a device's Composer implementation supports a field, + * it should be supported here. Over time these metadata fields will be moved out of + * Composer/BufferQueue/etc. and into the buffer's Metadata fields. * * @param buffer Buffer receiving desired metadata * @param metadataType MetadataType for the metadata value being set @@ -546,11 +547,12 @@ typedef struct AIMapperV5 { * particular Metadata field. * * The framework will attempt to set the following StandardMetadataType - * values: DATASPACE, SMPTE2086, CTA861_3, SMPTE2094_40 and BLEND_MODE. - * We require everyone to support setting those fields. If a device's Composer - * implementation supports a field, it should be supported here. Over time these - * metadata fields will be moved out of Composer/BufferQueue/etc. and into the - * buffer's Metadata fields. + * values: DATASPACE, SMPTE2086, CTA861_3, and BLEND_MODE. + * We require everyone to support setting those fields. Framework will also attempt to set + * SMPTE2094_40 and SMPTE2094_10 if available, and it is required to support setting those + * if it is possible to get them. If a device's Composer implementation supports a field, + * it should be supported here. Over time these metadata fields will be moved out of + * Composer/BufferQueue/etc. and into the buffer's Metadata fields. * * @param buffer Buffer receiving desired metadata * @param standardMetadataType StandardMetadataType for the metadata value being set diff --git a/graphics/mapper/stable-c/vts/VtsHalGraphicsMapperStableC_TargetTest.cpp b/graphics/mapper/stable-c/vts/VtsHalGraphicsMapperStableC_TargetTest.cpp index 6ab11a305d..326346cf8e 100644 --- a/graphics/mapper/stable-c/vts/VtsHalGraphicsMapperStableC_TargetTest.cpp +++ b/graphics/mapper/stable-c/vts/VtsHalGraphicsMapperStableC_TargetTest.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -66,6 +67,24 @@ struct YCbCr { int64_t verticalSubSampling; }; +constexpr const char* STANDARD_METADATA_NAME = + "android.hardware.graphics.common.StandardMetadataType"; + +static bool isStandardMetadata(AIMapper_MetadataType metadataType) { + return strcmp(STANDARD_METADATA_NAME, metadataType.name) == 0; +} + +static std::string toString(const std::vector types) { + std::stringstream buf; + buf << "["; + for (auto type : types) { + buf << toString(type) << ", "; + } + buf.seekp(-2, buf.cur); + buf << "]"; + return buf.str(); +} + class BufferHandle { AIMapper* mIMapper; buffer_handle_t mHandle = nullptr; @@ -1533,8 +1552,187 @@ TEST_P(GraphicsMapperStableCTests, GetSmpte2094_40) { auto bufferHandle = buffer->import(); ASSERT_TRUE(bufferHandle); auto value = getStandardMetadata(*bufferHandle); - ASSERT_TRUE(value.has_value()); - EXPECT_FALSE(value->has_value()); + if (value.has_value()) { + EXPECT_FALSE(value->has_value()); + } +} + +TEST_P(GraphicsMapperStableCTests, SupportsRequiredGettersSetters) { + auto buffer = allocateGeneric(); + ASSERT_TRUE(buffer); + auto bufferHandle = buffer->import(); + ASSERT_TRUE(bufferHandle); + const AIMapper_MetadataTypeDescription* descriptions = nullptr; + size_t descriptionCount = 0; + ASSERT_EQ(AIMAPPER_ERROR_NONE, + mapper()->v5.listSupportedMetadataTypes(&descriptions, &descriptionCount)); + std::vector requiredGetters = { + StandardMetadataType::BUFFER_ID, + StandardMetadataType::NAME, + StandardMetadataType::WIDTH, + StandardMetadataType::HEIGHT, + StandardMetadataType::LAYER_COUNT, + StandardMetadataType::PIXEL_FORMAT_REQUESTED, + StandardMetadataType::PIXEL_FORMAT_FOURCC, + StandardMetadataType::PIXEL_FORMAT_MODIFIER, + StandardMetadataType::USAGE, + StandardMetadataType::ALLOCATION_SIZE, + StandardMetadataType::PROTECTED_CONTENT, + StandardMetadataType::COMPRESSION, + StandardMetadataType::INTERLACED, + StandardMetadataType::CHROMA_SITING, + StandardMetadataType::PLANE_LAYOUTS, + StandardMetadataType::CROP, + StandardMetadataType::DATASPACE, + StandardMetadataType::BLEND_MODE, + StandardMetadataType::SMPTE2086, + StandardMetadataType::CTA861_3, + }; + + std::vector requiredSetters = { + StandardMetadataType::DATASPACE, + StandardMetadataType::BLEND_MODE, + StandardMetadataType::SMPTE2086, + StandardMetadataType::CTA861_3, + }; + + for (int i = 0; i < descriptionCount; i++) { + const auto& it = descriptions[i]; + if (isStandardMetadata(it.metadataType)) { + EXPECT_GT(it.metadataType.value, static_cast(StandardMetadataType::INVALID)); + EXPECT_LT(it.metadataType.value, + ndk::internal::enum_values.size()); + + if (it.isGettable) { + std::erase(requiredGetters, + static_cast(it.metadataType.value)); + } + if (it.isSettable) { + std::erase(requiredSetters, + static_cast(it.metadataType.value)); + } + } else { + EXPECT_NE(nullptr, it.description) << "Non-standard metadata must have a description"; + int len = strlen(it.description); + EXPECT_GE(len, 0) << "Non-standard metadata must have a description"; + } + } + + EXPECT_EQ(0, requiredGetters.size()) << "Missing required getters" << toString(requiredGetters); + EXPECT_EQ(0, requiredSetters.size()) << "Missing required setters" << toString(requiredSetters); +} + +/* + * Test that verifies that if the optional StandardMetadataTypes have getters, they have + * the required setters as well + */ +TEST_P(GraphicsMapperStableCTests, CheckRequiredSettersIfHasGetters) { + auto buffer = allocateGeneric(); + ASSERT_TRUE(buffer); + auto bufferHandle = buffer->import(); + ASSERT_TRUE(bufferHandle); + const AIMapper_MetadataTypeDescription* descriptions = nullptr; + size_t descriptionCount = 0; + ASSERT_EQ(AIMAPPER_ERROR_NONE, + mapper()->v5.listSupportedMetadataTypes(&descriptions, &descriptionCount)); + + for (int i = 0; i < descriptionCount; i++) { + const auto& it = descriptions[i]; + if (isStandardMetadata(it.metadataType)) { + const auto type = static_cast(it.metadataType.value); + switch (type) { + case StandardMetadataType::SMPTE2094_10: + case StandardMetadataType::SMPTE2094_40: + if (it.isGettable) { + EXPECT_TRUE(it.isSettable) + << "Type " << toString(type) << " must be settable if gettable"; + } + break; + default: + break; + } + } + } +} + +TEST_P(GraphicsMapperStableCTests, ListSupportedWorks) { + auto buffer = allocateGeneric(); + ASSERT_TRUE(buffer); + auto bufferHandle = buffer->import(); + ASSERT_TRUE(bufferHandle); + const AIMapper_MetadataTypeDescription* descriptions = nullptr; + size_t descriptionCount = 0; + ASSERT_EQ(AIMAPPER_ERROR_NONE, + mapper()->v5.listSupportedMetadataTypes(&descriptions, &descriptionCount)); + + std::vector metadataBuffer; + auto get = [&](AIMapper_MetadataType metadataType) -> int32_t { + int32_t size = mapper()->v5.getMetadata(*bufferHandle, metadataType, nullptr, 0); + if (size >= 0) { + metadataBuffer.resize(size); + size = mapper()->v5.getMetadata(*bufferHandle, metadataType, metadataBuffer.data(), + metadataBuffer.size()); + EXPECT_EQ(size, metadataBuffer.size()); + } + return size; + }; + + for (int i = 0; i < descriptionCount; i++) { + const auto& it = descriptions[i]; + if (!isStandardMetadata(it.metadataType)) { + continue; + } + if (!it.isGettable) { + EXPECT_FALSE(it.isSettable) + << "StandardMetadata that isn't gettable must not be settable"; + continue; + } + EXPECT_GE(get(it.metadataType), 0) + << "Get failed for claimed supported getter of " + << toString(static_cast(it.metadataType.value)); + if (it.isSettable) { + EXPECT_EQ(AIMAPPER_ERROR_NONE, + mapper()->v5.setMetadata(*bufferHandle, it.metadataType, + metadataBuffer.data(), metadataBuffer.size())) + << "Failed to set metadata for " + << toString(static_cast(it.metadataType.value)); + } + } +} + +TEST_P(GraphicsMapperStableCTests, GetMetadataBadValue) { + auto get = [this](StandardMetadataType type) -> AIMapper_Error { + // This is a _Nonnull parameter, but this is enough obfuscation to fool the linter + buffer_handle_t buffer = nullptr; + int32_t ret = + mapper()->v5.getStandardMetadata(buffer, static_cast(type), nullptr, 0); + return (ret < 0) ? (AIMapper_Error)-ret : AIMAPPER_ERROR_NONE; + }; + + for (auto type : ndk::enum_range()) { + if (type == StandardMetadataType::INVALID) { + continue; + } + EXPECT_EQ(AIMAPPER_ERROR_BAD_BUFFER, get(type)) << "Wrong error for " << toString(type); + } +} + +TEST_P(GraphicsMapperStableCTests, GetUnsupportedMetadata) { + auto buffer = allocateGeneric(); + ASSERT_TRUE(buffer); + auto bufferHandle = buffer->import(); + ASSERT_TRUE(bufferHandle); + + int result = mapper()->v5.getMetadata(*bufferHandle, {"Fake", 1}, nullptr, 0); + EXPECT_EQ(AIMAPPER_ERROR_UNSUPPORTED, -result); + + result = mapper()->v5.getStandardMetadata( + *bufferHandle, static_cast(StandardMetadataType::INVALID), nullptr, 0); + EXPECT_EQ(AIMAPPER_ERROR_UNSUPPORTED, -result); + + constexpr int64_t unknownStandardType = ndk::internal::enum_values.size(); + result = mapper()->v5.getStandardMetadata(*bufferHandle, unknownStandardType, nullptr, 0); + EXPECT_EQ(AIMAPPER_ERROR_UNSUPPORTED, -result); } std::vector>> getIAllocatorsAtLeastVersion(