[libcamera-devel] [PATCH v8 0/7] Add CrOS JEA implementation

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Dec 14 12:10:31 CET 2022


Hi Harvey,

Could you add the post-commit checkstyle hooks so you get notified about
checkstyle issues please?

  cp utils/hooks/post-commit .git/hooks/


I think the PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION indendation
we'd ignore, but the others in "Add JEA implementation" look appropriate
to fix.

--
Kieran



-------------------------------------------------------------------------
01b369fa09fb79aaca3c9a5e915e3652ae7d32df Allow inheritance of FrameBuffer
-------------------------------------------------------------------------
No issue detected

--------------------------------------------------------------------------------------------------
3458f84f864921cb45ca8e3a0d7e9e347a3b59a7 Add HALFrameBuffer and replace FrameBuffer in src/android
--------------------------------------------------------------------------------------------------
--- src/android/frame_buffer_allocator.h
+++ src/android/frame_buffer_allocator.h
@@ -36,21 +36,21 @@
 		int halPixelFormat, const libcamera::Size &size, uint32_t usage);
 };

-#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION			\
-PlatformFrameBufferAllocator::PlatformFrameBufferAllocator(		\
-	CameraDevice *const cameraDevice)				\
-	: Extensible(std::make_unique<Private>(cameraDevice))		\
-{									\
-}									\
-PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator()		\
-{									\
-}									\
-std::unique_ptr<HALFrameBuffer> 					\
-PlatformFrameBufferAllocator::allocate(int halPixelFormat,		\
-				       const libcamera::Size &size,	\
-				       uint32_t usage)			\
-{									\
-	return _d()->allocate(halPixelFormat, size, usage);		\
-}
+#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION                        \
+	PlatformFrameBufferAllocator::PlatformFrameBufferAllocator(         \
+		CameraDevice *const cameraDevice)                           \
+		: Extensible(std::make_unique<Private>(cameraDevice))       \
+	{                                                                   \
+	}                                                                   \
+	PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator()       \
+	{                                                                   \
+	}                                                                   \
+	std::unique_ptr<HALFrameBuffer>                                     \
+	PlatformFrameBufferAllocator::allocate(int halPixelFormat,          \
+					       const libcamera::Size &size, \
+					       uint32_t usage)              \
+	{                                                                   \
+		return _d()->allocate(halPixelFormat, size, usage);         \
+	}

 #endif /* __ANDROID_FRAME_BUFFER_ALLOCATOR_H__ */
---
1 potential issue detected, please review

----------------------------------------------------------------------------
214944ac387b6f22e30e16ea7950a4f40899047b Add meson.build in src/android/jpeg
----------------------------------------------------------------------------
No issue detected

----------------------------------------------------------------------------------------
2ed81d92f699b2c62c61e808066520f544fa5e40 Add an internal Encoder class in EncoderLibJpeg
----------------------------------------------------------------------------------------
--- src/android/jpeg/encoder_libjpeg.cpp
+++ src/android/jpeg/encoder_libjpeg.cpp
@@ -201,8 +201,8 @@
 }

 int EncoderLibJpeg::encode(const std::vector<Span<uint8_t>> &src,
-				    Span<uint8_t> dest, Span<const uint8_t> exifData,
-				    unsigned int quality)
+			   Span<uint8_t> dest, Span<const uint8_t> exifData,
+			   unsigned int quality)
 {
 	return encoder_.encode(src, std::move(dest), std::move(exifData), quality);
 }
---
1 potential issue detected, please review

-------------------------------------------------------------------------------------------------
f115685e544741d3084a603794328c4beb8f608c Move generateThumbnail from PostProcessorJpeg to Encoder
-------------------------------------------------------------------------------------------------
No issue detected

------------------------------------------------------------------------------
3113d70138d5204fd956ae1e0fc496a106a3e96f Pass StreamBuffer to Encoder::encoder
------------------------------------------------------------------------------
No issue detected

