<div dir="ltr"><div dir="ltr">Thank you for the patch!</div><div dir="ltr">Only nits of the format.</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">The usual way to signal errors in libcamera is to return an error code.<br>
Change the Encoder::generateThumbnail() function to return an int<br>
instead of signaling errors by not resizing the thumbnail vector. This<br>
allows propagating error codes to the callers.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
src/android/jpeg/encoder.h | 8 +--<br>
src/android/jpeg/encoder_libjpeg.cpp | 70 +++++++++++++-----------<br>
src/android/jpeg/encoder_libjpeg.h | 8 +--<br>
src/android/jpeg/post_processor_jpeg.cpp | 6 +-<br>
4 files changed, 48 insertions(+), 44 deletions(-)<br>
<br>
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h<br>
index 5f9ef890023a..d15f22e67830 100644<br>
--- a/src/android/jpeg/encoder.h<br>
+++ b/src/android/jpeg/encoder.h<br>
@@ -22,8 +22,8 @@ public:<br>
libcamera::Span<uint8_t> destination,<br>
libcamera::Span<const uint8_t> exifData,<br>
unsigned int quality) = 0;<br>
- virtual void generateThumbnail(const libcamera::FrameBuffer &source,<br>
- const libcamera::Size &targetSize,<br>
- unsigned int quality,<br>
- std::vector<unsigned char> *thumbnail) = 0;<br>
+ virtual int generateThumbnail(const libcamera::FrameBuffer &source,<br>
+ const libcamera::Size &targetSize,<br>
+ unsigned int quality,<br>
+ std::vector<unsigned char> *thumbnail) = 0;<br>
};<br>
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp<br>
index 9bbf1abbe09c..8f4df7899dfd 100644<br>
--- a/src/android/jpeg/encoder_libjpeg.cpp<br>
+++ b/src/android/jpeg/encoder_libjpeg.cpp<br>
@@ -89,53 +89,57 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,<br>
return captureEncoder_.encode(frame.planes(), dest, exifData, quality);<br>
}<br>
<br>
-void EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,<br>
- const libcamera::Size &targetSize,<br>
- unsigned int quality,<br>
- std::vector<unsigned char> *thumbnail)<br>
+int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,<br>
+ const libcamera::Size &targetSize,<br>
+ unsigned int quality,<br>
+ std::vector<unsigned char> *thumbnail)<br>
{<br>
/* Stores the raw scaled-down thumbnail bytes. */<br>
std::vector<unsigned char> rawThumbnail;<br>
<br>
thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);<br>
+ if (rawThumbnail.empty())<br>
+ return -EINVAL;<br>
<br>
StreamConfiguration thCfg;<br>
thCfg.size = targetSize;<br>
thCfg.pixelFormat = thumbnailer_.pixelFormat();<br>
int ret = thumbnailEncoder_.configure(thCfg);<br>
+ if (ret)<br>
+ return ret;<br>
<br>
- if (!rawThumbnail.empty() && !ret) {<br>
- /*<br>
- * \todo Avoid value-initialization of all elements of the<br>
- * vector.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- */<br>
- thumbnail->resize(rawThumbnail.size());<br>
+ /*<br>
+ * \todo Avoid value-initialization of all elements of the<br>
+ * vector.<br></blockquote><div><br class="gmail-Apple-interchange-newline">nit: I guess |vector| could be merged in the above line, as the line</div><div>is not that long now? (Is the limit 80 characters?)</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">
+ */<br>
+ thumbnail->resize(rawThumbnail.size());<br>
<br>
- /*<br>
- * Split planes manually as the encoder expects a vector of<br>
- * planes.<br></blockquote><div><br></div><div>ditto</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">
- *<br>
- * \todo Pass a vector of planes directly to<br>
- * Thumbnailer::createThumbnailer above and remove the manual<br>
- * planes split from here.<br>
- */<br>
- std::vector<Span<uint8_t>> thumbnailPlanes;<br>
- const PixelFormatInfo &formatNV12 =<br>
- PixelFormatInfo::info(formats::NV12);<br>
- size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);<br>
- size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);<br>
- thumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });<br>
- thumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize,<br>
- uvPlaneSize });<br>
+ /*<br>
+ * Split planes manually as the encoder expects a vector of<br>
+ * planes.<br>
+ *<br>
+ * \todo Pass a vector of planes directly to<br>
+ * Thumbnailer::createThumbnailer above and remove the manual<br>
+ * planes split from here.<br>
+ */<br>
+ std::vector<Span<uint8_t>> thumbnailPlanes;<br>
+ const PixelFormatInfo &formatNV12 =<br>
+ PixelFormatInfo::info(formats::NV12);<br>
+ size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);<br>
+ size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);<br>
+ thumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });<br>
+ thumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize,<br>
+ uvPlaneSize });<br>
<br>
- int jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail,<br>
- {}, quality);<br>
- thumbnail->resize(jpegSize);<br>
+ int jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail,<br>
+ {}, quality);<br>
+ thumbnail->resize(jpegSize);<br>
<br>
- LOG(JPEG, Debug)<br>
- << "Thumbnail compress returned "<br>
- << jpegSize << " bytes";<br>
- }<br></blockquote><div>ditto </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ LOG(JPEG, Debug)<br>
+ << "Thumbnail compress returned "<br>
+ << jpegSize << " bytes";<br>
+<br>
+ return 0;<br>
}<br>
<br>
EncoderLibJpeg::Encoder::Encoder()<br>
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h<br>
index a022a72e02bf..9cff4f1b42e3 100644<br>
--- a/src/android/jpeg/encoder_libjpeg.h<br>
+++ b/src/android/jpeg/encoder_libjpeg.h<br>
@@ -28,10 +28,10 @@ public:<br>
libcamera::Span<uint8_t> destination,<br>
libcamera::Span<const uint8_t> exifData,<br>
unsigned int quality) override;<br>
- void generateThumbnail(const libcamera::FrameBuffer &source,<br>
- const libcamera::Size &targetSize,<br>
- unsigned int quality,<br>
- std::vector<unsigned char> *thumbnail) 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>
class Encoder<br>
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp<br>
index 69b18a2e5945..5df89383e1af 100644<br>
--- a/src/android/jpeg/post_processor_jpeg.cpp<br>
+++ b/src/android/jpeg/post_processor_jpeg.cpp<br>
@@ -115,9 +115,9 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu<br>
<br>
if (thumbnailSize != Size(0, 0)) {<br>
std::vector<unsigned char> thumbnail;<br>
- encoder_->generateThumbnail(source, thumbnailSize,<br>
- quality, &thumbnail);<br>
- if (!thumbnail.empty())<br>
+ ret = encoder_->generateThumbnail(source, thumbnailSize,<br>
+ quality, &thumbnail);<br>
+ if (!ret)<br>
exif.setThumbnail(std::move(thumbnail), Exif::Compression::JPEG);<br>
}<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>