[libcamera-devel,v8,1/4] tests: Add IPADataSerializer test

11288 diff mbox series

Test the IPADataSerializer for controls, vectors, maps, and PODs of
built-in types.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
No change in v8

Changes in v7:
- remove printing values of vectors/maps
- simplify map and vector equality check
- return immediately on the first failure

Changes in v6:
- no longer need to initialize rpi ControlInfoMap
- no longer need to pass ControlInfoMap to the ControlList serializer

Changes in v5:
- use ControlInfoMap serializer instead of const ControlInfoMap
  serializer

Changes in v4:
- use RPi::controls instead RPi::Controls

Changes in v3:
- use re-namespaced RPi::Controls

New in v2
---
 .../ipa_data_serializer_test.cpp              | 378 ++++++++++++++++++
 test/serialization/meson.build                |   1 +
 2 files changed, 379 insertions(+)
 create mode 100644 test/serialization/ipa_data_serializer_test.cpp

Hi Paul,

Thank you for the patch.

On Sat, Feb 13, 2021 at 01:23:09PM +0900, Paul Elder wrote:
> Test the IPADataSerializer for controls, vectors, maps, and PODs of
> built-in types.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> No change in v8
> 
> Changes in v7:
> - remove printing values of vectors/maps
> - simplify map and vector equality check
> - return immediately on the first failure
> 
> Changes in v6:
> - no longer need to initialize rpi ControlInfoMap
> - no longer need to pass ControlInfoMap to the ControlList serializer
> 
> Changes in v5:
> - use ControlInfoMap serializer instead of const ControlInfoMap
>   serializer
> 
> Changes in v4:
> - use RPi::controls instead RPi::Controls
> 
> Changes in v3:
> - use re-namespaced RPi::Controls
> 
> New in v2
> ---
>  .../ipa_data_serializer_test.cpp              | 378 ++++++++++++++++++
>  test/serialization/meson.build                |   1 +
>  2 files changed, 379 insertions(+)
>  create mode 100644 test/serialization/ipa_data_serializer_test.cpp
> 
> diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp
> new file mode 100644
> index 00000000..024cdf79
> --- /dev/null
> +++ b/test/serialization/ipa_data_serializer_test.cpp
> @@ -0,0 +1,378 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_data_serializer_test.cpp - Test serializing/deserializing with IPADataSerializer
> + */
> +
> +#include <algorithm>
> +#include <fcntl.h>
> +#include <iostream>
> +#include <limits>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <tuple>
> +#include <unistd.h>
> +#include <vector>
> +
> +#include <libcamera/ipa/raspberrypi.h>

Could we avoid depending on this header ? The only reason we need it is
for RPi::Controls, and that will go away. You can create a custom
ControlInfoMap to replace it.

> +
> +#include "libcamera/internal/device_enumerator.h"
> +#include "libcamera/internal/ipa_data_serializer.h"
> +#include "libcamera/internal/ipa_manager.h"
> +#include "libcamera/internal/ipa_module.h"
> +#include "libcamera/internal/pipeline_handler.h"
> +#include "libcamera/internal/thread.h"
> +#include "libcamera/internal/timer.h"
> +
> +#include "serialization_test.h"
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +template<typename T>
> +bool isVectorEqual(vector<T> &vecA, vector<T> &vecB)
> +{
> +	if (vecA.size() != vecB.size())
> +		return false;
> +
> +	size_t len = vecA.size();
> +	for (unsigned int i = 0; i < len; i++)
> +		if (vecA[i] != vecB[i])
> +			return false;
> +
> +	return true;

This can be implemented as

	return vecA == vecB;

Furthermore, it's customary, in comparison functions, to name the two
parameters lhs and rhs.

> +}
> +
> +template<>
> +bool isVectorEqual<ControlInfoMap>(vector<ControlInfoMap> &vecA,
> +				   vector<ControlInfoMap> &vecB)
> +{
> +	if (vecA.size() != vecB.size())
> +		return false;
> +
> +	size_t len = vecA.size();
> +	for (unsigned int i = 0; i < len; i++)
> +		if (!SerializationTest::equals(vecA[i], vecB[i]))
> +			return false;
> +
> +	return true;
> +}

You could replace those two functions with

