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