[libcamera-devel] [PATCH v9 6/8] android: jpeg: Return an error code from generateThumbnail()
Cheng-Hao Yang
chenghaoyang at chromium.org
Tue Jan 17 11:27:21 CET 2023
Thank you for the patch!
Only nits of the format.
On Mon, Jan 16, 2023 at 8:28 AM Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> The usual way to signal errors in libcamera is to return an error code.
> Change the Encoder::generateThumbnail() function to return an int
> instead of signaling errors by not resizing the thumbnail vector. This
> allows propagating error codes to the callers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/android/jpeg/encoder.h | 8 +--
> src/android/jpeg/encoder_libjpeg.cpp | 70 +++++++++++++-----------
> src/android/jpeg/encoder_libjpeg.h | 8 +--
> src/android/jpeg/post_processor_jpeg.cpp | 6 +-
> 4 files changed, 48 insertions(+), 44 deletions(-)
>
> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
> index 5f9ef890023a..d15f22e67830 100644
> --- a/src/android/jpeg/encoder.h
> +++ b/src/android/jpeg/encoder.h
> @@ -22,8 +22,8 @@ public:
> libcamera::Span<uint8_t> destination,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality) = 0;
> - virtual void generateThumbnail(const libcamera::FrameBuffer
> &source,
> - const libcamera::Size &targetSize,
> - unsigned int quality,
> - std::vector<unsigned char>
> *thumbnail) = 0;
> + virtual int generateThumbnail(const libcamera::FrameBuffer &source,
> + const libcamera::Size &targetSize,
> + unsigned int quality,
> + std::vector<unsigned char>
> *thumbnail) = 0;
> };
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp
> b/src/android/jpeg/encoder_libjpeg.cpp
> index 9bbf1abbe09c..8f4df7899dfd 100644
> --- a/src/android/jpeg/encoder_libjpeg.cpp
> +++ b/src/android/jpeg/encoder_libjpeg.cpp
> @@ -89,53 +89,57 @@ int EncoderLibJpeg::encode(const FrameBuffer &source,
> Span<uint8_t> dest,
> return captureEncoder_.encode(frame.planes(), dest, exifData,
> quality);
> }
>
> -void EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer
> &source,
> - const libcamera::Size &targetSize,
> - unsigned int quality,
> - std::vector<unsigned char>
> *thumbnail)
> +int EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer
> &source,
> + const libcamera::Size &targetSize,
> + unsigned int quality,
> + std::vector<unsigned char>
> *thumbnail)
> {
> /* Stores the raw scaled-down thumbnail bytes. */
> std::vector<unsigned char> rawThumbnail;
>
> thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);
> + if (rawThumbnail.empty())
> + return -EINVAL;
>
> StreamConfiguration thCfg;
> thCfg.size = targetSize;
> thCfg.pixelFormat = thumbnailer_.pixelFormat();
> int ret = thumbnailEncoder_.configure(thCfg);
> + if (ret)
> + return ret;
>
> - if (!rawThumbnail.empty() && !ret) {
> - /*
> - * \todo Avoid value-initialization of all elements of the
> - * vector.
- */
> - thumbnail->resize(rawThumbnail.size());
> + /*
> + * \todo Avoid value-initialization of all elements of the
> + * vector.
>
nit: I guess |vector| could be merged in the above line, as the line
is not that long now? (Is the limit 80 characters?)
> + */
> + thumbnail->resize(rawThumbnail.size());
>
> - /*
> - * Split planes manually as the encoder expects a vector of
> - * planes.
>
ditto
> - *
> - * \todo Pass a vector of planes directly to
> - * Thumbnailer::createThumbnailer above and remove the
> manual
> - * planes split from here.
> - */
> - std::vector<Span<uint8_t>> thumbnailPlanes;
> - const PixelFormatInfo &formatNV12 =
> - PixelFormatInfo::info(formats::NV12);
> - size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);
> - size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);
> - thumbnailPlanes.push_back({ rawThumbnail.data(),
> yPlaneSize });
> - thumbnailPlanes.push_back({ rawThumbnail.data() +
> yPlaneSize,
> - uvPlaneSize });
> + /*
> + * Split planes manually as the encoder expects a vector of
> + * planes.
> + *
> + * \todo Pass a vector of planes directly to
> + * Thumbnailer::createThumbnailer above and remove the manual
> + * planes split from here.
> + */
> + std::vector<Span<uint8_t>> thumbnailPlanes;
> + const PixelFormatInfo &formatNV12 =
> + PixelFormatInfo::info(formats::NV12);
> + size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);
> + size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);
> + thumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });
> + thumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize,
> + uvPlaneSize });
>
> - int jpegSize = thumbnailEncoder_.encode(thumbnailPlanes,
> *thumbnail,
> - {}, quality);
> - thumbnail->resize(jpegSize);
> + int jpegSize = thumbnailEncoder_.encode(thumbnailPlanes,
> *thumbnail,
> + {}, quality);
> + thumbnail->resize(jpegSize);
>
> - LOG(JPEG, Debug)
> - << "Thumbnail compress returned "
> - << jpegSize << " bytes";
> - }
>
ditto
> + LOG(JPEG, Debug)
> + << "Thumbnail compress returned "
> + << jpegSize << " bytes";
> +
> + return 0;
> }
>
> EncoderLibJpeg::Encoder::Encoder()
> diff --git a/src/android/jpeg/encoder_libjpeg.h
> b/src/android/jpeg/encoder_libjpeg.h
> index a022a72e02bf..9cff4f1b42e3 100644
> --- a/src/android/jpeg/encoder_libjpeg.h
> +++ b/src/android/jpeg/encoder_libjpeg.h
> @@ -28,10 +28,10 @@ public:
> libcamera::Span<uint8_t> destination,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality) override;
> - void generateThumbnail(const libcamera::FrameBuffer &source,
> - const libcamera::Size &targetSize,
> - unsigned int quality,
> - std::vector<unsigned char> *thumbnail)
> override;
> + int generateThumbnail(const libcamera::FrameBuffer &source,
> + const libcamera::Size &targetSize,
> + unsigned int quality,
> + std::vector<unsigned char> *thumbnail)
> override;
>
> private:
> class Encoder
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp
> b/src/android/jpeg/post_processor_jpeg.cpp
> index 69b18a2e5945..5df89383e1af 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -115,9 +115,9 @@ void
> PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu
>
> if (thumbnailSize != Size(0, 0)) {
> std::vector<unsigned char> thumbnail;
> - encoder_->generateThumbnail(source, thumbnailSize,
> - quality, &thumbnail);
> - if (!thumbnail.empty())
> + ret = encoder_->generateThumbnail(source,
> thumbnailSize,
> + quality,
> &thumbnail);
> + if (!ret)
> exif.setThumbnail(std::move(thumbnail),
> Exif::Compression::JPEG);
> }
>
> --
> Regards,
>
> Laurent Pinchart
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230117/1735a3de/attachment.htm>
More information about the libcamera-devel
mailing list