[libcamera-devel] [PATCH] android; camera_device: Reset config_ if Camera::configure() fails

[Umang Jain]

Hi Laurent,

On 6/15/21 5:50 AM, Laurent Pinchart wrote:
> The config_ pointer is reset in all error paths of the
> CameraDevice::configureStreams() function, except when
> Camera::configure() fails. Fix it by using a local unique pointer to
> store the configuration until the end of the function, to avoid similar
> issues in the future.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>   src/android/camera_device.cpp | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f3fd38690e63..f0b5f87bc38f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1716,8 +1716,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>   	 * Generate an empty configuration, and construct a StreamConfiguration
>   	 * for each camera3_stream to add to it.
>   	 */
> -	config_ = camera_->generateConfiguration();
> -	if (!config_) {
> +	std::unique_ptr<CameraConfiguration> config = camera_->generateConfiguration();
> +	if (!config) {
>   		LOG(HAL, Error) << "Failed to generate camera configuration";
>   		return -EINVAL;
>   	}
> @@ -1855,29 +1855,27 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>   
>   	sortCamera3StreamConfigs(streamConfigs, jpegStream);
>   	for (const auto &streamConfig : streamConfigs) {
> -		config_->addConfiguration(streamConfig.config);
> +		config->addConfiguration(streamConfig.config);
>   
>   		for (auto &stream : streamConfig.streams) {
>   			streams_.emplace_back(this, stream.type, stream.stream,
> -					      config_->size() - 1);
> +					      config->size() - 1);
>   			stream.stream->priv = static_cast<void *>(&streams_.back());
>   		}
>   	}
>   
> -	switch (config_->validate()) {
> +	switch (config->validate()) {
>   	case CameraConfiguration::Valid:
>   		break;
>   	case CameraConfiguration::Adjusted:
>   		LOG(HAL, Info) << "Camera configuration adjusted";
>   
> -		for (const StreamConfiguration &cfg : *config_)
> +		for (const StreamConfiguration &cfg : *config)
>   			LOG(HAL, Info) << " - " << cfg.toString();
>   
> -		config_.reset();
>   		return -EINVAL;
>   	case CameraConfiguration::Invalid:
>   		LOG(HAL, Info) << "Camera configuration invalid";
> -		config_.reset();
>   		return -EINVAL;
>   	}
>   
> @@ -1885,7 +1883,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>   	 * Once the CameraConfiguration has been adjusted/validated
>   	 * it can be applied to the camera.
>   	 */
> -	int ret = camera_->configure(config_.get());
> +	int ret = camera_->configure(config.get());
>   	if (ret) {
>   		LOG(HAL, Error) << "Failed to configure camera '"
>   				<< camera_->id() << "'";
> @@ -1905,6 +1903,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>   		}
>   	}
>   
> +	config_ = std::move(config);
>   	return 0;
>   }
>