| Message ID | 20201002103416.53623-2-email@uajain.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Fri, Oct 02, 2020 at 04:04:14PM +0530, Umang Jain wrote: > Add a basic image thumbnailer for NV12 frames being captured. > It shall generate a thumbnail image to be embedded as a part of > EXIF metadata of the frame. The output of the thumbnail will still > be NV12. > > Signed-off-by: Umang Jain <email@uajain.com> > --- > src/android/jpeg/encoder_libjpeg.cpp | 12 +++ > src/android/jpeg/thumbnailer.cpp | 106 +++++++++++++++++++++++++++ > src/android/jpeg/thumbnailer.h | 34 +++++++++ > src/android/meson.build | 1 + > 4 files changed, 153 insertions(+) > create mode 100644 src/android/jpeg/thumbnailer.cpp > create mode 100644 src/android/jpeg/thumbnailer.h > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > index 510613c..7d097c6 100644 > --- a/src/android/jpeg/encoder_libjpeg.cpp > +++ b/src/android/jpeg/encoder_libjpeg.cpp > @@ -6,6 +6,7 @@ > */ > > #include "encoder_libjpeg.h" > +#include "thumbnailer.h" > > #include <fcntl.h> > #include <iomanip> > @@ -214,6 +215,17 @@ int EncoderLibJpeg::encode(const FrameBuffer *source, > LOG(JPEG, Debug) << "JPEG Encode Starting:" << compress_.image_width > << "x" << compress_.image_height; > > + Thumbnailer thScaler; > + libcamera::Span<uint8_t> thumbnail; > + thScaler.configure(Size (compress_.image_width, compress_.image_height), > + pixelFormatInfo_->format); > + thScaler.scaleBuffer(source, thumbnail); > + /* > + * \todo: Discuss if we require scaling here or one level above where > + * exif data is set (setThumbnail()?). > + * Setting thumbnailed scaled data seems a bit convulated initially. > + */ Here's a proposal. As the JPEG encoder could also be used to create the thumbnail, I would create an additional layer. A new JPEGPostProcessor (we can bikeshed the name) class would handle the whole JFIF image creation. It would would internally encode the image to JPEG using EncoderLibjpeg, downscale it to thumbnail size, compress the thumbnail with EncoderLibjpeg, generate the Exif, and store the JPEG-compressed thumbnail in the Exif. That way, JPEG encoding, Exif generation and thumbnail generation are all handled inside JPEGPostProcessor and not visible to the CameraDevice or CameraStream, and the JPEG encoder itself is kept generic enough to be used for both the main image and the thumbnail. > + > if (nv_) > compressNV(&frame); > else > diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp > new file mode 100644 > index 0000000..d706ea6 > --- /dev/null > +++ b/src/android/jpeg/thumbnailer.cpp > @@ -0,0 +1,106 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (C) 2020, Google Inc. > + * > + * thumbnailer.cpp - Basic image thumbnailer from NV12 > + */ > + > +#include "thumbnailer.h" > +#include "system/graphics.h" This header shouldn't be needed, the implementation of this class should not be Android-specific. > + > +#include "libcamera/internal/file.h" > +#include "libcamera/internal/log.h" > + > +using namespace libcamera; > + > +LOG_DEFINE_CATEGORY(Thumbnailer) > + > +Thumbnailer::Thumbnailer() > + : validConfiguration_(false) > +{ > +} > + > +void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat) > +{ > + sourceSize_ = sourceSize; > + pixelFormat_ = pixelFormat; > + > + if (pixelFormat_ .fourcc() == HAL_PIXEL_FORMAT_YV12) { Please don't use fourcc(), this is completely wrong. PixelFormat stores a DRM fourcc, and you're comparing it to a completely unrelated value. The PixelFormat class is meant to abstract pixel formats, you must compare it with formats::NV12 instead. And shouldn't the check be reversed ? This accepts all formats except NV12. > + LOG (Thumbnailer, Error) << "Failed to configure: Pixel Format " > + << pixelFormat_.toString() << " unsupported."; > + return; > + } > + > + validConfiguration_ = true; > +} I'd simplify the API by passing the pixel format and size to the scaleBuffer() functionn and remove the configure() function. > + > +/* Compute a thumbnail size with same aspect ratio as source. */ Could you please extend this comment to explain how the size is computed ? Not a description of the implementation, but the rules that the function implements. > +void Thumbnailer::computeThumbnailSize() > +{ > + unsigned int baseWidth = 160; > + > + unsigned int width = baseWidth * sourceSize_.width / sourceSize_.height; > + targetSize_.width = width - (width % 16); > + targetSize_.height = targetSize_.width * sourceSize_.height > + / sourceSize_.width; > + if (targetSize_.height & 1) > + targetSize_.height++; With a source aspect ratio of 4:3, this outputs 208x156 instead of the expected 160x120. > +} > + > +int Thumbnailer::scaleBuffer(const FrameBuffer *source, Span<uint8_t> &dest) > +{ > + MappedFrameBuffer frame(source, PROT_READ); Mappings are expensive. I think the caller should create the MappedFrameBuffer and pass it to both the JPEG encoder and the thumbnail generator. > + if (!frame.isValid()) { > + LOG(Thumbnailer, Error) << "Failed to map FrameBuffer : " > + << strerror(frame.error()); > + return frame.error(); > + } > + > + if (!validConfiguration_) { > + LOG(Thumbnailer, Error) << "config is unconfigured or invalid."; > + return -1; > + } > + > + computeThumbnailSize(); You can make the function return the size instead of storing it in a member variable. > + > + const unsigned int sw = sourceSize_.width; > + const unsigned int sh = sourceSize_.height; > + const unsigned int tw = targetSize_.width; > + const unsigned int th = targetSize_.height; > + > + /* Image scaling block implementing nearest-neighbour algorithm. */ > + unsigned char *src = static_cast<unsigned char *>(frame.maps()[0].data()); > + unsigned char *src_c = src + sh * sw; > + unsigned int cb_pos = 0; > + unsigned int cr_pos = 1; > + unsigned char *src_cb, *src_cr; > + > + size_t dstSize = (th * tw) + ((th/2) * tw); No need for parentheses. > + unsigned char *destination = static_cast<unsigned char *>(malloc(dstSize)); C++ uses new. > + unsigned char *dst = destination; > + unsigned char *dst_c = destination + th * tw; > + > + for (unsigned int y = 0; y < th; y+=2) { s/+=/ += / There are a few other locations below that need spaces around operators. > + unsigned int sourceY = (sh*y + th/2) / th; > + > + src_cb = src_c + (sourceY/2) * sw + cb_pos; > + src_cr = src_c + (sourceY/2) * sw + cr_pos; I'd create a new src_y variable to handle the luma plane the same way as the chroma plane. > + > + for (unsigned int x = 0; x < tw; x+=2) { > + unsigned int sourceX = (sw*x + tw/2) / tw; > + > + dst[y * tw + x] = src[sw * sourceY + sourceX]; > + dst[(y+1) * tw + x] = src[sw * (sourceY+1) + sourceX]; > + dst[y * tw + (x+1)] = src[sw * sourceY + (sourceX+1)]; > + dst[(y+1) * tw + (x+1)] = src[sw * (sourceY+1) + (sourceX+1)]; > + > + dst_c[(y/2) * tw + x + cb_pos] = src_cb[(sourceX/2) * 2]; > + dst_c[(y/2) * tw + x + cr_pos] = src_cr[(sourceX/2) * 2]; You can just do dst_c[(y/2) * tw + x + 0] = src_c[(sourceX/2) * 2 + 0]; dst_c[(y/2) * tw + x + 1] = src_c[(sourceX/2) * 2 + 1]; and drop cb_pos, cr_pos, src_cb and src_cr, as the only thing that matters it to copy both pixels. src_c should be handled the same way as src_cb (computed in the outer loop), and I would do the same for dst_c (and add a new dst_y). > + } > + } > + > + /* Write scaled pixels to dest */ > + dest = { destination, dstSize }; This is a very good way to cause memory leaks :-S There's nothing in the function signature that tells that it allocates memory internally. Let's create a better interface that makes leaks impossible. > + > + return 0; > +} > diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h > new file mode 100644 > index 0000000..8ba9816 > --- /dev/null > +++ b/src/android/jpeg/thumbnailer.h > @@ -0,0 +1,34 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (C) 2020, Google Inc. > + * > + * thumbnailer.h - Basic image thumbnailer from NV12 > + */ > +#ifndef __ANDROID_JPEG_THUMBNAILER_H__ > +#define __ANDROID_JPEG_THUMBNAILER_H__ > + > +#include <libcamera/geometry.h> > + > +#include "libcamera/internal/buffer.h" > +#include "libcamera/internal/formats.h" > + > +class Thumbnailer > +{ > +public: > + Thumbnailer(); > + > + void configure(const libcamera::Size &sourceSize, > + libcamera::PixelFormat pixelFormat); > + int scaleBuffer(const libcamera::FrameBuffer *source, libcamera::Span<uint8_t> &dest); > + > +private: > + void computeThumbnailSize(); > + > + libcamera::PixelFormat pixelFormat_; > + libcamera::Size sourceSize_; > + libcamera::Size targetSize_; > + > + bool validConfiguration_; > +}; > + > +#endif /* __ANDROID_JPEG_THUMBNAILER_H__ */ > diff --git a/src/android/meson.build b/src/android/meson.build > index 0293c20..f35ba04 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -22,6 +22,7 @@ android_hal_sources = files([ > 'camera_ops.cpp', > 'jpeg/encoder_libjpeg.cpp', > 'jpeg/exif.cpp', > + 'jpeg/thumbnailer.cpp', > ]) > > android_camera_metadata_sources = files([
Hi Umang, One more thing. On Sun, Oct 04, 2020 at 10:16:57PM +0300, Laurent Pinchart wrote: > On Fri, Oct 02, 2020 at 04:04:14PM +0530, Umang Jain wrote: > > Add a basic image thumbnailer for NV12 frames being captured. > > It shall generate a thumbnail image to be embedded as a part of > > EXIF metadata of the frame. The output of the thumbnail will still > > be NV12. > > > > Signed-off-by: Umang Jain <email@uajain.com> > > --- > > src/android/jpeg/encoder_libjpeg.cpp | 12 +++ > > src/android/jpeg/thumbnailer.cpp | 106 +++++++++++++++++++++++++++ > > src/android/jpeg/thumbnailer.h | 34 +++++++++ > > src/android/meson.build | 1 + > > 4 files changed, 153 insertions(+) > > create mode 100644 src/android/jpeg/thumbnailer.cpp > > create mode 100644 src/android/jpeg/thumbnailer.h > > > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > > index 510613c..7d097c6 100644 > > --- a/src/android/jpeg/encoder_libjpeg.cpp > > +++ b/src/android/jpeg/encoder_libjpeg.cpp > > @@ -6,6 +6,7 @@ > > */ > > > > #include "encoder_libjpeg.h" > > +#include "thumbnailer.h" > > > > #include <fcntl.h> > > #include <iomanip> > > @@ -214,6 +215,17 @@ int EncoderLibJpeg::encode(const FrameBuffer *source, > > LOG(JPEG, Debug) << "JPEG Encode Starting:" << compress_.image_width > > << "x" << compress_.image_height; > > > > + Thumbnailer thScaler; > > + libcamera::Span<uint8_t> thumbnail; > > + thScaler.configure(Size (compress_.image_width, compress_.image_height), > > + pixelFormatInfo_->format); > > + thScaler.scaleBuffer(source, thumbnail); > > + /* > > + * \todo: Discuss if we require scaling here or one level above where > > + * exif data is set (setThumbnail()?). > > + * Setting thumbnailed scaled data seems a bit convulated initially. > > + */ > > Here's a proposal. > > As the JPEG encoder could also be used to create the thumbnail, I would > create an additional layer. A new JPEGPostProcessor (we can bikeshed the > name) class would handle the whole JFIF image creation. It would would > internally encode the image to JPEG using EncoderLibjpeg, downscale it > to thumbnail size, compress the thumbnail with EncoderLibjpeg, generate > the Exif, and store the JPEG-compressed thumbnail in the Exif. That way, > JPEG encoding, Exif generation and thumbnail generation are all handled > inside JPEGPostProcessor and not visible to the CameraDevice or > CameraStream, and the JPEG encoder itself is kept generic enough to be > used for both the main image and the thumbnail. > > > + > > if (nv_) > > compressNV(&frame); > > else > > diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp > > new file mode 100644 > > index 0000000..d706ea6 > > --- /dev/null > > +++ b/src/android/jpeg/thumbnailer.cpp > > @@ -0,0 +1,106 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (C) 2020, Google Inc. > > + * > > + * thumbnailer.cpp - Basic image thumbnailer from NV12 > > + */ > > + > > +#include "thumbnailer.h" > > +#include "system/graphics.h" > > This header shouldn't be needed, the implementation of this class should > not be Android-specific. > > > + > > +#include "libcamera/internal/file.h" > > +#include "libcamera/internal/log.h" > > + > > +using namespace libcamera; > > + > > +LOG_DEFINE_CATEGORY(Thumbnailer) > > + > > +Thumbnailer::Thumbnailer() > > + : validConfiguration_(false) > > +{ > > +} > > + > > +void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat) > > +{ > > + sourceSize_ = sourceSize; > > + pixelFormat_ = pixelFormat; > > + > > + if (pixelFormat_ .fourcc() == HAL_PIXEL_FORMAT_YV12) { > > Please don't use fourcc(), this is completely wrong. PixelFormat stores > a DRM fourcc, and you're comparing it to a completely unrelated value. > The PixelFormat class is meant to abstract pixel formats, you must > compare it with formats::NV12 instead. > > And shouldn't the check be reversed ? This accepts all formats except > NV12. > > > + LOG (Thumbnailer, Error) << "Failed to configure: Pixel Format " > > + << pixelFormat_.toString() << " unsupported."; > > + return; > > + } > > + > > + validConfiguration_ = true; > > +} > > I'd simplify the API by passing the pixel format and size to the > scaleBuffer() functionn and remove the configure() function. > > > + > > +/* Compute a thumbnail size with same aspect ratio as source. */ > > Could you please extend this comment to explain how the size is computed > ? Not a description of the implementation, but the rules that the > function implements. > > > +void Thumbnailer::computeThumbnailSize() > > +{ > > + unsigned int baseWidth = 160; > > + > > + unsigned int width = baseWidth * sourceSize_.width / sourceSize_.height; > > + targetSize_.width = width - (width % 16); > > + targetSize_.height = targetSize_.width * sourceSize_.height > > + / sourceSize_.width; > > + if (targetSize_.height & 1) > > + targetSize_.height++; > > With a source aspect ratio of 4:3, this outputs 208x156 instead of the > expected 160x120. > > > +} > > + > > +int Thumbnailer::scaleBuffer(const FrameBuffer *source, Span<uint8_t> &dest) > > +{ > > + MappedFrameBuffer frame(source, PROT_READ); > > Mappings are expensive. I think the caller should create the > MappedFrameBuffer and pass it to both the JPEG encoder and the thumbnail > generator. > > > + if (!frame.isValid()) { > > + LOG(Thumbnailer, Error) << "Failed to map FrameBuffer : " > > + << strerror(frame.error()); > > + return frame.error(); > > + } > > + > > + if (!validConfiguration_) { > > + LOG(Thumbnailer, Error) << "config is unconfigured or invalid."; > > + return -1; > > + } > > + > > + computeThumbnailSize(); > > You can make the function return the size instead of storing it in a > member variable. > > > + > > + const unsigned int sw = sourceSize_.width; > > + const unsigned int sh = sourceSize_.height; > > + const unsigned int tw = targetSize_.width; > > + const unsigned int th = targetSize_.height; > > + > > + /* Image scaling block implementing nearest-neighbour algorithm. */ > > + unsigned char *src = static_cast<unsigned char *>(frame.maps()[0].data()); > > + unsigned char *src_c = src + sh * sw; The stride shouldn't be hardcoded to sh, but should come from the stream configuration. This doesn't have to be implemented yet, a \todo comment would be fine for now (see encoder_libjpeg.cpp). I would however recommend using PixelFormatInfo to calculate the stride for now (see encoder_libjpeg.cpp too :-)). > > + unsigned int cb_pos = 0; > > + unsigned int cr_pos = 1; > > + unsigned char *src_cb, *src_cr; > > + > > + size_t dstSize = (th * tw) + ((th/2) * tw); > > No need for parentheses. > > > + unsigned char *destination = static_cast<unsigned char *>(malloc(dstSize)); > > C++ uses new. > > > + unsigned char *dst = destination; > > + unsigned char *dst_c = destination + th * tw; > > + > > + for (unsigned int y = 0; y < th; y+=2) { > > s/+=/ += / > > There are a few other locations below that need spaces around operators. > > > + unsigned int sourceY = (sh*y + th/2) / th; > > + > > + src_cb = src_c + (sourceY/2) * sw + cb_pos; > > + src_cr = src_c + (sourceY/2) * sw + cr_pos; > > I'd create a new src_y variable to handle the luma plane the same way as > the chroma plane. > > > + > > + for (unsigned int x = 0; x < tw; x+=2) { > > + unsigned int sourceX = (sw*x + tw/2) / tw; > > + > > + dst[y * tw + x] = src[sw * sourceY + sourceX]; > > + dst[(y+1) * tw + x] = src[sw * (sourceY+1) + sourceX]; > > + dst[y * tw + (x+1)] = src[sw * sourceY + (sourceX+1)]; > > + dst[(y+1) * tw + (x+1)] = src[sw * (sourceY+1) + (sourceX+1)]; > > + > > + dst_c[(y/2) * tw + x + cb_pos] = src_cb[(sourceX/2) * 2]; > > + dst_c[(y/2) * tw + x + cr_pos] = src_cr[(sourceX/2) * 2]; > > You can just do > > dst_c[(y/2) * tw + x + 0] = src_c[(sourceX/2) * 2 + 0]; > dst_c[(y/2) * tw + x + 1] = src_c[(sourceX/2) * 2 + 1]; > > and drop cb_pos, cr_pos, src_cb and src_cr, as the only thing that > matters it to copy both pixels. > > src_c should be handled the same way as src_cb (computed in the outer > loop), and I would do the same for dst_c (and add a new dst_y). > > > + } > > + } > > + > > + /* Write scaled pixels to dest */ > > + dest = { destination, dstSize }; > > This is a very good way to cause memory leaks :-S There's nothing in the > function signature that tells that it allocates memory internally. Let's > create a better interface that makes leaks impossible. > > > + > > + return 0; > > +} > > diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h > > new file mode 100644 > > index 0000000..8ba9816 > > --- /dev/null > > +++ b/src/android/jpeg/thumbnailer.h > > @@ -0,0 +1,34 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (C) 2020, Google Inc. > > + * > > + * thumbnailer.h - Basic image thumbnailer from NV12 > > + */ > > +#ifndef __ANDROID_JPEG_THUMBNAILER_H__ > > +#define __ANDROID_JPEG_THUMBNAILER_H__ > > + > > +#include <libcamera/geometry.h> > > + > > +#include "libcamera/internal/buffer.h" > > +#include "libcamera/internal/formats.h" > > + > > +class Thumbnailer > > +{ > > +public: > > + Thumbnailer(); > > + > > + void configure(const libcamera::Size &sourceSize, > > + libcamera::PixelFormat pixelFormat); > > + int scaleBuffer(const libcamera::FrameBuffer *source, libcamera::Span<uint8_t> &dest); > > + > > +private: > > + void computeThumbnailSize(); > > + > > + libcamera::PixelFormat pixelFormat_; > > + libcamera::Size sourceSize_; > > + libcamera::Size targetSize_; > > + > > + bool validConfiguration_; > > +}; > > + > > +#endif /* __ANDROID_JPEG_THUMBNAILER_H__ */ > > diff --git a/src/android/meson.build b/src/android/meson.build > > index 0293c20..f35ba04 100644 > > --- a/src/android/meson.build > > +++ b/src/android/meson.build > > @@ -22,6 +22,7 @@ android_hal_sources = files([ > > 'camera_ops.cpp', > > 'jpeg/encoder_libjpeg.cpp', > > 'jpeg/exif.cpp', > > + 'jpeg/thumbnailer.cpp', > > ]) > > > > android_camera_metadata_sources = files([
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp index 510613c..7d097c6 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -6,6 +6,7 @@ */ #include "encoder_libjpeg.h" +#include "thumbnailer.h" #include <fcntl.h> #include <iomanip> @@ -214,6 +215,17 @@ int EncoderLibJpeg::encode(const FrameBuffer *source, LOG(JPEG, Debug) << "JPEG Encode Starting:" << compress_.image_width << "x" << compress_.image_height; + Thumbnailer thScaler; + libcamera::Span<uint8_t> thumbnail; + thScaler.configure(Size (compress_.image_width, compress_.image_height), + pixelFormatInfo_->format); + thScaler.scaleBuffer(source, thumbnail); + /* + * \todo: Discuss if we require scaling here or one level above where + * exif data is set (setThumbnail()?). + * Setting thumbnailed scaled data seems a bit convulated initially. + */ + if (nv_) compressNV(&frame); else diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp new file mode 100644 index 0000000..d706ea6 --- /dev/null +++ b/src/android/jpeg/thumbnailer.cpp @@ -0,0 +1,106 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * thumbnailer.cpp - Basic image thumbnailer from NV12 + */ + +#include "thumbnailer.h" +#include "system/graphics.h" + +#include "libcamera/internal/file.h" +#include "libcamera/internal/log.h" + +using namespace libcamera; + +LOG_DEFINE_CATEGORY(Thumbnailer) + +Thumbnailer::Thumbnailer() + : validConfiguration_(false) +{ +} + +void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat) +{ + sourceSize_ = sourceSize; + pixelFormat_ = pixelFormat; + + if (pixelFormat_ .fourcc() == HAL_PIXEL_FORMAT_YV12) { + LOG (Thumbnailer, Error) << "Failed to configure: Pixel Format " + << pixelFormat_.toString() << " unsupported."; + return; + } + + validConfiguration_ = true; +} + +/* Compute a thumbnail size with same aspect ratio as source. */ +void Thumbnailer::computeThumbnailSize() +{ + unsigned int baseWidth = 160; + + unsigned int width = baseWidth * sourceSize_.width / sourceSize_.height; + targetSize_.width = width - (width % 16); + targetSize_.height = targetSize_.width * sourceSize_.height + / sourceSize_.width; + if (targetSize_.height & 1) + targetSize_.height++; +} + +int Thumbnailer::scaleBuffer(const FrameBuffer *source, Span<uint8_t> &dest) +{ + MappedFrameBuffer frame(source, PROT_READ); + if (!frame.isValid()) { + LOG(Thumbnailer, Error) << "Failed to map FrameBuffer : " + << strerror(frame.error()); + return frame.error(); + } + + if (!validConfiguration_) { + LOG(Thumbnailer, Error) << "config is unconfigured or invalid."; + return -1; + } + + computeThumbnailSize(); + + const unsigned int sw = sourceSize_.width; + const unsigned int sh = sourceSize_.height; + const unsigned int tw = targetSize_.width; + const unsigned int th = targetSize_.height; + + /* Image scaling block implementing nearest-neighbour algorithm. */ + unsigned char *src = static_cast<unsigned char *>(frame.maps()[0].data()); + unsigned char *src_c = src + sh * sw; + unsigned int cb_pos = 0; + unsigned int cr_pos = 1; + unsigned char *src_cb, *src_cr; + + size_t dstSize = (th * tw) + ((th/2) * tw); + unsigned char *destination = static_cast<unsigned char *>(malloc(dstSize)); + unsigned char *dst = destination; + unsigned char *dst_c = destination + th * tw; + + for (unsigned int y = 0; y < th; y+=2) { + unsigned int sourceY = (sh*y + th/2) / th; + + src_cb = src_c + (sourceY/2) * sw + cb_pos; + src_cr = src_c + (sourceY/2) * sw + cr_pos; + + for (unsigned int x = 0; x < tw; x+=2) { + unsigned int sourceX = (sw*x + tw/2) / tw; + + dst[y * tw + x] = src[sw * sourceY + sourceX]; + dst[(y+1) * tw + x] = src[sw * (sourceY+1) + sourceX]; + dst[y * tw + (x+1)] = src[sw * sourceY + (sourceX+1)]; + dst[(y+1) * tw + (x+1)] = src[sw * (sourceY+1) + (sourceX+1)]; + + dst_c[(y/2) * tw + x + cb_pos] = src_cb[(sourceX/2) * 2]; + dst_c[(y/2) * tw + x + cr_pos] = src_cr[(sourceX/2) * 2]; + } + } + + /* Write scaled pixels to dest */ + dest = { destination, dstSize }; + + return 0; +} diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h new file mode 100644 index 0000000..8ba9816 --- /dev/null +++ b/src/android/jpeg/thumbnailer.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * thumbnailer.h - Basic image thumbnailer from NV12 + */ +#ifndef __ANDROID_JPEG_THUMBNAILER_H__ +#define __ANDROID_JPEG_THUMBNAILER_H__ + +#include <libcamera/geometry.h> + +#include "libcamera/internal/buffer.h" +#include "libcamera/internal/formats.h" + +class Thumbnailer +{ +public: + Thumbnailer(); + + void configure(const libcamera::Size &sourceSize, + libcamera::PixelFormat pixelFormat); + int scaleBuffer(const libcamera::FrameBuffer *source, libcamera::Span<uint8_t> &dest); + +private: + void computeThumbnailSize(); + + libcamera::PixelFormat pixelFormat_; + libcamera::Size sourceSize_; + libcamera::Size targetSize_; + + bool validConfiguration_; +}; + +#endif /* __ANDROID_JPEG_THUMBNAILER_H__ */ diff --git a/src/android/meson.build b/src/android/meson.build index 0293c20..f35ba04 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -22,6 +22,7 @@ android_hal_sources = files([ 'camera_ops.cpp', 'jpeg/encoder_libjpeg.cpp', 'jpeg/exif.cpp', + 'jpeg/thumbnailer.cpp', ]) android_camera_metadata_sources = files([
Add a basic image thumbnailer for NV12 frames being captured. It shall generate a thumbnail image to be embedded as a part of EXIF metadata of the frame. The output of the thumbnail will still be NV12. Signed-off-by: Umang Jain <email@uajain.com> --- src/android/jpeg/encoder_libjpeg.cpp | 12 +++ src/android/jpeg/thumbnailer.cpp | 106 +++++++++++++++++++++++++++ src/android/jpeg/thumbnailer.h | 34 +++++++++ src/android/meson.build | 1 + 4 files changed, 153 insertions(+) create mode 100644 src/android/jpeg/thumbnailer.cpp create mode 100644 src/android/jpeg/thumbnailer.h