[libcamera-devel] [PATCH v7 5/6] Pass StreamBuffer to Encoder::encoder
Cheng-Hao Yang
chenghaoyang at chromium.org
Wed Dec 14 10:35:48 CET 2022
Thanks Laurent!
Adopted.
On Wed, Dec 7, 2022 at 12:56 PM Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> Hi Harvey,
>
> Thank you for the patch.
>
> On Thu, Dec 01, 2022 at 09:27:32AM +0000, Harvey Yang via libcamera-devel
> 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|.
> >
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > ---
> > src/android/jpeg/encoder.h | 5 +++--
> > src/android/jpeg/encoder_libjpeg.cpp | 11 +++++++++++
> > src/android/jpeg/encoder_libjpeg.h | 7 +++++--
> > src/android/jpeg/post_processor_jpeg.cpp | 3 +--
> > 4 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
> > index 7dc1ee27..eeff87b1 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 int generateThumbnail(
> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp
> b/src/android/jpeg/encoder_libjpeg.cpp
> > index cc37fde3..d8d72fbd 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)
> > @@ -192,6 +194,15 @@ void EncoderLibJpeg::compressNV(const
> std::vector<Span<uint8_t>> &planes)
> > }
> > }
> >
> > +int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer
> *streamBuffer,
> > + libcamera::Span<const uint8_t> exifData,
> > + unsigned int quality)
> > +{
> > + return encode(*streamBuffer->srcBuffer,
> > + streamBuffer->dstBuffer.get()->plane(0), exifData,
> > + quality);
> > +}
>
> Do you need to create a wrapper around the encode() function that takes a
> FrameBuffer, or could you merge the two together ? It seems to be called
> from here only. The following diff compiles:
>
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp
> b/src/android/jpeg/encoder_libjpeg.cpp
> index d8d72fbdd6b8..6d7a3cbf586d 100644
> --- a/src/android/jpeg/encoder_libjpeg.cpp
> +++ b/src/android/jpeg/encoder_libjpeg.cpp
> @@ -198,9 +198,19 @@ int
> EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality)
> {
> - return encode(*streamBuffer->srcBuffer,
> - streamBuffer->dstBuffer.get()->plane(0), exifData,
> - quality);
> + const FrameBuffer &source = *streamBuffer->srcBuffer;
> + Span<uint8_t> dest = streamBuffer->dstBuffer.get()->plane(0);
> +
> + compress_ = &image_data_.compress;
> +
> + MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
> + if (!frame.isValid()) {
> + LOG(JPEG, Error) << "Failed to map FrameBuffer : "
> + << strerror(frame.error());
> + return frame.error();
> + }
> +
> + return encode(frame.planes(), dest, exifData, quality);
> }
>
> int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer
> &source,
> @@ -255,21 +265,6 @@ int EncoderLibJpeg::generateThumbnail(const
> libcamera::FrameBuffer &source,
> return -1;
> }
>
> -int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,
> - Span<const uint8_t> exifData, unsigned int
> quality)
> -{
> - compress_ = &image_data_.compress;
> -
> - MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
> - if (!frame.isValid()) {
> - LOG(JPEG, Error) << "Failed to map FrameBuffer : "
> - << strerror(frame.error());
> - return frame.error();
> - }
> -
> - return encode(frame.planes(), dest, exifData, quality);
> -}
> -
> int EncoderLibJpeg::encode(const std::vector<Span<uint8_t>> &src,
> Span<uint8_t> dest, Span<const uint8_t>
> exifData,
> unsigned int quality)
> diff --git a/src/android/jpeg/encoder_libjpeg.h
> b/src/android/jpeg/encoder_libjpeg.h
> index 6e9b65d4fc94..5928837367dc 100644
> --- a/src/android/jpeg/encoder_libjpeg.h
> +++ b/src/android/jpeg/encoder_libjpeg.h
> @@ -43,10 +43,6 @@ private:
> libcamera::PixelFormat pixelFormat,
> libcamera::Size size);
>
> - int encode(const libcamera::FrameBuffer &source,
> - libcamera::Span<uint8_t> destination,
> - libcamera::Span<const uint8_t> exifData,
> - unsigned int quality);
> int encode(const std::vector<libcamera::Span<uint8_t>> &planes,
> libcamera::Span<uint8_t> destination,
> libcamera::Span<const uint8_t> exifData,
>
>
> With that addressed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +
> > int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer
> &source,
> > const libcamera::Size &targetSize,
> > unsigned int quality,
> > diff --git a/src/android/jpeg/encoder_libjpeg.h
> b/src/android/jpeg/encoder_libjpeg.h
> > index 1ec2ba13..6e9b65d4 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;
> > int generateThumbnail(
> > @@ -44,6 +43,10 @@ private:
> > libcamera::PixelFormat pixelFormat,
> > libcamera::Size size);
> >
> > + int encode(const libcamera::FrameBuffer &source,
> > + libcamera::Span<uint8_t> destination,
> > + libcamera::Span<const uint8_t> exifData,
> > + unsigned int quality);
> > int encode(const std::vector<libcamera::Span<uint8_t>> &planes,
> > libcamera::Span<uint8_t> destination,
> > libcamera::Span<const uint8_t> exifData,
> > diff --git a/src/android/jpeg/post_processor_jpeg.cpp
> b/src/android/jpeg/post_processor_jpeg.cpp
> > index 60eebb11..10ac4666 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221214/ed38d9ed/attachment.htm>
More information about the libcamera-devel
mailing list