| Message ID | 20221214093330.3345421-6-chenghaoyang@google.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Harvey, Thank you for the patch. On Wed, Dec 14, 2022 at 09:33:28AM +0000, Harvey Yang via libcamera-devel wrote: > From: Harvey Yang <chenghaoyang@chromium.org> > > In the following patch, generateThumbnail will have a different > implementation in the jea encoder. Therefore, this patch moves the > generateThumbnail function from PostProcessorJpeg to Encoder. > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > --- > src/android/jpeg/encoder.h | 4 ++ > src/android/jpeg/encoder_libjpeg.cpp | 54 +++++++++++++++++++++--- > src/android/jpeg/encoder_libjpeg.h | 15 ++++--- > src/android/jpeg/post_processor_jpeg.cpp | 52 +---------------------- > src/android/jpeg/post_processor_jpeg.h | 11 +---- > 5 files changed, 66 insertions(+), 70 deletions(-) > > diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h > index b974d367..5f9ef890 100644 > --- a/src/android/jpeg/encoder.h > +++ b/src/android/jpeg/encoder.h > @@ -22,4 +22,8 @@ public: > libcamera::Span<uint8_t> destination, > libcamera::Span<const uint8_t> exifData, > unsigned int quality) = 0; > + virtual void 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 d849547f..cc5f37be 100644 > --- a/src/android/jpeg/encoder_libjpeg.cpp > +++ b/src/android/jpeg/encoder_libjpeg.cpp > @@ -74,7 +74,7 @@ EncoderLibJpeg::~EncoderLibJpeg() = default; > > int EncoderLibJpeg::configure(const StreamConfiguration &cfg) > { > - return encoder_.configure(cfg); > + return captureEncoder_.configure(cfg); > } > > EncoderLibJpeg::Encoder::Encoder() > @@ -197,14 +197,56 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest, > return frame.error(); > } > > - return encoder_.encode(frame.planes(), dest, exifData, quality); > + return captureEncoder_.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) > +void EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source, > + const libcamera::Size &targetSize, > + unsigned int quality, > + std::vector<unsigned char> *thumbnail) > { > - return encoder_.encode(src, std::move(dest), std::move(exifData), quality); > + /* 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 jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail, {}, > + quality); Nitpicking, int jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail, {}, quality); > + thumbnail->resize(jpegSize); > + > + LOG(JPEG, Debug) > + << "Thumbnail compress returned " > + << jpegSize << " bytes"; > + } > } > > int EncoderLibJpeg::Encoder::encode(const std::vector<Span<uint8_t>> &src, > diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h > index 97182b96..fa077b8d 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: > @@ -26,10 +28,10 @@ public: > libcamera::Span<uint8_t> destination, > libcamera::Span<const uint8_t> exifData, > unsigned int quality) override; > - int encode(const std::vector<libcamera::Span<uint8_t>> &planes, > - libcamera::Span<uint8_t> destination, > - libcamera::Span<const uint8_t> exifData, > - unsigned int quality); > + void generateThumbnail(const libcamera::FrameBuffer &source, > + const libcamera::Size &targetSize, > + unsigned int quality, > + std::vector<unsigned char> *thumbnail) override; > > private: > class Encoder > @@ -57,5 +59,8 @@ private: > bool nvSwap_; > }; > > - Encoder encoder_; > + Encoder captureEncoder_; > + Encoder thumbnailEncoder_; > + > + Thumbnailer thumbnailer_; > }; > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp > index 0cf56716..69b18a2e 100644 > --- a/src/android/jpeg/post_processor_jpeg.cpp > +++ b/src/android/jpeg/post_processor_jpeg.cpp > @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg, > > streamSize_ = outCfg.size; > > - thumbnailer_.configure(inCfg.size, inCfg.pixelFormat); > - > encoder_ = std::make_unique<EncoderLibJpeg>(); > > 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,7 +115,8 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu > > if (thumbnailSize != Size(0, 0)) { > std::vector<unsigned char> thumbnail; > - generateThumbnail(source, thumbnailSize, quality, &thumbnail); > + encoder_->generateThumbnail(source, thumbnailSize, > + quality, &thumbnail); > if (!thumbnail.empty()) > exif.setThumbnail(std::move(thumbnail), Exif::Compression::JPEG); > } > diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h > index 98309b01..55b23d7d 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" > + This can be replaced by a forward declaration. No need to resubmit for just this, I can also handle it when applying, adding my Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > class CameraDevice; > > class PostProcessorJpeg : public PostProcessor > @@ -25,14 +25,7 @@ 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); > - > CameraDevice *const cameraDevice_; > std::unique_ptr<Encoder> encoder_; > libcamera::Size streamSize_; > - EncoderLibJpeg thumbnailEncoder_; > - Thumbnailer thumbnailer_; > };
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h index b974d367..5f9ef890 100644 --- a/src/android/jpeg/encoder.h +++ b/src/android/jpeg/encoder.h @@ -22,4 +22,8 @@ public: libcamera::Span<uint8_t> destination, libcamera::Span<const uint8_t> exifData, unsigned int quality) = 0; + virtual void 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 d849547f..cc5f37be 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -74,7 +74,7 @@ EncoderLibJpeg::~EncoderLibJpeg() = default; int EncoderLibJpeg::configure(const StreamConfiguration &cfg) { - return encoder_.configure(cfg); + return captureEncoder_.configure(cfg); } EncoderLibJpeg::Encoder::Encoder() @@ -197,14 +197,56 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest, return frame.error(); } - return encoder_.encode(frame.planes(), dest, exifData, quality); + return captureEncoder_.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) +void EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source, + const libcamera::Size &targetSize, + unsigned int quality, + std::vector<unsigned char> *thumbnail) { - return encoder_.encode(src, std::move(dest), std::move(exifData), quality); + /* 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 jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail, {}, + quality); + thumbnail->resize(jpegSize); + + LOG(JPEG, Debug) + << "Thumbnail compress returned " + << jpegSize << " bytes"; + } } int EncoderLibJpeg::Encoder::encode(const std::vector<Span<uint8_t>> &src, diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h index 97182b96..fa077b8d 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: @@ -26,10 +28,10 @@ public: libcamera::Span<uint8_t> destination, libcamera::Span<const uint8_t> exifData, unsigned int quality) override; - int encode(const std::vector<libcamera::Span<uint8_t>> &planes, - libcamera::Span<uint8_t> destination, - libcamera::Span<const uint8_t> exifData, - unsigned int quality); + void generateThumbnail(const libcamera::FrameBuffer &source, + const libcamera::Size &targetSize, + unsigned int quality, + std::vector<unsigned char> *thumbnail) override; private: class Encoder @@ -57,5 +59,8 @@ private: bool nvSwap_; }; - Encoder encoder_; + Encoder captureEncoder_; + Encoder thumbnailEncoder_; + + Thumbnailer thumbnailer_; }; diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 0cf56716..69b18a2e 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg, streamSize_ = outCfg.size; - thumbnailer_.configure(inCfg.size, inCfg.pixelFormat); - encoder_ = std::make_unique<EncoderLibJpeg>(); 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,7 +115,8 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu if (thumbnailSize != Size(0, 0)) { std::vector<unsigned char> thumbnail; - generateThumbnail(source, thumbnailSize, quality, &thumbnail); + encoder_->generateThumbnail(source, thumbnailSize, + quality, &thumbnail); if (!thumbnail.empty()) exif.setThumbnail(std::move(thumbnail), Exif::Compression::JPEG); } diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 98309b01..55b23d7d 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,7 @@ 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); - CameraDevice *const cameraDevice_; std::unique_ptr<Encoder> encoder_; libcamera::Size streamSize_; - EncoderLibJpeg thumbnailEncoder_; - Thumbnailer thumbnailer_; };