| Message ID | 20210126102825.147026-9-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Tue, Jan 26, 2021 at 07:28:25PM +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> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > No change in v5 > Changes in v4: > - set jpeg quality to 95 by default > > New in v3 > --- > src/android/camera_device.cpp | 4 +++- > 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 + > 6 files changed, 27 insertions(+), 25 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index d297bdf4..a0865911 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -708,7 +708,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize() > * Currently: 53 entries, 782 bytes of static metadata > */ > uint32_t numEntries = 53; > - uint32_t byteSize = 802; > + uint32_t byteSize = 810; > > /* > * Calculate space occupation in bytes for dynamically built metadata > @@ -1272,6 +1272,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > ANDROID_JPEG_SIZE, > ANDROID_JPEG_QUALITY, > ANDROID_JPEG_ORIENTATION, > + ANDROID_JPEG_THUMBNAIL_QUALITY, > + ANDROID_JPEG_THUMBNAIL_SIZE, > }; > staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS, > availableResultKeys.data(), Should this be moved to patch 6/8 ? Apart from that all seems good. > 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 e990ba04..cac0087b 100644 > --- a/src/android/jpeg/post_processor_jpeg.cpp > +++ b/src/android/jpeg/post_processor_jpeg.cpp > @@ -52,6 +52,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. */ > @@ -72,7 +73,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) > @@ -135,20 +136,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); > @@ -169,7 +168,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; > @@ -197,10 +200,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 uint8_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_;
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index d297bdf4..a0865911 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -708,7 +708,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize() * Currently: 53 entries, 782 bytes of static metadata */ uint32_t numEntries = 53; - uint32_t byteSize = 802; + uint32_t byteSize = 810; /* * Calculate space occupation in bytes for dynamically built metadata @@ -1272,6 +1272,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() ANDROID_JPEG_SIZE, ANDROID_JPEG_QUALITY, ANDROID_JPEG_ORIENTATION, + ANDROID_JPEG_THUMBNAIL_QUALITY, + ANDROID_JPEG_THUMBNAIL_SIZE, }; staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_RESULT_KEYS, availableResultKeys.data(), 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 e990ba04..cac0087b 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -52,6 +52,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. */ @@ -72,7 +73,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) @@ -135,20 +136,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); @@ -169,7 +168,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; @@ -197,10 +200,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 uint8_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_;