Separated v4l2_metadata into two pieces.
v4l2_metadata and metadata/metadata. The previous design did not work
as intended for testing (it incorrectly assumed that a constructor
would call an overridden method of a child class), plus this way
makes more sense.
BUG: 30140438
Change-Id: I1293535932fed6daed766682421b1215739e17dd
TEST: Unit tests pass
diff --git a/modules/camera/3_4/Android.mk b/modules/camera/3_4/Android.mk
index 1ced4a5..b6f0fa6 100644
--- a/modules/camera/3_4/Android.mk
+++ b/modules/camera/3_4/Android.mk
@@ -39,6 +39,7 @@
v4l2_src_files := \
camera.cpp \
+ metadata/metadata.cpp \
stream.cpp \
stream_format.cpp \
v4l2_camera.cpp \
@@ -48,7 +49,7 @@
v4l2_wrapper.cpp \
v4l2_test_files := \
- v4l2_metadata_test.cpp \
+ metadata/metadata_test.cpp \
# V4L2 Camera HAL.
# ==============================================================================
diff --git a/modules/camera/3_4/metadata/metadata.cpp b/modules/camera/3_4/metadata/metadata.cpp
new file mode 100644
index 0000000..8867e4d
--- /dev/null
+++ b/modules/camera/3_4/metadata/metadata.cpp
@@ -0,0 +1,139 @@
+/*
+ * Copyright 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "metadata.h"
+
+#include <camera/CameraMetadata.h>
+
+#include "../common.h"
+
+namespace v4l2_camera_hal {
+
+Metadata::Metadata() { HAL_LOG_ENTER(); }
+
+Metadata::~Metadata() { HAL_LOG_ENTER(); }
+
+void Metadata::AddComponent(
+ std::unique_ptr<PartialMetadataInterface> component) {
+ HAL_LOG_ENTER();
+
+ components_.push_back(std::move(component));
+}
+
+int Metadata::FillStaticMetadata(camera_metadata_t** metadata) {
+ HAL_LOG_ENTER();
+
+ std::vector<int32_t> static_tags;
+ std::vector<int32_t> control_tags;
+ std::vector<int32_t> dynamic_tags;
+ int res = 0;
+
+ for (auto& component : components_) {
+ // Populate the fields.
+ res = component->PopulateStaticFields(metadata);
+ if (res) {
+ // Exit on error.
+ HAL_LOGE("Failed to get all static properties.");
+ return res;
+ }
+
+ // Note what tags the component adds.
+ const std::vector<int32_t>* tags = &component->StaticTags();
+ static_tags.insert(static_tags.end(), tags->begin(), tags->end());
+ tags = &component->ControlTags();
+ control_tags.insert(control_tags.end(), tags->begin(), tags->end());
+ tags = &component->DynamicTags();
+ dynamic_tags.insert(dynamic_tags.end(), tags->begin(), tags->end());
+ }
+
+ // 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());
+ 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());
+ 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());
+ 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) {
+ HAL_LOG_ENTER();
+
+ // Null means "use previous settings", which are inherently valid.
+ if (metadata == nullptr) return true;
+
+ for (auto& component : components_) {
+ // Check that all components support the values requested of them.
+ bool valid_request = component->SupportsRequestValues(metadata);
+ if (!valid_request) {
+ // Exit early if possible.
+ return false;
+ }
+ }
+
+ return true;
+}
+
+int Metadata::SetRequestSettings(const camera_metadata_t* metadata) {
+ HAL_LOG_ENTER();
+
+ // Null means "use previous settings".
+ if (metadata == nullptr) return 0;
+
+ for (auto& component : components_) {
+ int res = component->SetRequestValues(metadata);
+ if (res) {
+ // Exit early if possible.
+ HAL_LOGE("Failed to set all requested settings.");
+ return res;
+ }
+ }
+
+ return 0;
+}
+
+int Metadata::FillResultMetadata(camera_metadata_t** metadata) {
+ for (auto& component : components_) {
+ int res = component->PopulateDynamicFields(metadata);
+ if (res) {
+ // Exit early if possible.
+ HAL_LOGE("Failed to get all dynamic result fields.");
+ return res;
+ }
+ }
+
+ return 0;
+}
+
+} // namespace v4l2_camera_hal
diff --git a/modules/camera/3_4/metadata/metadata.h b/modules/camera/3_4/metadata/metadata.h
new file mode 100644
index 0000000..d25a040
--- /dev/null
+++ b/modules/camera/3_4/metadata/metadata.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef V4L2_CAMERA_HAL_METADATA_H_
+#define V4L2_CAMERA_HAL_METADATA_H_
+
+#include <hardware/camera3.h>
+
+#include "../common.h"
+#include "partial_metadata_interface.h"
+
+namespace v4l2_camera_hal {
+class Metadata {
+ public:
+ 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);
+
+ protected:
+ // Helper for the child constructors to fill in metadata components.
+ void AddComponent(std::unique_ptr<PartialMetadataInterface> component);
+
+ private:
+ // The overall metadata is broken down into several distinct pieces.
+ // Note: it is undefined behavior if multiple components share tags.
+ std::vector<std::unique_ptr<PartialMetadataInterface>> components_;
+
+ friend class MetadataTest;
+
+ DISALLOW_COPY_AND_ASSIGN(Metadata);
+};
+
+} // namespace v4l2_camera_hal
+
+#endif // V4L2_CAMERA_HAL_V4L2_METADATA_H_
diff --git a/modules/camera/3_4/v4l2_metadata_test.cpp b/modules/camera/3_4/metadata/metadata_test.cpp
similarity index 78%
rename from modules/camera/3_4/v4l2_metadata_test.cpp
rename to modules/camera/3_4/metadata/metadata_test.cpp
index 651d48c..d7da553 100644
--- a/modules/camera/3_4/v4l2_metadata_test.cpp
+++ b/modules/camera/3_4/metadata/metadata_test.cpp
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-#include "v4l2_metadata.h"
+#include "metadata.h"
#include <memory>
#include <set>
@@ -25,7 +25,6 @@
#include <gtest/gtest.h>
#include "metadata/partial_metadata_interface_mock.h"
-#include "v4l2_wrapper_mock.h"
using testing::AtMost;
using testing::Return;
@@ -35,23 +34,13 @@
namespace v4l2_camera_hal {
-class V4L2MetadataTest : public Test {
+class MetadataTest : public Test {
protected:
- // Create a test version of V4L2Metadata with no default components.
- class TestV4L2Metadata : public V4L2Metadata {
- public:
- TestV4L2Metadata(V4L2Wrapper* device) : V4L2Metadata(device){};
-
- private:
- void PopulateComponents() override{};
- };
-
virtual void SetUp() {
- device_.reset(new V4L2WrapperMock());
- dut_.reset(new TestV4L2Metadata(device_.get()));
+ dut_.reset(new Metadata());
- component1_.reset(new PartialMetadataInterfaceMock);
- component2_.reset(new PartialMetadataInterfaceMock);
+ component1_.reset(new PartialMetadataInterfaceMock());
+ component2_.reset(new PartialMetadataInterfaceMock());
}
// Once the component mocks have had expectations set,
@@ -65,23 +54,22 @@
virtual void CompareTags(const std::set<int32_t>& expected,
const camera_metadata_entry_t& actual) {
- EXPECT_EQ(expected.size(), actual.count);
+ ASSERT_EQ(expected.size(), actual.count);
for (size_t i = 0; i < actual.count; ++i) {
EXPECT_NE(expected.find(actual.data.i32[i]), expected.end());
}
}
// Device under test.
- std::unique_ptr<V4L2Metadata> dut_;
+ std::unique_ptr<Metadata> dut_;
// Mocks.
- std::unique_ptr<V4L2WrapperMock> device_;
std::unique_ptr<PartialMetadataInterfaceMock> component1_;
std::unique_ptr<PartialMetadataInterfaceMock> component2_;
// An empty vector to use as necessary.
std::vector<int32_t> empty_tags_;
};
-TEST_F(V4L2MetadataTest, FillStaticSuccess) {
+TEST_F(MetadataTest, FillStaticSuccess) {
android::CameraMetadata metadata(1);
camera_metadata_t* metadata_raw = metadata.release();
@@ -135,7 +123,7 @@
metadata.find(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS));
}
-TEST_F(V4L2MetadataTest, FillStaticFail) {
+TEST_F(MetadataTest, FillStaticFail) {
camera_metadata_t* metadata = nullptr;
int err = -99;
// Order undefined, and may or may not exit early; use AtMost.
@@ -169,8 +157,8 @@
EXPECT_EQ(dut_->FillStaticMetadata(&metadata), err);
}
-TEST_F(V4L2MetadataTest, IsValidSuccess) {
- camera_metadata_t* metadata = nullptr;
+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));
@@ -178,11 +166,13 @@
AddComponents();
// Should succeed.
- EXPECT_TRUE(dut_->IsValidRequest(metadata));
+ // Note: getAndLock is a lock against pointer invalidation, not concurrency,
+ // and unlocks on object destruction.
+ EXPECT_TRUE(dut_->IsValidRequest(metadata.getAndLock()));
}
-TEST_F(V4L2MetadataTest, IsValidFail) {
- camera_metadata_t* metadata = nullptr;
+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.
@@ -193,23 +183,40 @@
AddComponents();
// Should fail since one of the components failed.
- EXPECT_FALSE(dut_->IsValidRequest(metadata));
+ // Note: getAndLock is a lock against pointer invalidation, not concurrency,
+ // and unlocks on object destruction.
+ EXPECT_FALSE(dut_->IsValidRequest(metadata.getAndLock()));
}
-TEST_F(V4L2MetadataTest, SetSettingsSuccess) {
+TEST_F(MetadataTest, IsValidNull) {
camera_metadata_t* metadata = nullptr;
+ // 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.
+ EXPECT_CALL(*component1_, SupportsRequestValues(_)).Times(0);
+ EXPECT_CALL(*component2_, SupportsRequestValues(_)).Times(0);
+
+ AddComponents();
+ 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));
AddComponents();
// Should succeed.
- EXPECT_EQ(dut_->SetRequestSettings(metadata), 0);
+ // Note: getAndLock is a lock against pointer invalidation, not concurrency,
+ // and unlocks on object destruction.
+ EXPECT_EQ(dut_->SetRequestSettings(metadata.getAndLock()), 0);
}
-TEST_F(V4L2MetadataTest, SetSettingsFail) {
- camera_metadata_t* metadata = nullptr;
+TEST_F(MetadataTest, SetSettingsFail) {
+ android::CameraMetadata metadata(1);
int err = -99;
// Should check if all the components set successfully.
@@ -221,10 +228,25 @@
AddComponents();
// Should fail since one of the components failed.
- EXPECT_EQ(dut_->SetRequestSettings(metadata), err);
+ // Note: getAndLock is a lock against pointer invalidation, not concurrency,
+ // and unlocks on object destruction.
+ EXPECT_EQ(dut_->SetRequestSettings(metadata.getAndLock()), err);
}
-TEST_F(V4L2MetadataTest, FillResultSuccess) {
+TEST_F(MetadataTest, SetSettingsNull) {
+ camera_metadata_t* metadata = nullptr;
+
+ // 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);
+ EXPECT_CALL(*component2_, SetRequestValues(_)).Times(0);
+
+ AddComponents();
+ // Should succeed.
+ EXPECT_EQ(dut_->SetRequestSettings(metadata), 0);
+}
+
+TEST_F(MetadataTest, FillResultSuccess) {
camera_metadata_t* metadata = nullptr;
// Should check if all the components fill results successfully.
@@ -236,7 +258,7 @@
EXPECT_EQ(dut_->FillResultMetadata(&metadata), 0);
}
-TEST_F(V4L2MetadataTest, FillResultFail) {
+TEST_F(MetadataTest, FillResultFail) {
camera_metadata_t* metadata = nullptr;
int err = -99;
diff --git a/modules/camera/3_4/v4l2_metadata.cpp b/modules/camera/3_4/v4l2_metadata.cpp
index 54d2809..0745720 100644
--- a/modules/camera/3_4/v4l2_metadata.cpp
+++ b/modules/camera/3_4/v4l2_metadata.cpp
@@ -25,119 +25,11 @@
V4L2Metadata::V4L2Metadata(V4L2Wrapper* device) : device_(device) {
HAL_LOG_ENTER();
- PopulateComponents();
+ // TODO(b/30140438): Add all metadata components used by V4L2Camera here.
+ // Currently these are all the fixed properties. Will add the other properties
+ // as more PartialMetadata subclasses get implemented.
}
V4L2Metadata::~V4L2Metadata() { HAL_LOG_ENTER(); }
-void V4L2Metadata::PopulateComponents() {
- HAL_LOG_ENTER();
-
- // TODO(arihc): Add all default components.
-}
-
-void V4L2Metadata::AddComponent(
- std::unique_ptr<PartialMetadataInterface> component) {
- HAL_LOG_ENTER();
-
- components_.push_back(std::move(component));
-}
-
-int V4L2Metadata::FillStaticMetadata(camera_metadata_t** metadata) {
- HAL_LOG_ENTER();
-
- std::vector<int32_t> static_tags;
- std::vector<int32_t> control_tags;
- std::vector<int32_t> dynamic_tags;
- int res = 0;
-
- for (auto& component : components_) {
- // Populate the fields.
- res = component->PopulateStaticFields(metadata);
- if (res) {
- // Exit on error.
- HAL_LOGE("Failed to get all static properties.");
- return res;
- }
-
- // Note what tags the component adds.
- const std::vector<int32_t>* tags = &component->StaticTags();
- static_tags.insert(static_tags.end(), tags->begin(), tags->end());
- tags = &component->ControlTags();
- control_tags.insert(control_tags.end(), tags->begin(), tags->end());
- tags = &component->DynamicTags();
- dynamic_tags.insert(dynamic_tags.end(), tags->begin(), tags->end());
- }
-
- // 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());
- 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());
- 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());
- if (res != android::OK) {
- HAL_LOGE("Failed to add characteristics keys meta key.");
- return -ENODEV;
- }
-
- *metadata = metadata_wrapper.release();
- return 0;
-}
-
-bool V4L2Metadata::IsValidRequest(const camera_metadata_t* metadata) {
- HAL_LOG_ENTER();
-
- for (auto& component : components_) {
- // Check that all components support the values requested of them.
- bool valid_request = component->SupportsRequestValues(metadata);
- if (!valid_request) {
- // Exit early if possible.
- return false;
- }
- }
-
- return true;
-}
-
-int V4L2Metadata::SetRequestSettings(const camera_metadata_t* metadata) {
- HAL_LOG_ENTER();
-
- for (auto& component : components_) {
- int res = component->SetRequestValues(metadata);
- if (res) {
- // Exit early if possible.
- HAL_LOGE("Failed to set all requested settings.");
- return res;
- }
- }
-
- return 0;
-}
-
-int V4L2Metadata::FillResultMetadata(camera_metadata_t** metadata) {
- for (auto& component : components_) {
- int res = component->PopulateDynamicFields(metadata);
- if (res) {
- // Exit early if possible.
- HAL_LOGE("Failed to get all dynamic result fields.");
- return res;
- }
- }
-
- return 0;
-}
-
} // namespace v4l2_camera_hal
diff --git a/modules/camera/3_4/v4l2_metadata.h b/modules/camera/3_4/v4l2_metadata.h
index 573154a..ff157c0 100644
--- a/modules/camera/3_4/v4l2_metadata.h
+++ b/modules/camera/3_4/v4l2_metadata.h
@@ -20,36 +20,18 @@
#include <hardware/camera3.h>
#include "common.h"
-#include "metadata/partial_metadata_interface.h"
+#include "metadata/metadata.h"
#include "v4l2_wrapper.h"
namespace v4l2_camera_hal {
-class V4L2Metadata {
+class V4L2Metadata : public Metadata {
public:
V4L2Metadata(V4L2Wrapper* device);
virtual ~V4L2Metadata();
- 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);
-
- protected:
- // Helper for the constructor to fill in metadata components.
- // Virtual/protected so tests can subclass and override populating with mocks.
- virtual void PopulateComponents();
-
private:
- // Helper for the constructor to fill in metadata components.
- void AddComponent(std::unique_ptr<PartialMetadataInterface> component);
-
// Access to the device.
V4L2Wrapper* device_;
- // The overall metadata is broken down into several distinct pieces.
- // Note: it is undefined behavior if multiple components share tags.
- std::vector<std::unique_ptr<PartialMetadataInterface>> components_;
-
- friend class V4L2MetadataTest;
DISALLOW_COPY_AND_ASSIGN(V4L2Metadata);
};