Message ID | 20210123051704.188117-7-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Paul Elder |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Sat, Jan 23, 2021 at 02:17:02PM +0900, Paul Elder wrote: > Configure the thumbnailer based on the thumbnail parameters given by the > android request metadata. Only the thumbnail encoder needs to be > configured, and since it is only used at post-processing time, move the > configuration out of the post-processor constructor and into the > processing step. > > Also set the following android result metadata tags: > - ANDROID_JPEG_THUMBNAIL_SIZE > - ANDROID_JPEG_THUMBNAIL_QUALITY > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v3 > - split from "android: Set result metadata and EXIF fields based on > request" > --- > src/android/jpeg/post_processor_jpeg.cpp | 40 +++++++++++++++++------- > src/android/jpeg/post_processor_jpeg.h | 1 + > src/android/jpeg/thumbnailer.cpp | 25 ++------------- > src/android/jpeg/thumbnailer.h | 4 +-- > 4 files changed, 34 insertions(+), 36 deletions(-) > > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp > index b325a06a..e5e42d39 100644 > --- a/src/android/jpeg/post_processor_jpeg.cpp > +++ b/src/android/jpeg/post_processor_jpeg.cpp > @@ -41,12 +41,6 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg, > streamSize_ = outCfg.size; > > thumbnailer_.configure(inCfg.size, inCfg.pixelFormat); > - StreamConfiguration thCfg = inCfg; > - thCfg.size = thumbnailer_.size(); > - if (thumbnailEncoder_.configure(thCfg) != 0) { > - LOG(JPEG, Error) << "Failed to configure thumbnail encoder"; > - return -EINVAL; > - } > > encoder_ = std::make_unique<EncoderLibJpeg>(); > > @@ -54,14 +48,20 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg, > } > > void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, > + const Size &targetSize, > std::vector<unsigned char> *thumbnail) > { > /* Stores the raw scaled-down thumbnail bytes. */ > std::vector<unsigned char> rawThumbnail; > > - thumbnailer_.createThumbnail(source, &rawThumbnail); > + thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail); > + > + StreamConfiguration thCfg; > + thCfg.size = targetSize; > + thCfg.pixelFormat = thumbnailer_.pixelFormat(); > + int ret = thumbnailEncoder_.configure(thCfg); It may make sense to also pass the size and format to the encoder's encode function, but that's a possible improvement on top. > > - if (!rawThumbnail.empty()) { > + if (!rawThumbnail.empty() && !ret) { > /* > * \todo Avoid value-initialization of all elements of the > * vector. > @@ -125,10 +125,26 @@ int PostProcessorJpeg::process(const FrameBuffer &source, > entry.data.i64, 1); > } > > - std::vector<unsigned char> thumbnail; > - generateThumbnail(source, &thumbnail); > - if (!thumbnail.empty()) > - exif.setThumbnail(thumbnail, Exif::Compression::JPEG); > + ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_SIZE, &entry); > + if (ret) { > + const int32_t *data = entry.data.i32; > + Size thumbnailSize = { static_cast<uint32_t>(data[0]), > + static_cast<uint32_t>(data[1]) }; > + > + if (thumbnailSize != Size(0, 0)) { > + std::vector<unsigned char> thumbnail; > + generateThumbnail(source, thumbnailSize, &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); Line wrap ? > + } > > ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry); > if (ret) { > diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h > index d721d1b9..660b79b4 100644 > --- a/src/android/jpeg/post_processor_jpeg.h > +++ b/src/android/jpeg/post_processor_jpeg.h > @@ -31,6 +31,7 @@ public: > > private: > void generateThumbnail(const libcamera::FrameBuffer &source, > + const libcamera::Size &targetSize, > std::vector<unsigned char> *thumbnail); > > CameraDevice *const cameraDevice_; > diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp > index 5374538a..f709d343 100644 > --- a/src/android/jpeg/thumbnailer.cpp > +++ b/src/android/jpeg/thumbnailer.cpp > @@ -32,30 +32,11 @@ void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat) > return; > } > > - targetSize_ = computeThumbnailSize(); > - > valid_ = true; > } > > -/* > - * The Exif specification recommends the width of the thumbnail to be a > - * multiple of 16 (section 4.8.1). Hence, compute the corresponding height > - * keeping the aspect ratio same as of the source. > - */ > -Size Thumbnailer::computeThumbnailSize() const > -{ > - unsigned int targetHeight; > - constexpr unsigned int kTargetWidth = 160; > - > - targetHeight = kTargetWidth * sourceSize_.height / sourceSize_.width; > - > - if (targetHeight & 1) > - targetHeight++; > - > - return Size(kTargetWidth, targetHeight); > -} > - > void Thumbnailer::createThumbnail(const FrameBuffer &source, > + const Size &targetSize, > std::vector<unsigned char> *destination) > { > MappedFrameBuffer frame(&source, PROT_READ); > @@ -73,8 +54,8 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source, > > const unsigned int sw = sourceSize_.width; > const unsigned int sh = sourceSize_.height; > - const unsigned int tw = targetSize_.width; > - const unsigned int th = targetSize_.height; > + const unsigned int tw = targetSize.width; > + const unsigned int th = targetSize.height; > > ASSERT(tw % 2 == 0 && th % 2 == 0); > > diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h > index 98f11833..bbd9832d 100644 > --- a/src/android/jpeg/thumbnailer.h > +++ b/src/android/jpeg/thumbnailer.h > @@ -20,15 +20,15 @@ public: > void configure(const libcamera::Size &sourceSize, > libcamera::PixelFormat pixelFormat); > void createThumbnail(const libcamera::FrameBuffer &source, > + const libcamera::Size &targetSize, > std::vector<unsigned char> *dest); > - const libcamera::Size &size() const { return targetSize_; } > + const libcamera::PixelFormat &pixelFormat() const { return pixelFormat_; } > > private: > libcamera::Size computeThumbnailSize() const; This should be dropped as the function is removed. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > libcamera::PixelFormat pixelFormat_; > libcamera::Size sourceSize_; > - libcamera::Size targetSize_; > > bool valid_; > };
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index b325a06a..e5e42d39 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -41,12 +41,6 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg, streamSize_ = outCfg.size; thumbnailer_.configure(inCfg.size, inCfg.pixelFormat); - StreamConfiguration thCfg = inCfg; - thCfg.size = thumbnailer_.size(); - if (thumbnailEncoder_.configure(thCfg) != 0) { - LOG(JPEG, Error) << "Failed to configure thumbnail encoder"; - return -EINVAL; - } encoder_ = std::make_unique<EncoderLibJpeg>(); @@ -54,14 +48,20 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg, } void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, + const Size &targetSize, std::vector<unsigned char> *thumbnail) { /* Stores the raw scaled-down thumbnail bytes. */ std::vector<unsigned char> rawThumbnail; - thumbnailer_.createThumbnail(source, &rawThumbnail); + thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail); + + StreamConfiguration thCfg; + thCfg.size = targetSize; + thCfg.pixelFormat = thumbnailer_.pixelFormat(); + int ret = thumbnailEncoder_.configure(thCfg); - if (!rawThumbnail.empty()) { + if (!rawThumbnail.empty() && !ret) { /* * \todo Avoid value-initialization of all elements of the * vector. @@ -125,10 +125,26 @@ int PostProcessorJpeg::process(const FrameBuffer &source, entry.data.i64, 1); } - std::vector<unsigned char> thumbnail; - generateThumbnail(source, &thumbnail); - if (!thumbnail.empty()) - exif.setThumbnail(thumbnail, Exif::Compression::JPEG); + ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_SIZE, &entry); + if (ret) { + const int32_t *data = entry.data.i32; + Size thumbnailSize = { static_cast<uint32_t>(data[0]), + static_cast<uint32_t>(data[1]) }; + + if (thumbnailSize != Size(0, 0)) { + std::vector<unsigned char> thumbnail; + generateThumbnail(source, thumbnailSize, &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); if (ret) { diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index d721d1b9..660b79b4 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -31,6 +31,7 @@ public: private: void generateThumbnail(const libcamera::FrameBuffer &source, + const libcamera::Size &targetSize, std::vector<unsigned char> *thumbnail); CameraDevice *const cameraDevice_; diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp index 5374538a..f709d343 100644 --- a/src/android/jpeg/thumbnailer.cpp +++ b/src/android/jpeg/thumbnailer.cpp @@ -32,30 +32,11 @@ void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat) return; } - targetSize_ = computeThumbnailSize(); - valid_ = true; } -/* - * The Exif specification recommends the width of the thumbnail to be a - * multiple of 16 (section 4.8.1). Hence, compute the corresponding height - * keeping the aspect ratio same as of the source. - */ -Size Thumbnailer::computeThumbnailSize() const -{ - unsigned int targetHeight; - constexpr unsigned int kTargetWidth = 160; - - targetHeight = kTargetWidth * sourceSize_.height / sourceSize_.width; - - if (targetHeight & 1) - targetHeight++; - - return Size(kTargetWidth, targetHeight); -} - void Thumbnailer::createThumbnail(const FrameBuffer &source, + const Size &targetSize, std::vector<unsigned char> *destination) { MappedFrameBuffer frame(&source, PROT_READ); @@ -73,8 +54,8 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source, const unsigned int sw = sourceSize_.width; const unsigned int sh = sourceSize_.height; - const unsigned int tw = targetSize_.width; - const unsigned int th = targetSize_.height; + const unsigned int tw = targetSize.width; + const unsigned int th = targetSize.height; ASSERT(tw % 2 == 0 && th % 2 == 0); diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h index 98f11833..bbd9832d 100644 --- a/src/android/jpeg/thumbnailer.h +++ b/src/android/jpeg/thumbnailer.h @@ -20,15 +20,15 @@ public: void configure(const libcamera::Size &sourceSize, libcamera::PixelFormat pixelFormat); void createThumbnail(const libcamera::FrameBuffer &source, + const libcamera::Size &targetSize, std::vector<unsigned char> *dest); - const libcamera::Size &size() const { return targetSize_; } + const libcamera::PixelFormat &pixelFormat() const { return pixelFormat_; } private: libcamera::Size computeThumbnailSize() const; libcamera::PixelFormat pixelFormat_; libcamera::Size sourceSize_; - libcamera::Size targetSize_; bool valid_; };
Configure the thumbnailer based on the thumbnail parameters given by the android request metadata. Only the thumbnail encoder needs to be configured, and since it is only used at post-processing time, move the configuration out of the post-processor constructor and into the processing step. Also set the following android result metadata tags: - ANDROID_JPEG_THUMBNAIL_SIZE - ANDROID_JPEG_THUMBNAIL_QUALITY Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v3 - split from "android: Set result metadata and EXIF fields based on request" --- src/android/jpeg/post_processor_jpeg.cpp | 40 +++++++++++++++++------- src/android/jpeg/post_processor_jpeg.h | 1 + src/android/jpeg/thumbnailer.cpp | 25 ++------------- src/android/jpeg/thumbnailer.h | 4 +-- 4 files changed, 34 insertions(+), 36 deletions(-)