<div dir="ltr">Thanks Laurent!<br><div><br></div><div>Adopted.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 7, 2022 at 12:56 PM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey,<br>
<br>
Thank you for the patch.<br>
<br>
On Thu, Dec 01, 2022 at 09:27:32AM +0000, Harvey Yang via libcamera-devel wrote:<br>
> From: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> <br>
> To prepare for the JEA encoder in the following patches, StreamBuffer is<br>
> passed to Encoder::encoder, which contains the original FrameBuffer and<br>
> Span |destination|.<br>
> <br>
> Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> ---<br>
> src/android/jpeg/encoder.h | 5 +++--<br>
> src/android/jpeg/encoder_libjpeg.cpp | 11 +++++++++++<br>
> src/android/jpeg/encoder_libjpeg.h | 7 +++++--<br>
> src/android/jpeg/post_processor_jpeg.cpp | 3 +--<br>
> 4 files changed, 20 insertions(+), 6 deletions(-)<br>
> <br>
> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h<br>
> index 7dc1ee27..eeff87b1 100644<br>
> --- a/src/android/jpeg/encoder.h<br>
> +++ b/src/android/jpeg/encoder.h<br>
> @@ -12,14 +12,15 @@<br>
> #include <libcamera/framebuffer.h><br>
> #include <libcamera/stream.h><br>
> <br>
> +#include "../camera_request.h"<br>
> +<br>
> class Encoder<br>
> {<br>
> public:<br>
> virtual ~Encoder() = default;<br>
> <br>
> virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;<br>
> - virtual int encode(const libcamera::FrameBuffer &source,<br>
> - libcamera::Span<uint8_t> destination,<br>
> + virtual int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> libcamera::Span<const uint8_t> exifData,<br>
> unsigned int quality) = 0;<br>
> virtual int generateThumbnail(<br>
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp<br>
> index cc37fde3..d8d72fbd 100644<br>
> --- a/src/android/jpeg/encoder_libjpeg.cpp<br>
> +++ b/src/android/jpeg/encoder_libjpeg.cpp<br>
> @@ -24,6 +24,8 @@<br>
> #include "libcamera/internal/formats.h"<br>
> #include "libcamera/internal/mapped_framebuffer.h"<br>
> <br>
> +#include "../camera_buffer.h"<br>
> +<br>
> using namespace libcamera;<br>
> <br>
> LOG_DECLARE_CATEGORY(JPEG)<br>
> @@ -192,6 +194,15 @@ void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)<br>
> }<br>
> }<br>
> <br>
> +int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> + libcamera::Span<const uint8_t> exifData,<br>
> + unsigned int quality)<br>
> +{<br>
> + return encode(*streamBuffer->srcBuffer,<br>
> + streamBuffer->dstBuffer.get()->plane(0), exifData,<br>
> + quality);<br>
> +}<br>
<br>
Do you need to create a wrapper around the encode() function that takes a<br>
FrameBuffer, or could you merge the two together ? It seems to be called<br>
from here only. The following diff compiles:<br>
<br>
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp<br>
index d8d72fbdd6b8..6d7a3cbf586d 100644<br>
--- a/src/android/jpeg/encoder_libjpeg.cpp<br>
+++ b/src/android/jpeg/encoder_libjpeg.cpp<br>
@@ -198,9 +198,19 @@ int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
libcamera::Span<const uint8_t> exifData,<br>
unsigned int quality)<br>
{<br>
- return encode(*streamBuffer->srcBuffer,<br>
- streamBuffer->dstBuffer.get()->plane(0), exifData,<br>
- quality);<br>
+ const FrameBuffer &source = *streamBuffer->srcBuffer;<br>
+ Span<uint8_t> dest = streamBuffer->dstBuffer.get()->plane(0);<br>
+<br>
+ compress_ = &image_data_.compress;<br>
+<br>
+ MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);<br>
+ if (!frame.isValid()) {<br>
+ LOG(JPEG, Error) << "Failed to map FrameBuffer : "<br>
+ << strerror(frame.error());<br>
+ return frame.error();<br>
+ }<br>
+<br>
+ return encode(frame.planes(), dest, exifData, quality);<br>
}<br>
<br>
int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,<br>
@@ -255,21 +265,6 @@ int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,<br>
return -1;<br>
}<br>
<br>
-int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,<br>
- Span<const uint8_t> exifData, unsigned int quality)<br>
-{<br>
- compress_ = &image_data_.compress;<br>
-<br>
- MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);<br>
- if (!frame.isValid()) {<br>
- LOG(JPEG, Error) << "Failed to map FrameBuffer : "<br>
- << strerror(frame.error());<br>
- return frame.error();<br>
- }<br>
-<br>
- return encode(frame.planes(), dest, exifData, quality);<br>
-}<br>
-<br>
int EncoderLibJpeg::encode(const std::vector<Span<uint8_t>> &src,<br>
Span<uint8_t> dest, Span<const uint8_t> exifData,<br>
unsigned int quality)<br>
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h<br>
index 6e9b65d4fc94..5928837367dc 100644<br>
--- a/src/android/jpeg/encoder_libjpeg.h<br>
+++ b/src/android/jpeg/encoder_libjpeg.h<br>
@@ -43,10 +43,6 @@ private:<br>
libcamera::PixelFormat pixelFormat,<br>
libcamera::Size size);<br>
<br>
- int encode(const libcamera::FrameBuffer &source,<br>
- libcamera::Span<uint8_t> destination,<br>
- libcamera::Span<const uint8_t> exifData,<br>
- unsigned int quality);<br>
int encode(const std::vector<libcamera::Span<uint8_t>> &planes,<br>
libcamera::Span<uint8_t> destination,<br>
libcamera::Span<const uint8_t> exifData,<br>
<br>
<br>
With that addressed,<br>
<br>
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
<br>
> +<br>
> int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,<br>
> const libcamera::Size &targetSize,<br>
> unsigned int quality,<br>
> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h<br>
> index 1ec2ba13..6e9b65d4 100644<br>
> --- a/src/android/jpeg/encoder_libjpeg.h<br>
> +++ b/src/android/jpeg/encoder_libjpeg.h<br>
> @@ -24,8 +24,7 @@ public:<br>
> ~EncoderLibJpeg();<br>
> <br>
> int configure(const libcamera::StreamConfiguration &cfg) override;<br>
> - int encode(const libcamera::FrameBuffer &source,<br>
> - libcamera::Span<uint8_t> destination,<br>
> + int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> libcamera::Span<const uint8_t> exifData,<br>
> unsigned int quality) override;<br>
> int generateThumbnail(<br>
> @@ -44,6 +43,10 @@ private:<br>
> libcamera::PixelFormat pixelFormat,<br>
> libcamera::Size size);<br>
> <br>
> + int encode(const libcamera::FrameBuffer &source,<br>
> + libcamera::Span<uint8_t> destination,<br>
> + libcamera::Span<const uint8_t> exifData,<br>
> + unsigned int quality);<br>
> int encode(const std::vector<libcamera::Span<uint8_t>> &planes,<br>
> libcamera::Span<uint8_t> destination,<br>
> libcamera::Span<const uint8_t> exifData,<br>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp<br>
> index 60eebb11..10ac4666 100644<br>
> --- a/src/android/jpeg/post_processor_jpeg.cpp<br>
> +++ b/src/android/jpeg/post_processor_jpeg.cpp<br>
> @@ -146,8 +146,7 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu<br>
> const uint8_t quality = ret ? *entry.data.u8 : 95;<br>
> resultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality);<br>
> <br>
> - int jpeg_size = encoder_->encode(source, destination->plane(0),<br>
> - exif.data(), quality);<br>
> + int jpeg_size = encoder_->encode(streamBuffer, exif.data(), quality);<br>
> if (jpeg_size < 0) {<br>
> LOG(JPEG, Error) << "Failed to encode stream image";<br>
> processComplete.emit(streamBuffer, PostProcessor::Status::Error);<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div>