[libcamera-devel] [PATCH v8 0/7] Add CrOS JEA implementation
Cheng-Hao Yang
chenghaoyang at chromium.org
Wed Dec 14 15:47:27 CET 2022
Right. Thanks Kieran for catching that!
Added the post-commit script. Will try to prevent it from happening again.
I'll wait a bit for other comments/reviews before sending another version
of patches, to avoid the spamming.
BR,
Harvey
On Wed, Dec 14, 2022 at 7:10 PM Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:
> 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221214/dbadaa73/attachment.htm>
More information about the libcamera-devel
mailing list