Have Metadata use android::CameraMetadata.

Ownership is much more clear than with the various double
raw metadata pointers.

BUG: 30140438
TEST: unit tests pass

Change-Id: Ib0de3bb4b9e6dc30594230e46f40ffaea06df51d
This commit is contained in:
Ari Hausman-Cohen 2016-08-04 13:01:05 -07:00
parent 10481a3ec9
commit b1af4ff795
5 changed files with 75 additions and 65 deletions

View file

@ -33,7 +33,7 @@ void Metadata::AddComponent(
components_.push_back(std::move(component));
}
int Metadata::FillStaticMetadata(camera_metadata_t** metadata) {
int Metadata::FillStaticMetadata(android::CameraMetadata* metadata) {
HAL_LOG_ENTER();
std::vector<int32_t> static_tags;
@ -42,13 +42,22 @@ int Metadata::FillStaticMetadata(camera_metadata_t** metadata) {
int res = 0;
for (auto& component : components_) {
// Prevent components from potentially overriding others.
android::CameraMetadata additional_metadata;
// Populate the fields.
res = component->PopulateStaticFields(metadata);
res = component->PopulateStaticFields(&additional_metadata);
if (res) {
// Exit on error.
HAL_LOGE("Failed to get all static properties.");
return res;
}
// Add it to the overall result.
if (!additional_metadata.isEmpty()) {
res = metadata->append(additional_metadata);
if (res != android::OK) {
HAL_LOGE("Failed to append all static properties.");
return res;
}
}
// Note what tags the component adds.
const std::vector<int32_t>* tags = &component->StaticTags();
@ -60,38 +69,36 @@ int Metadata::FillStaticMetadata(camera_metadata_t** metadata) {
}
// Populate the meta fields.
android::CameraMetadata metadata_wrapper(*metadata);
static_tags.push_back(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS);
res = metadata_wrapper.update(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,
control_tags.data(), control_tags.size());
res = metadata->update(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,
control_tags.data(), control_tags.size());
if (res != android::OK) {
HAL_LOGE("Failed to add request keys meta key.");
return -ENODEV;
}
static_tags.push_back(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS);
res = metadata_wrapper.update(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS,
dynamic_tags.data(), dynamic_tags.size());
res = metadata->update(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS,
dynamic_tags.data(), dynamic_tags.size());
if (res != android::OK) {
HAL_LOGE("Failed to add result keys meta key.");
return -ENODEV;
}
static_tags.push_back(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS);
res = metadata_wrapper.update(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS,
static_tags.data(), static_tags.size());
res = metadata->update(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS,
static_tags.data(), static_tags.size());
if (res != android::OK) {
HAL_LOGE("Failed to add characteristics keys meta key.");
return -ENODEV;
}
*metadata = metadata_wrapper.release();
return 0;
}
bool Metadata::IsValidRequest(const camera_metadata_t* metadata) {
bool Metadata::IsValidRequest(const android::CameraMetadata& metadata) {
HAL_LOG_ENTER();
// Null means "use previous settings", which are inherently valid.
if (metadata == nullptr) return true;
// Empty means "use previous settings", which are inherently valid.
if (metadata.isEmpty()) return true;
for (auto& component : components_) {
// Check that all components support the values requested of them.
@ -105,11 +112,11 @@ bool Metadata::IsValidRequest(const camera_metadata_t* metadata) {
return true;
}
int Metadata::SetRequestSettings(const camera_metadata_t* metadata) {
int Metadata::SetRequestSettings(const android::CameraMetadata& metadata) {
HAL_LOG_ENTER();
// Null means "use previous settings".
if (metadata == nullptr) return 0;
// Empty means "use previous settings".
if (metadata.isEmpty()) return 0;
for (auto& component : components_) {
int res = component->SetRequestValues(metadata);
@ -123,14 +130,24 @@ int Metadata::SetRequestSettings(const camera_metadata_t* metadata) {
return 0;
}
int Metadata::FillResultMetadata(camera_metadata_t** metadata) {
int Metadata::FillResultMetadata(android::CameraMetadata* metadata) {
for (auto& component : components_) {
int res = component->PopulateDynamicFields(metadata);
// Prevent components from potentially overriding others.
android::CameraMetadata additional_metadata;
int res = component->PopulateDynamicFields(&additional_metadata);
if (res) {
// Exit early if possible.
HAL_LOGE("Failed to get all dynamic result fields.");
return res;
}
// Add it to the overall result.
if (!additional_metadata.isEmpty()) {
res = metadata->append(additional_metadata);
if (res != android::OK) {
HAL_LOGE("Failed to append all dynamic result fields.");
return res;
}
}
}
return 0;

View file

@ -17,6 +17,7 @@
#ifndef V4L2_CAMERA_HAL_METADATA_H_
#define V4L2_CAMERA_HAL_METADATA_H_
#include <camera/CameraMetadata.h>
#include <hardware/camera3.h>
#include "../common.h"
@ -28,10 +29,10 @@ class Metadata {
Metadata();
virtual ~Metadata();
int FillStaticMetadata(camera_metadata_t** metadata);
bool IsValidRequest(const camera_metadata_t* metadata);
int SetRequestSettings(const camera_metadata_t* metadata);
int FillResultMetadata(camera_metadata_t** metadata);
int FillStaticMetadata(android::CameraMetadata* metadata);
bool IsValidRequest(const android::CameraMetadata& metadata);
int SetRequestSettings(const android::CameraMetadata& metadata);
int FillResultMetadata(android::CameraMetadata* metadata);
protected:
// Helper for the child constructors to fill in metadata components.

View file

@ -41,6 +41,10 @@ class MetadataTest : public Test {
component1_.reset(new PartialMetadataInterfaceMock());
component2_.reset(new PartialMetadataInterfaceMock());
metadata_.reset(new android::CameraMetadata());
non_empty_metadata_.reset(new android::CameraMetadata());
uint8_t val = 1;
non_empty_metadata_->update(ANDROID_COLOR_CORRECTION_MODE, &val, 1);
}
// Once the component mocks have had expectations set,
@ -65,14 +69,14 @@ class MetadataTest : public Test {
// Mocks.
std::unique_ptr<PartialMetadataInterfaceMock> component1_;
std::unique_ptr<PartialMetadataInterfaceMock> component2_;
// Metadata.
std::unique_ptr<android::CameraMetadata> metadata_;
std::unique_ptr<android::CameraMetadata> non_empty_metadata_;
// An empty vector to use as necessary.
std::vector<int32_t> empty_tags_;
};
TEST_F(MetadataTest, FillStaticSuccess) {
android::CameraMetadata metadata(1);
camera_metadata_t* metadata_raw = metadata.release();
// Should populate all the component static pieces.
EXPECT_CALL(*component1_, PopulateStaticFields(_)).WillOnce(Return(0));
EXPECT_CALL(*component2_, PopulateStaticFields(_)).WillOnce(Return(0));
@ -93,7 +97,7 @@ TEST_F(MetadataTest, FillStaticSuccess) {
AddComponents();
// Should succeed. If it didn't, no reason to continue checking output.
ASSERT_EQ(dut_->FillStaticMetadata(&metadata_raw), 0);
ASSERT_EQ(dut_->FillStaticMetadata(metadata_.get()), 0);
// Meta keys should be filled correctly.
// Note: sets are used here, but it is undefined behavior if
@ -114,17 +118,15 @@ TEST_F(MetadataTest, FillStaticSuccess) {
static_tags.emplace(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS);
// Check against what was filled in in the metadata.
metadata.acquire(metadata_raw);
CompareTags(static_tags,
metadata.find(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS));
metadata_->find(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS));
CompareTags(control_tags,
metadata.find(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS));
metadata_->find(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS));
CompareTags(dynamic_tags,
metadata.find(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS));
metadata_->find(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS));
}
TEST_F(MetadataTest, FillStaticFail) {
camera_metadata_t* metadata = nullptr;
int err = -99;
// Order undefined, and may or may not exit early; use AtMost.
EXPECT_CALL(*component1_, PopulateStaticFields(_))
@ -154,12 +156,10 @@ TEST_F(MetadataTest, FillStaticFail) {
AddComponents();
// If any component errors, error should be returned
EXPECT_EQ(dut_->FillStaticMetadata(&metadata), err);
EXPECT_EQ(dut_->FillStaticMetadata(metadata_.get()), err);
}
TEST_F(MetadataTest, IsValidSuccess) {
android::CameraMetadata metadata(1);
// Should check if all the component request values are valid.
EXPECT_CALL(*component1_, SupportsRequestValues(_)).WillOnce(Return(true));
EXPECT_CALL(*component2_, SupportsRequestValues(_)).WillOnce(Return(true));
@ -168,12 +168,10 @@ TEST_F(MetadataTest, IsValidSuccess) {
// Should succeed.
// Note: getAndLock is a lock against pointer invalidation, not concurrency,
// and unlocks on object destruction.
EXPECT_TRUE(dut_->IsValidRequest(metadata.getAndLock()));
EXPECT_TRUE(dut_->IsValidRequest(*non_empty_metadata_));
}
TEST_F(MetadataTest, IsValidFail) {
android::CameraMetadata metadata(1);
// Should check if all the component request values are valid.
// Order undefined, and may or may not exit early; use AtMost.
EXPECT_CALL(*component1_, SupportsRequestValues(_))
@ -185,12 +183,10 @@ TEST_F(MetadataTest, IsValidFail) {
// Should fail since one of the components failed.
// Note: getAndLock is a lock against pointer invalidation, not concurrency,
// and unlocks on object destruction.
EXPECT_FALSE(dut_->IsValidRequest(metadata.getAndLock()));
EXPECT_FALSE(dut_->IsValidRequest(*non_empty_metadata_));
}
TEST_F(MetadataTest, IsValidNull) {
camera_metadata_t* metadata = nullptr;
TEST_F(MetadataTest, IsValidEmpty) {
// Setting null settings is a special case indicating to use the
// previous (valid) settings. As such it is inherently valid.
// Should not try to check any components.
@ -198,12 +194,10 @@ TEST_F(MetadataTest, IsValidNull) {
EXPECT_CALL(*component2_, SupportsRequestValues(_)).Times(0);
AddComponents();
EXPECT_TRUE(dut_->IsValidRequest(metadata));
EXPECT_TRUE(dut_->IsValidRequest(*metadata_));
}
TEST_F(MetadataTest, SetSettingsSuccess) {
android::CameraMetadata metadata(1);
// Should check if all the components set successfully.
EXPECT_CALL(*component1_, SetRequestValues(_)).WillOnce(Return(0));
EXPECT_CALL(*component2_, SetRequestValues(_)).WillOnce(Return(0));
@ -212,11 +206,10 @@ TEST_F(MetadataTest, SetSettingsSuccess) {
// Should succeed.
// Note: getAndLock is a lock against pointer invalidation, not concurrency,
// and unlocks on object destruction.
EXPECT_EQ(dut_->SetRequestSettings(metadata.getAndLock()), 0);
EXPECT_EQ(dut_->SetRequestSettings(*non_empty_metadata_), 0);
}
TEST_F(MetadataTest, SetSettingsFail) {
android::CameraMetadata metadata(1);
int err = -99;
// Should check if all the components set successfully.
@ -230,12 +223,10 @@ TEST_F(MetadataTest, SetSettingsFail) {
// Should fail since one of the components failed.
// Note: getAndLock is a lock against pointer invalidation, not concurrency,
// and unlocks on object destruction.
EXPECT_EQ(dut_->SetRequestSettings(metadata.getAndLock()), err);
EXPECT_EQ(dut_->SetRequestSettings(*non_empty_metadata_), err);
}
TEST_F(MetadataTest, SetSettingsNull) {
camera_metadata_t* metadata = nullptr;
TEST_F(MetadataTest, SetSettingsEmpty) {
// Setting null settings is a special case indicating to use the
// previous settings. Should not try to set any components.
EXPECT_CALL(*component1_, SetRequestValues(_)).Times(0);
@ -243,23 +234,20 @@ TEST_F(MetadataTest, SetSettingsNull) {
AddComponents();
// Should succeed.
EXPECT_EQ(dut_->SetRequestSettings(metadata), 0);
EXPECT_EQ(dut_->SetRequestSettings(*metadata_), 0);
}
TEST_F(MetadataTest, FillResultSuccess) {
camera_metadata_t* metadata = nullptr;
// Should check if all the components fill results successfully.
EXPECT_CALL(*component1_, PopulateDynamicFields(_)).WillOnce(Return(0));
EXPECT_CALL(*component2_, PopulateDynamicFields(_)).WillOnce(Return(0));
AddComponents();
// Should succeed.
EXPECT_EQ(dut_->FillResultMetadata(&metadata), 0);
EXPECT_EQ(dut_->FillResultMetadata(metadata_.get()), 0);
}
TEST_F(MetadataTest, FillResultFail) {
camera_metadata_t* metadata = nullptr;
int err = -99;
// Should check if all the components fill results successfully.
@ -271,7 +259,7 @@ TEST_F(MetadataTest, FillResultFail) {
AddComponents();
// Should fail since one of the components failed.
EXPECT_EQ(dut_->FillResultMetadata(&metadata), err);
EXPECT_EQ(dut_->FillResultMetadata(metadata_.get()), err);
}
} // namespace v4l2_camera_hal

View file

@ -19,6 +19,7 @@
#include <vector>
#include <camera/CameraMetadata.h>
#include <hardware/camera3.h>
namespace v4l2_camera_hal {
@ -36,17 +37,18 @@ class PartialMetadataInterface {
// Add all the static properties this partial metadata
// is responsible for to |metadata|.
virtual int PopulateStaticFields(camera_metadata_t** metadata) const = 0;
virtual int PopulateStaticFields(android::CameraMetadata* metadata) const = 0;
// Add all the dynamic states this partial metadata
// is responsible for to |metadata|.
virtual int PopulateDynamicFields(camera_metadata_t** metadata) const = 0;
virtual int PopulateDynamicFields(
android::CameraMetadata* metadata) const = 0;
// Check if the requested control values from |metadata| (for controls
// this partial metadata owns) are supported.
virtual bool SupportsRequestValues(
const camera_metadata_t* metadata) const = 0;
const android::CameraMetadata& metadata) const = 0;
// Set all the controls this partial metadata
// is responsible for from |metadata|.
virtual int SetRequestValues(const camera_metadata_t* metadata) = 0;
virtual int SetRequestValues(const android::CameraMetadata& metadata) = 0;
};
} // namespace v4l2_camera_hal

View file

@ -31,11 +31,13 @@ class PartialMetadataInterfaceMock : public PartialMetadataInterface {
MOCK_CONST_METHOD0(StaticTags, const std::vector<int32_t>&());
MOCK_CONST_METHOD0(ControlTags, const std::vector<int32_t>&());
MOCK_CONST_METHOD0(DynamicTags, const std::vector<int32_t>&());
MOCK_CONST_METHOD1(PopulateStaticFields, int(camera_metadata_t** metadata));
MOCK_CONST_METHOD1(PopulateDynamicFields, int(camera_metadata_t** metadata));
MOCK_CONST_METHOD1(PopulateStaticFields,
int(android::CameraMetadata* metadata));
MOCK_CONST_METHOD1(PopulateDynamicFields,
int(android::CameraMetadata* metadata));
MOCK_CONST_METHOD1(SupportsRequestValues,
bool(const camera_metadata_t* metadata));
MOCK_METHOD1(SetRequestValues, int(const camera_metadata_t* metadata));
bool(const android::CameraMetadata& metadata));
MOCK_METHOD1(SetRequestValues, int(const android::CameraMetadata& metadata));
};
} // namespace v4l2_camera_hal