[libcamera-devel] [PATCH v4 3/3] android: camera_device: Configure one stream for identical stream requests

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Wed, Sep 01, 2021 at 05:03:02PM +0900, Hirokazu Honda wrote:
> An Android HAL client may request identical stream requests. It is

s/identical stream requests/multiple identical streams/

> redundant that a native camera device produces a separate stream for
> each of the identical requests.
> Configure camera one stream configuration for the identical stream
> requests.

And here I'd write

Configure the camera with a single stream in that case. The other
identical HAL streams will be produced by the YUV post-processor.

> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 324b997f..51d5370e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -618,12 +618,38 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  			continue;
>  		}
>  
> +		/*
> +		 * Always add GRALLOC_USAGE_HW_CAMERA_WRITE to the usage on
> +		 * ChromeOS because cros::CameraBufferManager imports the stream
> +		 * buffer with the usage.

I'm still not sure to follow you. What happens if you don't set
GRALLOC_USAGE_HW_CAMERA_WRITE ?

> +		 */
> +#if defined(OS_CHROMEOS)
> +		stream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;
> +#endif
> +
> +		/*
> +		 * If a CameraStream with the same size and format of the
> +		 * current stream has already been requested, associate the two.
> +		 */
> +		auto iter = std::find_if(
> +			streamConfigs.begin(), streamConfigs.end(),
> +			[size, format](const Camera3StreamConfig &streamConfig) {
> +				return streamConfig.config.size == size &&
> +				       streamConfig.config.pixelFormat == format;
> +			});
> +		if (iter != streamConfigs.end()) {
> +			/* Add usage to copy the buffer in streams[0] to stream. */
> +			iter->streams[0].stream->usage |= GRALLOC_USAGE_SW_READ_OFTEN;
> +			stream->usage |= GRALLOC_USAGE_SW_WRITE_OFTEN;
> +			iter->streams.push_back({ stream, CameraStream::Type::Mapped });
> +			continue;
> +		}
> +
>  		Camera3StreamConfig streamConfig;
>  		streamConfig.streams = { { stream, CameraStream::Type::Direct } };
>  		streamConfig.config.size = size;
>  		streamConfig.config.pixelFormat = format;
>  		streamConfigs.push_back(std::move(streamConfig));
> -

This isn't needed.

>  		/* This stream will be produced by hardware. */
>  		stream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;
>  	}

-- 
Regards,

Laurent Pinchart