[libcamera-devel,v3,8/8] android: jpeg: Set thumbnail and JPEG quality based on request
diff mbox series

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

Commit Message

Paul Elder Jan. 23, 2021, 5:17 a.m. UTC 
Set the thumbnail quality and the JPEG quality based on the android
request metadata.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
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 | 25 ++++++++++++------------
 src/android/jpeg/post_processor_jpeg.h   |  1 +
 5 files changed, 25 insertions(+), 22 deletions(-)

Comments

Laurent Pinchart Jan. 23, 2021, 9:49 a.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Sat, Jan 23, 2021 at 02:17:04PM +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>
> 
> ---
> 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 | 25 ++++++++++++------------
>  src/android/jpeg/post_processor_jpeg.h   |  1 +
>  5 files changed, 25 insertions(+), 22 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 e5e42d39..8b1801b9 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -49,6 +49,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. */
> @@ -69,7 +70,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)
> @@ -131,19 +132,20 @@ 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);
> +		if (ret)
> +			resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, entry.data.u8, 1);

This should be set to 95 if ANDROID_JPEG_THUMBNAIL_QUALITY isn't
specified as that's the value we use below.

		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,
> +					  ret ? *entry.data.u8 : 95, &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);
> @@ -164,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 uint32_t jpeg_quality = ret ? *entry.data.u8 : 95;

s/jpeg_quality/jpegQuality/ (or just quality).

This should be a uint8_t otherwise you'll pass the wrong pointer to
->addEntry() and it will fail on big-endian machines.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
> +
> +	int jpeg_size = encoder_->encode(source, destination, exif.data(), jpeg_quality);
>  	if (jpeg_size < 0) {
>  		LOG(JPEG, Error) << "Failed to encode stream image";
>  		return jpeg_size;
> @@ -192,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 jpeg_quality = ret ? *entry.data.u8 : 95;
> -	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 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_;

Patch
diff mbox series 

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 e5e42d39..8b1801b9 100644
--- a/src/android/jpeg/post_processor_jpeg.cpp
+++ b/src/android/jpeg/post_processor_jpeg.cpp
@@ -49,6 +49,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. */
@@ -69,7 +70,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)
@@ -131,19 +132,20 @@  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);
+		if (ret)
+			resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, entry.data.u8, 1);
+
 		if (thumbnailSize != Size(0, 0)) {
 			std::vector<unsigned char> thumbnail;
-			generateThumbnail(source, thumbnailSize, &thumbnail);
+			generateThumbnail(source, thumbnailSize,
+					  ret ? *entry.data.u8 : 95, &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);
@@ -164,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 uint32_t jpeg_quality = ret ? *entry.data.u8 : 95;
+	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
+
+	int jpeg_size = encoder_->encode(source, destination, exif.data(), jpeg_quality);
 	if (jpeg_size < 0) {
 		LOG(JPEG, Error) << "Failed to encode stream image";
 		return jpeg_size;
@@ -192,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 jpeg_quality = ret ? *entry.data.u8 : 95;
-	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 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_;