diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h index 8a63ae44a13e..307ecba572fc 100644 --- a/include/libcamera/internal/control_serializer.h +++ b/include/libcamera/internal/control_serializer.h @@ -47,9 +47,13 @@ private: static void store(const ControlValue &value, ByteStreamBuffer &buffer); static void store(const ControlInfo &info, ByteStreamBuffer &buffer); + void populateControlValueEntry(struct ipa_control_value_entry &entry, + const ControlValue &value, + uint32_t offset); + ControlValue loadControlValue(ByteStreamBuffer &buffer, - bool isArray = false, unsigned int count = 1); - ControlInfo loadControlInfo(ByteStreamBuffer &buffer); + ControlType type, + bool isArray, unsigned int count); unsigned int serial_; unsigned int serialSeed_; diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h index 980668c86bcc..ecdf6051ffd9 100644 --- a/include/libcamera/ipa/ipa_controls.h +++ b/include/libcamera/ipa/ipa_controls.h @@ -15,7 +15,7 @@ namespace libcamera { extern "C" { #endif -#define IPA_CONTROLS_FORMAT_VERSION 1 +#define IPA_CONTROLS_FORMAT_VERSION 2 enum ipa_controls_id_map_type { IPA_CONTROL_ID_MAP_CONTROLS, @@ -34,20 +34,25 @@ struct ipa_controls_header { }; struct ipa_control_value_entry { - uint32_t id; uint8_t type; uint8_t is_array; uint16_t count; uint32_t offset; uint32_t padding[1]; + uint32_t reserved; +}; + +struct ipa_control_list_entry { + uint32_t id; + struct ipa_control_value_entry value; }; struct ipa_control_info_entry { uint32_t id; uint32_t type; - uint32_t offset; uint8_t direction; uint8_t padding[3]; + uint32_t reserved; }; #ifdef __cplusplus diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index 050f8512bd52..f576a868204f 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -144,7 +144,7 @@ void ControlSerializer::reset() size_t ControlSerializer::binarySize(const ControlValue &value) { - return sizeof(ControlType) + value.data().size_bytes(); + return value.data().size_bytes(); } size_t ControlSerializer::binarySize(const ControlInfo &info) @@ -164,7 +164,8 @@ size_t ControlSerializer::binarySize(const ControlInfo &info) size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap) { size_t size = sizeof(struct ipa_controls_header) - + infoMap.size() * sizeof(struct ipa_control_info_entry); + + infoMap.size() * (sizeof(struct ipa_control_info_entry) + + 3 * sizeof(struct ipa_control_value_entry)); for (const auto &ctrl : infoMap) size += binarySize(ctrl.second); @@ -184,7 +185,7 @@ size_t ControlSerializer::binarySize(const ControlInfoMap &infoMap) size_t ControlSerializer::binarySize(const ControlList &list) { size_t size = sizeof(struct ipa_controls_header) - + list.size() * sizeof(struct ipa_control_value_entry); + + list.size() * sizeof(struct ipa_control_list_entry); for (const auto &ctrl : list) size += binarySize(ctrl.second); @@ -195,16 +196,17 @@ size_t ControlSerializer::binarySize(const ControlList &list) void ControlSerializer::store(const ControlValue &value, ByteStreamBuffer &buffer) { - const ControlType type = value.type(); - buffer.write(&type); buffer.write(value.data()); } -void ControlSerializer::store(const ControlInfo &info, ByteStreamBuffer &buffer) +void ControlSerializer::populateControlValueEntry(struct ipa_control_value_entry &entry, + const ControlValue &value, + uint32_t offset) { - store(info.min(), buffer); - store(info.max(), buffer); - store(info.def(), buffer); + entry.type = value.type(); + entry.is_array = value.isArray(); + entry.count = value.numElements(); + entry.offset = offset; } /** @@ -232,7 +234,8 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, /* Compute entries and data required sizes. */ size_t entriesSize = infoMap.size() - * sizeof(struct ipa_control_info_entry); + * (sizeof(struct ipa_control_info_entry) + + 3 * sizeof(struct ipa_control_value_entry)); size_t valuesSize = 0; for (const auto &ctrl : infoMap) valuesSize += binarySize(ctrl.second); @@ -280,11 +283,32 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, struct ipa_control_info_entry entry; entry.id = id->id(); entry.type = id->type(); - entry.offset = values.offset(); entry.direction = static_cast(id->direction()); entries.write(&entry); - store(info, values); + /* + * Write the metadata for the ControlValue entries as well, + * since we need type, isArray, and numElements information for + * min/max/def of the ControlInfo. Doing it this way is the + * least intrusive in terms of changing the structs in + * ipa_controls.h + */ + struct ipa_control_value_entry valueEntry; + + populateControlValueEntry(valueEntry, info.min(), + values.offset()); + entries.write(&valueEntry); + store(info.min(), values); + + populateControlValueEntry(valueEntry, info.max(), + values.offset()); + entries.write(&valueEntry); + store(info.max(), values); + + populateControlValueEntry(valueEntry, info.def(), + values.offset()); + entries.write(&valueEntry); + store(info.def(), values); } if (buffer.overflow()) @@ -341,7 +365,7 @@ int ControlSerializer::serialize(const ControlList &list, else idMapType = IPA_CONTROL_ID_MAP_V4L2; - size_t entriesSize = list.size() * sizeof(struct ipa_control_value_entry); + size_t entriesSize = list.size() * sizeof(struct ipa_control_list_entry); size_t valuesSize = 0; for (const auto &ctrl : list) valuesSize += binarySize(ctrl.second); @@ -365,12 +389,9 @@ int ControlSerializer::serialize(const ControlList &list, unsigned int id = ctrl.first; const ControlValue &value = ctrl.second; - struct ipa_control_value_entry entry; + struct ipa_control_list_entry entry; entry.id = id; - entry.type = value.type(); - entry.is_array = value.isArray(); - entry.count = value.numElements(); - entry.offset = values.offset(); + populateControlValueEntry(entry.value, value, values.offset()); entries.write(&entry); store(value, values); @@ -383,12 +404,10 @@ int ControlSerializer::serialize(const ControlList &list, } ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer, + ControlType type, bool isArray, unsigned int count) { - ControlType type; - buffer.read(&type); - ControlValue value; value.reserve(type, isArray, count); @@ -397,15 +416,6 @@ ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer, return value; } -ControlInfo ControlSerializer::loadControlInfo(ByteStreamBuffer &b) -{ - ControlValue min = loadControlValue(b); - ControlValue max = loadControlValue(b); - ControlValue def = loadControlValue(b); - - return ControlInfo(min, max, def); -} - /** * \fn template T ControlSerializer::deserialize(ByteStreamBuffer &buffer) * \brief Deserialize an object from a binary buffer @@ -483,9 +493,11 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & ControlInfoMap::Map ctrls; for (unsigned int i = 0; i < hdr->entries; ++i) { - const struct ipa_control_info_entry *entry = - entries.read(); - if (!entry) { + const auto *entry = entries.read(); + const auto *min_entry = entries.read(); + const auto *max_entry = entries.read(); + const auto *def_entry = entries.read(); + if (!entry || !min_entry || !max_entry || !def_entry) { LOG(Serializer, Error) << "Out of data"; return {}; } @@ -511,15 +523,39 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & const ControlId *controlId = idMap->at(entry->id); ASSERT(controlId); - if (entry->offset != values.offset()) { + if (min_entry->offset != values.offset()) { LOG(Serializer, Error) - << "Bad data, entry offset mismatch (entry " + << "Bad data, entry offset mismatch (min entry " + << i << ")"; + return {}; + } + ControlValue min = + loadControlValue(values, static_cast(min_entry->type), + min_entry->is_array, min_entry->count); + + if (max_entry->offset != values.offset()) { + LOG(Serializer, Error) + << "Bad data, entry offset mismatch (max entry " << i << ")"; return {}; } + ControlValue max = + loadControlValue(values, static_cast(max_entry->type), + max_entry->is_array, max_entry->count); + + if (def_entry->offset != values.offset()) { + LOG(Serializer, Error) + << "Bad data, entry offset mismatch (def entry " + << i << ")"; + return {}; + } + ControlValue def = + loadControlValue(values, static_cast(def_entry->type), + def_entry->is_array, def_entry->count); + /* Create and store the ControlInfo. */ - ctrls.emplace(controlId, loadControlInfo(values)); + ctrls.emplace(controlId, ControlInfo(min, max, def)); } /* @@ -618,12 +654,12 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer ControlList ctrls(*idMap); for (unsigned int i = 0; i < hdr->entries; ++i) { - const struct ipa_control_value_entry *entry = - entries.read(); - if (!entry) { + auto *list_entry = entries.read(); + if (!list_entry) { LOG(Serializer, Error) << "Out of data"; return {}; } + const ipa_control_value_entry *entry = &list_entry->value; if (entry->offset != values.offset()) { LOG(Serializer, Error) @@ -632,8 +668,9 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer return {}; } - ctrls.set(entry->id, - loadControlValue(values, entry->is_array, entry->count)); + ctrls.set(list_entry->id, + loadControlValue(values, static_cast(entry->type), + entry->is_array, entry->count)); } return ctrls; diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp index 12d92ebe894d..8f73c8fda811 100644 --- a/src/libcamera/ipa_controls.cpp +++ b/src/libcamera/ipa_controls.cpp @@ -192,8 +192,6 @@ static_assert(sizeof(ipa_controls_header) == 32, /** * \struct ipa_control_value_entry * \brief Description of a serialized ControlValue entry - * \var ipa_control_value_entry::id - * The numerical ID of the control * \var ipa_control_value_entry::type * The type of the control (defined by enum ControlType) * \var ipa_control_value_entry::is_array @@ -205,11 +203,25 @@ static_assert(sizeof(ipa_controls_header) == 32, * value data (shall be a multiple of 8 bytes). * \var ipa_control_value_entry::padding * Padding bytes (shall be set to 0) + * \var ipa_control_value_entry::reserved + * Reserved for future extensions */ static_assert(sizeof(ipa_control_value_entry) == 16, "Invalid ABI size change for struct ipa_control_value_entry"); +/** + * \struct ipa_control_list_entry + * \brief Description of a serialized ControlList entry + * \var ipa_control_list_entry::id + * The numerical ID of the control + * \var ipa_control_list_entry::value + * The description of the serialized ControlValue + */ + +static_assert(sizeof(ipa_control_list_entry) == 20, + "Invalid ABI size change for struct ipa_control_list_entry"); + /** * \struct ipa_control_info_entry * \brief Description of a serialized ControlInfo entry @@ -217,8 +229,6 @@ static_assert(sizeof(ipa_control_value_entry) == 16, * The numerical ID of the control * \var ipa_control_info_entry::type * The type of the control (defined by enum ControlType) - * \var ipa_control_info_entry::offset - * The offset in bytes from the beginning of the data section to the control * info data (shall be a multiple of 8 bytes) * \var ipa_control_info_entry::direction * The directions in which the control is allowed to be sent. This is a flags @@ -226,6 +236,8 @@ static_assert(sizeof(ipa_control_value_entry) == 16, * metadata). \sa ControlId::Direction * \var ipa_control_info_entry::padding * Padding bytes (shall be set to 0) + * \var ipa_control_info_entry::reserved + * Reserved for future extensions */ static_assert(sizeof(ipa_control_info_entry) == 16,