[libcamera-devel] [PATCH] android: camera_device: Update gralloc usage flags for streams

[Jacopo Mondi]

Hi Laurent,

On Fri, Mar 05, 2021 at 01:56:28PM +0200, Laurent Pinchart wrote:
> When configuring streams, the camera HAL is supposed to update the
> gralloc usage flags to reflect the operations it will need to do on the
> stream buffers. Failure to do so leads to incorrect format selection by
> gralloc for the HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED format, as
> gralloc will not take into consideration the need of the camera to
> access the buffers.

You know how happy this makes me!

I've tested with OpenCamera and run CTS without noticing regressions,
so please add
Tested-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
 j


>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index ae01c362559e..3ee683705d00 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1539,6 +1539,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  		streamConfig.config.size = size;
>  		streamConfig.config.pixelFormat = format;
>  		streamConfigs.push_back(std::move(streamConfig));
> +
> +		/* This stream will be produced by hardware. */
> +		stream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;
>  	}
>
>  	/* Now handle the MJPEG streams, adding a new stream if required. */
> @@ -1548,7 +1551,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>
>  		/* Search for a compatible stream in the non-JPEG ones. */
>  		for (size_t i = 0; i < streamConfigs.size(); ++i) {
> -			const auto &cfg = streamConfigs[i].config;
> +			Camera3StreamConfig &streamConfig = streamConfigs[i];
> +			const auto &cfg = streamConfig.config;
>
>  			/*
>  			 * \todo The PixelFormat must also be compatible with
> @@ -1563,6 +1567,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>
>  			type = CameraStream::Type::Mapped;
>  			index = i;
> +
> +			/*
> +			 * The source stream will be read by software to
> +			 * produce the JPEG stream.
> +			 */
> +			camera3_stream_t *stream = streamConfig.streams[0].stream;
> +			stream->usage |= GRALLOC_USAGE_SW_READ_OFTEN;
>  			break;
>  		}
>
> @@ -1590,6 +1601,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  			index = streamConfigs.size() - 1;
>  		}
>
> +		/* The JPEG stream will be produced by software. */
> +		jpegStream->usage |= GRALLOC_USAGE_SW_WRITE_OFTEN;
> +
>  		streamConfigs[index].streams.push_back({ jpegStream, type });
>  	}
>
> --
> Regards,
>
> Laurent Pinchart
>