[libcamera-devel,v4,6/8] android: jpeg: Configure thumbnailer based on request metadata
diff mbox series

Message ID 20210125071444.26252-7-paul.elder@ideasonboard.com
State Superseded
Delegated to: Paul Elder
Headers show
Series
  • Fill in android result metadata and EXIF tags
Related show

Commit Message

Paul Elder Jan. 25, 2021, 7:14 a.m. UTC 
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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
Changes in v4:
- cosmetic changes

New in v3
- split from "android: Set result metadata and EXIF fields based on
  request"
---
 src/android/camera_device.cpp            |  6 ++--
 src/android/jpeg/post_processor_jpeg.cpp | 43 +++++++++++++++++-------
 src/android/jpeg/post_processor_jpeg.h   |  1 +
 src/android/jpeg/thumbnailer.cpp         | 25 ++------------
 src/android/jpeg/thumbnailer.h           |  6 ++--
 5 files changed, 40 insertions(+), 41 deletions(-)

Comments

Jacopo Mondi Jan. 25, 2021, 12:09 p.m. UTC | #1 
Hi Paul,

On Mon, Jan 25, 2021 at 04:14:42PM +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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> ---
> Changes in v4:
> - cosmetic changes
>
> New in v3
> - split from "android: Set result metadata and EXIF fields based on
>   request"
> ---
>  src/android/camera_device.cpp            |  6 ++--
>  src/android/jpeg/post_processor_jpeg.cpp | 43 +++++++++++++++++-------
>  src/android/jpeg/post_processor_jpeg.h   |  1 +
>  src/android/jpeg/thumbnailer.cpp         | 25 ++------------
>  src/android/jpeg/thumbnailer.h           |  6 ++--
>  5 files changed, 40 insertions(+), 41 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 2cd3c8a1..3068f89f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1915,7 +1915,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>
>  	/*
>  	 * \todo Keep this in sync with the actual number of entries.
> -	 * Currently: 38 entries, 147 bytes
> +	 * Currently: 40 entries, 156 bytes
>  	 *
>  	 * Reserve more space for the JPEG metadata set by the post-processor.
>  	 * Currently: ANDROID_JPEG_SIZE (int32_t), ANDROID_JPEG_QUALITY (byte),
> @@ -1923,11 +1923,13 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  	 * ANDROID_JPEG_GPS_PROCESSING_METHOD (byte x 32) = 32 bytes
>  	 * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes
>  	 * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.
> +	 * ANDROID_JPEG_THUMBNAIL_QUALITY (byte) = 1 byte
> +	 * ANDROID_JPEG_THUMBNAIL_SIZE (int32 x 2) = 8 bytes

Same comment as the previous patch. The new tags should be added to
availableResultKeys and the static metadata pack size updated
accordingly.

>  	 * ANDROID_LENS_APERTURE (float) = 4 bytes
>  	 * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes
>  	 */
>  	std::unique_ptr<CameraMetadata> resultMetadata =
> -		std::make_unique<CameraMetadata>(38, 147);
> +		std::make_unique<CameraMetadata>(40, 156);
>  	if (!resultMetadata->isValid()) {
>  		LOG(HAL, Error) << "Failed to allocate static metadata";
>  		return nullptr;
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 10c71a47..d35fe361 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -43,12 +43,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>();
>
> @@ -56,14 +50,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);
>
> -	if (!rawThumbnail.empty()) {
> +	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.
> @@ -127,6 +127,28 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  					 entry.data.i64, 1);
>  	}
>
> +	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) {
>  		exif.setGPSLocation(entry.data.d);
> @@ -142,11 +164,6 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  					 entry.data.u8, entry.count);
>  	}
>
> -	std::vector<unsigned char> thumbnail;
> -	generateThumbnail(source, &thumbnail);
> -	if (!thumbnail.empty())
> -		exif.setThumbnail(thumbnail, Exif::Compression::JPEG);
> -
>  	if (exif.generate() != 0)
>  		LOG(JPEG, Error) << "Failed to generate valid EXIF data";
>
> 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..4e9226c3 100644
> --- a/src/android/jpeg/thumbnailer.h
> +++ b/src/android/jpeg/thumbnailer.h
> @@ -20,15 +20,13 @@ 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_;
>  };
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 2cd3c8a1..3068f89f 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1915,7 +1915,7 @@  CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
 
 	/*
 	 * \todo Keep this in sync with the actual number of entries.
-	 * Currently: 38 entries, 147 bytes
+	 * Currently: 40 entries, 156 bytes
 	 *
 	 * Reserve more space for the JPEG metadata set by the post-processor.
 	 * Currently: ANDROID_JPEG_SIZE (int32_t), ANDROID_JPEG_QUALITY (byte),
@@ -1923,11 +1923,13 @@  CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
 	 * ANDROID_JPEG_GPS_PROCESSING_METHOD (byte x 32) = 32 bytes
 	 * ANDROID_JPEG_GPS_TIMESTAMP (int64) = 8 bytes
 	 * ANDROID_JPEG_ORIENTATION (int32_t) = 3 entries, 9 bytes.
+	 * ANDROID_JPEG_THUMBNAIL_QUALITY (byte) = 1 byte
+	 * ANDROID_JPEG_THUMBNAIL_SIZE (int32 x 2) = 8 bytes
 	 * ANDROID_LENS_APERTURE (float) = 4 bytes
 	 * ANDROID_LENS_FOCAL_LENGTH (float) = 4 bytes
 	 */
 	std::unique_ptr<CameraMetadata> resultMetadata =
-		std::make_unique<CameraMetadata>(38, 147);
+		std::make_unique<CameraMetadata>(40, 156);
 	if (!resultMetadata->isValid()) {
 		LOG(HAL, Error) << "Failed to allocate static metadata";
 		return nullptr;
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
index 10c71a47..d35fe361 100644
--- a/src/android/jpeg/post_processor_jpeg.cpp
+++ b/src/android/jpeg/post_processor_jpeg.cpp
@@ -43,12 +43,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>();
 
@@ -56,14 +50,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);
 
-	if (!rawThumbnail.empty()) {
+	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.
@@ -127,6 +127,28 @@  int PostProcessorJpeg::process(const FrameBuffer &source,
 					 entry.data.i64, 1);
 	}
 
+	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) {
 		exif.setGPSLocation(entry.data.d);
@@ -142,11 +164,6 @@  int PostProcessorJpeg::process(const FrameBuffer &source,
 					 entry.data.u8, entry.count);
 	}
 
-	std::vector<unsigned char> thumbnail;
-	generateThumbnail(source, &thumbnail);
-	if (!thumbnail.empty())
-		exif.setThumbnail(thumbnail, Exif::Compression::JPEG);
-
 	if (exif.generate() != 0)
 		LOG(JPEG, Error) << "Failed to generate valid EXIF data";
 
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..4e9226c3 100644
--- a/src/android/jpeg/thumbnailer.h
+++ b/src/android/jpeg/thumbnailer.h
@@ -20,15 +20,13 @@  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_;
 };