---------------------------------------------------------------
72e0597c7e3af7ac9e80ee2db9b7c212ab1172d8 Add JEA implementation
---------------------------------------------------------------
--- src/android/jpeg/encoder_jea.cpp
+++ src/android/jpeg/encoder_jea.cpp
@@ -56,9 +56,9 @@
 }

 void EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,
-				  const libcamera::Size &targetSize,
-				  unsigned int quality,
-				  std::vector<unsigned char> *thumbnail)
+				   const libcamera::Size &targetSize,
+				   unsigned int quality,
+				   std::vector<unsigned char> *thumbnail)
 {
 	if (!jpegCompressor_)
 		return;
@@ -71,11 +71,11 @@

 	// JEA needs consecutive memory.
 	unsigned long size = 0, index = 0;
-	for (const auto& plane : frame.planes())
+	for (const auto &plane : frame.planes())
 		size += plane.size();

 	std::vector<uint8_t> data(size);
-	for (const auto& plane : frame.planes()) {
+	for (const auto &plane : frame.planes()) {
 		memcpy(&data[index], plane.data(), plane.size());
 		index += plane.size();
 	}
---
2 potential issues detected, please review




Quoting Harvey Yang via libcamera-devel (2022-12-14 09:33:23)
> Hi all,
> 
> Updated based on Laurent's comments, and fixed a memory flaky
> issue in JEA's generateThumbnail function.
> 
> Sorry that the issue took me so long, with other issues in CrOS camera
> service mixed together :p
> 
> BR,
> Harvey
> 
> Harvey Yang (7):
>   Allow inheritance of FrameBuffer
>   Add HALFrameBuffer and replace FrameBuffer in src/android
>   Add meson.build in src/android/jpeg
>   Add an internal Encoder class in EncoderLibJpeg
>   Move generateThumbnail from PostProcessorJpeg to Encoder
>   Pass StreamBuffer to Encoder::encoder
>   Add JEA implementation
> 
>  include/libcamera/framebuffer.h               |   3 +-
>  src/android/camera_device.cpp                 |   5 +-
>  src/android/camera_device.h                   |   3 +-
>  src/android/camera_request.h                  |   3 +-
>  src/android/cros/camera3_hal.cpp              |   4 +-
>  src/android/cros_mojo_token.h                 |  12 ++
>  src/android/frame_buffer_allocator.h          |   7 +-
>  src/android/hal_framebuffer.cpp               |  22 ++++
>  src/android/hal_framebuffer.h                 |  26 +++++
>  src/android/jpeg/encoder.h                    |   9 +-
>  src/android/jpeg/encoder_jea.cpp              | 105 ++++++++++++++++++
>  src/android/jpeg/encoder_jea.h                |  35 ++++++
>  src/android/jpeg/encoder_libjpeg.cpp          |  85 ++++++++++++--
>  src/android/jpeg/encoder_libjpeg.h            |  46 +++++---
>  src/android/jpeg/meson.build                  |  17 +++
>  src/android/jpeg/post_processor_jpeg.cpp      |  63 ++---------
>  src/android/jpeg/post_processor_jpeg.h        |  11 +-
>  src/android/meson.build                       |   6 +-
>  .../mm/cros_frame_buffer_allocator.cpp        |   9 +-
>  .../mm/generic_frame_buffer_allocator.cpp     |  11 +-
>  20 files changed, 373 insertions(+), 109 deletions(-)
>  create mode 100644 src/android/cros_mojo_token.h
>  create mode 100644 src/android/hal_framebuffer.cpp
>  create mode 100644 src/android/hal_framebuffer.h
>  create mode 100644 src/android/jpeg/encoder_jea.cpp
>  create mode 100644 src/android/jpeg/encoder_jea.h
>  create mode 100644 src/android/jpeg/meson.build
> 
> -- 
> 2.39.0.rc1.256.g54fd8350bd-goog
>


More information about the libcamera-devel mailing list