<div dir="ltr"><div dir="ltr">Thanks Laurent!</div><div>Some nits below. After these are addressed, the patches work fine on Chromebook soraka.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 16, 2023 at 8:28 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@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">From: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
<br>
This patch adds JEA implementation to replace libjpeg in CrOS platform,<br>
where hardware accelerator is available.<br>
<br>
Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
---<br>
 src/android/cros/camera3_hal.cpp         |   4 +-<br>
 src/android/cros_mojo_token.h            |  12 +++<br>
 src/android/jpeg/encoder_jea.cpp         | 103 +++++++++++++++++++++++<br>
 src/android/jpeg/encoder_jea.h           |  35 ++++++++<br>
 src/android/jpeg/meson.build             |  13 ++-<br>
 src/android/jpeg/post_processor_jpeg.cpp |   8 ++<br>
 6 files changed, 172 insertions(+), 3 deletions(-)<br>
 create mode 100644 src/android/cros_mojo_token.h<br>
 create mode 100644 src/android/jpeg/encoder_jea.cpp<br>
 create mode 100644 src/android/jpeg/encoder_jea.h<br>
<br>
diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp<br>
index fb863b5f9aa9..71acb441b0d4 100644<br>
--- a/src/android/cros/camera3_hal.cpp<br>
+++ b/src/android/cros/camera3_hal.cpp<br>
@@ -8,9 +8,11 @@<br>
 #include <cros-camera/cros_camera_hal.h><br>
<br>
 #include "../camera_hal_manager.h"<br>
+#include "../cros_mojo_token.h"<br>
<br>
-static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)<br>
+static void set_up(cros::CameraMojoChannelManagerToken *token)<br>
 {<br>
+       gCrosMojoToken = token;<br>
 }<br>
<br>
 static void tear_down()<br>
diff --git a/src/android/cros_mojo_token.h b/src/android/cros_mojo_token.h<br>
new file mode 100644<br>
index 000000000000..043c752a3997<br>
--- /dev/null<br>
+++ b/src/android/cros_mojo_token.h<br>
@@ -0,0 +1,12 @@<br>
+/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
+/*<br>
+ * Copyright (C) 2022, Google Inc.<br>
+ *<br>
+ * cros_mojo_token.h - cros-specific mojo token<br>
+ */<br>
+<br>
+#pragma once<br>
+<br>
+#include <cros-camera/cros_camera_hal.h><br>
+<br>
+inline cros::CameraMojoChannelManagerToken *gCrosMojoToken = nullptr;<br>
diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp<br>
new file mode 100644<br>
index 000000000000..a7076039d0b7<br>
--- /dev/null<br>
+++ b/src/android/jpeg/encoder_jea.cpp<br>
@@ -0,0 +1,103 @@<br>
+/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
+/*<br>
+ * Copyright (C) 2022, Google Inc.<br>
+ *<br>
+ * encoder_jea.cpp - JPEG encoding using CrOS JEA<br>
+ */<br>
+<br>
+#include "encoder_jea.h"<br>
+<br>
+#include "libcamera/internal/mapped_framebuffer.h"<br>
+<br>
+#include <cros-camera/camera_mojo_channel_manager_token.h><br>
+<br>
+#include "../cros_mojo_token.h"<br>
+#include "../hal_framebuffer.h"<br>
+<br>
+EncoderJea::EncoderJea() = default;<br>
+<br>
+EncoderJea::~EncoderJea() = default;<br>
+<br>
+int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)<br>
+{<br>
+       size_ = cfg.size;<br>
+<br>
+       if (jpegCompressor_)<br>
+               return 0;<br>
+<br>
+       if (gCrosMojoToken == nullptr)<br>
+               return -ENOTSUP;<br>
+<br>
+       jpegCompressor_ = cros::JpegCompressor::GetInstance(gCrosMojoToken);<br>
+<br>
+       return 0;<br>
+}<br>
+<br>
+int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br></blockquote><div><br></div><div>Perhaps use |buffer| instead of |streamBuffer|, to be aligned with the base class.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+                      libcamera::Span<const uint8_t> exifData,<br>
+                      unsigned int quality)<br>
+{<br>
+       if (!jpegCompressor_)<br>
+               return -ENOTSUP;<br>
+<br>
+       uint32_t outDataSize = 0;<br>
+       const HALFrameBuffer *fb =<br>
+               dynamic_cast<const HALFrameBuffer *>(streamBuffer->srcBuffer);<br>
+<br>
+       if (!jpegCompressor_->CompressImageFromHandle(fb->handle(),<br>
+                                                     *streamBuffer->camera3Buffer,<br>
+                                                     size_.width, size_.height,<br>
+                                                     quality, exifData.data(),<br>
+                                                     exifData.size(),<br>
+                                                     &outDataSize))<br>
+               return -EBUSY;<br>
+<br>
+       return outDataSize;<br>
+}<br>
+<br>
+int EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,<br>
+                                 const libcamera::Size &targetSize,<br>
+                                 unsigned int quality,<br>
+                                 std::vector<unsigned char> *thumbnail)<br>
+{<br>
+       if (!jpegCompressor_)<br>
+               return -ENOTSUPP;<br>
+<br></blockquote><div><br></div><div>Should be `-ENOTSUP`.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+       libcamera::MappedFrameBuffer frame(&source,<br>
+                                          libcamera::MappedFrameBuffer::MapFlag::Read);<br>
+<br>
+       if (frame.planes().empty())<br>
+               return -EINVAL;<br>
+<br>
+       /* JEA needs consecutive memory. */<br>
+       unsigned long size = 0, index = 0;<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>
+               memcpy(&data[index], plane.data(), plane.size());<br>
+               index += plane.size();<br>
+       }<br>
+<br>
+       uint32_t outDataSize = 0;<br>
+<br>
+       /*<br>
+        * Since the structure of the App1 segment is like:<br>
+        *   0xFF [1 byte marker] [2 bytes size] [data]<br>
+        * And it should not be larger than 64K.<br>
+        */<br>
+       constexpr int kApp1MaxDataSize = 65532;<br>
+       thumbnail->resize(kApp1MaxDataSize);<br>
+<br>
+       if (!jpegCompressor_->GenerateThumbnail(data.data(),<br>
+                                               size_.width, size_.height,<br>
+                                               targetSize.width,<br>
+                                               targetSize.height, quality,<br>
+                                               thumbnail->size(),<br>
+                                               thumbnail->data(),<br>
+                                               &outDataSize))<br>
+               return -EBUSY;<br>
+<br>
+       thumbnail->resize(outDataSize);<br></blockquote><div><br class="gmail-Apple-interchange-newline">Add `return 0;` here</div><div>  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+}</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h<br>
new file mode 100644<br>
index 000000000000..2eba31c2f73c<br>
--- /dev/null<br>
+++ b/src/android/jpeg/encoder_jea.h<br>
@@ -0,0 +1,35 @@<br>
+/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
+/*<br>
+ * Copyright (C) 2022, Google Inc.<br>
+ *<br>
+ * encoder_jea.h - JPEG encoding using CrOS JEA<br>
+ */<br>
+<br>
+#pragma once<br>
+<br>
+#include <libcamera/geometry.h><br>
+<br>
+#include <cros-camera/jpeg_compressor.h><br>
+<br>
+#include "encoder.h"<br>
+<br>
+class EncoderJea : public Encoder<br>
+{<br>
+public:<br>
+       EncoderJea();<br>
+       ~EncoderJea();<br>
+<br>
+       int configure(const libcamera::StreamConfiguration &cfg) override;<br>
+       int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
+                  libcamera::Span<const uint8_t> exifData,<br>
+                  unsigned int quality) override;<br>
+       int generateThumbnail(const libcamera::FrameBuffer &source,<br>
+                             const libcamera::Size &targetSize,<br>
+                             unsigned int quality,<br>
+                             std::vector<unsigned char> *thumbnail) override;<br>
+<br>
+private:<br>
+       libcamera::Size size_;<br>
+<br>
+       std::unique_ptr<cros::JpegCompressor> jpegCompressor_;<br>
+};<br>
diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build<br>
index 08397a87bc46..2b68f54c4228 100644<br>
--- a/src/android/jpeg/meson.build<br>
+++ b/src/android/jpeg/meson.build<br>
@@ -1,8 +1,17 @@<br>
 # SPDX-License-Identifier: CC0-1.0<br>
