[libcamera-devel] [PATCH v2] android: camera_mode: Reserve 'data' vectors

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Dec 01, 2020 at 04:59:30PM +0100, Jacopo Mondi wrote:
> The CameraDevice::getStaticMetadata() function populates the
> entries for Android's static metadata by walking the ControlInfo
> supported values reported by the libcamera pipeline.
> 
> The number of entries to be passed to Android is computed using the
> vector's size which is initialized at vector creation time to the
> maximum number of available entries.
> 
> In order to report the correct number of metadata do not create the
> vector with the largest possible number of elements but only reserve
> space for them using std::vector::reserve() which does not modify the
> vector's size.

Indeed, I had even missed that we were creating vectors with a few
default-initiliazed entries, and added additional entries after that.

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

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

> ---
> This patch fixes cros_camera_test:
> Camera3DeviceTest/Camera3DeviceDefaultSettings.ConstructDefaultSettings/1

You could include this in the commit message.

> ---
>  src/android/camera_device.cpp | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4690346e05cb..4eb05df0fdc2 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -596,7 +596,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> 
>  	/* Color correction static metadata. */
>  	{
> -		std::vector<uint8_t> data(3);
> +		std::vector<uint8_t> data;
> +		data.reserve(3);
>  		const auto &infoMap = controlsInfo.find(&controls::draft::ColorCorrectionAberrationMode);
>  		if (infoMap != controlsInfo.end()) {
>  			for (const auto &value : infoMap->second.values())
> @@ -782,7 +783,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  &maxFaceCount, 1);
> 
>  	{
> -		std::vector<uint8_t> data(2);
> +		std::vector<uint8_t> data;
> +		data.reserve(2);
>  		const auto &infoMap = controlsInfo.find(&controls::draft::LensShadingMapMode);
>  		if (infoMap != controlsInfo.end()) {
>  			for (const auto &value : infoMap->second.values())
> @@ -850,7 +852,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> 
>  	/* Noise reduction modes. */
>  	{
> -		std::vector<uint8_t> data(5);
> +		std::vector<uint8_t> data;
> +		data.reserve(5);
>  		const auto &infoMap = controlsInfo.find(&controls::draft::NoiseReductionMode);
>  		if (infoMap != controlsInfo.end()) {
>  			for (const auto &value : infoMap->second.values())

-- 
Regards,

Laurent Pinchart