Message ID | 20210125071444.26252-9-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Paul Elder |
Headers | show |
Series |
|
Related | show |
Hi Paul, On Mon, Jan 25, 2021 at 04:14:44PM +0900, Paul Elder wrote: > Set the thumbnail quality and the JPEG quality based on the android > request metadata. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Looks good! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > --- > Changes in v4: > - set jpeg quality to 95 by default > > New in v3 > --- > src/android/jpeg/encoder.h | 3 ++- > src/android/jpeg/encoder_libjpeg.cpp | 10 ++++----- > src/android/jpeg/encoder_libjpeg.h | 8 ++++---- > src/android/jpeg/post_processor_jpeg.cpp | 26 +++++++++++------------- > src/android/jpeg/post_processor_jpeg.h | 1 + > 5 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h > index 027233dc..8d449369 100644 > --- a/src/android/jpeg/encoder.h > +++ b/src/android/jpeg/encoder.h > @@ -19,7 +19,8 @@ public: > virtual int configure(const libcamera::StreamConfiguration &cfg) = 0; > virtual int encode(const libcamera::FrameBuffer &source, > libcamera::Span<uint8_t> destination, > - libcamera::Span<const uint8_t> exifData) = 0; > + libcamera::Span<const uint8_t> exifData, > + unsigned int quality) = 0; > }; > > #endif /* __ANDROID_JPEG_ENCODER_H__ */ > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > index aed919b9..f006e1d1 100644 > --- a/src/android/jpeg/encoder_libjpeg.cpp > +++ b/src/android/jpeg/encoder_libjpeg.cpp > @@ -68,7 +68,6 @@ const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) > } /* namespace */ > > EncoderLibJpeg::EncoderLibJpeg() > - : quality_(95) > { > /* \todo Expand error handling coverage with a custom handler. */ > compress_.err = jpeg_std_error(&jerr_); > @@ -94,7 +93,6 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg) > compress_.input_components = info.colorSpace == JCS_GRAYSCALE ? 1 : 3; > > jpeg_set_defaults(&compress_); > - jpeg_set_quality(&compress_, quality_, TRUE); > > pixelFormatInfo_ = &info.pixelFormatInfo; > > @@ -180,7 +178,7 @@ void EncoderLibJpeg::compressNV(Span<const uint8_t> frame) > } > > int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest, > - Span<const uint8_t> exifData) > + Span<const uint8_t> exifData, unsigned int quality) > { > MappedFrameBuffer frame(&source, PROT_READ); > if (!frame.isValid()) { > @@ -189,15 +187,17 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest, > return frame.error(); > } > > - return encode(frame.maps()[0], dest, exifData); > + return encode(frame.maps()[0], dest, exifData, quality); > } > > int EncoderLibJpeg::encode(Span<const uint8_t> src, Span<uint8_t> dest, > - Span<const uint8_t> exifData) > + Span<const uint8_t> exifData, unsigned int quality) > { > unsigned char *destination = dest.data(); > unsigned long size = dest.size(); > > + jpeg_set_quality(&compress_, quality, TRUE); > + > /* > * The jpeg_mem_dest will reallocate if the required size is not > * sufficient. That means the output won't be written to the correct > diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h > index 070f56f8..838da772 100644 > --- a/src/android/jpeg/encoder_libjpeg.h > +++ b/src/android/jpeg/encoder_libjpeg.h > @@ -23,10 +23,12 @@ public: > int configure(const libcamera::StreamConfiguration &cfg) override; > int encode(const libcamera::FrameBuffer &source, > libcamera::Span<uint8_t> destination, > - libcamera::Span<const uint8_t> exifData) override; > + libcamera::Span<const uint8_t> exifData, > + unsigned int quality) override; > int encode(libcamera::Span<const uint8_t> source, > libcamera::Span<uint8_t> destination, > - libcamera::Span<const uint8_t> exifData); > + libcamera::Span<const uint8_t> exifData, > + unsigned int quality); > > private: > void compressRGB(libcamera::Span<const uint8_t> frame); > @@ -35,8 +37,6 @@ private: > struct jpeg_compress_struct compress_; > struct jpeg_error_mgr jerr_; > > - unsigned int quality_; > - > const libcamera::PixelFormatInfo *pixelFormatInfo_; > > bool nv_; > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp > index d35fe361..f52a791a 100644 > --- a/src/android/jpeg/post_processor_jpeg.cpp > +++ b/src/android/jpeg/post_processor_jpeg.cpp > @@ -51,6 +51,7 @@ int PostProcessorJpeg::configure(const StreamConfiguration &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. */ > @@ -71,7 +72,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, > thumbnail->resize(rawThumbnail.size()); > > int jpeg_size = thumbnailEncoder_.encode(rawThumbnail, > - *thumbnail, {}); > + *thumbnail, {}, quality); > thumbnail->resize(jpeg_size); > > LOG(JPEG, Debug) > @@ -133,20 +134,18 @@ int PostProcessorJpeg::process(const FrameBuffer &source, > Size thumbnailSize = { static_cast<uint32_t>(data[0]), > static_cast<uint32_t>(data[1]) }; > > + ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &entry); > + uint8_t quality = ret ? *entry.data.u8 : 95; > + resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &quality, 1); > + > if (thumbnailSize != Size(0, 0)) { > std::vector<unsigned char> thumbnail; > - generateThumbnail(source, thumbnailSize, &thumbnail); > + generateThumbnail(source, thumbnailSize, quality, &thumbnail); > if (!thumbnail.empty()) > exif.setThumbnail(thumbnail, Exif::Compression::JPEG); > } > > resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2); > - > - /* \todo Use this quality as a parameter to the encoder */ > - ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &entry); > - if (ret) > - resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, > - entry.data.u8, 1); > } > > ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry); > @@ -167,7 +166,11 @@ int PostProcessorJpeg::process(const FrameBuffer &source, > if (exif.generate() != 0) > LOG(JPEG, Error) << "Failed to generate valid EXIF data"; > > - int jpeg_size = encoder_->encode(source, destination, exif.data()); > + ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry); > + const uint8_t quality = ret ? *entry.data.u8 : 95; > + resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &quality, 1); > + > + int jpeg_size = encoder_->encode(source, destination, exif.data(), quality); > if (jpeg_size < 0) { > LOG(JPEG, Error) << "Failed to encode stream image"; > return jpeg_size; > @@ -195,10 +198,5 @@ int PostProcessorJpeg::process(const FrameBuffer &source, > /* Update the JPEG result Metadata. */ > resultMetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1); > > - /* \todo Configure JPEG encoder with this */ > - ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry); > - const uint32_t jpegQuality = ret ? *entry.data.u8 : 95; > - resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 1); > - > return 0; > } > diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h > index 660b79b4..d2dfa450 100644 > --- a/src/android/jpeg/post_processor_jpeg.h > +++ b/src/android/jpeg/post_processor_jpeg.h > @@ -32,6 +32,7 @@ public: > private: > void generateThumbnail(const libcamera::FrameBuffer &source, > const libcamera::Size &targetSize, > + unsigned int quality, > std::vector<unsigned char> *thumbnail); > > CameraDevice *const cameraDevice_; > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h index 027233dc..8d449369 100644 --- a/src/android/jpeg/encoder.h +++ b/src/android/jpeg/encoder.h @@ -19,7 +19,8 @@ public: virtual int configure(const libcamera::StreamConfiguration &cfg) = 0; virtual int encode(const libcamera::FrameBuffer &source, libcamera::Span<uint8_t> destination, - libcamera::Span<const uint8_t> exifData) = 0; + libcamera::Span<const uint8_t> exifData, + unsigned int quality) = 0; }; #endif /* __ANDROID_JPEG_ENCODER_H__ */ diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp index aed919b9..f006e1d1 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -68,7 +68,6 @@ const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) } /* namespace */ EncoderLibJpeg::EncoderLibJpeg() - : quality_(95) { /* \todo Expand error handling coverage with a custom handler. */ compress_.err = jpeg_std_error(&jerr_); @@ -94,7 +93,6 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg) compress_.input_components = info.colorSpace == JCS_GRAYSCALE ? 1 : 3; jpeg_set_defaults(&compress_); - jpeg_set_quality(&compress_, quality_, TRUE); pixelFormatInfo_ = &info.pixelFormatInfo; @@ -180,7 +178,7 @@ void EncoderLibJpeg::compressNV(Span<const uint8_t> frame) } int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest, - Span<const uint8_t> exifData) + Span<const uint8_t> exifData, unsigned int quality) { MappedFrameBuffer frame(&source, PROT_READ); if (!frame.isValid()) { @@ -189,15 +187,17 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest, return frame.error(); } - return encode(frame.maps()[0], dest, exifData); + return encode(frame.maps()[0], dest, exifData, quality); } int EncoderLibJpeg::encode(Span<const uint8_t> src, Span<uint8_t> dest, - Span<const uint8_t> exifData) + Span<const uint8_t> exifData, unsigned int quality) { unsigned char *destination = dest.data(); unsigned long size = dest.size(); + jpeg_set_quality(&compress_, quality, TRUE); + /* * The jpeg_mem_dest will reallocate if the required size is not * sufficient. That means the output won't be written to the correct diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h index 070f56f8..838da772 100644 --- a/src/android/jpeg/encoder_libjpeg.h +++ b/src/android/jpeg/encoder_libjpeg.h @@ -23,10 +23,12 @@ public: int configure(const libcamera::StreamConfiguration &cfg) override; int encode(const libcamera::FrameBuffer &source, libcamera::Span<uint8_t> destination, - libcamera::Span<const uint8_t> exifData) override; + libcamera::Span<const uint8_t> exifData, + unsigned int quality) override; int encode(libcamera::Span<const uint8_t> source, libcamera::Span<uint8_t> destination, - libcamera::Span<const uint8_t> exifData); + libcamera::Span<const uint8_t> exifData, + unsigned int quality); private: void compressRGB(libcamera::Span<const uint8_t> frame); @@ -35,8 +37,6 @@ private: struct jpeg_compress_struct compress_; struct jpeg_error_mgr jerr_; - unsigned int quality_; - const libcamera::PixelFormatInfo *pixelFormatInfo_; bool nv_; diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index d35fe361..f52a791a 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -51,6 +51,7 @@ int PostProcessorJpeg::configure(const StreamConfiguration &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. */ @@ -71,7 +72,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, thumbnail->resize(rawThumbnail.size()); int jpeg_size = thumbnailEncoder_.encode(rawThumbnail, - *thumbnail, {}); + *thumbnail, {}, quality); thumbnail->resize(jpeg_size); LOG(JPEG, Debug) @@ -133,20 +134,18 @@ int PostProcessorJpeg::process(const FrameBuffer &source, Size thumbnailSize = { static_cast<uint32_t>(data[0]), static_cast<uint32_t>(data[1]) }; + ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &entry); + uint8_t quality = ret ? *entry.data.u8 : 95; + resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &quality, 1); + if (thumbnailSize != Size(0, 0)) { std::vector<unsigned char> thumbnail; - generateThumbnail(source, thumbnailSize, &thumbnail); + generateThumbnail(source, thumbnailSize, quality, &thumbnail); if (!thumbnail.empty()) exif.setThumbnail(thumbnail, Exif::Compression::JPEG); } resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2); - - /* \todo Use this quality as a parameter to the encoder */ - ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &entry); - if (ret) - resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, - entry.data.u8, 1); } ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry); @@ -167,7 +166,11 @@ int PostProcessorJpeg::process(const FrameBuffer &source, if (exif.generate() != 0) LOG(JPEG, Error) << "Failed to generate valid EXIF data"; - int jpeg_size = encoder_->encode(source, destination, exif.data()); + ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry); + const uint8_t quality = ret ? *entry.data.u8 : 95; + resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &quality, 1); + + int jpeg_size = encoder_->encode(source, destination, exif.data(), quality); if (jpeg_size < 0) { LOG(JPEG, Error) << "Failed to encode stream image"; return jpeg_size; @@ -195,10 +198,5 @@ int PostProcessorJpeg::process(const FrameBuffer &source, /* Update the JPEG result Metadata. */ resultMetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1); - /* \todo Configure JPEG encoder with this */ - ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry); - const uint32_t jpegQuality = ret ? *entry.data.u8 : 95; - resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpegQuality, 1); - return 0; } diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 660b79b4..d2dfa450 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -32,6 +32,7 @@ public: private: void generateThumbnail(const libcamera::FrameBuffer &source, const libcamera::Size &targetSize, + unsigned int quality, std::vector<unsigned char> *thumbnail); CameraDevice *const cameraDevice_;