[{"id":14708,"web_url":"https://patchwork.libcamera.org/comment/14708/","msgid":"<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>","date":"2021-01-22T13:05:32","subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Hiro,\n\nWhen there's more than one patch, could you also add a cover letter\nplease? it helps grouping them, and provides a way to comment on the\nwhole series as well as individual patches.\n\nIt doesn't have to be extensive, just a title and a brief.\n\n\nI'm happy to see this work being done, but there's no usage of this post\nprocessor.\n\nDo you have another patch in progress to do that which would become a\npart of this series?\n\n\n\nOn 22/01/2021 09:23, Hirokazu Honda wrote:\n> This adds PostProcessorLibyuv. It supports NV12 buffer scaling.\n> \n> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> ---\n>  src/android/libyuv/post_processor_libyuv.cpp | 120 +++++++++++++++++++\n>  src/android/libyuv/post_processor_libyuv.h   |  33 +++++\n>  src/android/meson.build                      |   1 +\n>  3 files changed, 154 insertions(+)\n>  create mode 100644 src/android/libyuv/post_processor_libyuv.cpp\n>  create mode 100644 src/android/libyuv/post_processor_libyuv.h\n> \n> diff --git a/src/android/libyuv/post_processor_libyuv.cpp b/src/android/libyuv/post_processor_libyuv.cpp\n> new file mode 100644\n> index 00000000..7cfa0539\n> --- /dev/null\n> +++ b/src/android/libyuv/post_processor_libyuv.cpp\n> @@ -0,0 +1,120 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * post_processor_libyuv.cpp - Post Processor using libyuv\n> + */\n> +\n> +#include \"post_processor_libyuv.h\"\n> +\n> +#include <libyuv/scale.h>\n> +\n> +#include <libcamera/formats.h>\n> +#include <libcamera/geometry.h>\n> +#include <libcamera/internal/formats.h>\n> +#include <libcamera/internal/log.h>\n> +#include <libcamera/pixel_format.h>\n> +\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(LIBYUV)\n> +\n> +namespace {\n> +void getNV12Length(Size size, unsigned int length[2])\n> +{\n> +\tauto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n> +\tfor (unsigned int i = 0; i < 2; i++) {\n> +\t\tconst unsigned int vertSubSample =\n> +\t\t\tnv12Info.planes[i].verticalSubSampling;\n> +\t\tlength[i] = nv12Info.stride(size.width, i, /*align=*/1) *\n> +\t\t\t    ((size.height + vertSubSample - 1) / vertSubSample);\n> +\t}\n> +}\n> +\n> +unsigned int getNV12Stride(Size size, unsigned int i)\n> +{\n> +\tauto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n> +\treturn nv12Info.stride(size.width, i, 1);\n> +}\n> +} // namespace\n> +\n> +PostProcessorLibyuv::PostProcessorLibyuv() = default;\n> +\n> +int PostProcessorLibyuv::configure(const StreamConfiguration &inCfg,\n> +\t\t\t\t   const StreamConfiguration &outCfg)\n> +{\n> +\tif (inCfg.pixelFormat != outCfg.pixelFormat ||\n> +\t    inCfg.size < outCfg.size) {\n> +\t\tLOG(LIBYUV, Error) << \"Only down-scaling is supported\";\n\nThis will report \"Only down-scaling is supported\" if someone tries to\nuse it to do pixel format conversion.\n\n\n> +\t\treturn -EINVAL;\n> +\t}\n> +\tif (inCfg.pixelFormat == formats::NV12) {\n> +\t\tLOG(LIBYUV, Error) << \"Only NV12 is supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\tinCfg_ = inCfg;\n> +\toutCfg_ = outCfg;\n> +\treturn 0;\n> +}\n> +\n> +int PostProcessorLibyuv::process(const FrameBuffer &source,\n> +\t\t\t\t libcamera::MappedBuffer *destination,\n> +\t\t\t\t CameraMetadata * /*metadata*/)\n> +{\n> +\tif (!IsValidNV12Buffers(source, *destination)) {\n> +\t\tLOG(LIBYUV, Error) << \"Invalid source and destination\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tconst MappedFrameBuffer sourceMapped(&source, PROT_READ);\n> +\tif (!sourceMapped.isValid()) {\n> +\t\tLOG(LIBYUV, Error) << \"Failed to mmap camera frame buffer\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn 0 == libyuv::NV12Scale(sourceMapped.maps()[0].data(),\n> +\t\t\t\t      getNV12Stride(inCfg_.size, 0),\n> +\t\t\t\t      sourceMapped.maps()[1].data(),\n> +\t\t\t\t      getNV12Stride(inCfg_.size, 1),\n> +\t\t\t\t      inCfg_.size.height, inCfg_.size.width,\n\n> +\t\t\t\t      destination->maps()[0].data(),\n> +\t\t\t\t      getNV12Stride(outCfg_.size, 0),\n> +\t\t\t\t      destination->maps()[1].data(),\n> +\t\t\t\t      getNV12Stride(outCfg_.size, 1),\n> +\t\t\t\t      outCfg_.size.width, outCfg_.size.height,\n\n> +\t\t\t\t      libyuv::FilterMode::kFilterBilinear);\n\nEeek, that's quite terse to read, but that's more of a restriction on\nthe libyuv() I expect.\n\n\n> +}\n> +\n> +bool PostProcessorLibyuv::IsValidNV12Buffers(\n> +\tconst FrameBuffer &source,\n> +\tconst libcamera::MappedBuffer &destination) const\n> +{\n> +\tif (source.planes().size() != 2u) {\n> +\t\tLOG(LIBYUV, Error) << \"The number of source planes is not 2\";\n> +\t\treturn false;\n> +\t}\n> +\tif (destination.maps().size() != 2u) {\n> +\t\tLOG(LIBYUV, Error)\n> +\t\t\t<< \"The number of destination planes is not 2\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tunsigned int sourceLength[2] = {};\n> +\tunsigned int destinationLength[2] = {};\n> +\tgetNV12Length(inCfg_.size, sourceLength);\n> +\tgetNV12Length(outCfg_.size, destinationLength);\n\nI think these can be calculated once at configure() time rather than in\nevery frame.\n\n> +\n> +\tif (source.planes()[0].length < sourceLength[0] ||\n> +\t    source.planes()[1].length < sourceLength[1]) {\n> +\t\tLOG(LIBYUV, Error)\n> +\t\t\t<< \"The source planes lengths are too small\";\n> +\t\treturn false;\n> +\t}\n\nI would expect that we have an element of 'we can trust the sizes of a\nFrameBuffer()' ?\n\n\n> +\tif (destination.maps()[0].size() < destinationLength[0] ||\n> +\t    destination.maps()[1].size() < destinationLength[1]) {\n> +\t\tLOG(LIBYUV, Error)\n> +\t\t\t<< \"The destination planes lengths are too small\";\n> +\t\treturn false;\n> +\t}\n\nnew line here.\n\nThat's a lot of checks for each buffer. Do we need to do that on every\nbuffer? I suspect we might need to because we don't know what buffers\nwe're given before they come in ...\n\nBut I wonder if we can make assumptions that they are accurate.\n\n\n\n> +\treturn true;\n> +}\n> diff --git a/src/android/libyuv/post_processor_libyuv.h b/src/android/libyuv/post_processor_libyuv.h\n> new file mode 100644\n> index 00000000..860a664b\n> --- /dev/null\n> +++ b/src/android/libyuv/post_processor_libyuv.h\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * post_processor_libyuv.h - Post Processor using libyuv\n> + */\n> +#ifndef __ANDROID_POST_PROCESSOR_LIBYUV_H__\n> +#define __ANDROID_POST_PROCESSOR_LIBYUV_H__\n> +\n> +#include \"../post_processor.h\"\n> +\n> +class CameraDevice;\n> +\n> +class PostProcessorLibyuv : public PostProcessor\n> +{\n> +public:\n> +\tPostProcessorLibyuv();\n> +\n> +\tint configure(const libcamera::StreamConfiguration &incfg,\n> +\t\t      const libcamera::StreamConfiguration &outcfg) override;\n> +\tint process(const libcamera::FrameBuffer &source,\n> +\t\t    libcamera::MappedBuffer *destination,\n> +\t\t    CameraMetadata *metadata) override;\n> +\n> +private:\n> +\tbool IsValidNV12Buffers(const libcamera::FrameBuffer &source,\n> +\t\t\t\tconst libcamera::MappedBuffer &destination) const;\n> +\n> +\tlibcamera::StreamConfiguration inCfg_;\n> +\tlibcamera::StreamConfiguration outCfg_;\n> +};\n> +\n> +#endif /* __ANDROID_POST_PROCESSOR_LIBYUV_H__ */\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 3d4d3be4..ff067d12 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -26,6 +26,7 @@ android_hal_sources = files([\n>      'jpeg/exif.cpp',\n>      'jpeg/post_processor_jpeg.cpp',\n>      'jpeg/thumbnailer.cpp',\n> +    'libyuv/post_processor_libyuv.cpp'\n>  ])\n> \n>  android_camera_metadata_sources = files([\n> --\n> 2.30.0.280.ga3ce27912f-goog\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 9F9A0BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 22 Jan 2021 13:05:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21AF26826D;\n\tFri, 22 Jan 2021 14:05:37 +0100 (CET)","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 45E646825B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Jan 2021 14:05:36 +0100 (CET)","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 AD9A050E;\n\tFri, 22 Jan 2021 14:05:35 +0100 (CET)"],"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=\"tNA5HSp3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611320735;\n\tbh=ML47u8FXMGsiqZbPUz6zjSrvCV/mwyekPivEnGK5GjY=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=tNA5HSp32Emb597LSKU37LdKJPUj0fko32Z+P3iBxJnimYlAIcMQx8lQT3kz2ysc0\n\tx95DzTCanEuIEE6IoTjbHBmJ7EWZx5ALblNOMCAH9oJavdHrBk7IgEfYgKuNp+b3W6\n\tBgt0KPWi4qLcATB74acG9aYT/n8MINO/WUiqTnos=","To":"Hirokazu Honda <hiroh@chromium.org>, libcamera-devel@lists.libcamera.org","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<20210122092335.254626-2-hiroh@chromium.org>","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":"<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>","Date":"Fri, 22 Jan 2021 13:05:32 +0000","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":"<20210122092335.254626-2-hiroh@chromium.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","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","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":14735,"web_url":"https://patchwork.libcamera.org/comment/14735/","msgid":"<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.com>","date":"2021-01-25T05:21:40","subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Kieran,\n\nThanks for reviewing.\n\nOn Fri, Jan 22, 2021 at 10:05 PM Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Hi Hiro,\n>\n> When there's more than one patch, could you also add a cover letter\n> please? it helps grouping them, and provides a way to comment on the\n> whole series as well as individual patches.\n>\n> It doesn't have to be extensive, just a title and a brief.\n>\n\n\nI see. I will do in the next patch.\n\n>\n> I'm happy to see this work being done, but there's no usage of this post\n> processor.\n>\n> Do you have another patch in progress to do that which would become a\n> part of this series?\n>\n\nI don't yet have a patch to use this post processor.\nI think I have to complete a mapping task, which I have been working\nin parallel, to use this post processor for NV12 scaling.\nI wonder if it is allowed to check in these patches whereas it is not used now.\nIf it is not good, I will hold on this post processor patch until the\nmapping task is done.\n\n>\n>\n> On 22/01/2021 09:23, Hirokazu Honda wrote:\n> > This adds PostProcessorLibyuv. It supports NV12 buffer scaling.\n> >\n> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> > ---\n> >  src/android/libyuv/post_processor_libyuv.cpp | 120 +++++++++++++++++++\n> >  src/android/libyuv/post_processor_libyuv.h   |  33 +++++\n> >  src/android/meson.build                      |   1 +\n> >  3 files changed, 154 insertions(+)\n> >  create mode 100644 src/android/libyuv/post_processor_libyuv.cpp\n> >  create mode 100644 src/android/libyuv/post_processor_libyuv.h\n> >\n> > diff --git a/src/android/libyuv/post_processor_libyuv.cpp b/src/android/libyuv/post_processor_libyuv.cpp\n> > new file mode 100644\n> > index 00000000..7cfa0539\n> > --- /dev/null\n> > +++ b/src/android/libyuv/post_processor_libyuv.cpp\n> > @@ -0,0 +1,120 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Google Inc.\n> > + *\n> > + * post_processor_libyuv.cpp - Post Processor using libyuv\n> > + */\n> > +\n> > +#include \"post_processor_libyuv.h\"\n> > +\n> > +#include <libyuv/scale.h>\n> > +\n> > +#include <libcamera/formats.h>\n> > +#include <libcamera/geometry.h>\n> > +#include <libcamera/internal/formats.h>\n> > +#include <libcamera/internal/log.h>\n> > +#include <libcamera/pixel_format.h>\n> > +\n> > +using namespace libcamera;\n> > +\n> > +LOG_DEFINE_CATEGORY(LIBYUV)\n> > +\n> > +namespace {\n> > +void getNV12Length(Size size, unsigned int length[2])\n> > +{\n> > +     auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n> > +     for (unsigned int i = 0; i < 2; i++) {\n> > +             const unsigned int vertSubSample =\n> > +                     nv12Info.planes[i].verticalSubSampling;\n> > +             length[i] = nv12Info.stride(size.width, i, /*align=*/1) *\n> > +                         ((size.height + vertSubSample - 1) / vertSubSample);\n> > +     }\n> > +}\n> > +\n> > +unsigned int getNV12Stride(Size size, unsigned int i)\n> > +{\n> > +     auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n> > +     return nv12Info.stride(size.width, i, 1);\n> > +}\n> > +} // namespace\n> > +\n> > +PostProcessorLibyuv::PostProcessorLibyuv() = default;\n> > +\n> > +int PostProcessorLibyuv::configure(const StreamConfiguration &inCfg,\n> > +                                const StreamConfiguration &outCfg)\n> > +{\n> > +     if (inCfg.pixelFormat != outCfg.pixelFormat ||\n> > +         inCfg.size < outCfg.size) {\n> > +             LOG(LIBYUV, Error) << \"Only down-scaling is supported\";\n>\n> This will report \"Only down-scaling is supported\" if someone tries to\n> use it to do pixel format conversion.\n>\n\nSorry I may not get your point. Should I change the message to something?\n\n>\n> > +             return -EINVAL;\n> > +     }\n> > +     if (inCfg.pixelFormat == formats::NV12) {\n> > +             LOG(LIBYUV, Error) << \"Only NV12 is supported\";\n> > +             return -EINVAL;\n> > +     }\n> > +     inCfg_ = inCfg;\n> > +     outCfg_ = outCfg;\n> > +     return 0;\n> > +}\n> > +\n> > +int PostProcessorLibyuv::process(const FrameBuffer &source,\n> > +                              libcamera::MappedBuffer *destination,\n> > +                              CameraMetadata * /*metadata*/)\n> > +{\n> > +     if (!IsValidNV12Buffers(source, *destination)) {\n> > +             LOG(LIBYUV, Error) << \"Invalid source and destination\";\n> > +             return -EINVAL;\n> > +     }\n> > +\n> > +     const MappedFrameBuffer sourceMapped(&source, PROT_READ);\n> > +     if (!sourceMapped.isValid()) {\n> > +             LOG(LIBYUV, Error) << \"Failed to mmap camera frame buffer\";\n> > +             return -EINVAL;\n> > +     }\n> > +\n> > +     return 0 == libyuv::NV12Scale(sourceMapped.maps()[0].data(),\n> > +                                   getNV12Stride(inCfg_.size, 0),\n> > +                                   sourceMapped.maps()[1].data(),\n> > +                                   getNV12Stride(inCfg_.size, 1),\n> > +                                   inCfg_.size.height, inCfg_.size.width,\n>\n> > +                                   destination->maps()[0].data(),\n> > +                                   getNV12Stride(outCfg_.size, 0),\n> > +                                   destination->maps()[1].data(),\n> > +                                   getNV12Stride(outCfg_.size, 1),\n> > +                                   outCfg_.size.width, outCfg_.size.height,\n>\n> > +                                   libyuv::FilterMode::kFilterBilinear);\n>\n> Eeek, that's quite terse to read, but that's more of a restriction on\n> the libyuv() I expect.\n>\n>\n> > +}\n> > +\n> > +bool PostProcessorLibyuv::IsValidNV12Buffers(\n> > +     const FrameBuffer &source,\n> > +     const libcamera::MappedBuffer &destination) const\n> > +{\n> > +     if (source.planes().size() != 2u) {\n> > +             LOG(LIBYUV, Error) << \"The number of source planes is not 2\";\n> > +             return false;\n> > +     }\n> > +     if (destination.maps().size() != 2u) {\n> > +             LOG(LIBYUV, Error)\n> > +                     << \"The number of destination planes is not 2\";\n> > +             return false;\n> > +     }\n> > +\n> > +     unsigned int sourceLength[2] = {};\n> > +     unsigned int destinationLength[2] = {};\n> > +     getNV12Length(inCfg_.size, sourceLength);\n> > +     getNV12Length(outCfg_.size, destinationLength);\n>\n> I think these can be calculated once at configure() time rather than in\n> every frame.\n>\n> > +\n> > +     if (source.planes()[0].length < sourceLength[0] ||\n> > +         source.planes()[1].length < sourceLength[1]) {\n> > +             LOG(LIBYUV, Error)\n> > +                     << \"The source planes lengths are too small\";\n> > +             return false;\n> > +     }\n>\n> I would expect that we have an element of 'we can trust the sizes of a\n> FrameBuffer()' ?\n>\n>\n> > +     if (destination.maps()[0].size() < destinationLength[0] ||\n> > +         destination.maps()[1].size() < destinationLength[1]) {\n> > +             LOG(LIBYUV, Error)\n> > +                     << \"The destination planes lengths are too small\";\n> > +             return false;\n> > +     }\n>\n> new line here.\n>\n> That's a lot of checks for each buffer. Do we need to do that on every\n> buffer? I suspect we might need to because we don't know what buffers\n> we're given before they come in ...\n>\n> But I wonder if we can make assumptions that they are accurate.\n>\n\nI am not sure how much I should trust the client.\nI would like to keep the check because this class can write out of bounds.\n\nBest Regards,\n-Hiro\n>\n>\n> > +     return true;\n> > +}\n> > diff --git a/src/android/libyuv/post_processor_libyuv.h b/src/android/libyuv/post_processor_libyuv.h\n> > new file mode 100644\n> > index 00000000..860a664b\n> > --- /dev/null\n> > +++ b/src/android/libyuv/post_processor_libyuv.h\n> > @@ -0,0 +1,33 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Google Inc.\n> > + *\n> > + * post_processor_libyuv.h - Post Processor using libyuv\n> > + */\n> > +#ifndef __ANDROID_POST_PROCESSOR_LIBYUV_H__\n> > +#define __ANDROID_POST_PROCESSOR_LIBYUV_H__\n> > +\n> > +#include \"../post_processor.h\"\n> > +\n> > +class CameraDevice;\n> > +\n> > +class PostProcessorLibyuv : public PostProcessor\n> > +{\n> > +public:\n> > +     PostProcessorLibyuv();\n> > +\n> > +     int configure(const libcamera::StreamConfiguration &incfg,\n> > +                   const libcamera::StreamConfiguration &outcfg) override;\n> > +     int process(const libcamera::FrameBuffer &source,\n> > +                 libcamera::MappedBuffer *destination,\n> > +                 CameraMetadata *metadata) override;\n> > +\n> > +private:\n> > +     bool IsValidNV12Buffers(const libcamera::FrameBuffer &source,\n> > +                             const libcamera::MappedBuffer &destination) const;\n> > +\n> > +     libcamera::StreamConfiguration inCfg_;\n> > +     libcamera::StreamConfiguration outCfg_;\n> > +};\n> > +\n> > +#endif /* __ANDROID_POST_PROCESSOR_LIBYUV_H__ */\n> > diff --git a/src/android/meson.build b/src/android/meson.build\n> > index 3d4d3be4..ff067d12 100644\n> > --- a/src/android/meson.build\n> > +++ b/src/android/meson.build\n> > @@ -26,6 +26,7 @@ android_hal_sources = files([\n> >      'jpeg/exif.cpp',\n> >      'jpeg/post_processor_jpeg.cpp',\n> >      'jpeg/thumbnailer.cpp',\n> > +    'libyuv/post_processor_libyuv.cpp'\n> >  ])\n> >\n> >  android_camera_metadata_sources = files([\n> > --\n> > 2.30.0.280.ga3ce27912f-goog\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 AEAADBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jan 2021 05:21:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 10130682A0;\n\tMon, 25 Jan 2021 06:21:54 +0100 (CET)","from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com\n\t[IPv6:2a00:1450:4864:20::52f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5C3B6030B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jan 2021 06:21:51 +0100 (CET)","by mail-ed1-x52f.google.com with SMTP id d2so10151931edz.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 24 Jan 2021 21:21:51 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"DVL3oazJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=k4Hd0I27EAYvIDCptgA6uPl9gAnLXPJBvndksK088xY=;\n\tb=DVL3oazJPAoPeXtXFr9pjOEG3ksHKn8BVZE56ChF6cwyJ2XtMH5rGqLNTlfOdiUm0V\n\tkT1S+N6DxQvy+kxkR+0raR6aVwLoqtcUHZpk6bv3Og39ajfv6LAXmydIHgQO1p3TuWZ7\n\twPMOv82Zk4p9v994EcXqeY8bYJv+VSMOtkPy8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=k4Hd0I27EAYvIDCptgA6uPl9gAnLXPJBvndksK088xY=;\n\tb=PygE+3/B0DhR4p7oUGASx+4M8b8+XlU1e186/sF7s1f4Ioj/Cfv49ZdRpwC4PMhhwS\n\tEarN86pC8U+FShf/pFa9yeKf1MhPgXtaw6hE1hMjzYPSyRLiIfTsrVrpZnnZsQihum0F\n\tl+rv5NUebNTUojcez9JYB9BjxodL42NGyy+As9/ePZ64r6Kek1kQTiNewQL5JwMiVk+r\n\t86gYd0pz1aD/Dcix2BAdUFtq0paC4XVLtxY+7X2nQPnyCouGcBUSeblnuB4iKzEG0py8\n\tiDwvTq/TZxgInyCm8/R62E0YJUCgZTQvfdvrK9mB14ZWph08nzomSXgWFCYdXKwR9mNH\n\t1C0Q==","X-Gm-Message-State":"AOAM53166QDtcohsdr3Nko7oF8B6qCXfty9o0y7+lD008x/zEh/BV/C4\n\twb1USqogIqt2HTuwiOom1uRqLSzihFUYooQINdk1aQ==","X-Google-Smtp-Source":"ABdhPJxA3kOPClqBYzZHVDeIZaHPaZki7VDtAH00OSZEUJufW7uoGLwZHAvm6KlWDOrguhhPxvfIvBV9HQv1i6TzvQM=","X-Received":"by 2002:a05:6402:402:: with SMTP id\n\tq2mr87422edv.116.1611552111429; \n\tSun, 24 Jan 2021 21:21:51 -0800 (PST)","MIME-Version":"1.0","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<20210122092335.254626-2-hiroh@chromium.org>\n\t<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>","In-Reply-To":"<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Mon, 25 Jan 2021 14:21:40 +0900","Message-ID":"<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","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":14744,"web_url":"https://patchwork.libcamera.org/comment/14744/","msgid":"<a875b1c2-913f-2de5-726b-2eac166a1e46@ideasonboard.com>","date":"2021-01-25T10:25:53","subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Hiro,\n\nOn 25/01/2021 05:21, Hirokazu Honda wrote:\n> Hi Kieran,\n> \n> Thanks for reviewing.\n> \n> On Fri, Jan 22, 2021 at 10:05 PM Kieran Bingham\n> <kieran.bingham@ideasonboard.com> wrote:\n>>\n>> Hi Hiro,\n>>\n>> When there's more than one patch, could you also add a cover letter\n>> please? it helps grouping them, and provides a way to comment on the\n>> whole series as well as individual patches.\n>>\n>> It doesn't have to be extensive, just a title and a brief.\n>>\n> \n> \n> I see. I will do in the next patch.\n\nThank you, I find it helpful.\n\n\n>> I'm happy to see this work being done, but there's no usage of this post\n>> processor.\n>>\n>> Do you have another patch in progress to do that which would become a\n>> part of this series?\n>>\n> \n> I don't yet have a patch to use this post processor.\n\nHave you tested the code at all?\nOr is it purely compile tested?\n\n\n> I think I have to complete a mapping task, which I have been working\n> in parallel, to use this post processor for NV12 scaling.\n> I wonder if it is allowed to check in these patches whereas it is not used now.\n\nIdeally only code that is 'used' would be integrated, but at the least\nI'd like to know that it has been tested in some form.\n\nBut we don't easily have a way to add unit tests to the android layer\ncurrently ... :-(\n\n\n> If it is not good, I will hold on this post processor patch until the\n> mapping task is done.\n\nPosting for review is certainly beneficial already.\n\nI was also hoping there was something I could try and test the code with.\n\n\n>> On 22/01/2021 09:23, Hirokazu Honda wrote:\n>>> This adds PostProcessorLibyuv. It supports NV12 buffer scaling.\n>>>\n>>> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n>>> ---\n>>>  src/android/libyuv/post_processor_libyuv.cpp | 120 +++++++++++++++++++\n>>>  src/android/libyuv/post_processor_libyuv.h   |  33 +++++\n>>>  src/android/meson.build                      |   1 +\n>>>  3 files changed, 154 insertions(+)\n>>>  create mode 100644 src/android/libyuv/post_processor_libyuv.cpp\n>>>  create mode 100644 src/android/libyuv/post_processor_libyuv.h\n>>>\n>>> diff --git a/src/android/libyuv/post_processor_libyuv.cpp b/src/android/libyuv/post_processor_libyuv.cpp\n>>> new file mode 100644\n>>> index 00000000..7cfa0539\n>>> --- /dev/null\n>>> +++ b/src/android/libyuv/post_processor_libyuv.cpp\n>>> @@ -0,0 +1,120 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2021, Google Inc.\n>>> + *\n>>> + * post_processor_libyuv.cpp - Post Processor using libyuv\n>>> + */\n>>> +\n>>> +#include \"post_processor_libyuv.h\"\n>>> +\n>>> +#include <libyuv/scale.h>\n>>> +\n>>> +#include <libcamera/formats.h>\n>>> +#include <libcamera/geometry.h>\n>>> +#include <libcamera/internal/formats.h>\n>>> +#include <libcamera/internal/log.h>\n>>> +#include <libcamera/pixel_format.h>\n>>> +\n>>> +using namespace libcamera;\n>>> +\n>>> +LOG_DEFINE_CATEGORY(LIBYUV)\n>>> +\n>>> +namespace {\n>>> +void getNV12Length(Size size, unsigned int length[2])\n>>> +{\n>>> +     auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n>>> +     for (unsigned int i = 0; i < 2; i++) {\n>>> +             const unsigned int vertSubSample =\n>>> +                     nv12Info.planes[i].verticalSubSampling;\n>>> +             length[i] = nv12Info.stride(size.width, i, /*align=*/1) *\n>>> +                         ((size.height + vertSubSample - 1) / vertSubSample);\n>>> +     }\n>>> +}\n>>> +\n>>> +unsigned int getNV12Stride(Size size, unsigned int i)\n>>> +{\n>>> +     auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n>>> +     return nv12Info.stride(size.width, i, 1);\n>>> +}\n>>> +} // namespace\n>>> +\n>>> +PostProcessorLibyuv::PostProcessorLibyuv() = default;\n>>> +\n>>> +int PostProcessorLibyuv::configure(const StreamConfiguration &inCfg,\n>>> +                                const StreamConfiguration &outCfg)\n>>> +{\n>>> +     if (inCfg.pixelFormat != outCfg.pixelFormat ||\n>>> +         inCfg.size < outCfg.size) {\n>>> +             LOG(LIBYUV, Error) << \"Only down-scaling is supported\";\n>>\n>> This will report \"Only down-scaling is supported\" if someone tries to\n>> use it to do pixel format conversion.\n>>\n> \n> Sorry I may not get your point. Should I change the message to something?\n\nI think it would be helpful to the reader of the error to diagnose what\nhappened if either the error checking was split for size checking, and\nformat checking.\n\n\n>>\n>>> +             return -EINVAL;\n>>> +     }\n>>> +     if (inCfg.pixelFormat == formats::NV12) {\n>>> +             LOG(LIBYUV, Error) << \"Only NV12 is supported\";\n>>> +             return -EINVAL;\n>>> +     }\n\nIn fact, You could put the pixelformat check first, and combine it with\nthe output format checking?\n\nmaybe even printing the configration if it fails?:\n\n\tif (inCfg.pixelFormat != outCfg.pixelFormat != formats::NV12)\n\t{\n\t\tLOG(LIBYUV, Error) << \"Only NV12 is supported\"\n\t\t\t\t   << \" - In: \" << inCfg.toString()\n\t\t\t\t   << \" - Out: \" << inCfg.toString();\n\t\treturn -EINVAL;\n\t}\n\nOr do you later expect libyuv to support pixel format conversion too?\n\n\n>>> +     inCfg_ = inCfg;\n>>> +     outCfg_ = outCfg;\n>>> +     return 0;\n>>> +}\n>>> +\n>>> +int PostProcessorLibyuv::process(const FrameBuffer &source,\n>>> +                              libcamera::MappedBuffer *destination,\n>>> +                              CameraMetadata * /*metadata*/)\n>>> +{\n>>> +     if (!IsValidNV12Buffers(source, *destination)) {\n>>> +             LOG(LIBYUV, Error) << \"Invalid source and destination\";\n>>> +             return -EINVAL;\n>>> +     }\n>>> +\n>>> +     const MappedFrameBuffer sourceMapped(&source, PROT_READ);\n>>> +     if (!sourceMapped.isValid()) {\n>>> +             LOG(LIBYUV, Error) << \"Failed to mmap camera frame buffer\";\n>>> +             return -EINVAL;\n>>> +     }\n>>> +\n>>> +     return 0 == libyuv::NV12Scale(sourceMapped.maps()[0].data(),\n>>> +                                   getNV12Stride(inCfg_.size, 0),\n>>> +                                   sourceMapped.maps()[1].data(),\n>>> +                                   getNV12Stride(inCfg_.size, 1),\n>>> +                                   inCfg_.size.height, inCfg_.size.width,\n>>\n>>> +                                   destination->maps()[0].data(),\n>>> +                                   getNV12Stride(outCfg_.size, 0),\n>>> +                                   destination->maps()[1].data(),\n>>> +                                   getNV12Stride(outCfg_.size, 1),\n>>> +                                   outCfg_.size.width, outCfg_.size.height,\n>>\n>>> +                                   libyuv::FilterMode::kFilterBilinear);\n>>\n>> Eeek, that's quite terse to read, but that's more of a restriction on\n>> the libyuv() I expect.\n>>\n>>\n>>> +}\n>>> +\n>>> +bool PostProcessorLibyuv::IsValidNV12Buffers(\n>>> +     const FrameBuffer &source,\n>>> +     const libcamera::MappedBuffer &destination) const\n>>> +{\n>>> +     if (source.planes().size() != 2u) {\n>>> +             LOG(LIBYUV, Error) << \"The number of source planes is not 2\";\n>>> +             return false;\n>>> +     }\n>>> +     if (destination.maps().size() != 2u) {\n>>> +             LOG(LIBYUV, Error)\n>>> +                     << \"The number of destination planes is not 2\";\n>>> +             return false;\n>>> +     }\n>>> +\n>>> +     unsigned int sourceLength[2] = {};\n>>> +     unsigned int destinationLength[2] = {};\n>>> +     getNV12Length(inCfg_.size, sourceLength);\n>>> +     getNV12Length(outCfg_.size, destinationLength);\n>>\n>> I think these can be calculated once at configure() time rather than in\n>> every frame.\n>>\n>>> +\n>>> +     if (source.planes()[0].length < sourceLength[0] ||\n>>> +         source.planes()[1].length < sourceLength[1]) {\n>>> +             LOG(LIBYUV, Error)\n>>> +                     << \"The source planes lengths are too small\";\n>>> +             return false;\n>>> +     }\n>>\n>> I would expect that we have an element of 'we can trust the sizes of a\n>> FrameBuffer()' ?\n>>\n>>\n>>> +     if (destination.maps()[0].size() < destinationLength[0] ||\n>>> +         destination.maps()[1].size() < destinationLength[1]) {\n>>> +             LOG(LIBYUV, Error)\n>>> +                     << \"The destination planes lengths are too small\";\n>>> +             return false;\n>>> +     }\n>>\n>> new line here.\n>>\n>> That's a lot of checks for each buffer. Do we need to do that on every\n>> buffer? I suspect we might need to because we don't know what buffers\n>> we're given before they come in ...\n>>\n>> But I wonder if we can make assumptions that they are accurate.\n>>\n> \n> I am not sure how much I should trust the client.\n> I would like to keep the check because this class can write out of bounds.\n> \n> Best Regards,\n> -Hiro\n>>\n>>\n>>> +     return true;\n>>> +}\n>>> diff --git a/src/android/libyuv/post_processor_libyuv.h b/src/android/libyuv/post_processor_libyuv.h\n>>> new file mode 100644\n>>> index 00000000..860a664b\n>>> --- /dev/null\n>>> +++ b/src/android/libyuv/post_processor_libyuv.h\n>>> @@ -0,0 +1,33 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2021, Google Inc.\n>>> + *\n>>> + * post_processor_libyuv.h - Post Processor using libyuv\n>>> + */\n>>> +#ifndef __ANDROID_POST_PROCESSOR_LIBYUV_H__\n>>> +#define __ANDROID_POST_PROCESSOR_LIBYUV_H__\n>>> +\n>>> +#include \"../post_processor.h\"\n>>> +\n>>> +class CameraDevice;\n>>> +\n>>> +class PostProcessorLibyuv : public PostProcessor\n>>> +{\n>>> +public:\n>>> +     PostProcessorLibyuv();\n>>> +\n>>> +     int configure(const libcamera::StreamConfiguration &incfg,\n>>> +                   const libcamera::StreamConfiguration &outcfg) override;\n>>> +     int process(const libcamera::FrameBuffer &source,\n>>> +                 libcamera::MappedBuffer *destination,\n>>> +                 CameraMetadata *metadata) override;\n>>> +\n>>> +private:\n>>> +     bool IsValidNV12Buffers(const libcamera::FrameBuffer &source,\n>>> +                             const libcamera::MappedBuffer &destination) const;\n>>> +\n>>> +     libcamera::StreamConfiguration inCfg_;\n>>> +     libcamera::StreamConfiguration outCfg_;\n>>> +};\n>>> +\n>>> +#endif /* __ANDROID_POST_PROCESSOR_LIBYUV_H__ */\n>>> diff --git a/src/android/meson.build b/src/android/meson.build\n>>> index 3d4d3be4..ff067d12 100644\n>>> --- a/src/android/meson.build\n>>> +++ b/src/android/meson.build\n>>> @@ -26,6 +26,7 @@ android_hal_sources = files([\n>>>      'jpeg/exif.cpp',\n>>>      'jpeg/post_processor_jpeg.cpp',\n>>>      'jpeg/thumbnailer.cpp',\n>>> +    'libyuv/post_processor_libyuv.cpp'\n>>>  ])\n>>>\n>>>  android_camera_metadata_sources = files([\n>>> --\n>>> 2.30.0.280.ga3ce27912f-goog\n>>> _______________________________________________\n>>> libcamera-devel mailing list\n>>> libcamera-devel@lists.libcamera.org\n>>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 7F825C0F2B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jan 2021 10:26:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 056AB682B6;\n\tMon, 25 Jan 2021 11:26:00 +0100 (CET)","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 F2DF16030B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jan 2021 11:25:57 +0100 (CET)","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 D2D083E;\n\tMon, 25 Jan 2021 11:25:56 +0100 (CET)"],"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=\"QUKurZX/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611570357;\n\tbh=jE9BNtKxAkEwGHx9vV5lE2U3idGJBhGosm6XuUy5r2Y=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=QUKurZX/9GdwUH/LtR5R/GBQQO7lbOFNCTIfmpUsvPwCR9oEH1MrOT/MjtTD3MnaS\n\tB97+K5x1Hl6pd00M3qil0QwRQyg5lRVEkT+uDJQhA3dvl8Wdhw33DzNsL8PQj1XEdR\n\tE7VQTDdMz0HZjz6Jb9bTAQ9Pt6tLzt5BPnKPw6V0=","To":"Hirokazu Honda <hiroh@chromium.org>","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<20210122092335.254626-2-hiroh@chromium.org>\n\t<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>\n\t<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.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":"<a875b1c2-913f-2de5-726b-2eac166a1e46@ideasonboard.com>","Date":"Mon, 25 Jan 2021 10:25:53 +0000","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":"<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","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>"}},{"id":14810,"web_url":"https://patchwork.libcamera.org/comment/14810/","msgid":"<CAO5uPHOj8mXLZcKc6mp8hEkUoY4xfjUze04MM6C9d8M7pAbtTg@mail.gmail.com>","date":"2021-01-27T04:18:41","subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"On Mon, Jan 25, 2021 at 7:25 PM Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Hi Hiro,\n>\n> On 25/01/2021 05:21, Hirokazu Honda wrote:\n> > Hi Kieran,\n> >\n> > Thanks for reviewing.\n> >\n> > On Fri, Jan 22, 2021 at 10:05 PM Kieran Bingham\n> > <kieran.bingham@ideasonboard.com> wrote:\n> >>\n> >> Hi Hiro,\n> >>\n> >> When there's more than one patch, could you also add a cover letter\n> >> please? it helps grouping them, and provides a way to comment on the\n> >> whole series as well as individual patches.\n> >>\n> >> It doesn't have to be extensive, just a title and a brief.\n> >>\n> >\n> >\n> > I see. I will do in the next patch.\n>\n> Thank you, I find it helpful.\n>\n>\n> >> I'm happy to see this work being done, but there's no usage of this post\n> >> processor.\n> >>\n> >> Do you have another patch in progress to do that which would become a\n> >> part of this series?\n> >>\n> >\n> > I don't yet have a patch to use this post processor.\n>\n> Have you tested the code at all?\n> Or is it purely compile tested?\n>\n\nI haven't tested this code at all.\nI only confirmed the compilation was successful.\n\n\n>\n> > I think I have to complete a mapping task, which I have been working\n> > in parallel, to use this post processor for NV12 scaling.\n> > I wonder if it is allowed to check in these patches whereas it is not used now.\n>\n> Ideally only code that is 'used' would be integrated, but at the least\n> I'd like to know that it has been tested in some form.\n>\n> But we don't easily have a way to add unit tests to the android layer\n> currently ... :-(\n>\n>\n> > If it is not good, I will hold on this post processor patch until the\n> > mapping task is done.\n>\n> Posting for review is certainly beneficial already.\n>\n> I was also hoping there was something I could try and test the code with.\n>\n>\n> >> On 22/01/2021 09:23, Hirokazu Honda wrote:\n> >>> This adds PostProcessorLibyuv. It supports NV12 buffer scaling.\n> >>>\n> >>> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> >>> ---\n> >>>  src/android/libyuv/post_processor_libyuv.cpp | 120 +++++++++++++++++++\n> >>>  src/android/libyuv/post_processor_libyuv.h   |  33 +++++\n> >>>  src/android/meson.build                      |   1 +\n> >>>  3 files changed, 154 insertions(+)\n> >>>  create mode 100644 src/android/libyuv/post_processor_libyuv.cpp\n> >>>  create mode 100644 src/android/libyuv/post_processor_libyuv.h\n> >>>\n> >>> diff --git a/src/android/libyuv/post_processor_libyuv.cpp b/src/android/libyuv/post_processor_libyuv.cpp\n> >>> new file mode 100644\n> >>> index 00000000..7cfa0539\n> >>> --- /dev/null\n> >>> +++ b/src/android/libyuv/post_processor_libyuv.cpp\n> >>> @@ -0,0 +1,120 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2021, Google Inc.\n> >>> + *\n> >>> + * post_processor_libyuv.cpp - Post Processor using libyuv\n> >>> + */\n> >>> +\n> >>> +#include \"post_processor_libyuv.h\"\n> >>> +\n> >>> +#include <libyuv/scale.h>\n> >>> +\n> >>> +#include <libcamera/formats.h>\n> >>> +#include <libcamera/geometry.h>\n> >>> +#include <libcamera/internal/formats.h>\n> >>> +#include <libcamera/internal/log.h>\n> >>> +#include <libcamera/pixel_format.h>\n> >>> +\n> >>> +using namespace libcamera;\n> >>> +\n> >>> +LOG_DEFINE_CATEGORY(LIBYUV)\n> >>> +\n> >>> +namespace {\n> >>> +void getNV12Length(Size size, unsigned int length[2])\n> >>> +{\n> >>> +     auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n> >>> +     for (unsigned int i = 0; i < 2; i++) {\n> >>> +             const unsigned int vertSubSample =\n> >>> +                     nv12Info.planes[i].verticalSubSampling;\n> >>> +             length[i] = nv12Info.stride(size.width, i, /*align=*/1) *\n> >>> +                         ((size.height + vertSubSample - 1) / vertSubSample);\n> >>> +     }\n> >>> +}\n> >>> +\n> >>> +unsigned int getNV12Stride(Size size, unsigned int i)\n> >>> +{\n> >>> +     auto nv12Info = PixelFormatInfo::info(PixelFormat(formats::NV12));\n> >>> +     return nv12Info.stride(size.width, i, 1);\n> >>> +}\n> >>> +} // namespace\n> >>> +\n> >>> +PostProcessorLibyuv::PostProcessorLibyuv() = default;\n> >>> +\n> >>> +int PostProcessorLibyuv::configure(const StreamConfiguration &inCfg,\n> >>> +                                const StreamConfiguration &outCfg)\n> >>> +{\n> >>> +     if (inCfg.pixelFormat != outCfg.pixelFormat ||\n> >>> +         inCfg.size < outCfg.size) {\n> >>> +             LOG(LIBYUV, Error) << \"Only down-scaling is supported\";\n> >>\n> >> This will report \"Only down-scaling is supported\" if someone tries to\n> >> use it to do pixel format conversion.\n> >>\n> >\n> > Sorry I may not get your point. Should I change the message to something?\n>\n> I think it would be helpful to the reader of the error to diagnose what\n> happened if either the error checking was split for size checking, and\n> format checking.\n>\n>\n> >>\n> >>> +             return -EINVAL;\n> >>> +     }\n> >>> +     if (inCfg.pixelFormat == formats::NV12) {\n> >>> +             LOG(LIBYUV, Error) << \"Only NV12 is supported\";\n> >>> +             return -EINVAL;\n> >>> +     }\n>\n> In fact, You could put the pixelformat check first, and combine it with\n> the output format checking?\n>\n> maybe even printing the configration if it fails?:\n>\n>         if (inCfg.pixelFormat != outCfg.pixelFormat != formats::NV12)\n>         {\n>                 LOG(LIBYUV, Error) << \"Only NV12 is supported\"\n>                                    << \" - In: \" << inCfg.toString()\n>                                    << \" - Out: \" << inCfg.toString();\n>                 return -EINVAL;\n>         }\n>\n> Or do you later expect libyuv to support pixel format conversion too?\n>\n>\n> >>> +     inCfg_ = inCfg;\n> >>> +     outCfg_ = outCfg;\n> >>> +     return 0;\n> >>> +}\n> >>> +\n> >>> +int PostProcessorLibyuv::process(const FrameBuffer &source,\n> >>> +                              libcamera::MappedBuffer *destination,\n> >>> +                              CameraMetadata * /*metadata*/)\n> >>> +{\n> >>> +     if (!IsValidNV12Buffers(source, *destination)) {\n> >>> +             LOG(LIBYUV, Error) << \"Invalid source and destination\";\n> >>> +             return -EINVAL;\n> >>> +     }\n> >>> +\n> >>> +     const MappedFrameBuffer sourceMapped(&source, PROT_READ);\n> >>> +     if (!sourceMapped.isValid()) {\n> >>> +             LOG(LIBYUV, Error) << \"Failed to mmap camera frame buffer\";\n> >>> +             return -EINVAL;\n> >>> +     }\n> >>> +\n> >>> +     return 0 == libyuv::NV12Scale(sourceMapped.maps()[0].data(),\n> >>> +                                   getNV12Stride(inCfg_.size, 0),\n> >>> +                                   sourceMapped.maps()[1].data(),\n> >>> +                                   getNV12Stride(inCfg_.size, 1),\n> >>> +                                   inCfg_.size.height, inCfg_.size.width,\n> >>\n> >>> +                                   destination->maps()[0].data(),\n> >>> +                                   getNV12Stride(outCfg_.size, 0),\n> >>> +                                   destination->maps()[1].data(),\n> >>> +                                   getNV12Stride(outCfg_.size, 1),\n> >>> +                                   outCfg_.size.width, outCfg_.size.height,\n> >>\n> >>> +                                   libyuv::FilterMode::kFilterBilinear);\n> >>\n> >> Eeek, that's quite terse to read, but that's more of a restriction on\n> >> the libyuv() I expect.\n> >>\n> >>\n> >>> +}\n> >>> +\n> >>> +bool PostProcessorLibyuv::IsValidNV12Buffers(\n> >>> +     const FrameBuffer &source,\n> >>> +     const libcamera::MappedBuffer &destination) const\n> >>> +{\n> >>> +     if (source.planes().size() != 2u) {\n> >>> +             LOG(LIBYUV, Error) << \"The number of source planes is not 2\";\n> >>> +             return false;\n> >>> +     }\n> >>> +     if (destination.maps().size() != 2u) {\n> >>> +             LOG(LIBYUV, Error)\n> >>> +                     << \"The number of destination planes is not 2\";\n> >>> +             return false;\n> >>> +     }\n> >>> +\n> >>> +     unsigned int sourceLength[2] = {};\n> >>> +     unsigned int destinationLength[2] = {};\n> >>> +     getNV12Length(inCfg_.size, sourceLength);\n> >>> +     getNV12Length(outCfg_.size, destinationLength);\n> >>\n> >> I think these can be calculated once at configure() time rather than in\n> >> every frame.\n> >>\n> >>> +\n> >>> +     if (source.planes()[0].length < sourceLength[0] ||\n> >>> +         source.planes()[1].length < sourceLength[1]) {\n> >>> +             LOG(LIBYUV, Error)\n> >>> +                     << \"The source planes lengths are too small\";\n> >>> +             return false;\n> >>> +     }\n> >>\n> >> I would expect that we have an element of 'we can trust the sizes of a\n> >> FrameBuffer()' ?\n> >>\n> >>\n> >>> +     if (destination.maps()[0].size() < destinationLength[0] ||\n> >>> +         destination.maps()[1].size() < destinationLength[1]) {\n> >>> +             LOG(LIBYUV, Error)\n> >>> +                     << \"The destination planes lengths are too small\";\n> >>> +             return false;\n> >>> +     }\n> >>\n> >> new line here.\n> >>\n> >> That's a lot of checks for each buffer. Do we need to do that on every\n> >> buffer? I suspect we might need to because we don't know what buffers\n> >> we're given before they come in ...\n> >>\n> >> But I wonder if we can make assumptions that they are accurate.\n> >>\n> >\n> > I am not sure how much I should trust the client.\n> > I would like to keep the check because this class can write out of bounds.\n> >\n> > Best Regards,\n> > -Hiro\n> >>\n> >>\n> >>> +     return true;\n> >>> +}\n> >>> diff --git a/src/android/libyuv/post_processor_libyuv.h b/src/android/libyuv/post_processor_libyuv.h\n> >>> new file mode 100644\n> >>> index 00000000..860a664b\n> >>> --- /dev/null\n> >>> +++ b/src/android/libyuv/post_processor_libyuv.h\n> >>> @@ -0,0 +1,33 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2021, Google Inc.\n> >>> + *\n> >>> + * post_processor_libyuv.h - Post Processor using libyuv\n> >>> + */\n> >>> +#ifndef __ANDROID_POST_PROCESSOR_LIBYUV_H__\n> >>> +#define __ANDROID_POST_PROCESSOR_LIBYUV_H__\n> >>> +\n> >>> +#include \"../post_processor.h\"\n> >>> +\n> >>> +class CameraDevice;\n> >>> +\n> >>> +class PostProcessorLibyuv : public PostProcessor\n> >>> +{\n> >>> +public:\n> >>> +     PostProcessorLibyuv();\n> >>> +\n> >>> +     int configure(const libcamera::StreamConfiguration &incfg,\n> >>> +                   const libcamera::StreamConfiguration &outcfg) override;\n> >>> +     int process(const libcamera::FrameBuffer &source,\n> >>> +                 libcamera::MappedBuffer *destination,\n> >>> +                 CameraMetadata *metadata) override;\n> >>> +\n> >>> +private:\n> >>> +     bool IsValidNV12Buffers(const libcamera::FrameBuffer &source,\n> >>> +                             const libcamera::MappedBuffer &destination) const;\n> >>> +\n> >>> +     libcamera::StreamConfiguration inCfg_;\n> >>> +     libcamera::StreamConfiguration outCfg_;\n> >>> +};\n> >>> +\n> >>> +#endif /* __ANDROID_POST_PROCESSOR_LIBYUV_H__ */\n> >>> diff --git a/src/android/meson.build b/src/android/meson.build\n> >>> index 3d4d3be4..ff067d12 100644\n> >>> --- a/src/android/meson.build\n> >>> +++ b/src/android/meson.build\n> >>> @@ -26,6 +26,7 @@ android_hal_sources = files([\n> >>>      'jpeg/exif.cpp',\n> >>>      'jpeg/post_processor_jpeg.cpp',\n> >>>      'jpeg/thumbnailer.cpp',\n> >>> +    'libyuv/post_processor_libyuv.cpp'\n> >>>  ])\n> >>>\n> >>>  android_camera_metadata_sources = files([\n> >>> --\n> >>> 2.30.0.280.ga3ce27912f-goog\n> >>> _______________________________________________\n> >>> libcamera-devel mailing list\n> >>> libcamera-devel@lists.libcamera.org\n> >>> https://lists.libcamera.org/listinfo/libcamera-devel\n> >>>\n> >>\n> >> --\n> >> Regards\n> >> --\n> >> Kieran\n>\n> --\n> Regards\n> --\n> Kieran","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 E6DB3BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Jan 2021 04:18:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3BCC86833A;\n\tWed, 27 Jan 2021 05:18:53 +0100 (CET)","from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com\n\t[IPv6:2a00:1450:4864:20::52f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E11960309\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jan 2021 05:18:52 +0100 (CET)","by mail-ed1-x52f.google.com with SMTP id a14so761921edu.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Jan 2021 20:18:52 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"EocHQLaT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=zYKeu0cHH41IYjPv+fneczdp9sY9VZjAcOTs+NVf+6o=;\n\tb=EocHQLaTa1tPJW7TBGIKpPqtHE8qVcgXjyBjmN5LO3MyUPpROZkbcQ34U3xWMXW2Ii\n\tjVW9apkbGklI3QiNcxsEbBGOIgEwYLPl9UirDpd8TnJVJwTQRsxAYhQ9m5ibfLoQoGH7\n\tghzJxNZKZccs96TaKpoFM8ZeepeRFC7jlaO4U=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=zYKeu0cHH41IYjPv+fneczdp9sY9VZjAcOTs+NVf+6o=;\n\tb=qFOb1BJaXpQP7ukvcQewrmKi1uOfOpVZ4j2Gz3ECfxh2G3HLYkNMphYpWDEOsPwZj6\n\t8g5bTgvgcsVhLUzN4FRTyxNB2SAU79UYwutYylGfihtE9nPbD6Amw55+oaeCYsB81Bs/\n\ts1MlC2sXapMtgJG9rRrXn+EovAg6EJiwtBhpoOAWJ3Gv1RNyQLQLbNZZmiiFygLGaUWZ\n\tdCCyEvqXQOtIRCl18Png7sN7MML3sy3mTaiAuf+hnCpXOEYjM/ZLpzoOqqE0OGBHPw70\n\tWkykhaNr471ky+2zTfO4XZey1gAlmIXxM1OBinMxe/tSQRo0KllX4MbTYB5JGGJuJA8C\n\ta9Eg==","X-Gm-Message-State":"AOAM532233PKAqvcnwc5qbHr8r+0uEefUDzVvjPDLArhwHqX7ZG8NS/k\n\tP7mWNZYboW5wwdkmQ+NQkQfmvnCIsC0t36tTdEZn1FhomHXh4g==","X-Google-Smtp-Source":"ABdhPJxBhJ8l8m+7YjgrSFWkK1xkfkICg3D/er2wm1lhTCCtTRg/3BzABzmZZFQX7uTrUYmetQQb3notHbvvkUYq5tU=","X-Received":"by 2002:a05:6402:3116:: with SMTP id\n\tdc22mr6958233edb.325.1611721131753; \n\tTue, 26 Jan 2021 20:18:51 -0800 (PST)","MIME-Version":"1.0","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<20210122092335.254626-2-hiroh@chromium.org>\n\t<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>\n\t<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.com>\n\t<a875b1c2-913f-2de5-726b-2eac166a1e46@ideasonboard.com>","In-Reply-To":"<a875b1c2-913f-2de5-726b-2eac166a1e46@ideasonboard.com>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Wed, 27 Jan 2021 13:18:41 +0900","Message-ID":"<CAO5uPHOj8mXLZcKc6mp8hEkUoY4xfjUze04MM6C9d8M7pAbtTg@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","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":14819,"web_url":"https://patchwork.libcamera.org/comment/14819/","msgid":"<828c3aad-7f89-fe8c-f451-3997a55c23dc@ideasonboard.com>","date":"2021-01-27T09:52:50","subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Hiro,\n\nOn 27/01/2021 04:18, Hirokazu Honda wrote:\n> On Mon, Jan 25, 2021 at 7:25 PM Kieran Bingham\n<snip>\n>>>> I'm happy to see this work being done, but there's no usage of this post\n>>>> processor.\n>>>>\n>>>> Do you have another patch in progress to do that which would become a\n>>>> part of this series?\n>>>>\n>>>\n>>> I don't yet have a patch to use this post processor.\n>>\n>> Have you tested the code at all?\n>> Or is it purely compile tested?\n>>\n> \n> I haven't tested this code at all.\n> I only confirmed the compilation was successful.\n\nOk, thanks for the update.\n\nOrdinarily, I think it would be better for code to go in that is tested\nand known to be working. But this object will not regress other\ncomponents, and will not cause breakage as long as it compiles.\n\nSo if it helps you progress, and base your developments on top of it,\nwith all review actions completed as normal, I think we can let this in\nbefore there is a user.\n\nI'm looking forwards to seeing how this will be used :-)","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 10351BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Jan 2021 09:52:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 95F0B68356;\n\tWed, 27 Jan 2021 10:52:55 +0100 (CET)","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 ED29F68353\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jan 2021 10:52:53 +0100 (CET)","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 662C5240;\n\tWed, 27 Jan 2021 10:52:53 +0100 (CET)"],"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=\"t9oODU6o\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611741173;\n\tbh=TqhaVzwHlmjSG323ftlGyCJJ0JezEgwC6cewmdgDCZ4=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=t9oODU6o3eVFP+Ax/zYHE4QbwV0T25kACNnL57EcWlI7BF+PXoho77i3QIZdZ2pdO\n\tTtIX0ujYsIuqK5/9YbQOyn3mmTmDml7Ys9WDO6MXfTcZqff1daWcDOe0d9biVAaf4j\n\tfeYkFg50P+mwAAO9Y/2CNh5WfD5gKCnkAhwMNTk8=","To":"Hirokazu Honda <hiroh@chromium.org>","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<20210122092335.254626-2-hiroh@chromium.org>\n\t<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>\n\t<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.com>\n\t<a875b1c2-913f-2de5-726b-2eac166a1e46@ideasonboard.com>\n\t<CAO5uPHOj8mXLZcKc6mp8hEkUoY4xfjUze04MM6C9d8M7pAbtTg@mail.gmail.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":"<828c3aad-7f89-fe8c-f451-3997a55c23dc@ideasonboard.com>","Date":"Wed, 27 Jan 2021 09:52:50 +0000","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":"<CAO5uPHOj8mXLZcKc6mp8hEkUoY4xfjUze04MM6C9d8M7pAbtTg@mail.gmail.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","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>"}},{"id":14838,"web_url":"https://patchwork.libcamera.org/comment/14838/","msgid":"<CAO5uPHPshmXpDMFPjCeho2KLw2E9Jg9ehB3gE055s9McStC8Tw@mail.gmail.com>","date":"2021-01-28T00:55:04","subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"On Wed, Jan 27, 2021 at 6:52 PM Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Hi Hiro,\n>\n> On 27/01/2021 04:18, Hirokazu Honda wrote:\n> > On Mon, Jan 25, 2021 at 7:25 PM Kieran Bingham\n> <snip>\n> >>>> I'm happy to see this work being done, but there's no usage of this post\n> >>>> processor.\n> >>>>\n> >>>> Do you have another patch in progress to do that which would become a\n> >>>> part of this series?\n> >>>>\n> >>>\n> >>> I don't yet have a patch to use this post processor.\n> >>\n> >> Have you tested the code at all?\n> >> Or is it purely compile tested?\n> >>\n> >\n> > I haven't tested this code at all.\n> > I only confirmed the compilation was successful.\n>\n> Ok, thanks for the update.\n>\n> Ordinarily, I think it would be better for code to go in that is tested\n> and known to be working. But this object will not regress other\n> components, and will not cause breakage as long as it compiles.\n>\n> So if it helps you progress, and base your developments on top of it,\n> with all review actions completed as normal, I think we can let this in\n> before there is a user.\n>\n> I'm looking forwards to seeing how this will be used :-)\n\nThanks Kieran for your patience and understanding!\n\n-Hiro\n\n\n\n> --\n> Regards\n> --\n> Kieran","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 3A995C0F2B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Jan 2021 00:55:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94A6D6836F;\n\tThu, 28 Jan 2021 01:55:17 +0100 (CET)","from mail-ed1-x529.google.com (mail-ed1-x529.google.com\n\t[IPv6:2a00:1450:4864:20::529])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 66360682BC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jan 2021 01:55:16 +0100 (CET)","by mail-ed1-x529.google.com with SMTP id dj23so4640303edb.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jan 2021 16:55:16 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"doyXMnTZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=lxos58M/f/xH/kzmQkLRkPstaPMf22ywMfrB3oUwQLY=;\n\tb=doyXMnTZyAZ3OFH8hN9LrhLJsrRJrvr7YyJAfUbZjBCVpEbnhuSm16QgRhM22lASM5\n\t0om9UJy7pxK905aAqgEB4sxoAdq5xTrY9HqBucku8NQcHLfRdOO5aNfSv5xxfeCX+k0M\n\t0hJefRzZyWObH7SMQr66fNpl8L4GdIBdAwK3Y=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=lxos58M/f/xH/kzmQkLRkPstaPMf22ywMfrB3oUwQLY=;\n\tb=X/jNx+9Ag/NRxUkuXOPh0E4t5Tym8GUykILIGdG/AZCN9W2bbqBDYrNlLKkWORAsnB\n\trnY5fVUyOAOmsK9kkOAG+eRn2bUhjZloelReMFLNPIiQdpmqsJuWhIAiEkNuPUIJ4QfQ\n\tpApEQAJBbIE+hK+ktPQtJscBT2qSlRNe2MYECIkgetNQWaZqTxU1GocCgR192bR+TKWq\n\t9BXPOzk+mL87Ty8T7t60qU7NTGGJ8S1nwDVCauWjia0DbnQX58ZACm1pt5S6vyMdwP7O\n\tLNzKY6r320YqDbgU9x3QY2OhgKaLVmoIfH1NWLceJMfaQ+Q4KkWm9DA/b/aKThjY5mnq\n\tRghQ==","X-Gm-Message-State":"AOAM533AstF3K/RYJaVGMLrNysVkaCLDasiX0mns2FoD1+W7qfi+u9B9\n\tNerokWpTrz4pfw5PrQqhf7/lDigvOoqZMNhbtDLHlA==","X-Google-Smtp-Source":"ABdhPJyINIT+inag7fSvd5nr4LQKHh16Q1n1z9who+nV3KcDXXpS5BSyZ9wz+e9hFh+dPTwTiC7sIsMYxvOqo02vUb8=","X-Received":"by 2002:aa7:d1d4:: with SMTP id\n\tg20mr11582509edp.244.1611795316043; \n\tWed, 27 Jan 2021 16:55:16 -0800 (PST)","MIME-Version":"1.0","References":"<20210122092335.254626-1-hiroh@chromium.org>\n\t<20210122092335.254626-2-hiroh@chromium.org>\n\t<e7f335c2-33fa-ca86-6ab2-f1e263c15622@ideasonboard.com>\n\t<CAO5uPHMEdCitwOEX-_53MVm5JrDNFkchM2cVkMfBTaSit7H3bQ@mail.gmail.com>\n\t<a875b1c2-913f-2de5-726b-2eac166a1e46@ideasonboard.com>\n\t<CAO5uPHOj8mXLZcKc6mp8hEkUoY4xfjUze04MM6C9d8M7pAbtTg@mail.gmail.com>\n\t<828c3aad-7f89-fe8c-f451-3997a55c23dc@ideasonboard.com>","In-Reply-To":"<828c3aad-7f89-fe8c-f451-3997a55c23dc@ideasonboard.com>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Thu, 28 Jan 2021 09:55:04 +0900","Message-ID":"<CAO5uPHPshmXpDMFPjCeho2KLw2E9Jg9ehB3gE055s9McStC8Tw@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] android: libyuv: Introduce\n\tPostProcessorLibyuv","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>"}}]