[{"id":11859,"web_url":"https://patchwork.libcamera.org/comment/11859/","msgid":"<20200805015935.GZ6075@pendragon.ideasonboard.com>","date":"2020-08-05T01:59:35","subject":"Re: [libcamera-devel] [PATCH v3 12/13] android: Introduce JPEG\n\tcompression","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Tue, Aug 04, 2020 at 10:47:10PM +0100, Kieran Bingham wrote:\n> Provide a compressor interface and implement a JPEG compressor using\n> libjpeg.\n> \n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> v2:\n> \n>  - Convert to use the libcamera format information rather than duplicating it ourselves.\n>    - Not easy to get the horizontal subsampling\n>    - not easy to determine if we have an nvSwap...\n> \n> v3:\n>  - Fix frame.error return value (negative inversion)\n>  - Fix comments\n>  - constify data table\n>  - set jpeg image lenght to zero after free\n>  - set override on class interface functions.\n>  - Remove ununsed fucntion prototype\n>  - Rename pixelFormatInfo to pixelFormatInfo_\n> ---\n>  src/android/jpeg/compressor.h        |  28 ++++\n>  src/android/jpeg/compressor_jpeg.cpp | 217 +++++++++++++++++++++++++++\n>  src/android/jpeg/compressor_jpeg.h   |  45 ++++++\n>  src/android/meson.build              |   1 +\n>  src/libcamera/meson.build            |   2 +\n>  5 files changed, 293 insertions(+)\n>  create mode 100644 src/android/jpeg/compressor.h\n>  create mode 100644 src/android/jpeg/compressor_jpeg.cpp\n>  create mode 100644 src/android/jpeg/compressor_jpeg.h\n> \n> diff --git a/src/android/jpeg/compressor.h b/src/android/jpeg/compressor.h\n> new file mode 100644\n> index 000000000000..18d8f65eba02\n> --- /dev/null\n> +++ b/src/android/jpeg/compressor.h\n> @@ -0,0 +1,28 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * compressor.h - Image compression interface\n\ns/Image/JPEG/ ?\n\n> + */\n> +#ifndef __ANDROID_JPEG_COMPRESSOR_H__\n> +#define __ANDROID_JPEG_COMPRESSOR_H__\n> +\n> +#include <libcamera/buffer.h>\n> +#include <libcamera/stream.h>\n> +\n> +struct CompressedImage {\n> +\tunsigned char *data;\n> +\tunsigned long length;\n> +};\n\nThis can be replaced by a Span<uint8_t> (which MappedBuffer provides you\n:-)).\n\n> +\n> +class Compressor\n> +{\n> +public:\n> +\tvirtual ~Compressor(){};\n\ns/{}/ {}/\n\n> +\n> +\tvirtual int configure(const libcamera::StreamConfiguration &cfg) = 0;\n> +\tvirtual int compress(const libcamera::FrameBuffer *source, CompressedImage *image) = 0;\n\ns/image/dest/\n\n(maybe src and dst ?)\n\nIf we pass a span for the destination, the function should return the\nused size on success.\n\n> +\tvirtual void free(CompressedImage *image) = 0;\n\nThis is never used, I think you can drop it.\n\n> +};\n> +\n> +#endif /* __ANDROID_JPEG_COMPRESSOR_H__ */\n> diff --git a/src/android/jpeg/compressor_jpeg.cpp b/src/android/jpeg/compressor_jpeg.cpp\n> new file mode 100644\n> index 000000000000..9921a294128c\n> --- /dev/null\n> +++ b/src/android/jpeg/compressor_jpeg.cpp\n> @@ -0,0 +1,217 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * compressor_jpeg.cpp - JPEG compression using libjpeg native API\n> + */\n> +\n> +#include \"compressor_jpeg.h\"\n> +\n> +#include <fcntl.h>\n> +#include <iomanip>\n> +#include <iostream>\n> +#include <sstream>\n> +#include <string.h>\n> +#include <sys/mman.h>\n> +#include <unistd.h>\n> +#include <vector>\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/formats.h>\n> +#include <libcamera/pixel_format.h>\n> +\n> +#include \"libcamera/internal/formats.h\"\n> +#include \"libcamera/internal/log.h\"\n> +\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(JPEG)\n> +\n> +namespace {\n> +\n> +struct JPEGPixelFormatInfo {\n> +\tJ_COLOR_SPACE colorSpace;\n> +\tconst PixelFormatInfo &pixelFormatInfo;\n> +\tbool nvSwap;\n> +};\n> +\n> +const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{\n> +\t{ formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> +\n> +\t{ formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> +\t{ formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> +\n> +\t{ formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> +\t{ formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> +\t{ formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> +\t{ formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> +\t{ formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> +\t{ formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> +};\n> +\n> +const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)\n> +{\n> +\tstatic const struct JPEGPixelFormatInfo invalidPixelFormat {\n> +\t\tJCS_UNKNOWN, PixelFormatInfo(), false\n> +\t};\n> +\n> +\tconst auto iter = pixelInfo.find(format);\n> +\tif (iter == pixelInfo.end()) {\n> +\t\tLOG(JPEG, Error) << \"Unsupported pixel format for JPEG compressor: \"\n> +\t\t\t\t << format.toString();\n> +\t\treturn invalidPixelFormat;\n> +\t}\n> +\n> +\treturn iter->second;\n> +}\n> +\n> +} /* namespace */\n> +\n> +CompressorJPEG::CompressorJPEG()\n> +\t: quality_(95)\n> +{\n> +\t/* \\todo: Expand error handling coverage with a custom handler. */\n\ns/todo:/todo/\n\n> +\tcompress_.err = jpeg_std_error(&jerr_);\n> +\n> +\tjpeg_create_compress(&compress_);\n> +}\n> +\n> +CompressorJPEG::~CompressorJPEG()\n> +{\n> +\tjpeg_destroy_compress(&compress_);\n> +}\n> +\n> +int CompressorJPEG::configure(const StreamConfiguration &cfg)\n> +{\n> +\tconst struct JPEGPixelFormatInfo info = findPixelInfo(cfg.pixelFormat);\n> +\tif (info.colorSpace == JCS_UNKNOWN)\n> +\t\treturn -ENOTSUP;\n> +\n> +\tcompress_.image_width = cfg.size.width;\n> +\tcompress_.image_height = cfg.size.height;\n> +\tcompress_.in_color_space = info.colorSpace;\n> +\n> +\tcompress_.input_components = info.colorSpace == JCS_GRAYSCALE ? 1 : 3;\n> +\n> +\tjpeg_set_defaults(&compress_);\n> +\tjpeg_set_quality(&compress_, quality_, TRUE);\n> +\n> +\tpixelFormatInfo_ = &info.pixelFormatInfo;\n> +\n> +\tnv_ = pixelFormatInfo_->numPlanes() == 2;\n> +\tnvSwap_ = info.nvSwap;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void CompressorJPEG::compressRGB(const libcamera::MappedBuffer *frame)\n> +{\n> +\tunsigned char *src = static_cast<unsigned char *>(frame->maps()[0].data());\n> +\tunsigned int stride = pixelFormatInfo_->stride(compress_.image_width, 0);\n\nNote for later, we'll have to replace this with the stride coming from\nthe frame buffer when it will be available. Same for NV formats.\n\n> +\n> +\tJSAMPROW row_pointer[1];\n> +\n> +\twhile (compress_.next_scanline < compress_.image_height) {\n> +\t\trow_pointer[0] = &src[compress_.next_scanline * stride];\n> +\t\tjpeg_write_scanlines(&compress_, row_pointer, 1);\n> +\t}\n> +}\n> +\n> +/*\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> + * Utilisation of the RAW api will be implemented on top as a performance\n> + * improvement.\n\nYou can replace the second sentence with\n\n * \\todo Use the libjpeg RAW API to improve performances\n\n> + */\n> +void CompressorJPEG::compressNV(const libcamera::MappedBuffer *frame)\n> +{\n> +\tstd::vector<uint8_t> tmprowbuf(compress_.image_width * 3);\n\nHow about\n\n\tuint8_t tmprowbuf[compress_.image_width * 3];\n\nOr do you think it should be allocated on the heap ?\n\n> +\n> +\t/*\n> +\t * \\todo Use the raw api, and only unpack the cb/cr samples to new line\n> +\t * buffers. If possible, see if we can set appropriate pixel strides\n> +\t * too to save even that copy.\n\nAh it's here :-) So you can drop the second sentence above.\n\n> +\t *\n> +\t * Possible hints at:\n> +\t * https://sourceforge.net/p/libjpeg/mailman/message/30815123/\n> +\t */\n> +\tunsigned int y_stride = pixelFormatInfo_->stride(compress_.image_width, 0);\n> +\tunsigned int c_stride = pixelFormatInfo_->stride(compress_.image_width, 1);\n> +\n> +\tunsigned int horzSubSample = 2 * compress_.image_width / c_stride;\n> +\tunsigned int vertSubSample = pixelFormatInfo_->planes[1].verticalSubSampling;\n> +\n> +\tunsigned int c_inc = horzSubSample == 1 ? 2 : 0;\n> +\tunsigned int cb_pos = nvSwap_ ? 1 : 0;\n> +\tunsigned int cr_pos = nvSwap_ ? 0 : 1;\n> +\n> +\tconst unsigned char *src = static_cast<unsigned char *>(frame->maps()[0].data());\n> +\tconst unsigned char *src_c = src + y_stride * compress_.image_height;\n> +\n> +\tJSAMPROW row_pointer[1];\n> +\trow_pointer[0] = &tmprowbuf[0];\n> +\n> +\tfor (unsigned int y = 0; y < compress_.image_height; y++) {\n> +\t\tunsigned char *dst = &tmprowbuf[0];\n> +\n> +\t\tconst unsigned char *src_y = src + y * compress_.image_width;\n> +\t\tconst unsigned char *src_cb = src_c + (y / vertSubSample) * c_stride + cb_pos;\n> +\t\tconst unsigned char *src_cr = src_c + (y / vertSubSample) * c_stride + cr_pos;\n> +\n> +\t\tfor (unsigned int x = 0; x < compress_.image_width; x += 2) {\n> +\t\t\tdst[0] = *src_y;\n> +\t\t\tdst[1] = *src_cb;\n> +\t\t\tdst[2] = *src_cr;\n> +\t\t\tsrc_y++;\n> +\t\t\tsrc_cb += c_inc;\n> +\t\t\tsrc_cr += c_inc;\n> +\t\t\tdst += 3;\n> +\n> +\t\t\tdst[0] = *src_y;\n> +\t\t\tdst[1] = *src_cb;\n> +\t\t\tdst[2] = *src_cr;\n> +\t\t\tsrc_y++;\n> +\t\t\tsrc_cb += 2;\n> +\t\t\tsrc_cr += 2;\n> +\t\t\tdst += 3;\n> +\t\t}\n> +\n> +\t\tjpeg_write_scanlines(&compress_, row_pointer, 1);\n> +\t}\n> +}\n> +\n> +int CompressorJPEG::compress(const FrameBuffer *source, CompressedImage *jpeg)\n> +{\n> +\tMappedFrameBuffer frame(source, PROT_READ);\n> +\tif (!frame.isValid()) {\n> +\t\tLOG(JPEG, Error) << \"Failed to map FrameBuffer : \"\n> +\t\t\t\t << strerror(frame.error());\n> +\t\treturn frame.error();\n> +\t}\n> +\n> +\tjpeg_mem_dest(&compress_, &jpeg->data, &jpeg->length);\n\nIf your buffer isn't big enough, libjpeg will reallocate by default.\nWe'll need to implement a custom destination to avoid this, and return\nan error. Can you add a \\todo ?\n\n> +\n> +\tjpeg_start_compress(&compress_, TRUE);\n> +\n> +\tLOG(JPEG, Debug) << \"JPEG Compress Starting:\"\n> +\t\t\t << \" Width: \" << compress_.image_width\n\ns/Width/width/\n\n> +\t\t\t << \" height: \" << compress_.image_height;\n\nOr maybe\n\t\t<< \"JPEG Compress Starting: \" << compress_.image_width\n\t\t<< \"x\" << compress_.image_height;\n\n> +\n> +\tif (nv_)\n> +\t\tcompressNV(&frame);\n> +\telse\n> +\t\tcompressRGB(&frame);\n> +\n> +\tLOG(JPEG, Debug) << \"JPEG Compress Completed\";\n> +\n> +\tjpeg_finish_compress(&compress_);\n> +\n> +\treturn 0;\n> +}\n> +\n> +void CompressorJPEG::free(CompressedImage *jpeg)\n> +{\n> +\t::free(jpeg->data);\n> +\tjpeg->data = nullptr;\n> +\tjpeg->length = 0;\n> +}\n> diff --git a/src/android/jpeg/compressor_jpeg.h b/src/android/jpeg/compressor_jpeg.h\n> new file mode 100644\n> index 000000000000..2284e8a6bff1\n> --- /dev/null\n> +++ b/src/android/jpeg/compressor_jpeg.h\n\nMaybe compressor_libjpeg.h ?\n\n> @@ -0,0 +1,45 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * compressor_jpeg.h - JPEG compression using libjpeg\n> + */\n> +#ifndef __ANDROID_JPEG_COMPRESSOR_JPEG_H__\n\nand COMPRESSOR_LIBJPEG here too.\n\n> +#define __ANDROID_JPEG_COMPRESSOR_JPEG_H__\n> +\n> +#include \"compressor.h\"\n> +\n> +#include <libcamera/buffer.h>\n> +#include <libcamera/stream.h>\n\nThose two headers are already included by compressor.h. As you only need\nthem to declare the overridden functions, you can drop them here.\n\n> +\n> +#include \"libcamera/internal/buffer.h\"\n> +#include \"libcamera/internal/formats.h\"\n> +\n> +#include <jpeglib.h>\n> +\n> +class CompressorJPEG : public Compressor\n> +{\n> +public:\n> +\tCompressorJPEG();\n> +\t~CompressorJPEG();\n> +\n> +\tint configure(const libcamera::StreamConfiguration &cfg) override;\n> +\tint compress(const libcamera::FrameBuffer *source, CompressedImage *jpeg) override;\n> +\tvoid free(CompressedImage *jpeg) override;\n> +\n> +private:\n> +\tvoid compressRGB(const libcamera::MappedBuffer *frame);\n> +\tvoid compressNV(const libcamera::MappedBuffer *frame);\n> +\n> +\tstruct jpeg_compress_struct compress_;\n> +\tstruct jpeg_error_mgr jerr_;\n> +\n> +\tunsigned int quality_;\n> +\n> +\tconst libcamera::PixelFormatInfo *pixelFormatInfo_;\n> +\n> +\tbool nv_;\n> +\tbool nvSwap_;\n> +};\n> +\n> +#endif /* __ANDROID_JPEG_COMPRESSOR_JPEG_H__ */\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 822cad621f01..51dcd99ee62f 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -6,6 +6,7 @@ android_hal_sources = files([\n>      'camera_device.cpp',\n>      'camera_metadata.cpp',\n>      'camera_ops.cpp',\n> +    'jpeg/compressor_jpeg.cpp',\n>  ])\n>  \n>  android_camera_metadata_sources = files([\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 3aad4386ffc2..d78e2c1f6eb8 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -124,6 +124,8 @@ if get_option('android')\n>      libcamera_sources += android_hal_sources\n>      includes += android_includes\n>      libcamera_link_with += android_camera_metadata\n> +\n\nNo need for a blank line ?\n\n> +    libcamera_deps += dependency('libjpeg')\n\nShould this be provided via an android_deps set in\nsrc/android/meson.build ?\n\n>  endif\n>  \n>  # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 69345BD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 Aug 2020 01:59:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7E7A60547;\n\tWed,  5 Aug 2020 03:59:49 +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 1FCF460536\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 Aug 2020 03:59:48 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DD9D2C0;\n\tWed,  5 Aug 2020 03:59:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bQkV8NVu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596592787;\n\tbh=krsxt2c2PeaC4alnyv1iFZ41C8Rfs7ju7Qy4pSbBPjU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bQkV8NVuYKLFsdRnYuAVsWFphpMpZk3rHTUg+YQvfsADLqcO/SBlZhait0ZgXe2X+\n\te01bERz6FnqJsZfP5KpUJHFea81FMYr1Q7bG9qV3ia0Vl6bDzuMgFbEAWwb9qauxdh\n\tZnL1jjJdNZqpsA73sv+5pGorzJ+VchjTls6ceKDY=","Date":"Wed, 5 Aug 2020 04:59:35 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200805015935.GZ6075@pendragon.ideasonboard.com>","References":"<20200804214711.177645-1-kieran.bingham@ideasonboard.com>\n\t<20200804214711.177645-13-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200804214711.177645-13-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 12/13] android: Introduce JPEG\n\tcompression","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11881,"web_url":"https://patchwork.libcamera.org/comment/11881/","msgid":"<ea1aa9c0-0632-fe7f-1270-04433e83e1fb@ideasonboard.com>","date":"2020-08-05T13:32:29","subject":"Re: [libcamera-devel] [PATCH v3 12/13] android: Introduce JPEG\n\tcompression","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 05/08/2020 02:59, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Tue, Aug 04, 2020 at 10:47:10PM +0100, Kieran Bingham wrote:\n>> Provide a compressor interface and implement a JPEG compressor using\n>> libjpeg.\n>>\n>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>> v2:\n>>\n>>  - Convert to use the libcamera format information rather than duplicating it ourselves.\n>>    - Not easy to get the horizontal subsampling\n>>    - not easy to determine if we have an nvSwap...\n>>\n>> v3:\n>>  - Fix frame.error return value (negative inversion)\n>>  - Fix comments\n>>  - constify data table\n>>  - set jpeg image lenght to zero after free\n>>  - set override on class interface functions.\n>>  - Remove ununsed fucntion prototype\n>>  - Rename pixelFormatInfo to pixelFormatInfo_\n>> ---\n>>  src/android/jpeg/compressor.h        |  28 ++++\n>>  src/android/jpeg/compressor_jpeg.cpp | 217 +++++++++++++++++++++++++++\n>>  src/android/jpeg/compressor_jpeg.h   |  45 ++++++\n>>  src/android/meson.build              |   1 +\n>>  src/libcamera/meson.build            |   2 +\n>>  5 files changed, 293 insertions(+)\n>>  create mode 100644 src/android/jpeg/compressor.h\n>>  create mode 100644 src/android/jpeg/compressor_jpeg.cpp\n>>  create mode 100644 src/android/jpeg/compressor_jpeg.h\n>>\n>> diff --git a/src/android/jpeg/compressor.h b/src/android/jpeg/compressor.h\n>> new file mode 100644\n>> index 000000000000..18d8f65eba02\n>> --- /dev/null\n>> +++ b/src/android/jpeg/compressor.h\n>> @@ -0,0 +1,28 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * compressor.h - Image compression interface\n> \n> s/Image/JPEG/ ?\n\n\nThe actual compressor interface is not specific to JPEG. It is only used\nby JPEG for us, but it could also be used to compress other formats.\n\nSo I chose the generic word Image intentionally.\n\n\n>> + */\n>> +#ifndef __ANDROID_JPEG_COMPRESSOR_H__\n>> +#define __ANDROID_JPEG_COMPRESSOR_H__\n>> +\n>> +#include <libcamera/buffer.h>\n>> +#include <libcamera/stream.h>\n>> +\n>> +struct CompressedImage {\n>> +\tunsigned char *data;\n>> +\tunsigned long length;\n>> +};\n> \n> This can be replaced by a Span<uint8_t> (which MappedBuffer provides you\n> :-)).\n\nYes, I had been working towards that too. But I was hit by a design\nflaw/issue.\n\nCompressedImage allows you to both give a region of memory and return\nit, with the size set. I think I see later you already suggest changing\nthe return type to return the number of bytes used instead so that\nsolves that though.\n\nThe other use case was to pass in a null pointer, to let the compressor\nallocate the memory, but that's mostly been a usecase for me to use the\ncompressor with Cam to test locally (as is the free() method) - so it\ncould be removed from here.\n\n>> +\n>> +class Compressor\n>> +{\n>> +public:\n>> +\tvirtual ~Compressor(){};\n> \n> s/{}/ {}/\n\nHrm, that's odd, I thought I recalled reformatting that line already due\nto checkstyle.py.\n\nIndeed:\n\n--- src/android/jpeg/compressor.h\n+++ src/android/jpeg/compressor.h\n@@ -18,8 +18,7 @@\n class Compressor\n {\n public:\n-\tvirtual ~Compressor()\n-\t{};\n+\tvirtual ~Compressor(){};\n\nAnd:\n--- src/android/jpeg/compressor.h\n+++ src/android/jpeg/compressor.h\n@@ -18,7 +18,7 @@\n class Compressor\n {\n public:\n-\tvirtual ~Compressor() {};\n+\tvirtual ~Compressor(){};\n\n\nSo checkstyle really wants to hug those braces, yet the precedence\nreally is set:\n\n\ngit grep \"virtual.*{}\"\ninclude/libcamera/bound_method.h:       virtual ~BoundMethodPackBase() {}\ninclude/libcamera/bound_method.h:       virtual ~BoundMethodBase() {}\ninclude/libcamera/internal/control_validator.h: virtual\n~ControlValidator() {}\ninclude/libcamera/internal/ipa_proxy.h: virtual ~IPAProxyFactory() {}\ninclude/libcamera/internal/media_object.h:      virtual ~MediaObject() {}\ninclude/libcamera/internal/pipeline_handler.h:  virtual ~CameraData() {}\ninclude/libcamera/internal/pipeline_handler.h:  virtual\n~PipelineHandlerFactory() {}\ninclude/libcamera/ipa/ipa_interface.h:  virtual ~IPAInterface() {}\nsrc/android/jpeg/compressor.h:  virtual ~Compressor() {};\nsrc/cam/options.h:      virtual ~KeyValueParser() {}\nsrc/ipa/raspberrypi/controller/algorithm.hpp:   virtual ~Algorithm() {}\nsrc/ipa/raspberrypi/md_parser.hpp:      virtual ~MdParser() {}\nsrc/libcamera/pipeline/rkisp1/timeline.h:       virtual ~FrameAction() {}\nsrc/libcamera/pipeline/rkisp1/timeline.h:       virtual ~Timeline() {}\ntest/libtest/test.h:    virtual void cleanup() {}\n\n\n\n>> +\n>> +\tvirtual int configure(const libcamera::StreamConfiguration &cfg) = 0;\n>> +\tvirtual int compress(const libcamera::FrameBuffer *source, CompressedImage *image) = 0;\n> \n> s/image/dest/\n> \n> (maybe src and dst ?)\n> \n> If we pass a span for the destination, the function should return the\n> used size on success.\n> \n>> +\tvirtual void free(CompressedImage *image) = 0;\n> \n> This is never used, I think you can drop it.\n\nlibjpeg allows you to provide a nullptr and it would do the allocation,\nwhich you must then free.\n\nWhich is why this 'free' interface is here.\n\nI use that in my test code to compress a FrameBuffer and have libjpeg\ndeal with the allocation ... but I guess it can be cut out for now.\n\nAlso I see further suggestion to use a custom memory destination to\nprevent reallocation anyway, which makes sense, and also prevents this\nuse case - so I think it's clear it will be better to remove this.\n\n\n>> +};\n>> +\n>> +#endif /* __ANDROID_JPEG_COMPRESSOR_H__ */\n>> diff --git a/src/android/jpeg/compressor_jpeg.cpp b/src/android/jpeg/compressor_jpeg.cpp\n>> new file mode 100644\n>> index 000000000000..9921a294128c\n>> --- /dev/null\n>> +++ b/src/android/jpeg/compressor_jpeg.cpp\n>> @@ -0,0 +1,217 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * compressor_jpeg.cpp - JPEG compression using libjpeg native API\n>> + */\n>> +\n>> +#include \"compressor_jpeg.h\"\n>> +\n>> +#include <fcntl.h>\n>> +#include <iomanip>\n>> +#include <iostream>\n>> +#include <sstream>\n>> +#include <string.h>\n>> +#include <sys/mman.h>\n>> +#include <unistd.h>\n>> +#include <vector>\n>> +\n>> +#include <libcamera/camera.h>\n>> +#include <libcamera/formats.h>\n>> +#include <libcamera/pixel_format.h>\n>> +\n>> +#include \"libcamera/internal/formats.h\"\n>> +#include \"libcamera/internal/log.h\"\n>> +\n>> +using namespace libcamera;\n>> +\n>> +LOG_DEFINE_CATEGORY(JPEG)\n>> +\n>> +namespace {\n>> +\n>> +struct JPEGPixelFormatInfo {\n>> +\tJ_COLOR_SPACE colorSpace;\n>> +\tconst PixelFormatInfo &pixelFormatInfo;\n>> +\tbool nvSwap;\n>> +};\n>> +\n>> +const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{\n>> +\t{ formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n>> +\n>> +\t{ formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n>> +\t{ formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n>> +\n>> +\t{ formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n>> +\t{ formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n>> +\t{ formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n>> +\t{ formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n>> +\t{ formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n>> +\t{ formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n>> +};\n>> +\n>> +const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)\n>> +{\n>> +\tstatic const struct JPEGPixelFormatInfo invalidPixelFormat {\n>> +\t\tJCS_UNKNOWN, PixelFormatInfo(), false\n>> +\t};\n>> +\n>> +\tconst auto iter = pixelInfo.find(format);\n>> +\tif (iter == pixelInfo.end()) {\n>> +\t\tLOG(JPEG, Error) << \"Unsupported pixel format for JPEG compressor: \"\n>> +\t\t\t\t << format.toString();\n>> +\t\treturn invalidPixelFormat;\n>> +\t}\n>> +\n>> +\treturn iter->second;\n>> +}\n>> +\n>> +} /* namespace */\n>> +\n>> +CompressorJPEG::CompressorJPEG()\n>> +\t: quality_(95)\n>> +{\n>> +\t/* \\todo: Expand error handling coverage with a custom handler. */\n> \n> s/todo:/todo/\n\n\\me guess I just dislike the doxygen syntax.\n\\me: Guess I just dislike the doxygen syntax.\n\n\n\n>> +\tcompress_.err = jpeg_std_error(&jerr_);\n>> +\n>> +\tjpeg_create_compress(&compress_);\n>> +}\n>> +\n>> +CompressorJPEG::~CompressorJPEG()\n>> +{\n>> +\tjpeg_destroy_compress(&compress_);\n>> +}\n>> +\n>> +int CompressorJPEG::configure(const StreamConfiguration &cfg)\n>> +{\n>> +\tconst struct JPEGPixelFormatInfo info = findPixelInfo(cfg.pixelFormat);\n>> +\tif (info.colorSpace == JCS_UNKNOWN)\n>> +\t\treturn -ENOTSUP;\n>> +\n>> +\tcompress_.image_width = cfg.size.width;\n>> +\tcompress_.image_height = cfg.size.height;\n>> +\tcompress_.in_color_space = info.colorSpace;\n>> +\n>> +\tcompress_.input_components = info.colorSpace == JCS_GRAYSCALE ? 1 : 3;\n>> +\n>> +\tjpeg_set_defaults(&compress_);\n>> +\tjpeg_set_quality(&compress_, quality_, TRUE);\n>> +\n>> +\tpixelFormatInfo_ = &info.pixelFormatInfo;\n>> +\n>> +\tnv_ = pixelFormatInfo_->numPlanes() == 2;\n>> +\tnvSwap_ = info.nvSwap;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +void CompressorJPEG::compressRGB(const libcamera::MappedBuffer *frame)\n>> +{\n>> +\tunsigned char *src = static_cast<unsigned char *>(frame->maps()[0].data());\n>> +\tunsigned int stride = pixelFormatInfo_->stride(compress_.image_width, 0);\n> \n> Note for later, we'll have to replace this with the stride coming from\n> the frame buffer when it will be available. Same for NV formats.\n\nI wonder if this is related to the issue Niklas' is seeing from\nOpenCamera ...(though not 'here', elsewhere).\n\nI think it looked like either a stride issue or pixelformat issue.\n\nAnyway, that deserves a\n\n \\todo Stride information should come from buffer configuration.\n\nCould we expect stride to be different for each buffer? or would the\nvalues be correct for the stream. Otherwise we're going to have to pass\nmore information about each buffer around.\n\n\n>> +\n>> +\tJSAMPROW row_pointer[1];\n>> +\n>> +\twhile (compress_.next_scanline < compress_.image_height) {\n>> +\t\trow_pointer[0] = &src[compress_.next_scanline * stride];\n>> +\t\tjpeg_write_scanlines(&compress_, row_pointer, 1);\n>> +\t}\n>> +}\n>> +\n>> +/*\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>> + * Utilisation of the RAW api will be implemented on top as a performance\n>> + * improvement.\n> \n> You can replace the second sentence with\n> \n>  * \\todo Use the libjpeg RAW API to improve performances\n> \n>> + */\n>> +void CompressorJPEG::compressNV(const libcamera::MappedBuffer *frame)\n>> +{\n>> +\tstd::vector<uint8_t> tmprowbuf(compress_.image_width * 3);\n> \n> How about\n> \n> \tuint8_t tmprowbuf[compress_.image_width * 3];\n> \n> Or do you think it should be allocated on the heap ?\nBecause Variable Length Arrays are not part of C++?\n\n(But yes, they are an extension provided by both GCC and clang)\n\nHrm, I've just added -Werror=vla, and I see we're already using them ...\nso I guess in this instance we can again, and revisit later if we change\nour minds on VLA usage.\n\n\n>> +\n>> +\t/*\n>> +\t * \\todo Use the raw api, and only unpack the cb/cr samples to new line\n>> +\t * buffers. If possible, see if we can set appropriate pixel strides\n>> +\t * too to save even that copy.\n> \n> Ah it's here :-) So you can drop the second sentence above.\n> \n>> +\t *\n>> +\t * Possible hints at:\n>> +\t * https://sourceforge.net/p/libjpeg/mailman/message/30815123/\n>> +\t */\n>> +\tunsigned int y_stride = pixelFormatInfo_->stride(compress_.image_width, 0);\n>> +\tunsigned int c_stride = pixelFormatInfo_->stride(compress_.image_width, 1);\n>> +\n>> +\tunsigned int horzSubSample = 2 * compress_.image_width / c_stride;\n>> +\tunsigned int vertSubSample = pixelFormatInfo_->planes[1].verticalSubSampling;\n>> +\n>> +\tunsigned int c_inc = horzSubSample == 1 ? 2 : 0;\n>> +\tunsigned int cb_pos = nvSwap_ ? 1 : 0;\n>> +\tunsigned int cr_pos = nvSwap_ ? 0 : 1;\n>> +\n>> +\tconst unsigned char *src = static_cast<unsigned char *>(frame->maps()[0].data());\n>> +\tconst unsigned char *src_c = src + y_stride * compress_.image_height;\n>> +\n>> +\tJSAMPROW row_pointer[1];\n>> +\trow_pointer[0] = &tmprowbuf[0];\n>> +\n>> +\tfor (unsigned int y = 0; y < compress_.image_height; y++) {\n>> +\t\tunsigned char *dst = &tmprowbuf[0];\n>> +\n>> +\t\tconst unsigned char *src_y = src + y * compress_.image_width;\n>> +\t\tconst unsigned char *src_cb = src_c + (y / vertSubSample) * c_stride + cb_pos;\n>> +\t\tconst unsigned char *src_cr = src_c + (y / vertSubSample) * c_stride + cr_pos;\n>> +\n>> +\t\tfor (unsigned int x = 0; x < compress_.image_width; x += 2) {\n>> +\t\t\tdst[0] = *src_y;\n>> +\t\t\tdst[1] = *src_cb;\n>> +\t\t\tdst[2] = *src_cr;\n>> +\t\t\tsrc_y++;\n>> +\t\t\tsrc_cb += c_inc;\n>> +\t\t\tsrc_cr += c_inc;\n>> +\t\t\tdst += 3;\n>> +\n>> +\t\t\tdst[0] = *src_y;\n>> +\t\t\tdst[1] = *src_cb;\n>> +\t\t\tdst[2] = *src_cr;\n>> +\t\t\tsrc_y++;\n>> +\t\t\tsrc_cb += 2;\n>> +\t\t\tsrc_cr += 2;\n>> +\t\t\tdst += 3;\n>> +\t\t}\n>> +\n>> +\t\tjpeg_write_scanlines(&compress_, row_pointer, 1);\n>> +\t}\n>> +}\n>> +\n>> +int CompressorJPEG::compress(const FrameBuffer *source, CompressedImage *jpeg)\n>> +{\n>> +\tMappedFrameBuffer frame(source, PROT_READ);\n>> +\tif (!frame.isValid()) {\n>> +\t\tLOG(JPEG, Error) << \"Failed to map FrameBuffer : \"\n>> +\t\t\t\t << strerror(frame.error());\n>> +\t\treturn frame.error();\n>> +\t}\n>> +\n>> +\tjpeg_mem_dest(&compress_, &jpeg->data, &jpeg->length);\n> \n> If your buffer isn't big enough, libjpeg will reallocate by default.\n> We'll need to implement a custom destination to avoid this, and return\n> an error. Can you add a \\todo ?\n\nOh, I thought it would fail in that case.\n\n/*\n * Prepare for output to a memory buffer.\n * The caller may supply an own initial buffer with appropriate size.\n * Otherwise, or when the actual data output exceeds the given size,\n * the library adapts the buffer size as necessary.\n * The standard library functions malloc/free are used for allocating\n * larger memory, so the buffer is available to the application after\n * finishing compression, and then the application is responsible for\n * freeing the requested memory.\n * Note:  An initial buffer supplied by the caller is expected to be\n * managed by the application.  The library does not free such buffer\n * when allocating a larger buffer.\n */\n\n\nUrgh, that's a bit horrible. So not only does the caller have to track\nwhich buffer actually came back, they have to free the original if it\nchanged too.\n\n\nIndeed, lets have our own custom destination manager to remove all of that.\n\n\\todo Implement our own custom memory destination to prevent\nreallocation and prefer failure with correct reporting.\n\n\n>> +\n>> +\tjpeg_start_compress(&compress_, TRUE);\n>> +\n>> +\tLOG(JPEG, Debug) << \"JPEG Compress Starting:\"\n>> +\t\t\t << \" Width: \" << compress_.image_width\n> \n> s/Width/width/\n> \n>> +\t\t\t << \" height: \" << compress_.image_height;\n> \n> Or maybe\n> \t\t<< \"JPEG Compress Starting: \" << compress_.image_width\n> \t\t<< \"x\" << compress_.image_height;\n\nI like the shorter 640x480 ;-)\n\n\n>> +\n>> +\tif (nv_)\n>> +\t\tcompressNV(&frame);\n>> +\telse\n>> +\t\tcompressRGB(&frame);\n>> +\n>> +\tLOG(JPEG, Debug) << \"JPEG Compress Completed\";\n\nI think I can drop this one now too ;-)\n\n\n>> +\n>> +\tjpeg_finish_compress(&compress_);\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +void CompressorJPEG::free(CompressedImage *jpeg)\n>> +{\n>> +\t::free(jpeg->data);\n>> +\tjpeg->data = nullptr;\n>> +\tjpeg->length = 0;\n>> +}\n>> diff --git a/src/android/jpeg/compressor_jpeg.h b/src/android/jpeg/compressor_jpeg.h\n>> new file mode 100644\n>> index 000000000000..2284e8a6bff1\n>> --- /dev/null\n>> +++ b/src/android/jpeg/compressor_jpeg.h\n> \n> Maybe compressor_libjpeg.h ?\n\nYes, that is probably more accurate indeed.\n\nEqually the compressor_jpeg.cpp of course.\n\n\n\n>> @@ -0,0 +1,45 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * compressor_jpeg.h - JPEG compression using libjpeg\n>> + */\n>> +#ifndef __ANDROID_JPEG_COMPRESSOR_JPEG_H__\n> \n> and COMPRESSOR_LIBJPEG here too.\n> \n>> +#define __ANDROID_JPEG_COMPRESSOR_JPEG_H__\n>> +\n>> +#include \"compressor.h\"\n>> +\n>> +#include <libcamera/buffer.h>\n>> +#include <libcamera/stream.h>\n> \n> Those two headers are already included by compressor.h. As you only need\n> them to declare the overridden functions, you can drop them here.\n\nDropped.\n\n\n>> +\n>> +#include \"libcamera/internal/buffer.h\"\n>> +#include \"libcamera/internal/formats.h\"\n>> +\n>> +#include <jpeglib.h>\n>> +\n>> +class CompressorJPEG : public Compressor\n\nWell, I guess this needs to be class CompressorLibJPEG now too ?\n\n\n>> +{\n>> +public:\n>> +\tCompressorJPEG();\n>> +\t~CompressorJPEG();\n>> +\n>> +\tint configure(const libcamera::StreamConfiguration &cfg) override;\n>> +\tint compress(const libcamera::FrameBuffer *source, CompressedImage *jpeg) override;\n>> +\tvoid free(CompressedImage *jpeg) override;\n>> +\n>> +private:\n>> +\tvoid compressRGB(const libcamera::MappedBuffer *frame);\n>> +\tvoid compressNV(const libcamera::MappedBuffer *frame);\n>> +\n>> +\tstruct jpeg_compress_struct compress_;\n>> +\tstruct jpeg_error_mgr jerr_;\n>> +\n>> +\tunsigned int quality_;\n>> +\n>> +\tconst libcamera::PixelFormatInfo *pixelFormatInfo_;\n>> +\n>> +\tbool nv_;\n>> +\tbool nvSwap_;\n>> +};\n>> +\n>> +#endif /* __ANDROID_JPEG_COMPRESSOR_JPEG_H__ */\n>> diff --git a/src/android/meson.build b/src/android/meson.build\n>> index 822cad621f01..51dcd99ee62f 100644\n>> --- a/src/android/meson.build\n>> +++ b/src/android/meson.build\n>> @@ -6,6 +6,7 @@ android_hal_sources = files([\n>>      'camera_device.cpp',\n>>      'camera_metadata.cpp',\n>>      'camera_ops.cpp',\n>> +    'jpeg/compressor_jpeg.cpp',\n>>  ])\n>>  \n>>  android_camera_metadata_sources = files([\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index 3aad4386ffc2..d78e2c1f6eb8 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -124,6 +124,8 @@ if get_option('android')\n>>      libcamera_sources += android_hal_sources\n>>      includes += android_includes\n>>      libcamera_link_with += android_camera_metadata\n>> +\n> \n> No need for a blank line ?\n> \n>> +    libcamera_deps += dependency('libjpeg')\n> \n> Should this be provided via an android_deps set in\n> src/android/meson.build ?\n\nI think I prefer that indeed.\nEspecially as other dependencies are coming in too.\n\nThis is already masked by the if get_option('android') of course, but it\nwould be better to describe the dependencies in that subdir.\n\n\n\n> \n>>  endif\n>>  \n>>  # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D15F5BD87A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 Aug 2020 13:32:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6529A605AC;\n\tWed,  5 Aug 2020 15:32:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BF3B26039D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 Aug 2020 15:32:34 +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 114D02C0;\n\tWed,  5 Aug 2020 15:32:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FQEJdaSM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596634354;\n\tbh=LXE5Gr0752hpgoGB6ZktxDJp8Ekb7v1lvNH4VOSvaks=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=FQEJdaSMiEBvBkxbGdSiX/J7lO9wZAkIAFAT/u/qk0Yv6NpMFiqIApDj6gJWllLgq\n\tSBAIOnp2MPaWFDP4N27li1QmuMrSA0Vi9YssTU1F2Yp03t4V52hOnhtIayCFHlFzRp\n\tn9g1mH3AYJwKs+JKN4gkiMyTMEVc6yuCerRUsXAs=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20200804214711.177645-1-kieran.bingham@ideasonboard.com>\n\t<20200804214711.177645-13-kieran.bingham@ideasonboard.com>\n\t<20200805015935.GZ6075@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<ea1aa9c0-0632-fe7f-1270-04433e83e1fb@ideasonboard.com>","Date":"Wed, 5 Aug 2020 14:32:29 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200805015935.GZ6075@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 12/13] android: Introduce JPEG\n\tcompression","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]