<div dir="ltr">Right. Thanks Kieran for catching that!<div>Added the post-commit script. Will try to prevent it from happening again.<br><div>I'll wait a bit for other comments/reviews before sending another version of patches, to avoid the spamming.</div><div><br></div><div>BR,</div><div>Harvey</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 14, 2022 at 7:10 PM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey,<br>
<br>
Could you add the post-commit checkstyle hooks so you get notified about<br>
checkstyle issues please?<br>
<br>
cp utils/hooks/post-commit .git/hooks/<br>
<br>
<br>
I think the PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION indendation<br>
we'd ignore, but the others in "Add JEA implementation" look appropriate<br>
to fix.<br>
<br>
--<br>
Kieran<br>
<br>
<br>
<br>
-------------------------------------------------------------------------<br>
01b369fa09fb79aaca3c9a5e915e3652ae7d32df Allow inheritance of FrameBuffer<br>
-------------------------------------------------------------------------<br>
No issue detected<br>
<br>
--------------------------------------------------------------------------------------------------<br>
3458f84f864921cb45ca8e3a0d7e9e347a3b59a7 Add HALFrameBuffer and replace FrameBuffer in src/android<br>
--------------------------------------------------------------------------------------------------<br>
--- src/android/frame_buffer_allocator.h<br>
+++ src/android/frame_buffer_allocator.h<br>
@@ -36,21 +36,21 @@<br>
int halPixelFormat, const libcamera::Size &size, uint32_t usage);<br>
};<br>
<br>
-#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION \<br>
-PlatformFrameBufferAllocator::PlatformFrameBufferAllocator( \<br>
- CameraDevice *const cameraDevice) \<br>
- : Extensible(std::make_unique<Private>(cameraDevice)) \<br>
-{ \<br>
-} \<br>
-PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator() \<br>
-{ \<br>
-} \<br>
-std::unique_ptr<HALFrameBuffer> \<br>
-PlatformFrameBufferAllocator::allocate(int halPixelFormat, \<br>
- const libcamera::Size &size, \<br>
- uint32_t usage) \<br>
-{ \<br>
- return _d()->allocate(halPixelFormat, size, usage); \<br>
-}<br>
+#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION \<br>
+ PlatformFrameBufferAllocator::PlatformFrameBufferAllocator( \<br>
+ CameraDevice *const cameraDevice) \<br>
+ : Extensible(std::make_unique<Private>(cameraDevice)) \<br>
+ { \<br>
+ } \<br>
+ PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator() \<br>
+ { \<br>
+ } \<br>
+ std::unique_ptr<HALFrameBuffer> \<br>
+ PlatformFrameBufferAllocator::allocate(int halPixelFormat, \<br>
+ const libcamera::Size &size, \<br>
+ uint32_t usage) \<br>
+ { \<br>
+ return _d()->allocate(halPixelFormat, size, usage); \<br>
+ }<br>
<br>
#endif /* __ANDROID_FRAME_BUFFER_ALLOCATOR_H__ */<br>
---<br>
1 potential issue detected, please review<br>
<br>
----------------------------------------------------------------------------<br>
214944ac387b6f22e30e16ea7950a4f40899047b Add meson.build in src/android/jpeg<br>
----------------------------------------------------------------------------<br>
No issue detected<br>
<br>
----------------------------------------------------------------------------------------<br>
2ed81d92f699b2c62c61e808066520f544fa5e40 Add an internal Encoder class in EncoderLibJpeg<br>
----------------------------------------------------------------------------------------<br>
--- src/android/jpeg/encoder_libjpeg.cpp<br>
+++ src/android/jpeg/encoder_libjpeg.cpp<br>
@@ -201,8 +201,8 @@<br>
}<br>
<br>
int EncoderLibJpeg::encode(const std::vector<Span<uint8_t>> &src,<br>
- Span<uint8_t> dest, Span<const uint8_t> exifData,<br>
- unsigned int quality)<br>
+ Span<uint8_t> dest, Span<const uint8_t> exifData,<br>
+ unsigned int quality)<br>
{<br>
return encoder_.encode(src, std::move(dest), std::move(exifData), quality);<br>
}<br>
---<br>
1 potential issue detected, please review<br>
<br>
-------------------------------------------------------------------------------------------------<br>
f115685e544741d3084a603794328c4beb8f608c Move generateThumbnail from PostProcessorJpeg to Encoder<br>
-------------------------------------------------------------------------------------------------<br>
No issue detected<br>
<br>
------------------------------------------------------------------------------<br>
3113d70138d5204fd956ae1e0fc496a106a3e96f Pass StreamBuffer to Encoder::encoder<br>
------------------------------------------------------------------------------<br>
No issue detected<br>
<br>
---------------------------------------------------------------<br>
72e0597c7e3af7ac9e80ee2db9b7c212ab1172d8 Add JEA implementation<br>
---------------------------------------------------------------<br>
--- src/android/jpeg/encoder_jea.cpp<br>
+++ src/android/jpeg/encoder_jea.cpp<br>
@@ -56,9 +56,9 @@<br>
}<br>
<br>
void EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,<br>
- const libcamera::Size &targetSize,<br>
- unsigned int quality,<br>
- std::vector<unsigned char> *thumbnail)<br>
+ const libcamera::Size &targetSize,<br>
+ unsigned int quality,<br>
+ std::vector<unsigned char> *thumbnail)<br>
{<br>
if (!jpegCompressor_)<br>
return;<br>
@@ -71,11 +71,11 @@<br>
<br>
// JEA needs consecutive memory.<br>
unsigned long size = 0, index = 0;<br>
- for (const auto& plane : frame.planes())<br>
+ for (const auto &plane : frame.planes())<br>
size += plane.size();<br>
<br>
std::vector<uint8_t> data(size);<br>
- for (const auto& plane : frame.planes()) {<br>
+ for (const auto &plane : frame.planes()) {<br>
memcpy(&data[index], plane.data(), plane.size());<br>
index += plane.size();<br>
}<br>
---<br>
2 potential issues detected, please review<br>
<br>
<br>
<br>
<br>
Quoting Harvey Yang via libcamera-devel (2022-12-14 09:33:23)<br>
> Hi all,<br>
> <br>
> Updated based on Laurent's comments, and fixed a memory flaky<br>
> issue in JEA's generateThumbnail function.<br>
> <br>
> Sorry that the issue took me so long, with other issues in CrOS camera<br>
> service mixed together :p<br>
> <br>
> BR,<br>
> Harvey<br>
> <br>
> Harvey Yang (7):<br>
> Allow inheritance of FrameBuffer<br>
> Add HALFrameBuffer and replace FrameBuffer in src/android<br>
> Add meson.build in src/android/jpeg<br>
> Add an internal Encoder class in EncoderLibJpeg<br>
> Move generateThumbnail from PostProcessorJpeg to Encoder<br>
> Pass StreamBuffer to Encoder::encoder<br>
> Add JEA implementation<br>
> <br>
> include/libcamera/framebuffer.h | 3 +-<br>
> src/android/camera_device.cpp | 5 +-<br>
> src/android/camera_device.h | 3 +-<br>
> src/android/camera_request.h | 3 +-<br>
> src/android/cros/camera3_hal.cpp | 4 +-<br>
> src/android/cros_mojo_token.h | 12 ++<br>
> src/android/frame_buffer_allocator.h | 7 +-<br>
> src/android/hal_framebuffer.cpp | 22 ++++<br>
> src/android/hal_framebuffer.h | 26 +++++<br>
> src/android/jpeg/encoder.h | 9 +-<br>
> src/android/jpeg/encoder_jea.cpp | 105 ++++++++++++++++++<br>
> src/android/jpeg/encoder_jea.h | 35 ++++++<br>
> src/android/jpeg/encoder_libjpeg.cpp | 85 ++++++++++++--<br>
> src/android/jpeg/encoder_libjpeg.h | 46 +++++---<br>
> src/android/jpeg/meson.build | 17 +++<br>
> src/android/jpeg/post_processor_jpeg.cpp | 63 ++---------<br>
> src/android/jpeg/post_processor_jpeg.h | 11 +-<br>
> src/android/meson.build | 6 +-<br>
> .../mm/cros_frame_buffer_allocator.cpp | 9 +-<br>
> .../mm/generic_frame_buffer_allocator.cpp | 11 +-<br>
> 20 files changed, 373 insertions(+), 109 deletions(-)<br>
> create mode 100644 src/android/cros_mojo_token.h<br>
> create mode 100644 src/android/hal_framebuffer.cpp<br>
> create mode 100644 src/android/hal_framebuffer.h<br>
> create mode 100644 src/android/jpeg/encoder_jea.cpp<br>
> create mode 100644 src/android/jpeg/encoder_jea.h<br>
> create mode 100644 src/android/jpeg/meson.build<br>
> <br>
> -- <br>
> 2.39.0.rc1.256.g54fd8350bd-goog<br>
><br>
</blockquote></div>