[libcamera-devel] [PATCH v2] android: jpeg: Split and pass the thumbnail planes to encoder
Hirokazu Honda
hiroh at chromium.org
Wed Sep 8 12:02:58 CEST 2021
Hi Umang, thank you for the patch.
On Wed, Sep 8, 2021 at 6:50 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> After multi-planar support was introduced for jpeg encoding as well,
> EncoderLibJpeg::encode() expects a vector of planes as the source of
> framebuffer to be encoded. Currently, we are passing a contiguous buffer
> which is treated as only one plane (instead of two, as thumbnail is NV12).
>
> Hence, split the thumbnail data into respective planes according to NV12.
> This fixes a crash in encoding of thumbnails.
>
> Fixes: 894ca69f6043("android: jpeg: Support multi-planar buffers")
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> Changes in v2:
> - Fix basic split logic error.
> - Add Jacopo's suggestion as a \todo since it's a superior method to fix
> this.
> ---
> src/android/jpeg/post_processor_jpeg.cpp | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 68d74842..1bf507a4 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -66,13 +66,23 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
> int ret = thumbnailEncoder_.configure(thCfg);
>
> if (!rawThumbnail.empty() && !ret) {
> + thumbnail->resize(rawThumbnail.size());
> +
> /*
> - * \todo Avoid value-initialization of all elements of the
> - * vector.
I think this comment should not be removed.
> + * 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.
> */
> - thumbnail->resize(rawThumbnail.size());
> + std::vector<Span<uint8_t>> thumbnailPlanes;
> + unsigned int thumbnailSize = targetSize.height * targetSize.width;
> + thumbnailPlanes.push_back({ rawThumbnail.data(), thumbnailSize });
> + thumbnailPlanes.push_back({ rawThumbnail.data() + thumbnailSize,
> + thumbnailSize / 2 });
Shall PixelFormatInfo::planeSize() be used?
With nits,
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
-Hiro
>
> - int jpeg_size = thumbnailEncoder_.encode({ rawThumbnail },
> + int jpeg_size = thumbnailEncoder_.encode(thumbnailPlanes,
> *thumbnail, {}, quality);
> thumbnail->resize(jpeg_size);
>
> --
> 2.31.0
>
More information about the libcamera-devel
mailing list