[libcamera-devel] [PATCH v6 1/4] tests: Add IPADataSerializer test

Niklas Söderlund niklas.soderlund at ragnatech.se
Tue Dec 29 22:29:09 CET 2020


Hi Paul,

Thanks for your test.

On 2020-12-24 17:17:10 +0900, Paul Elder wrote:
> Test the IPADataSerializer for controls, vectors, maps, and PODs of
> built-in types.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Change since 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              | 464 ++++++++++++++++++
>  test/serialization/meson.build                |   3 +-
>  2 files changed, 466 insertions(+), 1 deletion(-)
>  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..914663ca
> --- /dev/null
> +++ b/test/serialization/ipa_data_serializer_test.cpp
> @@ -0,0 +1,464 @@
> +/* 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>
> +
> +#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])
> +			goto nequal;
> +
> +	return true;
> +
> +nequal:
> +	cerr << "lhs: { ";
> +	for (const auto &value : vecA)
> +		cerr << value << ", ";
> +	cerr << "}" << endl;
> +
> +	cerr << "rhs: { ";
> +	for (const auto &value : vecB)
> +		cerr << value << ", ";
> +	cerr << "}" << endl;

I don't think it's needed to print the vectors if the test fails, as it 
should not happen after development is done right ;-) If you feel 
strongly about it I think all vector compare functions should print on 
fail.

> +
> +	return false;
> +}
> +
> +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;
> +}
> +
> +template<typename K, typename V>
> +bool isMapEqual(map<K, V> &mapA, map<K, V> &mapB)
> +{
> +	if (mapA == mapB)
> +		return true;
> +
> +	cerr << "lhs: { ";
> +	for (const auto &value : mapA)
> +		cerr << value.first << " : " << value.second << ", ";
> +	cerr << "}" << endl;
> +
> +	cerr << "rhs: { ";
> +	for (const auto &value : mapB)
> +		cerr << value.first << " : " << value.second << ", ";
> +	cerr << "}" << endl;

Same comment as above about printing on failure.

> +
> +	return false;
> +}
> +
> +template<typename K, typename V>
> +bool isMapToCimEqual(map<K, V> &mapA, map<K, V> &mapB)
> +{
> +	bool isEqual = true;
> +
> +	auto itA = mapA.begin();
> +	auto itB = mapB.begin();
> +	while (true) {
> +		bool endA = (itA == mapA.end());
> +		bool endB = (itB == mapB.end());

I think you could check mapA.size() != mapB.size() before the loop to 
remove the need for these flags.

> +
> +		if (endA and endB)
> +			break;
> +
> +		if (!endA && (endB || itA->first < itB->first)) {
> +			cerr << "key: " << itA->first << " not in mapB" << endl;
> +			isEqual = false;
> +			itA++;
> +			continue;
> +		}
> +
> +		if (endA || itB->first < itA->first) {
> +			cerr << "key: " << itB->first << " not in mapA" << endl;
> +			isEqual = false;
> +			itB++;
> +			continue;
> +		}
> +
> +		if (!SerializationTest::equals(itA->second, itB->second)) {
> +			cerr << "key " << itA->first
> +			     << " has different values" << endl;
> +			isEqual = false;
> +		}
> +
> +		itA++;
> +		itB++;
> +	}
> +
> +	return isEqual;

Would it make sens to do redice this funnction to something like this 
(not tested)?

    if (mapA.size() != mapB.size())
        return false;

    for (cont auto &itA : mapA) {
        itB = mapB.find(itA->first);

        if (itB == mapB.end())
            return false;

        if (!SerializationTest::equals(itA->second, itB->second))
            return false;
    }

    return true;

> +}
> +
> +template<typename K, typename V>
> +bool isMapToVecEqual(map<K, V> &mapA, map<K, V> &mapB)
> +{
> +	if (mapA == mapB)
> +		return true;
> +
> +	cerr << "lhs: { ";
> +	for (const auto &value : mapA) {
> +		cerr << value.first << " : { ";
> +		for (const auto &v : value.second)
> +			cerr << v << ", ";
> +		cerr << "}" << endl;
> +	}
> +	cerr << "}" << endl;
> +
> +	cerr << "rhs: { ";
> +	for (const auto &value : mapB) {
> +		cerr << value.first << " : { ";
> +		for (const auto &v : value.second)
> +			cerr << v << ", ";
> +		cerr << "}" << endl;
> +	}
> +	cerr << "}" << endl;

Print or no print on fail ;-)

> +
> +	return false;
> +}
> +
> +class IPADataSerializerTest : public CameraTest, public Test
> +{
> +public:
> +	IPADataSerializerTest()
> +		: CameraTest("platform/vimc.0 Sensor B")
> +	{
> +	}
> +
> +protected:
> +	int init() override
> +	{
> +		return status_;
> +	}
> +
> +	int run() override
> +	{
> +		int finalRet = TestPass;
> +		int ret;
> +
> +		ret = testControls();
> +		if (ret != TestPass)
> +			finalRet = ret;
> +
> +		ret = testVector();
> +		if (ret != TestPass)
> +			finalRet = ret;
> +
> +		ret = testMap();
> +		if (ret != TestPass)
> +			finalRet = ret;
> +
> +		ret = testPod();
> +		if (ret != TestPass)
> +			finalRet = ret;
> +
> +		return finalRet;

I think it's OK to fail as early as possible and just drop the finalRet 
variable.

> +	}
> +
> +private:
> +	ControlList generateControlListA()
> +	{
> +		/* Create a control list with three controls. */
> +		const ControlInfoMap &infoMap = camera_->controls();
> +		ControlList list(infoMap);

