[libcamera-devel] [PATCH v3 2/2] android: CameraDevice: Add stop()

[Jacopo Mondi]

Hi Hiro,

On Mon, Mar 29, 2021 at 03:11:19PM +0900, Hirokazu Honda wrote:
> This adds CameraDevice::stop(), which cleans up the member
> variables of CameraDevice. It is called in CameraDevice::close()
> and CameraDevice::configureStreams().
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/camera_device.cpp | 17 +++++++++--------
>  src/android/camera_device.h   |  2 ++
>  2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 02d3bfb2..d5447d8e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -659,12 +659,17 @@ int CameraDevice::open(const hw_module_t *hardwareModule)
>
>  void CameraDevice::close()
>  {
> -	streams_.clear();
> +	stop();
> +
> +	camera_->release();
> +}
>
> +void CameraDevice::stop()
> +{
>  	worker_.stop();
>  	camera_->stop();

I think the camera shall be stopped only if (running_). Otherwise
you'll get an error message from the Camera state maching if I'm not
mistaken.

> -	camera_->release();
>
> +	descriptors_.clear();

Ideally, this should be done as:
1) Introduce stop() in CameraDevice
2) Introduce descriptors_ and clear it in stop()

As this bundle two changes in one patch.

A minor though.

The patch is fine, but I think the discussion with Laurent on what has
triggered this issue in first place is still a bit on-going, right ?

>  	running_ = false;
>  }
>
> @@ -1547,12 +1552,8 @@ PixelFormat CameraDevice::toPixelFormat(int format) const
>   */
>  int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  {
> -	/* Before any configuration attempt, stop the camera if it's running. */
> -	if (running_) {
> -		worker_.stop();
> -		camera_->stop();
> -		running_ = false;
> -	}
> +	/* Before any configuration attempt, stop the camera. */
> +	stop();
>
>  	/*
>  	 * Generate an empty configuration, and construct a StreamConfiguration
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 5a2d22d6..cc12fe20 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -87,6 +87,8 @@ private:
>  		int androidFormat;
>  	};
>
> +	void stop();
> +
>  	int initializeStreamConfigurations();
>  	std::vector<libcamera::Size>
>  	getYUVResolutions(libcamera::CameraConfiguration *cameraConfig,
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel