[libcamera-devel] [PATCH v3 3/4] Rework JPEG encoder API and update PostProcessorJpeg and EncoderLibJpeg
Cheng-Hao Yang
chenghaoyang at chromium.org
Thu Sep 1 23:07:28 CEST 2022
Hi Laurent,
On Fri, Sep 2, 2022 at 4:09 AM Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> Hi Harvey,
>
> On Wed, Jun 22, 2022 at 08:37:58PM +0800, Cheng-Hao Yang wrote:
> > Hi Laurent,
> >
> > Uploaded PATCH v5 and fixed the following:
> >
> > > > > int configure(const libcamera::StreamConfiguration &cfg)
> override;
> > > > > + int encode(Camera3RequestDescriptor::StreamBuffer
> *streamBuffer,
> > > > > + libcamera::Span<const uint8_t> exifData,
> > > > > + unsigned int quality) override;
> > > > > + int generateThumbnail(
> > > > > + const libcamera::FrameBuffer &source,
> > > > > + const libcamera::Size &targetSize,
> > > > > + unsigned int quality,
> > > > > + std::vector<unsigned char> *thumbnail) override;
> > > >
> > > > Same here.
> > >
> > > I think I need your help on the indentation format. Any doc or rule I
> can
> > > study? Thanks!
> >
> > I was using gmail to view your comment. Now I use `
> > https://patchwork.libcamera.org/patch`
> <https://patchwork.libcamera.org/patch>, and the tabs are correctly
> aligned.
> > Done.
>
> I'm afraid I can't really help with getting gmail to display the e-mail
> contents properly. Maybe you have more leverage than I do there ;-)
> Jokes aside, I think most people avoid webmail for patch review.
>
> While on the topic of e-mails, could you plese try to reply inline in
> the patch instead of copying content and answers to the beginning ? It
> gets difficult to get the whole context otherwise.
>
>
Right, I'll start to do that from now on. Thanks for letting me know :)
> > > > You now use the same libjpeg encoder instance for both the main image
> > > > and the thumbnail. This leads to EncoderLibJpeg::internalConfigure()
> being
> > > > called twice for every frame. The function looks fairly cheap so it
> should
> > > > be fine, but could you confirm that jpeg_set_defaults() doesn't
> cause a
> > > > large overhead ? Please mention this change in the commit message of
> the
> > > > appropriate patch, it's an important one.
> > >
> > > Originally for each frame we need at least one call to
> > > jpeg_set_defaults() to encode the thumbnail, and we need two now with
> patch
> > > v3, so I thought it was fine in terms of the complexity, while you're
> right
> > > that I'm not sure about the complexity of jpeg_set_defaults(), even
> looking
> > > into the implementation [1]. I can also modify the code not to
> configure
> > > twice per frame. WDYT?
> >
> > Done with only one jpeg_set_defaults() being called per frame. Please
> check.
>
> Thank you.
>
> > On Tue, May 3, 2022 at 11:29 AM Cheng-Hao Yang wrote:
> >
> > > Hi Laurent,
> > >
> > > Sorry I forgot to send this reply before going on vacation...
> > >
> > > > The e-mail address should use "@" instead of " at ".
> > > Oh I see! I checked the libcamera archives for examples, and thought
> > > that was intentional haha.
> > > Updated.
> > >
> > > > cfg_ is only used here, I suppose to reconfigure the libjpeg encoder
> for
> > > the
> > > Is this a deprecated comment?
> > >
> > > > > int configure(const libcamera::StreamConfiguration &cfg)
> > > override;
> > > > > + int encode(Camera3RequestDescriptor::StreamBuffer
> *streamBuffer,
> > > > > + libcamera::Span<const uint8_t> exifData,
> > > > > + unsigned int quality) override;
> > > > > + int generateThumbnail(
> > > > > + const libcamera::FrameBuffer &source,
> > > > > + const libcamera::Size &targetSize,
> > > > > + unsigned int quality,
> > > > > + std::vector<unsigned char> *thumbnail) override;
> > >
> > > > Same here.
> > > I think I need your help on the indentation format. Any doc or rule I
> can
> > > study? Thanks!
> > >
> > > > I'm tempted to simplify this by using conditional compilation (we
> have a
> > > OS_CHROMEOS macro), the function could then go to
> post_processor_jpeg.cpp.
> > > Oh I didn't notice there's such a macro to use. Thanks! I think we can
> get
> > > rid of the PostProcessorJpeg::createEncoder() then.
> > >
> > > > You now use the same libjpeg encoder instance for both the main image
> > > and the thumbnail. This leads to EncoderLibJpeg::internalConfigure()
> being
> > > called twice for every frame. The function looks fairly cheap so it
> should
> > > be fine, but could you confirm that jpeg_set_defaults() doesn't cause a
> > > large overhead ? Please mention this change in the commit message of
> the
> > > appropriate patch, it's an important one.
> > > Originally for each frame we need at least one call to
> jpeg_set_defaults()
> > > to encode the thumbnail, and we need two now with patch v3, so I
> thought it
> > > was fine in terms of the complexity, while you're right that I'm not
> sure
> > > about the complexity of jpeg_set_defaults(), even looking into the
> > > implementation [1]. I can also modify the code not to configure twice
> per
> > > frame. WDYT?
> > >
> > > [1]: https://github.com/kornelski/libjpeg/blob/master/jcparam.c#L268
> > >
> > >
> > > On Wed, Apr 27, 2022 at 7:44 AM Laurent Pinchart <
> > > laurent.pinchart at ideasonboard.com> wrote:
> > >
> > >> Hi Harvey,
> > >>
> > >> Thank you for the patch.
> > >>
> > >> On Tue, Apr 26, 2022 at 09:14:08AM +0000, Harvey Yang via
> libcamera-devel
> > >> wrote:
> > >> > To add JEA in the next CL, this CL reworks JPEG encoder API and move
> > >>
> > >> Same comment as in 1/4 for "CL" (I won't repeat it for the other
> patches
> > >> in this series, please update them as applicable).
> > >>
> > >> > thumbnail encoding code into EncoderLibJpeg's implementation, as JEA
> > >> > supports that as well.
> > >> >
> > >> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > >>
> > >> The e-mail address should use "@" instead of " at ".
> > >>
> > >> > ---
> > >> > src/android/jpeg/encoder.h | 9 ++-
> > >> > src/android/jpeg/encoder_libjpeg.cpp | 70
> +++++++++++++++++++
> > >> > src/android/jpeg/encoder_libjpeg.h | 21 +++++-
> > >> > .../jpeg/generic_post_processor_jpeg.cpp | 14 ++++
> > >> > src/android/jpeg/meson.build | 9 +++
> > >> > src/android/jpeg/post_processor_jpeg.cpp | 60 ++--------------
> > >> > src/android/jpeg/post_processor_jpeg.h | 11 +--
> > >> > src/android/meson.build | 5 +-
> > >>
> > >> This is more reviewable than in the previous version, but there are
> > >> still quite a few changes bundled in the same patch. As a v4 will be
> > >> needed anyway, could you split this further as follows ? Feel free to
> > >> change the order as appropriate.
> > >>
> > >> - Add meson.build file in jpeg/ directory
> > >> - Move thumbnail generate to Encoder class
> > >> - Pass streamBuffer to encode() to prepare for JPEG encoders that need
> > >> access to the HAL buffer handle
> > >> - Add PostProcessorJpeg::SetEncoder()
> > >>
> > >> > 8 files changed, 128 insertions(+), 71 deletions(-)
> > >> > create mode 100644 src/android/jpeg/generic_post_processor_jpeg.cpp
> > >> > create mode 100644 src/android/jpeg/meson.build
> > >> >
> > >> > diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
> > >> > index b974d367..6d527d91 100644
> > >> > --- a/src/android/jpeg/encoder.h
> > >> > +++ b/src/android/jpeg/encoder.h
> > >> > @@ -12,14 +12,19 @@
> > >> > #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(const libcamera::FrameBuffer
> > >> &source,
> > >> > + const libcamera::Size
> &targetSize,
> > >> > + unsigned int quality,
> > >> > + std::vector<unsigned char>
> > >> *thumbnail) = 0;
> > >> > };
> > >> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp
> > >> b/src/android/jpeg/encoder_libjpeg.cpp
> > >> > index 21a3b33d..b5591e33 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)
> > >> > @@ -82,8 +84,17 @@ EncoderLibJpeg::~EncoderLibJpeg()
> > >> > }
> > >> >
> > >> > int EncoderLibJpeg::configure(const StreamConfiguration &cfg)
> > >> > +{
> > >> > + thumbnailer_.configure(cfg.size, cfg.pixelFormat);
> > >> > + cfg_ = cfg;
> > >>
> > >> I'd store the size and pixel format only, not the full
> > >> StreamConfiguration, as they are all you need. internalConfigure()
> > >> should then take a size and pixel format, which will make it easier to
> > >> use in generateThumbnail().
> > >>
> > >> > +
> > >> > + return internalConfigure(cfg);
> > >> > +}
> > >> > +
> > >> > +int EncoderLibJpeg::internalConfigure(const StreamConfiguration
> &cfg)
> > >>
> > >> Let's name the function configureEncoder().
> > >>
> > >> > {
> > >> > const struct JPEGPixelFormatInfo info =
> > >> findPixelInfo(cfg.pixelFormat);
> > >> > +
> > >> > if (info.colorSpace == JCS_UNKNOWN)
> > >> > return -ENOTSUP;
> > >> >
> > >> > @@ -178,6 +189,65 @@ 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)
> > >> > +{
> > >> > + internalConfigure(cfg_);
> > >>
> > >> cfg_ is only used here, I suppose to reconfigure the libjpeg encoder
> for
> > >> the
> > >>
> > >> > + return encode(*streamBuffer->srcBuffer,
> > >> streamBuffer->dstBuffer.get()->plane(0), exifData, quality);
> > >>
> > >> Let's shorten the line a bit (we aim for 80 columns as a soft target)
> > >>
> > >> return encode(*streamBuffer->srcBuffer,
> > >> streamBuffer->dstBuffer.get()->plane(0),
> > >> exifData, quality);
> > >>
> > >> > +}
> > >> > +
> > >> > +int EncoderLibJpeg::generateThumbnail(
> > >> > + const libcamera::FrameBuffer &source,
> > >> > + const libcamera::Size &targetSize,
> > >> > + unsigned int quality,
> > >> > + std::vector<unsigned char> *thumbnail)
> > >>
> > >> int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer
> > >> &source,
> > >> const libcamera::Size
> &targetSize,
> > >> unsigned int quality,
> > >> std::vector<unsigned char>
> > >> *thumbnail)
> > >>
> > >> to match the coding style.
> > >>
> > >> > +{
> > >> > + /* Stores the raw scaled-down thumbnail bytes. */
> > >> > + std::vector<unsigned char> rawThumbnail;
> > >> > +
> > >> > + thumbnailer_.createThumbnail(source, targetSize,
> &rawThumbnail);
> > >> > +
> > >> > + StreamConfiguration thCfg;
> > >> > + thCfg.size = targetSize;
> > >> > + thCfg.pixelFormat = thumbnailer_.pixelFormat();
> > >> > + int ret = internalConfigure(thCfg);
> > >>
> > >> You now use the same libjpeg encoder instance for both the main image
> > >> and the thumbnail. This leads to EncoderLibJpeg::internalConfigure()
> > >> being called twice for every frame. The function looks fairly cheap so
> > >> it should be fine, but could you confirm that jpeg_set_defaults()
> > >> doesn't cause a large overhead ? Please mention this change in the
> > >> commit message of the appropriate patch, it's an important one.
> > >>
> > >> > +
> > >> > + if (!rawThumbnail.empty() && !ret) {
> > >> > + /*
> > >> > + * \todo Avoid value-initialization of all elements
> of the
> > >> > + * vector.
> > >> > + */
> > >> > + thumbnail->resize(rawThumbnail.size());
> > >> > +
> > >> > + /*
> > >> > + * Split planes manually as the encoder expects a
> vector
> > >> of
> > >> > + * planes.
> > >> > + *
> > >> > + * \todo Pass a vector of planes directly to
> > >> > + * Thumbnailer::createThumbnailer above and remove the
> > >> manual
> > >> > + * planes split from here.
> > >> > + */
> > >> > + std::vector<Span<uint8_t>> thumbnailPlanes;
> > >> > + const PixelFormatInfo &formatNV12 =
> > >> PixelFormatInfo::info(formats::NV12);
> > >> > + size_t yPlaneSize = formatNV12.planeSize(targetSize,
> 0);
> > >> > + size_t uvPlaneSize = formatNV12.planeSize(targetSize,
> 1);
> > >> > + thumbnailPlanes.push_back({ rawThumbnail.data(),
> > >> yPlaneSize });
> > >> > + thumbnailPlanes.push_back({ rawThumbnail.data() +
> > >> yPlaneSize, uvPlaneSize });
> > >> > +
> > >> > + int jpeg_size = encode(thumbnailPlanes, *thumbnail,
> {},
> > >> quality);
> > >> > + thumbnail->resize(jpeg_size);
> > >> > +
> > >> > + LOG(JPEG, Debug)
> > >> > + << "Thumbnail compress returned "
> > >> > + << jpeg_size << " bytes";
> > >> > +
> > >> > + return jpeg_size;
> > >> > + }
> > >> > +
> > >> > + return -1;
> > >> > +}
> > >> > +
> > >> > int EncoderLibJpeg::encode(const FrameBuffer &source, 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 1b3ac067..56b27bae 100644
> > >> > --- a/src/android/jpeg/encoder_libjpeg.h
> > >> > +++ b/src/android/jpeg/encoder_libjpeg.h
> > >> > @@ -15,6 +15,8 @@
> > >> >
> > >> > #include <jpeglib.h>
> > >> >
> > >> > +#include "thumbnailer.h"
> > >> > +
> > >> > class EncoderLibJpeg : public Encoder
> > >> > {
> > >> > public:
> > >> > @@ -22,19 +24,32 @@ public:
> > >> > ~EncoderLibJpeg();
> > >> >
> > >> > int configure(const libcamera::StreamConfiguration &cfg)
> override;
> > >> > + int encode(Camera3RequestDescriptor::StreamBuffer
> *streamBuffer,
> > >> > + libcamera::Span<const uint8_t> exifData,
> > >> > + unsigned int quality) override;
> > >> > + int generateThumbnail(
> > >> > + const libcamera::FrameBuffer &source,
> > >> > + const libcamera::Size &targetSize,
> > >> > + unsigned int quality,
> > >> > + std::vector<unsigned char> *thumbnail) override;
> > >>
> > >> Same here.
> > >>
> > >> > +
> > >> > +private:
> > >> > + int internalConfigure(const libcamera::StreamConfiguration
> &cfg);
> > >> > +
> > >> > int encode(const libcamera::FrameBuffer &source,
> > >> > libcamera::Span<uint8_t> destination,
> > >> > libcamera::Span<const uint8_t> exifData,
> > >> > - unsigned int quality) override;
> > >> > + unsigned int quality);
> > >> > int encode(const std::vector<libcamera::Span<uint8_t>>
> &planes,
> > >> > libcamera::Span<uint8_t> destination,
> > >> > libcamera::Span<const uint8_t> exifData,
> > >> > unsigned int quality);
> > >> >
> > >> > -private:
> > >> > void compressRGB(const std::vector<libcamera::Span<uint8_t>>
> > >> &planes);
> > >> > void compressNV(const std::vector<libcamera::Span<uint8_t>>
> > >> &planes);
> > >> >
> > >> > + libcamera::StreamConfiguration cfg_;
> > >> > +
> > >> > struct jpeg_compress_struct compress_;
> > >> > struct jpeg_error_mgr jerr_;
> > >> >
> > >> > @@ -42,4 +57,6 @@ private:
> > >> >
> > >> > bool nv_;
> > >> > bool nvSwap_;
> > >> > +
> > >> > + Thumbnailer thumbnailer_;
> > >> > };
> > >> > diff --git a/src/android/jpeg/generic_post_processor_jpeg.cpp
> > >> b/src/android/jpeg/generic_post_processor_jpeg.cpp
> > >> > new file mode 100644
> > >> > index 00000000..890f6972
> > >> > --- /dev/null
> > >> > +++ b/src/android/jpeg/generic_post_processor_jpeg.cpp
> > >> > @@ -0,0 +1,14 @@
> > >> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > >> > +/*
> > >> > + * Copyright (C) 2022, Google Inc.
> > >> > + *
> > >> > + * generic_post_processor_jpeg.cpp - Generic JPEG Post Processor
> > >> > + */
> > >> > +
> > >> > +#include "encoder_libjpeg.h"
> > >> > +#include "post_processor_jpeg.h"
> > >> > +
> > >> > +void PostProcessorJpeg::SetEncoder()
> > >>
> > >> We use camelCase for function names, not CamelCase. Let's name the
> > >> function createEncoder(), as it doesn't just set it but actually
> creates
> > >> it.
> > >>
> > >> > +{
> > >> > + encoder_ = std::make_unique<EncoderLibJpeg>();
> > >>
> > >> I'm tempted to simplify this by using conditional compilation (we
> have a
> > >> OS_CHROMEOS macro), the function could then go to
> > >> post_processor_jpeg.cpp.
> > >>
> > >> > +}
> > >> > diff --git a/src/android/jpeg/meson.build
> b/src/android/jpeg/meson.build
> > >> > new file mode 100644
> > >> > index 00000000..94522dc0
> > >> > --- /dev/null
> > >> > +++ b/src/android/jpeg/meson.build
> > >> > @@ -0,0 +1,9 @@
> > >> > +# SPDX-License-Identifier: CC0-1.0
> > >> > +
> > >> > +android_hal_sources += files([
> > >> > + 'exif.cpp',
> > >> > + 'post_processor_jpeg.cpp'])
> > >>
> > >> The '])' should be on the next line.
> > >>
> > >> > +
> > >> > +android_hal_sources += files(['encoder_libjpeg.cpp',
> > >> > + 'generic_post_processor_jpeg.cpp',
> > >> > + 'thumbnailer.cpp'])
> > >>
> > >> Similar comment here,
> > >>
> > >> android_hal_sources += files([
> > >> 'encoder_libjpeg.cpp',
> > >> 'generic_post_processor_jpeg.cpp',
> > >> 'thumbnailer.cpp',
> > >> ])
> > >>
> > >> > diff --git a/src/android/jpeg/post_processor_jpeg.cpp
> > >> b/src/android/jpeg/post_processor_jpeg.cpp
> > >> > index d72ebc3c..7ceba60e 100644
> > >> > --- a/src/android/jpeg/post_processor_jpeg.cpp
> > >> > +++ b/src/android/jpeg/post_processor_jpeg.cpp
> > >> > @@ -9,10 +9,10 @@
> > >> >
> > >> > #include <chrono>
> > >> >
> > >> > +#include "../android_framebuffer.h"
> > >>
> > >> Is this needed ?
> > >>
> > >> > #include "../camera_device.h"
> > >> > #include "../camera_metadata.h"
> > >> > #include "../camera_request.h"
> > >> > -#include "encoder_libjpeg.h"
> > >> > #include "exif.h"
> > >> >
> > >> > #include <libcamera/base/log.h>
> > >> > @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const
> > >> StreamConfiguration &inCfg,
> > >> >
> > >> > streamSize_ = outCfg.size;
> > >> >
> > >> > - thumbnailer_.configure(inCfg.size, inCfg.pixelFormat);
> > >> > -
> > >> > - encoder_ = std::make_unique<EncoderLibJpeg>();
> > >> > + SetEncoder();
> > >> >
> > >> > return encoder_->configure(inCfg);
> > >> > }
> > >> >
> > >> > -void PostProcessorJpeg::generateThumbnail(const FrameBuffer
> &source,
> > >> > - const Size &targetSize,
> > >> > - unsigned int quality,
> > >> > - std::vector<unsigned char>
> > >> *thumbnail)
> > >> > -{
> > >> > - /* Stores the raw scaled-down thumbnail bytes. */
> > >> > - std::vector<unsigned char> rawThumbnail;
> > >> > -
> > >> > - thumbnailer_.createThumbnail(source, targetSize,
> &rawThumbnail);
> > >> > -
> > >> > - StreamConfiguration thCfg;
> > >> > - thCfg.size = targetSize;
> > >> > - thCfg.pixelFormat = thumbnailer_.pixelFormat();
> > >> > - int ret = thumbnailEncoder_.configure(thCfg);
> > >> > -
> > >> > - if (!rawThumbnail.empty() && !ret) {
> > >> > - /*
> > >> > - * \todo Avoid value-initialization of all elements
> of the
> > >> > - * vector.
> > >> > - */
> > >> > - thumbnail->resize(rawThumbnail.size());
> > >> > -
> > >> > - /*
> > >> > - * Split planes manually as the encoder expects a
> vector
> > >> of
> > >> > - * planes.
> > >> > - *
> > >> > - * \todo Pass a vector of planes directly to
> > >> > - * Thumbnailer::createThumbnailer above and remove the
> > >> manual
> > >> > - * planes split from here.
> > >> > - */
> > >> > - std::vector<Span<uint8_t>> thumbnailPlanes;
> > >> > - const PixelFormatInfo &formatNV12 =
> > >> PixelFormatInfo::info(formats::NV12);
> > >> > - size_t yPlaneSize = formatNV12.planeSize(targetSize,
> 0);
> > >> > - size_t uvPlaneSize = formatNV12.planeSize(targetSize,
> 1);
> > >> > - thumbnailPlanes.push_back({ rawThumbnail.data(),
> > >> yPlaneSize });
> > >> > - thumbnailPlanes.push_back({ rawThumbnail.data() +
> > >> yPlaneSize, uvPlaneSize });
> > >> > -
> > >> > - int jpeg_size =
> thumbnailEncoder_.encode(thumbnailPlanes,
> > >> > - *thumbnail,
> {},
> > >> quality);
> > >> > - thumbnail->resize(jpeg_size);
> > >> > -
> > >> > - LOG(JPEG, Debug)
> > >> > - << "Thumbnail compress returned "
> > >> > - << jpeg_size << " bytes";
> > >> > - }
> > >> > -}
> > >> > -
> > >> > void
> PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer
> > >> *streamBuffer)
> > >> > {
> > >> > ASSERT(encoder_);
> > >> > @@ -164,8 +115,8 @@ void
> > >> PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer
> *streamBu
> > >> >
> > >> > if (thumbnailSize != Size(0, 0)) {
> > >> > std::vector<unsigned char> thumbnail;
> > >> > - generateThumbnail(source, thumbnailSize,
> quality,
> > >> &thumbnail);
> > >> > - if (!thumbnail.empty())
> > >> > + ret = encoder_->generateThumbnail(source,
> > >> thumbnailSize, quality, &thumbnail);
> > >> > + if (ret > 0 && !thumbnail.empty())
> > >> > exif.setThumbnail(thumbnail,
> > >> Exif::Compression::JPEG);
> > >> > }
> > >> >
> > >> > @@ -194,8 +145,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);
> > >> > diff --git a/src/android/jpeg/post_processor_jpeg.h
> > >> b/src/android/jpeg/post_processor_jpeg.h
> > >> > index 98309b01..a09f8798 100644
> > >> > --- a/src/android/jpeg/post_processor_jpeg.h
> > >> > +++ b/src/android/jpeg/post_processor_jpeg.h
> > >> > @@ -8,11 +8,11 @@
> > >> > #pragma once
> > >> >
> > >> > #include "../post_processor.h"
> > >> > -#include "encoder_libjpeg.h"
> > >> > -#include "thumbnailer.h"
> > >> >
> > >> > #include <libcamera/geometry.h>
> > >> >
> > >> > +#include "encoder.h"
> > >> > +
> > >> > class CameraDevice;
> > >> >
> > >> > class PostProcessorJpeg : public PostProcessor
> > >> > @@ -25,14 +25,9 @@ public:
> > >> > void process(Camera3RequestDescriptor::StreamBuffer
> > >> *streamBuffer) override;
> > >> >
> > >> > private:
> > >> > - void generateThumbnail(const libcamera::FrameBuffer &source,
> > >> > - const libcamera::Size &targetSize,
> > >> > - unsigned int quality,
> > >> > - std::vector<unsigned char> *thumbnail);
> > >> > + void SetEncoder();
> > >> >
> > >> > CameraDevice *const cameraDevice_;
> > >> > std::unique_ptr<Encoder> encoder_;
> > >> > libcamera::Size streamSize_;
> > >> > - EncoderLibJpeg thumbnailEncoder_;
> > >> > - Thumbnailer thumbnailer_;
> > >> > };
> > >> > diff --git a/src/android/meson.build b/src/android/meson.build
> > >> > index 27be27bb..026b8b3c 100644
> > >> > --- a/src/android/meson.build
> > >> > +++ b/src/android/meson.build
> > >> > @@ -48,10 +48,6 @@ android_hal_sources = files([
> > >> > 'camera_ops.cpp',
> > >> > 'camera_request.cpp',
> > >> > 'camera_stream.cpp',
> > >> > - 'jpeg/encoder_libjpeg.cpp',
> > >> > - 'jpeg/exif.cpp',
> > >> > - 'jpeg/post_processor_jpeg.cpp',
> > >> > - 'jpeg/thumbnailer.cpp',
> > >> > 'yuv/post_processor_yuv.cpp'
> > >> > ])
> > >> >
> > >> > @@ -59,6 +55,7 @@ android_cpp_args = []
> > >> >
> > >> > subdir('cros')
> > >> > subdir('mm')
> > >> > +subdir('jpeg')
> > >>
> > >> Please keep these alphabetically sorted.
> > >>
> > >> >
> > >> > android_camera_metadata_sources = files([
> > >> > 'metadata/camera_metadata.c',
> > >>
> > >> --
> > >> Regards,
> > >>
> > >> Laurent Pinchart
> > >>
> > >
>
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220902/e7315f87/attachment.htm>
More information about the libcamera-devel
mailing list