[{"id":19502,"web_url":"https://patchwork.libcamera.org/comment/19502/","msgid":"<297dc0d6-a8bd-e489-3f30-a282ae615d86@ideasonboard.com>","date":"2021-09-07T10:12:18","subject":"Re: [libcamera-devel] [PATCH v3 20/30] android: jpeg: Support\n\tmulti-planar buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 06/09/2021 23:56, Laurent Pinchart wrote:\n> The JPEG post-processor uses MappedFrameBuffer to access pixel data, but\n> only uses data from the first plane. Pass the vector of planes to the\n> encode() function to correctly handle multi-planar formats (currently\n> limited to NV12).\n> \n> Signed-off-by: Laurent Pinchart \n> Acked-by: Umang Jain \n> Tested-by: Umang Jain \n> Reviewed-by: Hirokazu Honda \n> ---\n> src/android/jpeg/encoder_libjpeg.cpp | 17 +++++++++--------\n> src/android/jpeg/encoder_libjpeg.h | 8 +++++---\n> src/android/jpeg/post_processor_jpeg.cpp | 2 +-\n> src/android/jpeg/thumbnailer.cpp | 4 ++--\n> 4 files changed, 17 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index 807a0949a8fc..d0b1da5016d2 100644\n> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> @@ -103,9 +103,9 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg)\n> \treturn 0;\n> }\n> \n> -void EncoderLibJpeg::compressRGB(Span frame)\n> +void EncoderLibJpeg::compressRGB(const std::vector> &planes)\n> {\n> -\tunsigned char *src = const_cast(frame.data());\n> +\tunsigned char *src = const_cast(planes[0].data());\n> \t/* \\todo Stride information should come from buffer configuration. */\n> \tunsigned int stride = pixelFormatInfo_->stride(compress_.image_width, 0);\n> \n> @@ -121,7 +121,7 @@ void EncoderLibJpeg::compressRGB(Span frame)\n> * Compress the incoming buffer from a supported NV format.\n> * This naively unpacks the semi-planar NV12 to a YUV888 format for libjpeg.\n> */\n> -void EncoderLibJpeg::compressNV(Span frame)\n> +void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> {\n\nShould we ASSERT(planes.size() >= 2); here? (or even == 2?)\n\n> \tuint8_t tmprowbuf[compress_.image_width * 3];\n> \n> @@ -143,8 +143,8 @@ void EncoderLibJpeg::compressNV(Span frame)\n> \tunsigned int cb_pos = nvSwap_ ? 1 : 0;\n> \tunsigned int cr_pos = nvSwap_ ? 0 : 1;\n> \n> -\tconst unsigned char *src = frame.data();\n> -\tconst unsigned char *src_c = src + y_stride * compress_.image_height;\n> +\tconst unsigned char *src = planes[0].data();\n> +\tconst unsigned char *src_c = planes[1].data();\n> \n> \tJSAMPROW row_pointer[1];\n> \trow_pointer[0] = &tmprowbuf[0];\n> @@ -188,11 +188,12 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span dest,\n> \t\treturn frame.error();\n> \t}\n> \n> -\treturn encode(frame.planes()[0], dest, exifData, quality);\n> +\treturn encode(frame.planes(), dest, exifData, quality);\n> }\n> \n> -int EncoderLibJpeg::encode(Span src, Span dest,\n> -\t\t\t Span exifData, unsigned int quality)\n> +int EncoderLibJpeg::encode(const std::vector> &src,\n> +\t\t\t Span dest, Span exifData,\n> +\t\t\t unsigned int quality)\n> {\n> \tunsigned char *destination = dest.data();\n> \tunsigned long size = dest.size();\n> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h\n> index 61fbd1a69278..45ffbd7fae5d 100644\n> --- a/src/android/jpeg/encoder_libjpeg.h\n> +++ b/src/android/jpeg/encoder_libjpeg.h\n> @@ -9,6 +9,8 @@\n> \n> #include \"encoder.h\"\n> \n> +#include \n> +\n> #include \"libcamera/internal/formats.h\"\n> \n> #include \n> @@ -24,14 +26,14 @@ public:\n> \t\t libcamera::Span destination,\n> \t\t libcamera::Span exifData,\n> \t\t unsigned int quality) override;\n> -\tint encode(libcamera::Span source,\n> +\tint encode(const std::vector> &planes,\n> \t\t libcamera::Span destination,\n> \t\t libcamera::Span exifData,\n> \t\t unsigned int quality);\n> \n> private:\n> -\tvoid compressRGB(libcamera::Span frame);\n> -\tvoid compressNV(libcamera::Span frame);\n> +\tvoid compressRGB(const std::vector> &planes);\n> +\tvoid compressNV(const std::vector> &planes);\n> \n> \tstruct jpeg_compress_struct compress_;\n> \tstruct jpeg_error_mgr jerr_;\n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 3160a784419c..68d74842925d 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -72,7 +72,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> \t\t */\n> \t\tthumbnail->resize(rawThumbnail.size());\n> \n> -\t\tint jpeg_size = thumbnailEncoder_.encode(rawThumbnail,\n> +\t\tint jpeg_size = thumbnailEncoder_.encode({ rawThumbnail },\n> \t\t\t\t\t\t\t *thumbnail, {}, quality);\n> \t\tthumbnail->resize(jpeg_size);\n> \n> diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp\n> index 043c7b33f06a..2c341535de9e 100644\n> --- a/src/android/jpeg/thumbnailer.cpp\n> +++ b/src/android/jpeg/thumbnailer.cpp\n> @@ -62,8 +62,8 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,\n> \tASSERT(tw % 2 == 0 && th % 2 == 0);\n> \n\nPossibly ASSERT(frame.planes().size() >= 2 here too.\n\nBut either way,\n\nReviewed-by: Kieran Bingham \n\n> \t/* Image scaling block implementing nearest-neighbour algorithm. */\n> -\tunsigned char *src = static_cast(frame.planes()[0].data());\n> -\tunsigned char *srcC = src + sh * sw;\n> +\tunsigned char *src = frame.planes()[0].data();\n> +\tunsigned char *srcC = frame.planes()[1].data();\n> \tunsigned char *srcCb, *srcCr;\n> \tunsigned char *dstY, *srcY;\n> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8F5E4BE175\n\tfor ;\n\tTue, 7 Sep 2021 10:12:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 15C2260252;\n\tTue, 7 Sep 2021 12:12:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCE9D60251\n\tfor ;\n\tTue, 7 Sep 2021 12:12:20 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4F626499;\n\tTue, 7 Sep 2021 12:12:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vzqAjRsL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631009540;\n\tbh=D2DXz/qeiyNiDMRP4OHEWEWDybKKGDhxoxA3vpi/b2s=;\n\th=To:References:From:Subject:Date:In-Reply-To:From;\n\tb=vzqAjRsL8jDYYkuQfQPE9NUp3CIdUvhlzvAsMg/+j7zdDnI9J65Vby8RjLKj36bwG\n\t4/SFK053WBCyrVf6+P+kon8RO5Ksb/ORJCzKB4KQ54x2qZ7v1kkdbG0RWgP6D4syN/\n\tSY1FE6MIoAAPb1Os5mrYLsaLjE+SMT89f5GYKUi4=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210906225420.13275-1-laurent.pinchart@ideasonboard.com>\n\t<20210906225636.14683-20-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Message-ID":"<297dc0d6-a8bd-e489-3f30-a282ae615d86@ideasonboard.com>","Date":"Tue, 7 Sep 2021 11:12:18 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210906225636.14683-20-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 20/30] android: jpeg: Support\n\tmulti-planar buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19514,"web_url":"https://patchwork.libcamera.org/comment/19514/","msgid":"","date":"2021-09-07T14:16:37","subject":"Re: [libcamera-devel] [PATCH v3 20/30] android: jpeg: Support\n\tmulti-planar buffers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Sep 07, 2021 at 11:12:18AM +0100, Kieran Bingham wrote:\n> On 06/09/2021 23:56, Laurent Pinchart wrote:\n> > The JPEG post-processor uses MappedFrameBuffer to access pixel data, but\n> > only uses data from the first plane. Pass the vector of planes to the\n> > encode() function to correctly handle multi-planar formats (currently\n> > limited to NV12).\n> > \n> > Signed-off-by: Laurent Pinchart \n> > Acked-by: Umang Jain \n> > Tested-by: Umang Jain \n> > Reviewed-by: Hirokazu Honda \n> > ---\n> > src/android/jpeg/encoder_libjpeg.cpp | 17 +++++++++--------\n> > src/android/jpeg/encoder_libjpeg.h | 8 +++++---\n> > src/android/jpeg/post_processor_jpeg.cpp | 2 +-\n> > src/android/jpeg/thumbnailer.cpp | 4 ++--\n> > 4 files changed, 17 insertions(+), 14 deletions(-)\n> > \n> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > index 807a0949a8fc..d0b1da5016d2 100644\n> > --- a/src/android/jpeg/encoder_libjpeg.cpp\n> > +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> > @@ -103,9 +103,9 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg)\n> > \treturn 0;\n> > }\n> > \n> > -void EncoderLibJpeg::compressRGB(Span frame)\n> > +void EncoderLibJpeg::compressRGB(const std::vector> &planes)\n> > {\n> > -\tunsigned char *src = const_cast(frame.data());\n> > +\tunsigned char *src = const_cast(planes[0].data());\n> > \t/* \\todo Stride information should come from buffer configuration. */\n> > \tunsigned int stride = pixelFormatInfo_->stride(compress_.image_width, 0);\n> > \n> > @@ -121,7 +121,7 @@ void EncoderLibJpeg::compressRGB(Span frame)\n> > * Compress the incoming buffer from a supported NV format.\n> > * This naively unpacks the semi-planar NV12 to a YUV888 format for libjpeg.\n> > */\n> > -void EncoderLibJpeg::compressNV(Span frame)\n> > +void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> > {\n> \n> Should we ASSERT(planes.size() >= 2); here? (or even == 2?)\n\nI'll add\n\n\tASSERT(src.size() == pixelFormatInfo_->numPlanes());\n\nin EncoderLibJpeg::encode() instead, that will cover the RGB case too.\n\n> > \tuint8_t tmprowbuf[compress_.image_width * 3];\n> > \n> > @@ -143,8 +143,8 @@ void EncoderLibJpeg::compressNV(Span frame)\n> > \tunsigned int cb_pos = nvSwap_ ? 1 : 0;\n> > \tunsigned int cr_pos = nvSwap_ ? 0 : 1;\n> > \n> > -\tconst unsigned char *src = frame.data();\n> > -\tconst unsigned char *src_c = src + y_stride * compress_.image_height;\n> > +\tconst unsigned char *src = planes[0].data();\n> > +\tconst unsigned char *src_c = planes[1].data();\n> > \n> > \tJSAMPROW row_pointer[1];\n> > \trow_pointer[0] = &tmprowbuf[0];\n> > @@ -188,11 +188,12 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span dest,\n> > \t\treturn frame.error();\n> > \t}\n> > \n> > -\treturn encode(frame.planes()[0], dest, exifData, quality);\n> > +\treturn encode(frame.planes(), dest, exifData, quality);\n> > }\n> > \n> > -int EncoderLibJpeg::encode(Span src, Span dest,\n> > -\t\t\t Span exifData, unsigned int quality)\n> > +int EncoderLibJpeg::encode(const std::vector> &src,\n> > +\t\t\t Span dest, Span exifData,\n> > +\t\t\t unsigned int quality)\n> > {\n> > \tunsigned char *destination = dest.data();\n> > \tunsigned long size = dest.size();\n> > diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h\n> > index 61fbd1a69278..45ffbd7fae5d 100644\n> > --- a/src/android/jpeg/encoder_libjpeg.h\n> > +++ b/src/android/jpeg/encoder_libjpeg.h\n> > @@ -9,6 +9,8 @@\n> > \n> > #include \"encoder.h\"\n> > \n> > +#include \n> > +\n> > #include \"libcamera/internal/formats.h\"\n> > \n> > #include \n> > @@ -24,14 +26,14 @@ public:\n> > \t\t libcamera::Span destination,\n> > \t\t libcamera::Span exifData,\n> > \t\t unsigned int quality) override;\n> > -\tint encode(libcamera::Span source,\n> > +\tint encode(const std::vector> &planes,\n> > \t\t libcamera::Span destination,\n> > \t\t libcamera::Span exifData,\n> > \t\t unsigned int quality);\n> > \n> > private:\n> > -\tvoid compressRGB(libcamera::Span frame);\n> > -\tvoid compressNV(libcamera::Span frame);\n> > +\tvoid compressRGB(const std::vector> &planes);\n> > +\tvoid compressNV(const std::vector> &planes);\n> > \n> > \tstruct jpeg_compress_struct compress_;\n> > \tstruct jpeg_error_mgr jerr_;\n> > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> > index 3160a784419c..68d74842925d 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -72,7 +72,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> > \t\t */\n> > \t\tthumbnail->resize(rawThumbnail.size());\n> > \n> > -\t\tint jpeg_size = thumbnailEncoder_.encode(rawThumbnail,\n> > +\t\tint jpeg_size = thumbnailEncoder_.encode({ rawThumbnail },\n> > \t\t\t\t\t\t\t *thumbnail, {}, quality);\n> > \t\tthumbnail->resize(jpeg_size);\n> > \n> > diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp\n> > index 043c7b33f06a..2c341535de9e 100644\n> > --- a/src/android/jpeg/thumbnailer.cpp\n> > +++ b/src/android/jpeg/thumbnailer.cpp\n> > @@ -62,8 +62,8 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,\n> > \tASSERT(tw % 2 == 0 && th % 2 == 0);\n> > \n> \n> Possibly ASSERT(frame.planes().size() >= 2 here too.\n> \n> But either way,\n> \n> Reviewed-by: Kieran Bingham \n> \n> > \t/* Image scaling block implementing nearest-neighbour algorithm. */\n> > -\tunsigned char *src = static_cast(frame.planes()[0].data());\n> > -\tunsigned char *srcC = src + sh * sw;\n> > +\tunsigned char *src = frame.planes()[0].data();\n> > +\tunsigned char *srcC = frame.planes()[1].data();\n> > \tunsigned char *srcCb, *srcCr;\n> > \tunsigned char *dstY, *srcY;\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7D105BE175\n\tfor ;\n\tTue, 7 Sep 2021 14:16:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6D2560252;\n\tTue, 7 Sep 2021 16:16:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCA2660251\n\tfor ;\n\tTue, 7 Sep 2021 16:16:56 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5237224F;\n\tTue, 7 Sep 2021 16:16:56 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FNfVQNKZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631024216;\n\tbh=RHN3PUx8Lf5j2rPgCXCVhENsQVrrOeOndQF/8tMvuPA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FNfVQNKZc6Krzem8Gg6DIlMcZwNnycT9jwST1DinMtf32At/13v/FqEjWJYMnwyDw\n\tsCGrvCioQaVihafoMOcCjjHqdATicm8ddWuOEFLV/deGEEwxh+jVeaVZBb8oTda/bX\n\tMZD1dtTSZ7tHCul5ejUVWUdHGZwl7BXSUxLSaOuk=","Date":"Tue, 7 Sep 2021 17:16:37 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210906225420.13275-1-laurent.pinchart@ideasonboard.com>\n\t<20210906225636.14683-20-laurent.pinchart@ideasonboard.com>\n\t<297dc0d6-a8bd-e489-3f30-a282ae615d86@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<297dc0d6-a8bd-e489-3f30-a282ae615d86@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 20/30] android: jpeg: Support\n\tmulti-planar buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]