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

[Jacopo Mondi]

Hi Laurent,

On Tue, Dec 01, 2020 at 05:06:30PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Dec 01, 2020 at 04:00:56PM +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.
> >
> > For each entry a vector of the size of the maximum number of possible
> > entries is reserved, populated and then stored in the Android's metadata
> > pack.
> >
> > The number of actual entries to be passed to Android is computed using
> > the vector's size which, for this reason, shall be resized to the actual
> > number of entries it stores.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > This patch fixes cros_camera_test:
> > Camera3DeviceTest/Camera3DeviceDefaultSettings.ConstructDefaultSettings/1
> > ---
> >  src/android/camera_device.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index d43db3600b20..d559f0fc4b81 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -602,8 +602,10 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		if (infoMap != controlsInfo.end()) {
> >  			for (const auto &value : infoMap->second.values())
> >  				data.push_back(value.get<int32_t>());
> > +			data.resize(infoMap->second.values().size());
> >  		} else {
> >  			data.push_back(ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF);
> > +			data.resize(1);
> >  		}
>
> Wouldn't it be better to avoid creating the vector with a too large size
> then ? Maybe something along the lines of the following ?
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4690346e05cb..a4d4914c2f80 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);

Oh, std::vector::reserve doesn't change the size of the vector.

That's better than resizing (even if the number of possible relocation
is still 0 if I'm not mistaken)

>  		const auto &infoMap = controlsInfo.find(&controls::draft::ColorCorrectionAberrationMode);
>  		if (infoMap != controlsInfo.end()) {
>  			for (const auto &value : infoMap->second.values())
>
>
> >  		staticMetadata_->addEntry(ANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,
> >  					  data.data(), data.size());
> > @@ -803,8 +805,10 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		if (infoMap != controlsInfo.end()) {
> >  			for (const auto &value : infoMap->second.values())
> >  				data.push_back(value.get<int32_t>());
> > +			data.resize(infoMap->second.values().size());
> >  		} else {
> >  			data.push_back(ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF);
> > +			data.resize(1);
> >  		}
> >  		staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_LENS_SHADING_MAP_MODES,
> >  					  data.data(), data.size());
> > @@ -871,8 +875,10 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		if (infoMap != controlsInfo.end()) {
> >  			for (const auto &value : infoMap->second.values())
> >  				data.push_back(value.get<int32_t>());
> > +			data.resize(infoMap->second.values().size());
> >  		} else {
> >  			data.push_back(ANDROID_NOISE_REDUCTION_MODE_OFF);
> > +			data.resize(1);
> >  		}
> >  		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> >  					  data.data(), data.size());
>
> --
> Regards,
>
> Laurent Pinchart