namespace libcamera {

static bool operator==(const ControlInfoMap &lhs, const ControlInfoMap &rhs)
{
	return SerializationTest::equals(lhs, rhs);
}

} /* namespace libcamera */

and use == below instead of isVectorEqual().

> +
> +template<typename K, typename V>
> +bool isMapToCimEqual(map<K, V> &mapA, map<K, V> &mapB)
> +{
> +	if (mapA.size() != mapB.size())
> +		return false;
> +
> +	for (const auto &itA : mapA) {
> +		auto itB = mapB.find(itA.first);
> +
> +		if (itB == mapB.end())
> +			return false;
> +
> +		if (!SerializationTest::equals(itA.second, itB->second))
> +			return false;
> +	}
> +
> +	return true;
> +}

And you can then drop this function too, and use == as well. Isn't C++
amazing ? ;-)

> +
> +class IPADataSerializerTest : public CameraTest, public Test
> +{
> +public:
> +	IPADataSerializerTest()
> +		: CameraTest("platform/vimc.0 Sensor B")
> +	{
> +	}
> +
> +protected:
> +	int init() override
> +	{
> +		return status_;
> +	}
> +
> +	int run() override
> +	{
> +		int ret;
> +
> +		ret = testControls();
> +		if (ret != TestPass)
> +			return ret;
> +
> +		ret = testVector();
> +		if (ret != TestPass)
> +			return ret;
> +
> +		ret = testMap();
> +		if (ret != TestPass)
> +			return ret;
> +
> +		ret = testPod();
> +		if (ret != TestPass)
> +			return ret;
> +
> +		return TestPass;
> +	}
> +
> +private:
> +	ControlList generateControlList(const ControlInfoMap &infoMap)
> +	{
> +		/* Create a control list with three controls. */
> +		ControlList list(infoMap);
> +
> +		list.set(controls::Brightness, 0.5f);
> +		list.set(controls::Contrast, 1.2f);
> +		list.set(controls::Saturation, 0.2f);
> +
> +		return list;
> +	}
> +
> +	int testControls()
> +	{
> +		ControlSerializer cs;
> +
> +		const ControlInfoMap &infoMap = camera_->controls();
> +		ControlList list = generateControlList(infoMap);
> +
> +		vector<uint8_t> infoMapBuf;
> +		tie(infoMapBuf, ignore) =
> +			IPADataSerializer<ControlInfoMap>::serialize(infoMap, &cs);

We've used the std:: namespace prefix explicitly in tests for anything
other than cout, cerr and endl. That's mostly historical, and the plan
is to drop usage for iostream for a more approriate test result
reporting infrastructure. At that point I'd like to drop the using
namespace std. Could we already qualify members of the std namespace
other than cout, cerr and endl explicitly ?

> +
> +		vector<uint8_t> listBuf;
> +		tie(listBuf, ignore) =
> +			IPADataSerializer<ControlList>::serialize(list, &cs);
> +
> +		const ControlInfoMap infoMapOut =
> +			IPADataSerializer<ControlInfoMap>::deserialize(infoMapBuf, &cs);
> +
> +		ControlList listOut = IPADataSerializer<ControlList>::deserialize(listBuf, &cs);
> +
> +		if (!SerializationTest::equals(infoMap, infoMapOut)) {
> +			cerr << "Deserialized map doesn't match original" << endl;
> +			return TestFail;
> +		}
> +
> +		if (!SerializationTest::equals(list, listOut)) {
> +			cerr << "Deserialized list doesn't match original" << endl;
> +			return TestFail;
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	int testVector()
> +	{
> +
> +#define TEST_VEC_SERDES(type, vec, cs)					\
> +tie(buf, fds) = IPADataSerializer<vector<type>>::serialize(vec, cs);	\
> +vector<type> vec##Out =							\
> +	IPADataSerializer<vector<type>>::deserialize(buf, fds, cs);	\
> +ret = isVectorEqual<type>(vec, vec##Out);				\
> +if (!ret) {								\
> +	cerr << "Deserialized vector " << #vec << " doesn't match original" << endl;\
> +	return TestFail;						\
> +}

This is sooooo plain C :-) Here's the C++ way:

#include <cxxabi.h>
#include <stdlib.h>

	template<typename T>
	int testVectorSerdes(const std::vector<T> &in,
			     ControlSerializer *cs = nullptr)
	{
		std::vector<uint8_t> buf;
		std::vector<int32_t> fds;

		std::tie(buf, fds) = IPADataSerializer<std::vector<T>>::serialize(in, cs);
		std::vector<T> out = IPADataSerializer<std::vector<T>>::deserialize(buf, fds, cs);
		if (in == out)
			return TestPass;

		char *name = abi::__cxa_demangle(typeid(T).name(), nullptr,
						 nullptr, nullptr);
		cerr << "Deserialized std::vector<" << name
		     << "> doesn't match original" << endl;
		free(name);
		return TestFail;
	}

Granted, print the name of T as a string is more complicated than with a
macro.

> +
> +		ControlSerializer cs;
> +
> +		/*
> +		 * We don't test FileDescriptor serdes because it dup()s, so we
> +		 * can't check for equality.
> +		 */
> +		vector<uint8_t>  vecUint8  = { 1, 2, 3, 4, 5, 6 };
> +		vector<uint16_t> vecUint16 = { 1, 2, 3, 4, 5, 6 };
> +		vector<uint32_t> vecUint32 = { 1, 2, 3, 4, 5, 6 };
> +		vector<uint64_t> vecUint64 = { 1, 2, 3, 4, 5, 6 };
> +		vector<int8_t>   vecInt8   = { 1, 2, 3, -4, 5, -6 };
> +		vector<int16_t>  vecInt16  = { 1, 2, 3, -4, 5, -6 };
> +		vector<int32_t>  vecInt32  = { 1, 2, 3, -4, 5, -6 };
> +		vector<int64_t>  vecInt64  = { 1, 2, 3, -4, 5, -6 };
> +		vector<float>    vecFloat  = { 1.1, 2.2, 3.3, -4.4, 5.5, -6.6 };
> +		vector<double>   vecDouble = { 1.1, 2.2, 3.3, -4.4, 5.5, -6.6 };
> +		vector<bool>     vecBool   = { true, true, false, false, true, false };
> +		vector<string>   vecString = { "foo", "bar", "baz" };
> +		vector<ControlInfoMap> vecControlInfoMap = {
> +			camera_->controls(),
> +			RPi::Controls,
> +		};
> +
> +		vector<uint8_t> buf;
> +		vector<int32_t> fds;
> +		int ret;
> +
> +		TEST_VEC_SERDES(uint8_t,  vecUint8,  nullptr);
> +		TEST_VEC_SERDES(uint16_t, vecUint16, nullptr);
> +		TEST_VEC_SERDES(uint32_t, vecUint32, nullptr);
> +		TEST_VEC_SERDES(uint64_t, vecUint64, nullptr);
> +		TEST_VEC_SERDES(int8_t,   vecInt8,   nullptr);
> +		TEST_VEC_SERDES(int16_t,  vecInt16,  nullptr);
> +		TEST_VEC_SERDES(int32_t,  vecInt32,  nullptr);
> +		TEST_VEC_SERDES(int64_t,  vecInt64,  nullptr);
> +		TEST_VEC_SERDES(float,    vecFloat,  nullptr);
> +		TEST_VEC_SERDES(double,   vecDouble, nullptr);
> +		TEST_VEC_SERDES(bool,     vecBool,   nullptr);
> +		TEST_VEC_SERDES(string,   vecString, nullptr);
> +		TEST_VEC_SERDES(ControlInfoMap, vecControlInfoMap, &cs);

And here

		if (testVectorSerdes(vecUint8) != TestPass)
			return TestFail;
		...

> +
> +		return TestPass;
> +	}
> +
> +	int testMap()
> +	{
> +
> +#define TEST_MAP_SERDES(ktype, vtype, m, cs)				\
> +tie(buf, fds) = IPADataSerializer<map<ktype, vtype>>::serialize(m, cs);	\
> +map<ktype, vtype> m##Out =						\
> +	IPADataSerializer<map<ktype, vtype>>::deserialize(buf, fds, cs);\
> +ret = m == m##Out;							\
> +if (!ret) {								\
> +	cerr << "Deserialized map " << #m << " doesn't match original" << endl;\
> +	return TestFail;						\
> +}
> +
> +#define TEST_MAP_CIM_SERDES(ktype, vtype, m, cs)			\
> +tie(buf, fds) = IPADataSerializer<map<ktype, vtype>>::serialize(m, cs);	\
> +map<ktype, vtype> m##Out =						\
> +	IPADataSerializer<map<ktype, vtype>>::deserialize(buf, fds, cs);\
> +ret = isMapToCimEqual<ktype, vtype>(m, m##Out);				\
> +if (!ret) {								\
> +	cerr << "Deserialized map " << #m << " doesn't match original" << endl;\
> +	return TestFail;						\
> +}
> +
> +#define TEST_MAP_VEC_SERDES(ktype, vtype, m, cs)			\
> +tie(buf, fds) = IPADataSerializer<map<ktype, vtype>>::serialize(m, cs);	\
> +map<ktype, vtype> m##Out =						\
> +	IPADataSerializer<map<ktype, vtype>>::deserialize(buf, fds, cs);\
> +ret = m == m##Out;							\
> +if (!ret) {								\
> +	cerr << "Deserialized map " << #m << " doesn't match original" << endl;\
> +	return TestFail;						\
> +}

How to use templates instead of macros here is an exercise left to the
reader :-) (Note that the second macro is identical to the first one after
replacing isMapToCimEqual with operator==, and the third macro is
already identical to the first one.)

> +
> +		ControlSerializer cs;
> +
> +		/*
> +		 * Realistically, only string and integral keys.
> +		 * Test simple, complex, and nested compound value.
> +		 */
> +		map<uint64_t, string> mapUintStr =
> +			{ {101, "foo"}, {102, "bar"}, {103, "baz"} };

Our coding style adds a space within the curly braces (I actually
wouldn't mind dropping that, to match both the cppreference.com style
and the Google C++ style, but that's a separate topic).

> +		map<int64_t, string> mapIntStr =
> +			{ {101, "foo"}, {-102, "bar"}, {-103, "baz"} };
> +		map<string, string> mapStrStr =
> +			{ {"a", "foo"}, {"b", "bar"}, {"c", "baz"} };
> +		map<uint64_t, ControlInfoMap> mapUintCIM =
> +			{ {201, camera_->controls()}, {202, RPi::Controls} };
> +		map<int64_t, ControlInfoMap> mapIntCIM =
> +			{ {201, camera_->controls()}, {-202, RPi::Controls} };
> +		map<string, ControlInfoMap> mapStrCIM =
> +			{ {"a", camera_->controls()}, {"b", RPi::Controls} };
> +		map<uint64_t, vector<uint8_t>> mapUintBVec =
> +			{ {301, { 1, 2, 3 }}, {302, {4, 5, 6}}, {303, {7, 8, 9}} };
> +		map<int64_t, vector<uint8_t>> mapIntBVec =
> +			{ {301, { 1, 2, 3 }}, {-302, {4, 5, 6}}, {-303, {7, 8, 9}} };
> +		map<string, vector<uint8_t>> mapStrBVec =
> +			{ {"a", { 1, 2, 3 }}, {"b", {4, 5, 6}}, {"c", {7, 8, 9}} };
> +
> +		vector<uint8_t> buf;
> +		vector<int32_t> fds;
> +		int ret;
> +
> +		TEST_MAP_SERDES(uint64_t, string,          mapUintStr, nullptr);
> +		TEST_MAP_SERDES(int64_t,  string,          mapIntStr,  nullptr);
> +		TEST_MAP_SERDES(string,   string,          mapStrStr,  nullptr);
> +		TEST_MAP_CIM_SERDES(uint64_t, ControlInfoMap,  mapUintCIM, &cs);
> +		TEST_MAP_CIM_SERDES(int64_t,  ControlInfoMap,  mapIntCIM,  &cs);
> +		TEST_MAP_CIM_SERDES(string,   ControlInfoMap,  mapStrCIM,  &cs);
> +		TEST_MAP_VEC_SERDES(uint64_t, vector<uint8_t>, mapUintBVec, nullptr);
> +		TEST_MAP_VEC_SERDES(int64_t,  vector<uint8_t>, mapIntBVec,  nullptr);
> +		TEST_MAP_VEC_SERDES(string,   vector<uint8_t>, mapStrBVec,  nullptr);
> +
> +		return TestPass;
> +	}
> +
> +	int testPod()
> +	{
> +
> +#define TEST_POD_SERDES(type, var)				\
> +tie(buf, fds) = IPADataSerializer<type>::serialize(var);	\
> +type var##Out =							\
> +	IPADataSerializer<type>::deserialize(buf, fds);		\
> +ret = (var == var##Out);					\
> +if (!ret) {							\
> +	cerr << "Deserialized " << #var << " as " << var##Out	\
> +	     << ", expected " << var << endl;			\
> +	return TestFail;					\
> +}
> +
> +		uint32_t u32min = numeric_limits<uint32_t>::min();
> +		uint32_t u32max = numeric_limits<uint32_t>::max();
> +		uint32_t u32one = 1;
> +		int32_t  i32min = numeric_limits<int32_t>::min();
> +		int32_t  i32max = numeric_limits<int32_t>::max();
> +		int32_t  i32one = 1;
> +
> +		uint64_t u64min = numeric_limits<uint64_t>::min();
> +		uint64_t u64max = numeric_limits<uint64_t>::max();
> +		uint64_t u64one = 1;
> +		int64_t  i64min = numeric_limits<int64_t>::min();
> +		int64_t  i64max = numeric_limits<int64_t>::max();
> +		int64_t  i64one = 1;
> +
> +		float  flow = numeric_limits<float>::lowest();
> +		float  fmin = numeric_limits<float>::min();
> +		float  fmax = numeric_limits<float>::max();
> +		float  falmostOne = 1 + 1.0e-37;
> +		double dlow = numeric_limits<double>::lowest();
> +		double dmin = numeric_limits<double>::min();
> +		double dmax = numeric_limits<double>::max();
> +		double dalmostOne = 1 + 1.0e-307;
> +
> +		bool t = true;
> +		bool f = false;
> +
> +		stringstream ss;
> +		for (unsigned int i = 0; i < (1 << 21); i++)
> +			ss << "0123456789";

Wow, that's a very long string, 20MB. Maybe we could keep it a bit
shorter or reduce the test running time ?

> +
> +		string strLong = ss.str();
> +		string strEmpty = "";
> +
> +		vector<uint8_t> buf;
> +		vector<int32_t> fds;
> +		int ret;
> +
> +		TEST_POD_SERDES(uint32_t, u32min);
> +		TEST_POD_SERDES(uint32_t, u32max);
> +		TEST_POD_SERDES(uint32_t, u32one);
> +		TEST_POD_SERDES(int32_t,  i32min);
> +		TEST_POD_SERDES(int32_t,  i32max);
> +		TEST_POD_SERDES(int32_t,  i32one);
> +		TEST_POD_SERDES(uint64_t, u64min);
> +		TEST_POD_SERDES(uint64_t, u64max);
> +		TEST_POD_SERDES(uint64_t, u64one);
> +		TEST_POD_SERDES(int64_t,  i64min);
> +		TEST_POD_SERDES(int64_t,  i64max);
> +		TEST_POD_SERDES(int64_t,  i64one);
> +		TEST_POD_SERDES(float,    flow);
> +		TEST_POD_SERDES(float,    fmin);
> +		TEST_POD_SERDES(float,    fmax);
> +		TEST_POD_SERDES(float,    falmostOne);
> +		TEST_POD_SERDES(double,   dlow);
> +		TEST_POD_SERDES(double,   dmin);
> +		TEST_POD_SERDES(double,   dmax);
> +		TEST_POD_SERDES(double,   dalmostOne);
> +		TEST_POD_SERDES(bool,     t);
> +		TEST_POD_SERDES(bool,     f);
> +		TEST_POD_SERDES(string,   strLong);
> +		TEST_POD_SERDES(string,   strEmpty);
> +
> +		return TestPass;
> +	}
> +};
> +
> +TEST_REGISTER(IPADataSerializerTest)
> diff --git a/test/serialization/meson.build b/test/serialization/meson.build
> index 6fc54f6b..a4636337 100644
> --- a/test/serialization/meson.build
> +++ b/test/serialization/meson.build
> @@ -2,6 +2,7 @@
>  
>  serialization_tests = [
>      ['control_serialization',     'control_serialization.cpp'],
> +    ['ipa_data_serializer_test',  'ipa_data_serializer_test.cpp'],
>  ]
>  
>  foreach t : serialization_tests