[libcamera-devel] [PATCH v8 6/7] Pass StreamBuffer to Encoder::encoder

[Laurent Pinchart]

Hi Harvey,

Thank you for the patch.

On Wed, Dec 14, 2022 at 09:33:29AM +0000, Harvey Yang wrote:
> From: Harvey Yang <chenghaoyang at chromium.org>
> 
> To prepare for the JEA encoder in the following patches, StreamBuffer is
> passed to Encoder::encoder, which contains the original FrameBuffer and
> Span |destination|.

We can explain why:

To prepare for support of the JEA encoder in a following commit, which
will need to access the buffer_handle_t of the destination buffer, pass
the StreamBuffer to the Encoder::encoder() function. As the StreamBuffer
contains the source FrameBuffer and the destination Span, drop them from
the function arguments and access them directly from the StreamBuffer.

> 
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
>  src/android/jpeg/encoder.h               |  5 +++--
>  src/android/jpeg/encoder_libjpeg.cpp     | 11 +++++++----
>  src/android/jpeg/encoder_libjpeg.h       |  3 +--
>  src/android/jpeg/post_processor_jpeg.cpp |  3 +--
>  4 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
> index 5f9ef890..14b1ab45 100644
> --- a/src/android/jpeg/encoder.h
> +++ b/src/android/jpeg/encoder.h
> @@ -12,14 +12,15 @@
>  #include <libcamera/framebuffer.h>
>  #include <libcamera/stream.h>
>  
> +#include "../camera_request.h"
> +
>  class Encoder
>  {
>  public:
>  	virtual ~Encoder() = default;
>  
>  	virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;
> -	virtual int encode(const libcamera::FrameBuffer &source,
> -			   libcamera::Span<uint8_t> destination,
> +	virtual int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
>  			   libcamera::Span<const uint8_t> exifData,
>  			   unsigned int quality) = 0;
>  	virtual void generateThumbnail(const libcamera::FrameBuffer &source,
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
> index cc5f37be..8bbba6e0 100644
> --- a/src/android/jpeg/encoder_libjpeg.cpp
> +++ b/src/android/jpeg/encoder_libjpeg.cpp
> @@ -24,6 +24,8 @@
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/mapped_framebuffer.h"
>  
> +#include "../camera_buffer.h"
> +
>  using namespace libcamera;
>  
>  LOG_DECLARE_CATEGORY(JPEG)
> @@ -187,17 +189,18 @@ void EncoderLibJpeg::Encoder::compressNV(const std::vector<Span<uint8_t>> &plane
>  	}
>  }
>  
> -int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,
> -			   Span<const uint8_t> exifData, unsigned int quality)
> +int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,

I'd name the parameter just `buffer` to shorten lines.

> +			   libcamera::Span<const uint8_t> exifData,
> +			   unsigned int quality)
>  {
> -	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
> +	MappedFrameBuffer frame(streamBuffer->srcBuffer, MappedFrameBuffer::MapFlag::Read);

Line wrap.

>  	if (!frame.isValid()) {
>  		LOG(JPEG, Error) << "Failed to map FrameBuffer : "
>  				 << strerror(frame.error());
>  		return frame.error();
>  	}
>  
> -	return captureEncoder_.encode(frame.planes(), dest, exifData, quality);
> +	return captureEncoder_.encode(frame.planes(), streamBuffer->dstBuffer->plane(0), exifData, quality);

And here especially.

As before, no need to resubmit just for this, I can handle it and add my

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  }
>  
>  void EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,
> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
> index fa077b8d..f4bf99ba 100644
> --- a/src/android/jpeg/encoder_libjpeg.h
> +++ b/src/android/jpeg/encoder_libjpeg.h
> @@ -24,8 +24,7 @@ public:
>  	~EncoderLibJpeg();
>  
>  	int configure(const libcamera::StreamConfiguration &cfg) override;
> -	int encode(const libcamera::FrameBuffer &source,
> -		   libcamera::Span<uint8_t> destination,
> +	int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
>  		   libcamera::Span<const uint8_t> exifData,
>  		   unsigned int quality) override;
>  	void generateThumbnail(const libcamera::FrameBuffer &source,
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 69b18a2e..918ab492 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -146,8 +146,7 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu
>  	const uint8_t quality = ret ? *entry.data.u8 : 95;
>  	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality);
>  
> -	int jpeg_size = encoder_->encode(source, destination->plane(0),
> -					 exif.data(), quality);
> +	int jpeg_size = encoder_->encode(streamBuffer, exif.data(), quality);
>  	if (jpeg_size < 0) {
>  		LOG(JPEG, Error) << "Failed to encode stream image";
>  		processComplete.emit(streamBuffer, PostProcessor::Status::Error);

-- 
Regards,

Laurent Pinchart