<br>
 android_hal_sources += files([<br>
-    'encoder_libjpeg.cpp',<br>
     'exif.cpp',<br>
     'post_processor_jpeg.cpp',<br>
-    'thumbnailer.cpp'<br>
 ])<br>
+<br>
+platform = get_option('android_platform')<br>
+if platform == 'generic'<br>
+    android_hal_sources += files([<br>
+      'encoder_libjpeg.cpp',<br>
+      'thumbnailer.cpp'<br>
+    ])<br>
+elif platform == 'cros'<br>
+    android_hal_sources += files(['encoder_jea.cpp'])<br>
+    android_deps += [dependency('libcros_camera')]<br>
+endif<br>
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp<br>
index 2a22b4a88f4a..f7cc70de1ef1 100644<br>
--- a/src/android/jpeg/post_processor_jpeg.cpp<br>
+++ b/src/android/jpeg/post_processor_jpeg.cpp<br>
@@ -12,7 +12,11 @@<br>
 #include "../camera_device.h"<br>
 #include "../camera_metadata.h"<br>
 #include "../camera_request.h"<br>
+#if defined(OS_CHROMEOS)<br>
+#include "encoder_jea.h"<br>
+#else /* !defined(OS_CHROMEOS) */<br>
 #include "encoder_libjpeg.h"<br>
+#endif<br>
 #include "exif.h"<br>
<br>
 #include <libcamera/base/log.h><br>
@@ -44,7 +48,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,<br>
<br>
        streamSize_ = outCfg.size;<br>
<br>
+#if defined(OS_CHROMEOS)<br>
+       encoder_ = std::make_unique<EncoderJea>();<br>
+#else /* !defined(OS_CHROMEOS) */<br>
        encoder_ = std::make_unique<EncoderLibJpeg>();<br>
+#endif<br>
<br>
        return encoder_->configure(inCfg);<br>
 }<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>