nit: You could post the infoMap as an argument as you fetch it just 
before the only caller.

> +
> +		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 = generateControlListA();
> +
> +		vector<uint8_t> infoMapBuf;
> +		tie(infoMapBuf, ignore) =
> +			IPADataSerializer<ControlInfoMap>::serialize(infoMap, &cs);
> +
> +		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;\
> +	finalRet = TestFail;						\

I would fail early here with a 'return TestFail'.

> +}
> +
> +		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 finalRet = TestPass;
> +		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);
> +
> +		return finalRet;
> +	}
> +
> +	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 = isMapEqual<ktype, vtype>(m, m##Out);				\
> +if (!ret) {								\
> +	cerr << "Deserialized map " << #m << " doesn't match original" << endl;\
> +	finalRet = TestFail;						\

Same as above, fail early IMHO.

> +}
> +
> +#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;\
> +	finalRet = TestFail;						\

Ditto.

> +}
> +
> +#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 = isMapToVecEqual<ktype, vtype>(m, m##Out);				\
> +if (!ret) {								\
> +	cerr << "Deserialized map " << #m << " doesn't match original" << endl;\
> +	finalRet = TestFail;						\

Ditto.

> +}
> +
> +		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"} };
> +		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 finalRet = TestPass;
> +		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 finalRet;
> +	}
> +
> +	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;			\
> +	finalRet = 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";
> +
> +		string strLong = ss.str();
> +		string strEmpty = "";
> +
> +		vector<uint8_t> buf;
> +		vector<int32_t> fds;
> +		int finalRet = TestPass;
> +		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 finalRet;
> +	}
> +};
> +
> +TEST_REGISTER(IPADataSerializerTest)
> diff --git a/test/serialization/meson.build b/test/serialization/meson.build
> index a9d9cbcb..c140a31c 100644
> --- a/test/serialization/meson.build
> +++ b/test/serialization/meson.build
> @@ -1,7 +1,8 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  serialization_tests = [
> -    [ 'control_serialization',    'control_serialization.cpp' ],
> +    [ 'control_serialization',     'control_serialization.cpp' ],
> +    [ 'ipa_data_serializer_test',  'ipa_data_serializer_test.cpp' ],
>  ]
>  
>  foreach t : serialization_tests
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list