[libcamera-devel] [PATCH 3/4] android: camera_device: Provide a toPixelFormat helper
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 29 22:20:43 CEST 2020
Hi Kieran,
Thank you for the patch.
On Mon, Jun 29, 2020 at 05:39:15PM +0100, Kieran Bingham wrote:
> Rather than converting pixelformats through the map, and then
> dereferencing the iterator later, create a helper to explicitly return a
> PixelFormat type.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 29 +++++++++++++++++++++--------
> src/android/camera_device.h | 1 +
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f788c11e7254..a6410f42fee8 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -923,6 +923,20 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)
> return requestTemplate->get();
> }
>
> +PixelFormat CameraDevice::toPixelFormat(int format)
> +{
> + /* Translate Android format code to libcamera pixel format. */
> + auto it = formatsMap_.find(format);
> + if (it == formatsMap_.end()) {
> + LOG(HAL, Error) << "Requested format "
> + << utils::hex(format)
> + << " not supported";
You could reduce the number of lines with
LOG(HAL, Error)
<< "Requested format " << utils::hex(format)
<< " not supported";
> + return PixelFormat();
> + }
> +
> + return it->second;
> +}
> +
> /*
> * Inspect the stream_list to produce a list of StreamConfiguration to
> * be use to configure the Camera.
> @@ -932,11 +946,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> for (unsigned int i = 0; i < stream_list->num_streams; ++i) {
> camera3_stream_t *stream = stream_list->streams[i];
>
> + PixelFormat format = toPixelFormat(stream->format);
> +
> LOG(HAL, Info) << "Stream #" << i
> << ", direction: " << stream->stream_type
> << ", width: " << stream->width
> << ", height: " << stream->height
> - << ", format: " << utils::hex(stream->format);
> + << ", format: " << utils::hex(stream->format)
> + << " (" << format.toString() << ")";
I wonder if it's really needed here, but I suppose it doesn't hurt
either.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> }
>
> /* Only one stream is supported. */
> @@ -947,13 +964,9 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> camera3_stream_t *camera3Stream = stream_list->streams[0];
>
> /* Translate Android format code to libcamera pixel format. */
> - auto it = formatsMap_.find(camera3Stream->format);
> - if (it == formatsMap_.end()) {
> - LOG(HAL, Error) << "Requested format "
> - << utils::hex(camera3Stream->format)
> - << " not supported";
> + PixelFormat format = toPixelFormat(camera3Stream->format);
> + if (!format.isValid())
> return -EINVAL;
> - }
>
> /*
> * Hardcode viewfinder role, replacing the generated configuration
> @@ -969,7 +982,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> StreamConfiguration *streamConfiguration = &config_->at(0);
> streamConfiguration->size.width = camera3Stream->width;
> streamConfiguration->size.height = camera3Stream->height;
> - streamConfiguration->pixelFormat = it->second;
> + streamConfiguration->pixelFormat = format;
>
> switch (config_->validate()) {
> case CameraConfiguration::Valid:
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index ed11410a5577..5bd6cf416156 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -72,6 +72,7 @@ private:
> std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
> void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
> + libcamera::PixelFormat toPixelFormat(int format);
> std::unique_ptr<CameraMetadata> getResultMetadata(int frame_number,
> int64_t timestamp);
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list