[{"id":27846,"web_url":"https://patchwork.libcamera.org/comment/27846/","msgid":"","date":"2023-09-22T10:58:21","subject":"Re: [libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support\n\tfor other pixel formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Mattijs\n\nOn Fri, Sep 15, 2023 at 09:57:27AM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> PostProcessorYuv assumes that both source and destination pixel formats\n> will always be formats::NV12.\n> This might change in the future, for example when doing pixel format\n> conversion via libyuv.\n>\n> Add the necessary plumbing so that other formats can be added easily in\n> the future.\n>\n> Also increase plane number to 3, since some YUV format [1] are\n> fully planar (and thus have 3 planes).\n>\n> [1] https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html#fully-planar-yuv-formats\n> Signed-off-by: Mattijs Korpershoek \n> ---\n> src/android/yuv/post_processor_yuv.cpp | 26 +++++++++++++++-----------\n> src/android/yuv/post_processor_yuv.h | 12 ++++++++----\n> 2 files changed, 23 insertions(+), 15 deletions(-)\n>\n> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp\n> index d58090db14ee..734bb85b7351 100644\n> --- a/src/android/yuv/post_processor_yuv.cpp\n> +++ b/src/android/yuv/post_processor_yuv.cpp\n> @@ -90,18 +90,18 @@ void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n> bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n> \t\t\t\t const CameraBuffer &destination) const\n> {\n> -\tif (source.planes().size() != 2) {\n> +\tif (source.planes().size() != sourceNumPlanes_) {\n> \t\tLOG(YUV, Error) << \"Invalid number of source planes: \"\n> \t\t\t\t<< source.planes().size();\n> \t\treturn false;\n> \t}\n> -\tif (destination.numPlanes() != 2) {\n> +\tif (destination.numPlanes() != destinationNumPlanes_) {\n> \t\tLOG(YUV, Error) << \"Invalid number of destination planes: \"\n> \t\t\t\t<< destination.numPlanes();\n> \t\treturn false;\n> \t}\n>\n> -\tfor (unsigned int i = 0; i < 2; i++) {\n> +\tfor (unsigned int i = 0; i < sourceNumPlanes_; i++) {\n> \t\tif (source.planes()[i].length < sourceLength_[i]) {\n> \t\t\tLOG(YUV, Error)\n> \t\t\t\t<< \"The source planes lengths are too small, \"\n> @@ -112,7 +112,7 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n> \t\t\treturn false;\n> \t\t}\n> \t}\n> -\tfor (unsigned int i = 0; i < 2; i++) {\n> +\tfor (unsigned int i = 0; i < destinationNumPlanes_; i++) {\n> \t\tif (destination.plane(i).size() < destinationLength_[i]) {\n> \t\t\tLOG(YUV, Error)\n> \t\t\t\t<< \"The destination planes lengths are too small, \"\n> @@ -132,18 +132,22 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,\n> {\n> \tsourceSize_ = inCfg.size;\n> \tdestinationSize_ = outCfg.size;\n> +\tsourceFormat_ = inCfg.pixelFormat;\n> +\tdestinationFormat_ = outCfg.pixelFormat;\n>\n> -\tconst PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);\n> -\tfor (unsigned int i = 0; i < 2; i++) {\n> +\tconst PixelFormatInfo &sourceInfo = PixelFormatInfo::info(sourceFormat_);\n> +\tsourceNumPlanes_ = sourceInfo.numPlanes();\n> +\tfor (unsigned int i = 0; i < sourceInfo.numPlanes(); i++) {\n\nCan't you use the just assigned sourceNumPlanes_ ?\n\n> \t\tsourceStride_[i] = inCfg.stride;\n> \t\tsourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,\n> \t\t\t\t\t\t\tsourceStride_[i]);\n> \t}\n>\n> -\tconst PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);\n> -\tfor (unsigned int i = 0; i < 2; i++) {\n> -\t\tdestinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);\n> -\t\tdestinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,\n> -\t\t\t\t\t\t\t destinationStride_[i]);\n> +\tconst PixelFormatInfo &destinationInfo = PixelFormatInfo::info(destinationFormat_);\n> +\tdestinationNumPlanes_ = destinationInfo.numPlanes();\n> +\tfor (unsigned int i = 0; i < destinationInfo.numPlanes(); i++) {\n\nditto\n\n> +\t\tdestinationStride_[i] = destinationInfo.stride(destinationSize_.width, i, 1);\n> +\t\tdestinationLength_[i] = destinationInfo.planeSize(destinationSize_.height, i,\n> +\t\t\t\t\t\t\t\t destinationStride_[i]);\n> \t}\n> }\n> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h\n> index a7ac17c564b6..bfe35f46c6dc 100644\n> --- a/src/android/yuv/post_processor_yuv.h\n> +++ b/src/android/yuv/post_processor_yuv.h\n> @@ -28,8 +28,12 @@ private:\n>\n> \tlibcamera::Size sourceSize_;\n> \tlibcamera::Size destinationSize_;\n> -\tunsigned int sourceLength_[2] = {};\n> -\tunsigned int destinationLength_[2] = {};\n> -\tunsigned int sourceStride_[2] = {};\n> -\tunsigned int destinationStride_[2] = {};\n> +\tlibcamera::PixelFormat sourceFormat_;\n> +\tlibcamera::PixelFormat destinationFormat_;\n> +\tunsigned int sourceLength_[3] = {};\n> +\tunsigned int destinationLength_[3] = {};\n> +\tunsigned int sourceStride_[3] = {};\n> +\tunsigned int destinationStride_[3] = {};\n> +\tunsigned int sourceNumPlanes_;\n> +\tunsigned int destinationNumPlanes_;\n\nThe rest looks good.\nReviewed-by: Jacopo Mondi \n\n> };\n>\n> --\n> 2.41.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D0C55BE080\n\tfor ;\n\tFri, 22 Sep 2023 10:58:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D95262931;\n\tFri, 22 Sep 2023 12:58:27 +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 06FB96291F\n\tfor ;\n\tFri, 22 Sep 2023 12:58:25 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC8BF891;\n\tFri, 22 Sep 2023 12:56:46 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695380307;\n\tbh=ev7SkNG6IxLIY8wY/obSXGPy2vBJbaH12Fob4lIJVYw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=X8pcas7nFSyMjmBZ9k6wQgYY15IBz+bN92te0cqeS5920prlKKCFZtrITlkjstOZ5\n\tcIRKZxeLiBXlG8QA5C3WO0J4PwYS+38ozpuIXUPBMAuUPhVt5/11VDdW0BdVDA+h8r\n\tl9uJ9XvBITbuKqwog3AZdSSaMvjOiAS1uVDROpkfjlIk7AaAN9sol577ek87prbCAd\n\tw9yr9OwOwmdrLPgSUrZrLZQWhYv/njRO1zK1vtkpUZU9ZZDbyqv3jTNyar1ChrRlnJ\n\tOQAzDv+j9SLfvTwo+VDHpJJRgXn37m3KATQ60EbJHNS++t1V7nE/mKa0/+edr3SSTB\n\taERVaCa6dWdZQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695380207;\n\tbh=ev7SkNG6IxLIY8wY/obSXGPy2vBJbaH12Fob4lIJVYw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hyaSqvUUtZzVfQFnkS/12BvYaCfTmEutKSXbnZLunTqZcowFBSmyNWsJtU+JkEgw0\n\tGhKsj6J216H6o3WtqxXbSgPBNuRF/ZYLr/lvBP67UgnwACMhl9OIjkivxfSOlt/5N6\n\tKO+UOu1jeKZY8BRbQ5z9y8EDJBCx2kQF7rvFaGXc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hyaSqvUU\"; dkim-atps=neutral","Date":"Fri, 22 Sep 2023 12:58:21 +0200","To":"Mattijs Korpershoek ","Message-ID":"","References":"<20230915-libyuv-convert-v1-0-1e5bcf68adac@baylibre.com>\n\t<20230915-libyuv-convert-v1-3-1e5bcf68adac@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230915-libyuv-convert-v1-3-1e5bcf68adac@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support\n\tfor other pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27853,"web_url":"https://patchwork.libcamera.org/comment/27853/","msgid":"<87cyy7ix2u.fsf@baylibre.com>","date":"2023-09-24T12:18:17","subject":"Re: [libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support\n\tfor other pixel formats","submitter":{"id":153,"url":"https://patchwork.libcamera.org/api/people/153/","name":"Mattijs Korpershoek","email":"mkorpershoek@baylibre.com"},"content":"Hi Jacopo,\n\nThank you for your review.\n\nOn ven., sept. 22, 2023 at 12:58, Jacopo Mondi wrote:\n\n> Hi Mattijs\n>\n> On Fri, Sep 15, 2023 at 09:57:27AM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n>> PostProcessorYuv assumes that both source and destination pixel formats\n>> will always be formats::NV12.\n>> This might change in the future, for example when doing pixel format\n>> conversion via libyuv.\n>>\n>> Add the necessary plumbing so that other formats can be added easily in\n>> the future.\n>>\n>> Also increase plane number to 3, since some YUV format [1] are\n>> fully planar (and thus have 3 planes).\n>>\n>> [1] https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html#fully-planar-yuv-formats\n>> Signed-off-by: Mattijs Korpershoek \n>> ---\n>> src/android/yuv/post_processor_yuv.cpp | 26 +++++++++++++++-----------\n>> src/android/yuv/post_processor_yuv.h | 12 ++++++++----\n>> 2 files changed, 23 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp\n>> index d58090db14ee..734bb85b7351 100644\n>> --- a/src/android/yuv/post_processor_yuv.cpp\n>> +++ b/src/android/yuv/post_processor_yuv.cpp\n>> @@ -90,18 +90,18 @@ void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n>> bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n>> \t\t\t\t const CameraBuffer &destination) const\n>> {\n>> -\tif (source.planes().size() != 2) {\n>> +\tif (source.planes().size() != sourceNumPlanes_) {\n>> \t\tLOG(YUV, Error) << \"Invalid number of source planes: \"\n>> \t\t\t\t<< source.planes().size();\n>> \t\treturn false;\n>> \t}\n>> -\tif (destination.numPlanes() != 2) {\n>> +\tif (destination.numPlanes() != destinationNumPlanes_) {\n>> \t\tLOG(YUV, Error) << \"Invalid number of destination planes: \"\n>> \t\t\t\t<< destination.numPlanes();\n>> \t\treturn false;\n>> \t}\n>>\n>> -\tfor (unsigned int i = 0; i < 2; i++) {\n>> +\tfor (unsigned int i = 0; i < sourceNumPlanes_; i++) {\n>> \t\tif (source.planes()[i].length < sourceLength_[i]) {\n>> \t\t\tLOG(YUV, Error)\n>> \t\t\t\t<< \"The source planes lengths are too small, \"\n>> @@ -112,7 +112,7 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n>> \t\t\treturn false;\n>> \t\t}\n>> \t}\n>> -\tfor (unsigned int i = 0; i < 2; i++) {\n>> +\tfor (unsigned int i = 0; i < destinationNumPlanes_; i++) {\n>> \t\tif (destination.plane(i).size() < destinationLength_[i]) {\n>> \t\t\tLOG(YUV, Error)\n>> \t\t\t\t<< \"The destination planes lengths are too small, \"\n>> @@ -132,18 +132,22 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,\n>> {\n>> \tsourceSize_ = inCfg.size;\n>> \tdestinationSize_ = outCfg.size;\n>> +\tsourceFormat_ = inCfg.pixelFormat;\n>> +\tdestinationFormat_ = outCfg.pixelFormat;\n>>\n>> -\tconst PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);\n>> -\tfor (unsigned int i = 0; i < 2; i++) {\n>> +\tconst PixelFormatInfo &sourceInfo = PixelFormatInfo::info(sourceFormat_);\n>> +\tsourceNumPlanes_ = sourceInfo.numPlanes();\n>> +\tfor (unsigned int i = 0; i < sourceInfo.numPlanes(); i++) {\n>\n> Can't you use the just assigned sourceNumPlanes_ ?\n\nWill do for v2\n\n>\n>> \t\tsourceStride_[i] = inCfg.stride;\n>> \t\tsourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,\n>> \t\t\t\t\t\t\tsourceStride_[i]);\n>> \t}\n>>\n>> -\tconst PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);\n>> -\tfor (unsigned int i = 0; i < 2; i++) {\n>> -\t\tdestinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);\n>> -\t\tdestinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,\n>> -\t\t\t\t\t\t\t destinationStride_[i]);\n>> +\tconst PixelFormatInfo &destinationInfo = PixelFormatInfo::info(destinationFormat_);\n>> +\tdestinationNumPlanes_ = destinationInfo.numPlanes();\n>> +\tfor (unsigned int i = 0; i < destinationInfo.numPlanes(); i++) {\n>\n> ditto\n\nWill do for v2\n\n>\n>> +\t\tdestinationStride_[i] = destinationInfo.stride(destinationSize_.width, i, 1);\n>> +\t\tdestinationLength_[i] = destinationInfo.planeSize(destinationSize_.height, i,\n>> +\t\t\t\t\t\t\t\t destinationStride_[i]);\n>> \t}\n>> }\n>> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h\n>> index a7ac17c564b6..bfe35f46c6dc 100644\n>> --- a/src/android/yuv/post_processor_yuv.h\n>> +++ b/src/android/yuv/post_processor_yuv.h\n>> @@ -28,8 +28,12 @@ private:\n>>\n>> \tlibcamera::Size sourceSize_;\n>> \tlibcamera::Size destinationSize_;\n>> -\tunsigned int sourceLength_[2] = {};\n>> -\tunsigned int destinationLength_[2] = {};\n>> -\tunsigned int sourceStride_[2] = {};\n>> -\tunsigned int destinationStride_[2] = {};\n>> +\tlibcamera::PixelFormat sourceFormat_;\n>> +\tlibcamera::PixelFormat destinationFormat_;\n>> +\tunsigned int sourceLength_[3] = {};\n>> +\tunsigned int destinationLength_[3] = {};\n>> +\tunsigned int sourceStride_[3] = {};\n>> +\tunsigned int destinationStride_[3] = {};\n>> +\tunsigned int sourceNumPlanes_;\n>> +\tunsigned int destinationNumPlanes_;\n>\n> The rest looks good.\n> Reviewed-by: Jacopo Mondi \n\nWill apply your tag\n\n>\n>> };\n>>\n>> --\n>> 2.41.0\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BF7ECC326B\n\tfor ;\n\tSun, 24 Sep 2023 12:18:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70C8562944;\n\tSun, 24 Sep 2023 14:18:22 +0200 (CEST)","from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com\n\t[IPv6:2a00:1450:4864:20::52c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A71F162916\n\tfor ;\n\tSun, 24 Sep 2023 14:18:20 +0200 (CEST)","by mail-ed1-x52c.google.com with SMTP id\n\t4fb4d7f45d1cf-51e28cac164so13539472a12.1\n\tfor ;\n\tSun, 24 Sep 2023 05:18:20 -0700 (PDT)","from localhost ([89.207.171.157]) by smtp.gmail.com with ESMTPSA id\n\tp6-20020a170906228600b009adc743340fsm4958259eja.197.2023.09.24.05.18.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 24 Sep 2023 05:18:19 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695557902;\n\tbh=TP9+M/mZITJqU3ypvhF5BQK+mGb18CH+x4E4hV/QsWg=;\n\th=To:In-Reply-To:References:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ywLaNfyLgrHruOxoEF99Ff0p6YFLmCQKgKfp0bvMIr1qOs9TmHcqhfAkZv27wv7fz\n\ta6T3GCYF1HLETeBi47vDTYq6KAVm6asgZJKaiRxp7sAPNBfEqVJZK3B+mxFihB/rxf\n\tvJJfK19Kfl+Vbq6qYdzRmqusw3HPqhdyS/g8yu/8e1SZXyi4rEdNThe9r2sGWkozZN\n\tBHRlwQ76V3dJh3he5B5/1IweX4qas23/3gpi21jtcewxS3LWrvI3+nHpqsS8aViiBA\n\ts4IAqEE0DOmzxCpX3zp7pVwR2R0cTMKLt182C1HhMNbtcwINwpFqit1ynSyTHGbSqw\n\tJI8oWj6z9vjGA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1695557900;\n\tx=1696162700; darn=lists.libcamera.org; \n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:from:to:cc:subject:date:message-id:reply-to;\n\tbh=CiA3e2vWFFx0gluZSNIIvgbD32zvbRtprWIrCFIgdpw=;\n\tb=fFu2C52e4GY5TfRNWZ43Y630N/+ateCyLbPpeAm9ihqSVCTmcKK0zweIzGSlc2Dpw+\n\tOTam+Xsc7JLAMThNzJgUf/1bU7RfOq8AdLTDCoN+VlWcK8jDZ6RNE0eygrXzKHzakIpf\n\t8QYwDXHINn/fE5rjSwoDyGdAL07YJ9dPlZMqOuq58kukPvm+kFDdqmVy0L7S+A5/FljX\n\tMsC/lt5u/NOExNB07rRV0Fbf+mScR+8FOtMlOnifEj7iUhh6VQq2kzaYZ+Br+f+Skt9x\n\tM0uFpk00N6jBb4M4MKLBvJlMPRrqC87rMBGvcwukLHASHO8LzhDYK24S69fH66889Gci\n\tSINQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=baylibre-com.20230601.gappssmtp.com\n\theader.i=@baylibre-com.20230601.gappssmtp.com header.b=\"fFu2C52e\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695557900; x=1696162700;\n\th=mime-version:message-id:date:references:in-reply-to:subject:cc:to\n\t:from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; \n\tbh=CiA3e2vWFFx0gluZSNIIvgbD32zvbRtprWIrCFIgdpw=;\n\tb=sy3cI/dhKTJfuC6xIiLkrMyKbZUBbRtV0fpn9zog7o9hXoDQtbSC2Yj+X3ZnYPhDwy\n\tYdtQaTbkp+ukXmip1DmZ2q3WluCKPWSBW8MMw5VX0yaIrhHqsQVDS85257T8F9b0+hdW\n\tN14BdMQUayE5mOSSfUZCx+kiv6ztXuih9mK0QPrcHCMGgJC8yAICy7zLbTrox+dXgnGc\n\tHB4DEHhR/nqO+jNRFbx1MuZf4gmcyWIitO3hmqjg1JB6ZPYsZzHxgKTpz7REgmt+FeGS\n\tj8zp1oq7sHqGIdxthVvSQlWrWgIWiQWWVajGdtCaJBftEaLTp9ycdG5QNMGGQIwk69fB\n\tS2Hg==","X-Gm-Message-State":"AOJu0YwO9C+xWZPe6BT9DHTHp331AvhimLO79jpdfZb0r/BdFWEmUDy7\n\trqqmIwrtY1lKiDA/YdCtH9ZHdw==","X-Google-Smtp-Source":"AGHT+IFE8GFRDPtSxJYHHbd1FwXdk7ipbEucZt7tPVkzIq11mBCgfzD2pPZ++tqMoWUBPOjeG52mow==","X-Received":"by 2002:a17:906:2099:b0:9a5:9f3c:961e with SMTP id\n\t25-20020a170906209900b009a59f3c961emr6604602ejq.18.1695557900129; \n\tSun, 24 Sep 2023 05:18:20 -0700 (PDT)","To":"Jacopo Mondi ","In-Reply-To":"","References":"<20230915-libyuv-convert-v1-0-1e5bcf68adac@baylibre.com>\n\t<20230915-libyuv-convert-v1-3-1e5bcf68adac@baylibre.com>\n\t","Date":"Sun, 24 Sep 2023 14:18:17 +0200","Message-ID":"<87cyy7ix2u.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support\n\tfor other pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Mattijs Korpershoek via libcamera-devel\n\t","Reply-To":"Mattijs Korpershoek ","Cc":"libcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27859,"web_url":"https://patchwork.libcamera.org/comment/27859/","msgid":"<20230924133329.GR19112@pendragon.ideasonboard.com>","date":"2023-09-24T13:33:29","subject":"Re: [libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support\n\tfor other pixel formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Sep 24, 2023 at 02:18:17PM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> On ven., sept. 22, 2023 at 12:58, Jacopo Mondi wrote:\n> > On Fri, Sep 15, 2023 at 09:57:27AM +0200, Mattijs Korpershoek via libcamera-devel wrote:\n> >> PostProcessorYuv assumes that both source and destination pixel formats\n> >> will always be formats::NV12.\n> >> This might change in the future, for example when doing pixel format\n\ns/might/will/\ns/for example //\n\n> >> conversion via libyuv.\n\nIf you intend to have two separate paragraphs, you need a blank line\nbetween the first and second sentence. Otherwise (which I think is what\nyou intended here), you shouldn't have a line break after the first\nsentence.\n\n> >>\n> >> Add the necessary plumbing so that other formats can be added easily in\n> >> the future.\n> >>\n> >> Also increase plane number to 3, since some YUV format [1] are\n> >> fully planar (and thus have 3 planes).\n> >>\n> >> [1] https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html#fully-planar-yuv-formats\n> >> Signed-off-by: Mattijs Korpershoek \n> >> ---\n> >> src/android/yuv/post_processor_yuv.cpp | 26 +++++++++++++++-----------\n> >> src/android/yuv/post_processor_yuv.h | 12 ++++++++----\n> >> 2 files changed, 23 insertions(+), 15 deletions(-)\n> >>\n> >> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp\n> >> index d58090db14ee..734bb85b7351 100644\n> >> --- a/src/android/yuv/post_processor_yuv.cpp\n> >> +++ b/src/android/yuv/post_processor_yuv.cpp\n> >> @@ -90,18 +90,18 @@ void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n> >> bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n> >> \t\t\t\t const CameraBuffer &destination) const\n> >> {\n> >> -\tif (source.planes().size() != 2) {\n> >> +\tif (source.planes().size() != sourceNumPlanes_) {\n> >> \t\tLOG(YUV, Error) << \"Invalid number of source planes: \"\n> >> \t\t\t\t<< source.planes().size();\n> >> \t\treturn false;\n> >> \t}\n> >> -\tif (destination.numPlanes() != 2) {\n> >> +\tif (destination.numPlanes() != destinationNumPlanes_) {\n> >> \t\tLOG(YUV, Error) << \"Invalid number of destination planes: \"\n> >> \t\t\t\t<< destination.numPlanes();\n> >> \t\treturn false;\n> >> \t}\n> >>\n> >> -\tfor (unsigned int i = 0; i < 2; i++) {\n> >> +\tfor (unsigned int i = 0; i < sourceNumPlanes_; i++) {\n> >> \t\tif (source.planes()[i].length < sourceLength_[i]) {\n> >> \t\t\tLOG(YUV, Error)\n> >> \t\t\t\t<< \"The source planes lengths are too small, \"\n> >> @@ -112,7 +112,7 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n> >> \t\t\treturn false;\n> >> \t\t}\n> >> \t}\n> >> -\tfor (unsigned int i = 0; i < 2; i++) {\n> >> +\tfor (unsigned int i = 0; i < destinationNumPlanes_; i++) {\n> >> \t\tif (destination.plane(i).size() < destinationLength_[i]) {\n> >> \t\t\tLOG(YUV, Error)\n> >> \t\t\t\t<< \"The destination planes lengths are too small, \"\n> >> @@ -132,18 +132,22 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,\n> >> {\n> >> \tsourceSize_ = inCfg.size;\n> >> \tdestinationSize_ = outCfg.size;\n> >> +\tsourceFormat_ = inCfg.pixelFormat;\n> >> +\tdestinationFormat_ = outCfg.pixelFormat;\n> >>\n> >> -\tconst PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);\n> >> -\tfor (unsigned int i = 0; i < 2; i++) {\n> >> +\tconst PixelFormatInfo &sourceInfo = PixelFormatInfo::info(sourceFormat_);\n> >> +\tsourceNumPlanes_ = sourceInfo.numPlanes();\n> >> +\tfor (unsigned int i = 0; i < sourceInfo.numPlanes(); i++) {\n> >\n> > Can't you use the just assigned sourceNumPlanes_ ?\n> \n> Will do for v2\n> \n> >> \t\tsourceStride_[i] = inCfg.stride;\n> >> \t\tsourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,\n> >> \t\t\t\t\t\t\tsourceStride_[i]);\n> >> \t}\n> >>\n> >> -\tconst PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);\n> >> -\tfor (unsigned int i = 0; i < 2; i++) {\n> >> -\t\tdestinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);\n> >> -\t\tdestinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,\n> >> -\t\t\t\t\t\t\t destinationStride_[i]);\n> >> +\tconst PixelFormatInfo &destinationInfo = PixelFormatInfo::info(destinationFormat_);\n> >> +\tdestinationNumPlanes_ = destinationInfo.numPlanes();\n> >> +\tfor (unsigned int i = 0; i < destinationInfo.numPlanes(); i++) {\n> >\n> > ditto\n> \n> Will do for v2\n> \n> >> +\t\tdestinationStride_[i] = destinationInfo.stride(destinationSize_.width, i, 1);\n> >> +\t\tdestinationLength_[i] = destinationInfo.planeSize(destinationSize_.height, i,\n> >> +\t\t\t\t\t\t\t\t destinationStride_[i]);\n> >> \t}\n> >> }\n> >> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h\n> >> index a7ac17c564b6..bfe35f46c6dc 100644\n> >> --- a/src/android/yuv/post_processor_yuv.h\n> >> +++ b/src/android/yuv/post_processor_yuv.h\n> >> @@ -28,8 +28,12 @@ private:\n> >>\n> >> \tlibcamera::Size sourceSize_;\n> >> \tlibcamera::Size destinationSize_;\n> >> -\tunsigned int sourceLength_[2] = {};\n> >> -\tunsigned int destinationLength_[2] = {};\n> >> -\tunsigned int sourceStride_[2] = {};\n> >> -\tunsigned int destinationStride_[2] = {};\n> >> +\tlibcamera::PixelFormat sourceFormat_;\n> >> +\tlibcamera::PixelFormat destinationFormat_;\n> >> +\tunsigned int sourceLength_[3] = {};\n> >> +\tunsigned int destinationLength_[3] = {};\n> >> +\tunsigned int sourceStride_[3] = {};\n> >> +\tunsigned int destinationStride_[3] = {};\n> >> +\tunsigned int sourceNumPlanes_;\n> >> +\tunsigned int destinationNumPlanes_;\n\nYou can use an std::vector to store sourceLength_, and\ndrop the sourceNumPlanes_ variable. Same for the destination.\n\nActually, I would probably make it\n\n\tstruct PlaneLayout {\n\t\tunsigned int length;\n\t\tunsigned int stride;\n\t};\n\n\tstd::vector sourcePlanes_;\n\tstd::vector destinationPlanes_;\n\n(names to be bikeshedded)\n\n> > The rest looks good.\n> > Reviewed-by: Jacopo Mondi \n> \n> Will apply your tag\n> \n> >> };\n> >>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D0B4EC326B\n\tfor ;\n\tSun, 24 Sep 2023 13:33:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 49E0A62944;\n\tSun, 24 Sep 2023 15:33:20 +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 56C7F62916\n\tfor ;\n\tSun, 24 Sep 2023 15:33:18 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9EAE0128D;\n\tSun, 24 Sep 2023 15:31:38 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695562400;\n\tbh=pG9xY9GSRZ4OUwA0FnvnBqO1CEXPYqNphd7nz6/N+YU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=nm0wxm/bEiqRFJ0Rxx2krQupDnl60NzoCXYfDKNwyXPk1GfZt4W/UcFYBV7Fb9EJ/\n\tSXTXc0pkYo9P2BIV/xf7jCiTsTuiShCoXdXlENIdnbuiVv541F9O8NfaRYpIv2fMCa\n\tlKR0PQxjmztggiDHSV8Mon2g7ALix+UWzCEgNyRFInxAGa9LcPI+blN9vmJuOM5isq\n\t/9rl1+/ADu94Q5WU2FI11MmVQWbejdoazlO21u3WOKvRCoRJRrWfwGFG+8aSUkgxpG\n\tXDwBMebp2XqjY6KVUUeIBVzQopqW0Jkyj65jqu2z626wLJ5kZPW6Hp5OIj5toRTL3u\n\tEkEmQK5t8xMWw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695562298;\n\tbh=pG9xY9GSRZ4OUwA0FnvnBqO1CEXPYqNphd7nz6/N+YU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=klXm19sCN0WOxP5Ma8PyUToz+/pjgTiCDsFwK86TtFFCdNoSKo2Q5Uh90Wm9+eBhF\n\tqqLn5VdSqNX/fhynpojsFcQjLjPc5KJEmcdM0IRhiGvifgy/oTO/wVHZ3CZd+vjzVd\n\tMpR4JHxqLLtsYLmaJghIPgJa4pIlAEpyQ7GZAgAM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"klXm19sC\"; dkim-atps=neutral","Date":"Sun, 24 Sep 2023 16:33:29 +0300","To":"Mattijs Korpershoek ","Message-ID":"<20230924133329.GR19112@pendragon.ideasonboard.com>","References":"<20230915-libyuv-convert-v1-0-1e5bcf68adac@baylibre.com>\n\t<20230915-libyuv-convert-v1-3-1e5bcf68adac@baylibre.com>\n\t\n\t<87cyy7ix2u.fsf@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87cyy7ix2u.fsf@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support\n\tfor other pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org,\n\tGuillaume La Roque ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]