libcamera: control_serializer: Remove unnecessary allocation
diff mbox series

Message ID 20260107103830.1415267-1-paul.elder@ideasonboard.com
State New
Headers show
Series
  • libcamera: control_serializer: Remove unnecessary allocation
Related show

Commit Message

Paul Elder Jan. 7, 2026, 10:38 a.m. UTC 
In between versions of the patch "libcamera: control_serializer: Add
array info to serialized ControlValue", ipa_control_value_entry was
changed to be members of ipa_control_info_entry as opposed to being
serialized at the same level. The binarySize/entriesSize computation was
not updated, however, leaving some extra memory allocated for the
serialized form of ControlInfoMap.

Fix this by removing the extra size for 3 * ipa_control_value_entry..

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/libcamera/control_serializer.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Dan Scally Jan. 7, 2026, 10:50 a.m. UTC | #1 
Hi Paul

On 07/01/2026 10:38, Paul Elder wrote:
> In between versions of the patch "libcamera: control_serializer: Add
> array info to serialized ControlValue", ipa_control_value_entry was
> changed to be members of ipa_control_info_entry as opposed to being
> serialized at the same level. The binarySize/entriesSize computation was
> not updated, however, leaving some extra memory allocated for the
> serialized form of ControlInfoMap.
> 
> Fix this by removing the extra size for 3 * ipa_control_value_entry..
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>   src/libcamera/control_serializer.cpp | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 843e2772f848..8812e6e23d5a 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -164,8 +164,7 @@ 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) +
> -					3 * sizeof(struct ipa_control_value_entry));
> +		    + infoMap.size() * (sizeof(struct ipa_control_info_entry));
>   
>   	for (const auto &ctrl : infoMap)
>   		size += binarySize(ctrl.second);
> @@ -234,8 +233,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
>   
>   	/* Compute entries and data required sizes. */
>   	size_t entriesSize = infoMap.size()
> -			   * (sizeof(struct ipa_control_info_entry) +
> -			      3 * sizeof(struct ipa_control_value_entry));
> +			   * (sizeof(struct ipa_control_info_entry));
>   	size_t valuesSize = 0;
>   	for (const auto &ctrl : infoMap)
>   		valuesSize += binarySize(ctrl.second);
Barnabás Pőcze Jan. 7, 2026, 11:32 a.m. UTC | #2 
Hi

2026. 01. 07. 11:38 keltezéssel, Paul Elder írta:
> In between versions of the patch "libcamera: control_serializer: Add
> array info to serialized ControlValue", ipa_control_value_entry was
> changed to be members of ipa_control_info_entry as opposed to being
> serialized at the same level. The binarySize/entriesSize computation was
> not updated, however, leaving some extra memory allocated for the
> serialized form of ControlInfoMap.
> 
> Fix this by removing the extra size for 3 * ipa_control_value_entry..

Extra `.` at the end?


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

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/libcamera/control_serializer.cpp | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 843e2772f848..8812e6e23d5a 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -164,8 +164,7 @@ 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) +
> -					3 * sizeof(struct ipa_control_value_entry));
> +		    + infoMap.size() * (sizeof(struct ipa_control_info_entry));
>   
>   	for (const auto &ctrl : infoMap)
>   		size += binarySize(ctrl.second);
> @@ -234,8 +233,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
>   
>   	/* Compute entries and data required sizes. */
>   	size_t entriesSize = infoMap.size()
> -			   * (sizeof(struct ipa_control_info_entry) +
> -			      3 * sizeof(struct ipa_control_value_entry));
> +			   * (sizeof(struct ipa_control_info_entry));
>   	size_t valuesSize = 0;
>   	for (const auto &ctrl : infoMap)
>   		valuesSize += binarySize(ctrl.second);
Laurent Pinchart Jan. 7, 2026, 1:26 p.m. UTC | #3 
Hi Paul,

Thank you for the patch.

On Wed, Jan 07, 2026 at 07:38:30PM +0900, Paul Elder wrote:
> In between versions of the patch "libcamera: control_serializer: Add
> array info to serialized ControlValue", ipa_control_value_entry was
> changed to be members of ipa_control_info_entry as opposed to being
> serialized at the same level. The binarySize/entriesSize computation was
> not updated, however, leaving some extra memory allocated for the
> serialized form of ControlInfoMap.
> 
> Fix this by removing the extra size for 3 * ipa_control_value_entry..
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/libcamera/control_serializer.cpp | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 843e2772f848..8812e6e23d5a 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -164,8 +164,7 @@ 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) +
> -					3 * sizeof(struct ipa_control_value_entry));
> +		    + infoMap.size() * (sizeof(struct ipa_control_info_entry));

You can remove the parenthesis around sizeof.

>  
>  	for (const auto &ctrl : infoMap)
>  		size += binarySize(ctrl.second);
> @@ -234,8 +233,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
>  
>  	/* Compute entries and data required sizes. */
>  	size_t entriesSize = infoMap.size()
> -			   * (sizeof(struct ipa_control_info_entry) +
> -			      3 * sizeof(struct ipa_control_value_entry));
> +			   * (sizeof(struct ipa_control_info_entry));

Same here.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	size_t valuesSize = 0;
>  	for (const auto &ctrl : infoMap)
>  		valuesSize += binarySize(ctrl.second);

Patch
diff mbox series 

diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
index 843e2772f848..8812e6e23d5a 100644
--- a/src/libcamera/control_serializer.cpp
+++ b/src/libcamera/control_serializer.cpp
@@ -164,8 +164,7 @@  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) +
-					3 * sizeof(struct ipa_control_value_entry));
+		    + infoMap.size() * (sizeof(struct ipa_control_info_entry));
 
 	for (const auto &ctrl : infoMap)
 		size += binarySize(ctrl.second);
@@ -234,8 +233,7 @@  int ControlSerializer::serialize(const ControlInfoMap &infoMap,
 
 	/* Compute entries and data required sizes. */
 	size_t entriesSize = infoMap.size()
-			   * (sizeof(struct ipa_control_info_entry) +
-			      3 * sizeof(struct ipa_control_value_entry));
+			   * (sizeof(struct ipa_control_info_entry));
 	size_t valuesSize = 0;
 	for (const auto &ctrl : infoMap)
 		valuesSize += binarySize(ctrl.second);