[libcamera-devel] [PATCH v3 1/2] android: post_processor: Change the type destination in process()

[Jacopo Mondi]

Hi Hiro,

On Thu, Jan 28, 2021 at 10:42:15PM +0000, Hirokazu Honda wrote:
> The type of the destination buffer in PostProcessor::process() is
> libcamera::Span. libcamera::Span is used for one dimension buffer
> (e.g. blob buffer). The destination can be multiple dimensions
> buffer (e.g. yuv frame). Therefore, this changes the type of the
> destination buffer to MappedFrameBuffer.

Thanks, this will make the recalculation of the JPEG buffer size
easier.

>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
>  src/android/camera_stream.cpp            | 4 ++--
>  src/android/camera_stream.h              | 6 ++++--
>  src/android/jpeg/post_processor_jpeg.cpp | 6 +++---
>  src/android/jpeg/post_processor_jpeg.h   | 2 +-
>  src/android/post_processor.h             | 7 ++++---
>  5 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 4c8020e5..611ec0d1 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -96,14 +96,14 @@ int CameraStream::configure()
>  }
>
>  int CameraStream::process(const libcamera::FrameBuffer &source,
> -			  MappedCamera3Buffer *dest,
> +			  libcamera::MappedBuffer *destination,
>  			  const CameraMetadata &requestMetadata,
>  			  CameraMetadata *resultMetadata)
>  {
>  	if (!postProcessor_)
>  		return 0;
>
> -	return postProcessor_->process(source, dest->maps()[0],
> +	return postProcessor_->process(source, destination,
>  				       requestMetadata, resultMetadata);
>  }
>
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 298ffbf6..73bac0ba 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -19,9 +19,10 @@
>  #include <libcamera/geometry.h>
>  #include <libcamera/pixel_format.h>
>
> +#include <libcamera/internal/buffer.h>
> +
>  class CameraDevice;
>  class CameraMetadata;
> -class MappedCamera3Buffer;
>  class PostProcessor;
>
>  class CameraStream
> @@ -119,9 +120,10 @@ public:
>
>  	int configure();
>  	int process(const libcamera::FrameBuffer &source,
> -		    MappedCamera3Buffer *dest,
> +		    libcamera::MappedBuffer *destination,
>  		    const CameraMetadata &requestMetadata,
>  		    CameraMetadata *resultMetadata);
> +
>  	libcamera::FrameBuffer *getBuffer();
>  	void putBuffer(libcamera::FrameBuffer *buffer);
>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index cac0087b..74e81c6f 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -83,7 +83,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
>  }
>
>  int PostProcessorJpeg::process(const FrameBuffer &source,
> -			       Span<uint8_t> destination,
> +			       libcamera::MappedBuffer *destination,
>  			       const CameraMetadata &requestMetadata,
>  			       CameraMetadata *resultMetadata)
>  {
> @@ -171,8 +171,8 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  	ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry);
>  	const uint8_t quality = ret ? *entry.data.u8 : 95;
>  	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &quality, 1);
> +	int jpeg_size = encoder_->encode(source, destination->maps()[0], exif.data(), quality);

I would not move this

>
> -	int jpeg_size = encoder_->encode(source, destination, exif.data(), quality);
>  	if (jpeg_size < 0) {
>  		LOG(JPEG, Error) << "Failed to encode stream image";
>  		return jpeg_size;
> @@ -190,7 +190,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  	 * \todo Investigate if the buffer size mismatch is an issue or
>  	 * expected behaviour.
>  	 */
> -	uint8_t *resultPtr = destination.data() +
> +	uint8_t *resultPtr = destination->maps()[0].data() +
>  			     cameraDevice_->maxJpegBufferSize() -
>  			     sizeof(struct camera3_jpeg_blob);
>  	auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index d2dfa450..7689de73 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -25,7 +25,7 @@ public:
>  	int configure(const libcamera::StreamConfiguration &incfg,
>  		      const libcamera::StreamConfiguration &outcfg) override;
>  	int process(const libcamera::FrameBuffer &source,
> -		    libcamera::Span<uint8_t> destination,
> +		    libcamera::MappedBuffer *destination,
>  		    const CameraMetadata &requestMetadata,
>  		    CameraMetadata *resultMetadata) override;
>
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index bda93bb4..f5c99f03 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -8,9 +8,10 @@
>  #define __ANDROID_POST_PROCESSOR_H__
>
>  #include <libcamera/buffer.h>
> -#include <libcamera/span.h>
>  #include <libcamera/stream.h>
>
> +#include <libcamera/internal/buffer.h>
> +
>  class CameraMetadata;
>
>  class PostProcessor
> @@ -21,9 +22,9 @@ public:
>  	virtual int configure(const libcamera::StreamConfiguration &inCfg,
>  			      const libcamera::StreamConfiguration &outCfg) = 0;
>  	virtual int process(const libcamera::FrameBuffer &source,
> -			    libcamera::Span<uint8_t> destination,
> +			    libcamera::MappedBuffer *destination,
>  			    const CameraMetadata &requestMetadata,
> -			    CameraMetadata *resultMetadata) = 0;
> +			    CameraMetadata *metadata) = 0;

Unrelated and not updated in the sub-classes. I would drop.

I think both can be fixed when applying, the rest looks good
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  };
>
>  #endif /* __ANDROID_POST_PROCESSOR_H__ */
> --
> 2.30.0.365.g02bc693789-goog
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel