[{"id":30573,"web_url":"https://patchwork.libcamera.org/comment/30573/","msgid":"<172284791414.2725865.10320885849259481853@ping.linuxembedded.co.uk>","date":"2024-08-05T08:51:54","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-08-04 14:36:05)\n> libcamera is implemented in C++, use std::vector<> to manage the\n> dynamically allocated line buffers instead of malloc() and free(). This\n> simplifies the code and improves memory safety by ensuring no allocation\n> will be leaked.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n> \n> - Use std::vector instead of new[]() and delete[]()\n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n> 2 files changed, 13 insertions(+), 20 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index f8d2677d657a..077f7f4bc45b 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> /* Initialize color lookup tables */\n> for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n> red_[i] = green_[i] = blue_[i] = i;\n> -\n> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> - lineBuffers_[i] = nullptr;\n> }\n> \n> -DebayerCpu::~DebayerCpu()\n> -{\n> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> - free(lineBuffers_[i]);\n> -}\n> +DebayerCpu::~DebayerCpu() = default;\n> \n> #define DECLARE_SRC_POINTERS(pixel_t) \\\n> const pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> lineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> lineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n> 2 * lineBufferPadding_;\n> - for (unsigned int i = 0;\n> - i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> - i++) {\n> - free(lineBuffers_[i]);\n> - lineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n> - if (!lineBuffers_[i])\n> - return -ENOMEM;\n> +\n> + if (enableInputMemcpy_) {\n> + for (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n> + lineBuffers_[i].resize(lineBufferLength_);\n\nthat's easier.\n\nI don't think the previous implementation was doing it, but should any\nfollowing lines be resized to 0 if the configuration shrinks?\n\nWorst case, this could grow to full allocation, then the later lines\nwould remain allocated indefinitely on any system with a long running\ncamera manager (i.e. pipewire). The memory wouldn't be lost, as it would\nbe released when the camera manager is destroyed, but the memory\nwouldn't be usable...\n\nCould be a patch on top as the previous implementation didn't do this\nand it's just a conversion patch here.\n\nSo I think this patch still steps in a good way forwards:\n\n\nReviewed-by: Kieran Bingham \n\n\n> }\n> \n> measuredFrames_ = 0;\n> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n> return;\n> \n> for (unsigned int i = 0; i < patternHeight; i++) {\n> - memcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n> + memcpy(lineBuffers_[i].data(),\n> + linePointers[i + 1] - lineBufferPadding_,\n> lineBufferLength_);\n> - linePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n> + linePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n> }\n> \n> /* Point lineBufferIndex_ to first unused lineBuffer */\n> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n> if (!enableInputMemcpy_)\n> return;\n> \n> - memcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n> + memcpy(lineBuffers_[lineBufferIndex_].data(),\n> + linePointers[patternHeight] - lineBufferPadding_,\n> lineBufferLength_);\n> - linePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n> + linePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n> + + lineBufferPadding_;\n> \n> lineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> }\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 1dac64351f1d..8237a64be733 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -146,7 +146,7 @@ private:\n> DebayerInputConfig inputConfig_;\n> DebayerOutputConfig outputConfig_;\n> std::unique_ptr stats_;\n> - uint8_t *lineBuffers_[kMaxLineBuffers];\n> + std::vector lineBuffers_[kMaxLineBuffers];\n> unsigned int lineBufferLength_;\n> unsigned int lineBufferPadding_;\n> unsigned int lineBufferIndex_;\n> \n> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 E8533BE173\n\tfor ;\n\tMon, 5 Aug 2024 08:52:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EFEBF63380;\n\tMon, 5 Aug 2024 10:51:59 +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 8F13E6191F\n\tfor ;\n\tMon, 5 Aug 2024 10:51:57 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 25528581;\n\tMon, 5 Aug 2024 10:51:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gIBb9fdK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722847866;\n\tbh=P034uEybB666qpv61EYwNzNbG+GXgEMhieLnQdbmGKU=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=gIBb9fdKboUjX8V/p0YZzmvhH3aO9eSClLN7TNJrdQ/zWWHTzVwditkUxVDBq81i3\n\tdnH3bKtugoLtxbDu4Hq25JEpXtQ4GCWHAxvhCS4xPnmC6SHNH/k/3D9HrX6656oEXq\n\tKY7Pjz9hMo4AYr2rMd0mxiEL7GZvxJNVHWRpgZUQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 05 Aug 2024 09:51:54 +0100","Message-ID":"<172284791414.2725865.10320885849259481853@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30580,"web_url":"https://patchwork.libcamera.org/comment/30580/","msgid":"","date":"2024-08-05T10:22:02","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 8/4/24 3:36 PM, Laurent Pinchart wrote:\n> libcamera is implemented in C++, use std::vector<> to manage the\n> dynamically allocated line buffers instead of malloc() and free(). This\n> simplifies the code and improves memory safety by ensuring no allocation\n> will be leaked.\n> \n> Signed-off-by: Laurent Pinchart \n\nI have given this a spin with IPU6 + the softISP and everything still works:\n\nTested-by: Hans de Goede \n\nRegards,\n\nHans\n\np.s.\n\nI do see an *unrelated* performance regression where the softISP processing\ntime for 1920x1080 goes up from 3 ms / frame to 6 ms / frame. 1 ms of that\nis caused by the new 32bpp RGB sparse output support which qcam prefers\nover the previous 24bpp packed support forcing 24 bpp packed output fmt\nresolves the 1 ms difference which is expected since 32 bpp sparse writes\n33% more data. But that still leaves 2 ms unaccounted for.\n\nI'll try to dig into where those 2 ms are coming from. With moderen CPU\nturboing behavior it is hard to tell really. Might even be a kernel issue...\n\n\n\n\n\n\n> ---\n> Changes since v1:\n> \n> - Use std::vector instead of new[]() and delete[]()\n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n> 2 files changed, 13 insertions(+), 20 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index f8d2677d657a..077f7f4bc45b 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> \t/* Initialize color lookup tables */\n> \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n> \t\tred_[i] = green_[i] = blue_[i] = i;\n> -\n> -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> -\t\tlineBuffers_[i] = nullptr;\n> }\n> \n> -DebayerCpu::~DebayerCpu()\n> -{\n> -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> -\t\tfree(lineBuffers_[i]);\n> -}\n> +DebayerCpu::~DebayerCpu() = default;\n> \n> #define DECLARE_SRC_POINTERS(pixel_t) \\\n> \tconst pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> \tlineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> \tlineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n> \t\t\t 2 * lineBufferPadding_;\n> -\tfor (unsigned int i = 0;\n> -\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> -\t i++) {\n> -\t\tfree(lineBuffers_[i]);\n> -\t\tlineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n> -\t\tif (!lineBuffers_[i])\n> -\t\t\treturn -ENOMEM;\n> +\n> +\tif (enableInputMemcpy_) {\n> +\t\tfor (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n> +\t\t\tlineBuffers_[i].resize(lineBufferLength_);\n> \t}\n> \n> \tmeasuredFrames_ = 0;\n> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n> \t\treturn;\n> \n> \tfor (unsigned int i = 0; i < patternHeight; i++) {\n> -\t\tmemcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n> +\t\tmemcpy(lineBuffers_[i].data(),\n> +\t\t linePointers[i + 1] - lineBufferPadding_,\n> \t\t lineBufferLength_);\n> -\t\tlinePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n> +\t\tlinePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n> \t}\n> \n> \t/* Point lineBufferIndex_ to first unused lineBuffer */\n> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n> \tif (!enableInputMemcpy_)\n> \t\treturn;\n> \n> -\tmemcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n> +\tmemcpy(lineBuffers_[lineBufferIndex_].data(),\n> +\t linePointers[patternHeight] - lineBufferPadding_,\n> \t lineBufferLength_);\n> -\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n> +\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n> +\t\t\t\t + lineBufferPadding_;\n> \n> \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> }\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 1dac64351f1d..8237a64be733 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -146,7 +146,7 @@ private:\n> \tDebayerInputConfig inputConfig_;\n> \tDebayerOutputConfig outputConfig_;\n> \tstd::unique_ptr stats_;\n> -\tuint8_t *lineBuffers_[kMaxLineBuffers];\n> +\tstd::vector lineBuffers_[kMaxLineBuffers];\n> \tunsigned int lineBufferLength_;\n> \tunsigned int lineBufferPadding_;\n> \tunsigned int lineBufferIndex_;\n> \n> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5","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 1EFF7BE173\n\tfor ;\n\tMon, 5 Aug 2024 10:22:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A48963369;\n\tMon, 5 Aug 2024 12:22:10 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 582FB6195D\n\tfor ;\n\tMon, 5 Aug 2024 12:22:08 +0200 (CEST)","from mail-ed1-f70.google.com (mail-ed1-f70.google.com\n\t[209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-446-iG4tMlV5MiiFSQDDD2hYXw-1; Mon, 05 Aug 2024 06:22:06 -0400","by mail-ed1-f70.google.com with SMTP id\n\t4fb4d7f45d1cf-5a7c6a0f440so6818308a12.0\n\tfor ;\n\tMon, 05 Aug 2024 03:22:05 -0700 (PDT)","from [10.40.98.157] ([78.108.130.194])\n\tby smtp.gmail.com with ESMTPSA id\n\t4fb4d7f45d1cf-5b83b82c068sm4691923a12.60.2024.08.05.03.22.03\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 05 Aug 2024 03:22:03 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"IIn806Z+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722853327;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=jvsqalO5humhn+qzMhRmMoblhgJ8mvovFyUpUGU1h+c=;\n\tb=IIn806Z+yCpTULfTWxmHOJGNpdfd79K6l5PDpU7It+tesRYeLCRenzpGaLCQphcPvy8ceb\n\tIM699fDrlgO+sjixyxYeRqRppR4m63tFxeGSyg6dTREu5XRPacmvijWYzzeaxJ1mr2xIn8\n\tORSPNYeH65AHAzXL/B0PIANb4cZHy7o=","X-MC-Unique":"iG4tMlV5MiiFSQDDD2hYXw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722853324; x=1723458124;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=jvsqalO5humhn+qzMhRmMoblhgJ8mvovFyUpUGU1h+c=;\n\tb=bz9ZSQchCjTBXP/QvK+/09JnDOlbIcyN3aREyLA83ak4y7pMvXzmhnPlcnogU9rOyc\n\tJQEh5SIgFID0hw8tcb8rwJj15mGrp4KAKeDkOZoDggALhCBJ0VdrE2kBYNcbzvs2NhRJ\n\t1GhE0oRyU5ESZFPPzonqMiZh7pOVOgUyYnQC5gFSrPqt/hknQV3ad98DMVBcIMi0Lzhd\n\tq5JYwODHQnnKBuAQilU3cAW78nVAEl5Z/Grll4Zjnw+hwGZaQQ6LpuF5M+fJhCMMfRJk\n\ttQxMhS88AWrJCFXdotIvxiu4yIEFH0MoKp3yLfeMzYtpOuJtvbrbcLQu53LvEMBIJDpX\n\tR+eQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVw4OgvPxzqVHoCWmUUc8HBlo10rusRqtphH+S2TZgK82AbsMmwN9fYrY7zjpZLnutTrU1cNKbS5thMBiNU+r32Wqav+vhR+XRgtQj1pJix+u409A==","X-Gm-Message-State":"AOJu0Yy8QPlxR7SgKGNmAHe7S5HbnyiFb7i1AVB2hhkhjb6VtIdS8cPt\n\ty6C8/5BlU+5OXcS1wUWqRcUje1Gx4KznmfbaUIKxHhC36+hksul/sEd7+VsSYQjtjWdrcC4afp0\n\thCfruxQq27JNUQqZ99lpV3nESBGf1B3tSopcPtRK3Z97QUy3zY2Cy7rIBLu6tjeMAPMCSz335Dm\n\tqbuxA=","X-Received":["by 2002:a50:e602:0:b0:5a1:faf:e5ac with SMTP id\n\t4fb4d7f45d1cf-5b7f540abb6mr7800754a12.26.1722853324387; \n\tMon, 05 Aug 2024 03:22:04 -0700 (PDT)","by 2002:a50:e602:0:b0:5a1:faf:e5ac with SMTP id\n\t4fb4d7f45d1cf-5b7f540abb6mr7800733a12.26.1722853323857; \n\tMon, 05 Aug 2024 03:22:03 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IH6YBL3NVwHU1Sot/iZ7Uh6/YLJZuxh6ae9r02QFYEAHkf9SUmcu63TyaNW07YlXZnsYFLC8w==","Message-ID":"","Date":"Mon, 5 Aug 2024 12:22:02 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>","From":"Hans de Goede ","In-Reply-To":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30581,"web_url":"https://patchwork.libcamera.org/comment/30581/","msgid":"<20240805103259.GB21286@pendragon.ideasonboard.com>","date":"2024-08-05T10:32:59","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hans,\n\nOn Mon, Aug 05, 2024 at 12:22:02PM +0200, Hans de Goede wrote:\n> On 8/4/24 3:36 PM, Laurent Pinchart wrote:\n> > libcamera is implemented in C++, use std::vector<> to manage the\n> > dynamically allocated line buffers instead of malloc() and free(). This\n> > simplifies the code and improves memory safety by ensuring no allocation\n> > will be leaked.\n> > \n> > Signed-off-by: Laurent Pinchart \n> \n> I have given this a spin with IPU6 + the softISP and everything still works:\n> \n> Tested-by: Hans de Goede \n\nThank you.\n\n> p.s.\n> \n> I do see an *unrelated* performance regression where the softISP processing\n> time for 1920x1080 goes up from 3 ms / frame to 6 ms / frame. 1 ms of that\n> is caused by the new 32bpp RGB sparse output support which qcam prefers\n> over the previous 24bpp packed support forcing 24 bpp packed output fmt\n> resolves the 1 ms difference which is expected since 32 bpp sparse writes\n> 33% more data. But that still leaves 2 ms unaccounted for.\n> \n> I'll try to dig into where those 2 ms are coming from. With moderen CPU\n> turboing behavior it is hard to tell really. Might even be a kernel issue...\n\nIs that a regression introduced by this patch, or a regression between a\nparticular point in the past and the last master branch ?\n\n> > ---\n> > Changes since v1:\n> > \n> > - Use std::vector instead of new[]() and delete[]()\n> > ---\n> > src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n> > src/libcamera/software_isp/debayer_cpu.h | 2 +-\n> > 2 files changed, 13 insertions(+), 20 deletions(-)\n> > \n> > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> > index f8d2677d657a..077f7f4bc45b 100644\n> > --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> > +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> > @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> > \t/* Initialize color lookup tables */\n> > \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n> > \t\tred_[i] = green_[i] = blue_[i] = i;\n> > -\n> > -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> > -\t\tlineBuffers_[i] = nullptr;\n> > }\n> > \n> > -DebayerCpu::~DebayerCpu()\n> > -{\n> > -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> > -\t\tfree(lineBuffers_[i]);\n> > -}\n> > +DebayerCpu::~DebayerCpu() = default;\n> > \n> > #define DECLARE_SRC_POINTERS(pixel_t) \\\n> > \tconst pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n> > @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> > \tlineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> > \tlineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n> > \t\t\t 2 * lineBufferPadding_;\n> > -\tfor (unsigned int i = 0;\n> > -\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> > -\t i++) {\n> > -\t\tfree(lineBuffers_[i]);\n> > -\t\tlineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n> > -\t\tif (!lineBuffers_[i])\n> > -\t\t\treturn -ENOMEM;\n> > +\n> > +\tif (enableInputMemcpy_) {\n> > +\t\tfor (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n> > +\t\t\tlineBuffers_[i].resize(lineBufferLength_);\n> > \t}\n> > \n> > \tmeasuredFrames_ = 0;\n> > @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n> > \t\treturn;\n> > \n> > \tfor (unsigned int i = 0; i < patternHeight; i++) {\n> > -\t\tmemcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n> > +\t\tmemcpy(lineBuffers_[i].data(),\n> > +\t\t linePointers[i + 1] - lineBufferPadding_,\n> > \t\t lineBufferLength_);\n> > -\t\tlinePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n> > +\t\tlinePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n> > \t}\n> > \n> > \t/* Point lineBufferIndex_ to first unused lineBuffer */\n> > @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n> > \tif (!enableInputMemcpy_)\n> > \t\treturn;\n> > \n> > -\tmemcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n> > +\tmemcpy(lineBuffers_[lineBufferIndex_].data(),\n> > +\t linePointers[patternHeight] - lineBufferPadding_,\n> > \t lineBufferLength_);\n> > -\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n> > +\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n> > +\t\t\t\t + lineBufferPadding_;\n> > \n> > \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> > }\n> > diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> > index 1dac64351f1d..8237a64be733 100644\n> > --- a/src/libcamera/software_isp/debayer_cpu.h\n> > +++ b/src/libcamera/software_isp/debayer_cpu.h\n> > @@ -146,7 +146,7 @@ private:\n> > \tDebayerInputConfig inputConfig_;\n> > \tDebayerOutputConfig outputConfig_;\n> > \tstd::unique_ptr stats_;\n> > -\tuint8_t *lineBuffers_[kMaxLineBuffers];\n> > +\tstd::vector lineBuffers_[kMaxLineBuffers];\n> > \tunsigned int lineBufferLength_;\n> > \tunsigned int lineBufferPadding_;\n> > \tunsigned int lineBufferIndex_;\n> > \n> > base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5","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 D8C0AC323E\n\tfor ;\n\tMon, 5 Aug 2024 10:33:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF93563382;\n\tMon, 5 Aug 2024 12:33:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B6B8E6195D\n\tfor ;\n\tMon, 5 Aug 2024 12:33:21 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0473D1A2;\n\tMon, 5 Aug 2024 12:32:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uBYZLadR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722853950;\n\tbh=O5sA8KFrJn09TJjeXK7Io/Rw6ZUD8mYCPiDyXS7ZOsg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uBYZLadRifJoNSl6Vj6YGxEcmi/Qx4qfuoi6BaZs9TmRwoVTlEZuw2XjalwNbfTv0\n\tg7J4TiTkk64lXuLHoTgrz/jx4ZqvT3+ZF98bAsh7Ad9y1GtyWMMZGpoq7J7oh8I+HE\n\tVpSlPkgX/+UtuwK6podobie8UggZjl1M2pw4EFeE=","Date":"Mon, 5 Aug 2024 13:32:59 +0300","From":"Laurent Pinchart ","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","Message-ID":"<20240805103259.GB21286@pendragon.ideasonboard.com>","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30582,"web_url":"https://patchwork.libcamera.org/comment/30582/","msgid":"<877ccvp88y.fsf@redhat.com>","date":"2024-08-05T10:35:41","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for the patch.\n\nLaurent Pinchart writes:\n\n> libcamera is implemented in C++, use std::vector<> to manage the\n> dynamically allocated line buffers instead of malloc() and free(). This\n> simplifies the code and improves memory safety by ensuring no allocation\n> will be leaked.\n>\n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Milan Zamazal \n\n(I thought that malloc was used here to be able to detect allocation\nfailures when exceptions are not used but it's probably not an issue\nwith total size of line buffers not being much more than ~100 kB\nusually).\n\n> ---\n> Changes since v1:\n>\n> - Use std::vector instead of new[]() and delete[]()\n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n> 2 files changed, 13 insertions(+), 20 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index f8d2677d657a..077f7f4bc45b 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> \t/* Initialize color lookup tables */\n> \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n> \t\tred_[i] = green_[i] = blue_[i] = i;\n> -\n> -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> -\t\tlineBuffers_[i] = nullptr;\n> }\n> \n> -DebayerCpu::~DebayerCpu()\n> -{\n> -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> -\t\tfree(lineBuffers_[i]);\n> -}\n> +DebayerCpu::~DebayerCpu() = default;\n> \n> #define DECLARE_SRC_POINTERS(pixel_t) \\\n> \tconst pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> \tlineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> \tlineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n> \t\t\t 2 * lineBufferPadding_;\n> -\tfor (unsigned int i = 0;\n> -\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> -\t i++) {\n> -\t\tfree(lineBuffers_[i]);\n> -\t\tlineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n> -\t\tif (!lineBuffers_[i])\n> -\t\t\treturn -ENOMEM;\n> +\n> +\tif (enableInputMemcpy_) {\n> +\t\tfor (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n> +\t\t\tlineBuffers_[i].resize(lineBufferLength_);\n> \t}\n> \n> \tmeasuredFrames_ = 0;\n> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n> \t\treturn;\n> \n> \tfor (unsigned int i = 0; i < patternHeight; i++) {\n> -\t\tmemcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n> +\t\tmemcpy(lineBuffers_[i].data(),\n> +\t\t linePointers[i + 1] - lineBufferPadding_,\n> \t\t lineBufferLength_);\n> -\t\tlinePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n> +\t\tlinePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n> \t}\n> \n> \t/* Point lineBufferIndex_ to first unused lineBuffer */\n> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n> \tif (!enableInputMemcpy_)\n> \t\treturn;\n> \n> -\tmemcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n> +\tmemcpy(lineBuffers_[lineBufferIndex_].data(),\n> +\t linePointers[patternHeight] - lineBufferPadding_,\n> \t lineBufferLength_);\n> -\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n> +\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n> +\t\t\t\t + lineBufferPadding_;\n> \n> \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> }\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 1dac64351f1d..8237a64be733 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -146,7 +146,7 @@ private:\n> \tDebayerInputConfig inputConfig_;\n> \tDebayerOutputConfig outputConfig_;\n> \tstd::unique_ptr stats_;\n> -\tuint8_t *lineBuffers_[kMaxLineBuffers];\n> +\tstd::vector lineBuffers_[kMaxLineBuffers];\n> \tunsigned int lineBufferLength_;\n> \tunsigned int lineBufferPadding_;\n> \tunsigned int lineBufferIndex_;\n>\n> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5","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 5EDD3BE173\n\tfor ;\n\tMon, 5 Aug 2024 10:35:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 486456337E;\n\tMon, 5 Aug 2024 12:35:50 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CA666195D\n\tfor ;\n\tMon, 5 Aug 2024 12:35:47 +0200 (CEST)","from mail-wr1-f72.google.com (mail-wr1-f72.google.com\n\t[209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-8-pHz9wMWpPaS5oTcO0Dx2MQ-1; Mon, 05 Aug 2024 06:35:45 -0400","by mail-wr1-f72.google.com with SMTP id\n\tffacd0b85a97d-368448dfe12so2543021f8f.1\n\tfor ;\n\tMon, 05 Aug 2024 03:35:45 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-36bbd01efe5sm9438061f8f.46.2024.08.05.03.35.42\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 05 Aug 2024 03:35:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"AAqntNXg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722854146;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=8Pt/d1BpurE+QPCWBgiceUj+uC2QKAyqws4hSblB+rg=;\n\tb=AAqntNXg29jyaOMxAYsO7DSGkR9eL/3hsgP5wksCopF+Wh5jiWE0XNYUJU+xFCz0kUMKLJ\n\t2oEjuRsS3kQvPjWZz/a0uXFncG1bQAZLjjUKTkV++e6aqXURoEgoSio4BUL1//jV4gffeE\n\tuh1QDP3coNuM1ZtqG51EL7ahQreCpV8=","X-MC-Unique":"pHz9wMWpPaS5oTcO0Dx2MQ-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722854144; x=1723458944;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=8Pt/d1BpurE+QPCWBgiceUj+uC2QKAyqws4hSblB+rg=;\n\tb=fQ94L1r7w3kbpoxD7WLU1QPS3gKSlrf/XRckdNCSIaPZz4bPNgzQ4dncfVGXHcu6Au\n\t4dm3NexhbBnj6dTtNRuqn9jfauOuEmHju4JQQWJCLvGDKAyzTLHIU1In3yFfsbEAUYQV\n\t5XcVdhvGIBBIHdUh0+xs8IbodGBTO3DvCQ68rPvldXwwwOzhxCkW62qpFwauTS/RwK/g\n\t5qTMtRFlg6Q6Ztf9z2g9+eGITFNneU3xHreY/kcFoQHPxqxZex8b/eW/PzTCdlCp6J1l\n\tuO7axnHfEriz9deXz/XjjFH80kb4ayTXlxnHMKapB03OWxKhAGogsYjhAYkserg6bWTJ\n\tBHBg==","X-Gm-Message-State":"AOJu0Ywk/BNghdhZCH6N0xbLFzIunNCmIQf0q2QHl+VuxppvBSFrx6D9\n\tjZxwL1nnfC+pOMz2pWkBG9KlqJ18POApI+ot/PbPAtIATIa4pSxJSEHToPqRmOo5kq5uxh96X9B\n\tRG9eKLtOk314fQ590JBnYo/N0nxVQ4hKUYFXZFDs2m/pmgm+mRcv/rvHx603m6Oq4Bj+6YxipnL\n\tdis7gzSjqtWbD2I1Fyv045g1/wq0vXZdela+LOedo3Oormr+IodrUJ3q8=","X-Received":["by 2002:a5d:5f84:0:b0:367:40eb:a3c3 with SMTP id\n\tffacd0b85a97d-36bbbeefadbmr10961990f8f.34.1722854143603; \n\tMon, 05 Aug 2024 03:35:43 -0700 (PDT)","by 2002:a5d:5f84:0:b0:367:40eb:a3c3 with SMTP id\n\tffacd0b85a97d-36bbbeefadbmr10961964f8f.34.1722854143032; \n\tMon, 05 Aug 2024 03:35:43 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHtkRSZ9SjwMS+lylv8sSKOIrVmFaHZFUXNq1NSYal0rJe47JhbLdUR2f6BAgBTYVKiXjj7sw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","In-Reply-To":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Sun, 4 Aug 2024 16:36:05 +0300\")","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>","Date":"Mon, 05 Aug 2024 12:35:41 +0200","Message-ID":"<877ccvp88y.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30583,"web_url":"https://patchwork.libcamera.org/comment/30583/","msgid":"<8734njp85d.fsf@redhat.com>","date":"2024-08-05T10:37:50","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Kieran Bingham writes:\n\n> Quoting Laurent Pinchart (2024-08-04 14:36:05)\n>> libcamera is implemented in C++, use std::vector<> to manage the\n>> dynamically allocated line buffers instead of malloc() and free(). This\n>\n>> simplifies the code and improves memory safety by ensuring no allocation\n>> will be leaked.\n>> \n>> Signed-off-by: Laurent Pinchart \n>> ---\n>> Changes since v1:\n>> \n>> - Use std::vector instead of new[]() and delete[]()\n>> ---\n>> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n>> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n>> 2 files changed, 13 insertions(+), 20 deletions(-)\n>> \n>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>> index f8d2677d657a..077f7f4bc45b 100644\n>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n>> /* Initialize color lookup tables */\n>> for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n>> red_[i] = green_[i] = blue_[i] = i;\n>> -\n>> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n>> - lineBuffers_[i] = nullptr;\n>> }\n>> \n>> -DebayerCpu::~DebayerCpu()\n>> -{\n>> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n>> - free(lineBuffers_[i]);\n>> -}\n>> +DebayerCpu::~DebayerCpu() = default;\n>> \n>> #define DECLARE_SRC_POINTERS(pixel_t) \\\n>> const pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n>> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n>> lineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n>> lineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n>> 2 * lineBufferPadding_;\n>> - for (unsigned int i = 0;\n>> - i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n>> - i++) {\n>> - free(lineBuffers_[i]);\n>> - lineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n>> - if (!lineBuffers_[i])\n>> - return -ENOMEM;\n>> +\n>> + if (enableInputMemcpy_) {\n>> + for (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n>> + lineBuffers_[i].resize(lineBufferLength_);\n>\n> that's easier.\n>\n> I don't think the previous implementation was doing it, but should any\n> following lines be resized to 0 if the configuration shrinks?\n>\n> Worst case, this could grow to full allocation, then the later lines\n> would remain allocated indefinitely on any system with a long running\n> camera manager (i.e. pipewire). The memory wouldn't be lost, as it would\n> be released when the camera manager is destroyed, but the memory\n> wouldn't be usable...\n\nCan the Bayer pattern actually change when changing the configuration?\n\n> Could be a patch on top as the previous implementation didn't do this\n> and it's just a conversion patch here.\n>\n> So I think this patch still steps in a good way forwards:\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n>> }\n>> \n>> measuredFrames_ = 0;\n>> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n>> return;\n>> \n>> for (unsigned int i = 0; i < patternHeight; i++) {\n>> - memcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n>> + memcpy(lineBuffers_[i].data(),\n>> + linePointers[i + 1] - lineBufferPadding_,\n>> lineBufferLength_);\n>> - linePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n>> + linePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n>> }\n>> \n>> /* Point lineBufferIndex_ to first unused lineBuffer */\n>> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>> if (!enableInputMemcpy_)\n>> return;\n>> \n>> - memcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n>> + memcpy(lineBuffers_[lineBufferIndex_].data(),\n>> + linePointers[patternHeight] - lineBufferPadding_,\n>> lineBufferLength_);\n>> - linePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n>> + linePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n>> + + lineBufferPadding_;\n>> \n>> lineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n>> }\n>> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n>> index 1dac64351f1d..8237a64be733 100644\n>> --- a/src/libcamera/software_isp/debayer_cpu.h\n>> +++ b/src/libcamera/software_isp/debayer_cpu.h\n>> @@ -146,7 +146,7 @@ private:\n>> DebayerInputConfig inputConfig_;\n>> DebayerOutputConfig outputConfig_;\n>> std::unique_ptr stats_;\n>> - uint8_t *lineBuffers_[kMaxLineBuffers];\n>> + std::vector lineBuffers_[kMaxLineBuffers];\n>> unsigned int lineBufferLength_;\n>> unsigned int lineBufferPadding_;\n>> unsigned int lineBufferIndex_;\n>> \n>> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5\n>> -- \n>> Regards,\n>> \n>> Laurent Pinchart\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 1B9EABE173\n\tfor ;\n\tMon, 5 Aug 2024 10:37:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5BC5A6337E;\n\tMon, 5 Aug 2024 12:37:58 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 531546195D\n\tfor ;\n\tMon, 5 Aug 2024 12:37:56 +0200 (CEST)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-467-Bt14YOZtM9SyQ5yCn0gKFw-1; Mon, 05 Aug 2024 06:37:53 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-a7275e17256so1036933066b.0\n\tfor ;\n\tMon, 05 Aug 2024 03:37:53 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a7dc9bc3c9asm436824366b.37.2024.08.05.03.37.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 05 Aug 2024 03:37:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"EDGTL2po\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722854275;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=Uv69xEGHqzm+jNdCZ79/OeaSac79ydVY+qHxR4poIw0=;\n\tb=EDGTL2poWXkNXqHjze8ALLzYCzbs5OKWv7RACuoVyWmq2QZGPxKGh2sTmpwxA6XQaPo+S+\n\tYnYv0m3+7Cfn6qb7qjb3hCIC9ZIRP8GX1mnLPZ5VJepaFfEfOGSQT/+BCd4Ri+BSCiKcWh\n\t5xSdyRbCk6XDwfi0acHvo3l2h9ThEC0=","X-MC-Unique":"Bt14YOZtM9SyQ5yCn0gKFw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722854272; x=1723459072;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=Uv69xEGHqzm+jNdCZ79/OeaSac79ydVY+qHxR4poIw0=;\n\tb=gHBi5U5+5MqcjzYpJJZs/J1hLJtGBboFJAGxAfbETWBBurnq0fiuoeJMj6eHuOKEF7\n\t/CcaNLTJyLT/Pb2MTavaGOynya/m4sCwVbi1P0szh43p/ZW//Zz/EypIP/1jvG1A6a/3\n\thGJubfiODrC/l4DE1MOQzgxNJPoz5CFZhJ7DQGRoccL/G3OpzCaVU1bREEpp73snocsq\n\tsdlDmxqmhqpnKD260y4RTvHYid63bEsWdW7HoU9CFgGO/pTZiLVcj+92hBsluGhGRfM2\n\ttdCY/MDNN+8ML5b8mwIplGE5EGlcyxhACyhWKayA5CqSGx7YEmmB8uixnCXJ13M8j1r5\n\twF3Q==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCW4xQSme1HEj3zDaHVAouACA/zl0yYfWwa1+FofqUeNfmstD1mBCvH7U5qFJoqgTFUXVLUx+6x6U363SHIZa0oOiQ4j0SMh8SjTOsmfgzzS2O1mIw==","X-Gm-Message-State":"AOJu0Yzu7KF7ifjJdbN6Y+b6gWcx7f1A7V1wf90vPy41fYG411sorKSm\n\tkPJ1zhR+3wr2O/qNUa352R6NRsjWlpWzAeKYXY3IenhZFYAfsdDHgu4xNZJ3pFyUx7NWw16Z0nH\n\tLXiTqsrllL3JMdiYkEo1/R70rbDYzmS11I6Tod6HhLaMa2W08kUWlSbD04lbh1Pc7cdIt3rnqI4\n\t8hL04ZIi6c9yL3uGZ8vXrm7HIXfMSEDL9+EFV454QzNZ1fDeHbxmPhjQc=","X-Received":["by 2002:a17:907:d8b:b0:a7a:9447:3e8b with SMTP id\n\ta640c23a62f3a-a7dc4eac568mr712823266b.25.1722854272223; \n\tMon, 05 Aug 2024 03:37:52 -0700 (PDT)","by 2002:a17:907:d8b:b0:a7a:9447:3e8b with SMTP id\n\ta640c23a62f3a-a7dc4eac568mr712820866b.25.1722854271618; \n\tMon, 05 Aug 2024 03:37:51 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG8D4hJnzVhDqwv0UHOejoH/lEJFmKDugRqEtzswByYKBOgUkHCGWBroYsmcMyCzvYUhL1U1A==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","In-Reply-To":"<172284791414.2725865.10320885849259481853@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Mon, 05 Aug 2024 09:51:54 +0100\")","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>\n\t<172284791414.2725865.10320885849259481853@ping.linuxembedded.co.uk>","Date":"Mon, 05 Aug 2024 12:37:50 +0200","Message-ID":"<8734njp85d.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30586,"web_url":"https://patchwork.libcamera.org/comment/30586/","msgid":"<917108fa-f502-4372-83b7-6f23d764d8ee@redhat.com>","date":"2024-08-05T10:56:06","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 8/5/24 12:32 PM, Laurent Pinchart wrote:\n> Hi Hans,\n> \n> On Mon, Aug 05, 2024 at 12:22:02PM +0200, Hans de Goede wrote:\n>> On 8/4/24 3:36 PM, Laurent Pinchart wrote:\n>>> libcamera is implemented in C++, use std::vector<> to manage the\n>>> dynamically allocated line buffers instead of malloc() and free(). This\n>>> simplifies the code and improves memory safety by ensuring no allocation\n>>> will be leaked.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>\n>> I have given this a spin with IPU6 + the softISP and everything still works:\n>>\n>> Tested-by: Hans de Goede \n> \n> Thank you.\n> \n>> p.s.\n>>\n>> I do see an *unrelated* performance regression where the softISP processing\n>> time for 1920x1080 goes up from 3 ms / frame to 6 ms / frame. 1 ms of that\n>> is caused by the new 32bpp RGB sparse output support which qcam prefers\n>> over the previous 24bpp packed support forcing 24 bpp packed output fmt\n>> resolves the 1 ms difference which is expected since 32 bpp sparse writes\n>> 33% more data. But that still leaves 2 ms unaccounted for.\n>>\n>> I'll try to dig into where those 2 ms are coming from. With moderen CPU\n>> turboing behavior it is hard to tell really. Might even be a kernel issue...\n> \n> Is that a regression introduced by this patch, or a regression between a\n> particular point in the past and the last master branch ?\n\nThe latter: \"a regression between a particular point in the past and\nthe last master branch\". I was hoping that the \"*unrelated*\" above made\nthat clear.\n\nIt could even be something outside of libcamera, like e.g. the kernel\nscheduling the softISP thread on the E-cores instead of on the P-cores...\n\nIOW this is homework for me to figure out where this comes from,\nthis does show that the builtin mini benchmark really is quite useful.\n\nRegards,\n\nHans\n\n\n\n \n>>> ---\n>>> Changes since v1:\n>>>\n>>> - Use std::vector instead of new[]() and delete[]()\n>>> ---\n>>> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n>>> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n>>> 2 files changed, 13 insertions(+), 20 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>>> index f8d2677d657a..077f7f4bc45b 100644\n>>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>>> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n>>> \t/* Initialize color lookup tables */\n>>> \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n>>> \t\tred_[i] = green_[i] = blue_[i] = i;\n>>> -\n>>> -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n>>> -\t\tlineBuffers_[i] = nullptr;\n>>> }\n>>> \n>>> -DebayerCpu::~DebayerCpu()\n>>> -{\n>>> -\tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n>>> -\t\tfree(lineBuffers_[i]);\n>>> -}\n>>> +DebayerCpu::~DebayerCpu() = default;\n>>> \n>>> #define DECLARE_SRC_POINTERS(pixel_t) \\\n>>> \tconst pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n>>> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n>>> \tlineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n>>> \tlineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n>>> \t\t\t 2 * lineBufferPadding_;\n>>> -\tfor (unsigned int i = 0;\n>>> -\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n>>> -\t i++) {\n>>> -\t\tfree(lineBuffers_[i]);\n>>> -\t\tlineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n>>> -\t\tif (!lineBuffers_[i])\n>>> -\t\t\treturn -ENOMEM;\n>>> +\n>>> +\tif (enableInputMemcpy_) {\n>>> +\t\tfor (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n>>> +\t\t\tlineBuffers_[i].resize(lineBufferLength_);\n>>> \t}\n>>> \n>>> \tmeasuredFrames_ = 0;\n>>> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n>>> \t\treturn;\n>>> \n>>> \tfor (unsigned int i = 0; i < patternHeight; i++) {\n>>> -\t\tmemcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n>>> +\t\tmemcpy(lineBuffers_[i].data(),\n>>> +\t\t linePointers[i + 1] - lineBufferPadding_,\n>>> \t\t lineBufferLength_);\n>>> -\t\tlinePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n>>> +\t\tlinePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n>>> \t}\n>>> \n>>> \t/* Point lineBufferIndex_ to first unused lineBuffer */\n>>> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>>> \tif (!enableInputMemcpy_)\n>>> \t\treturn;\n>>> \n>>> -\tmemcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n>>> +\tmemcpy(lineBuffers_[lineBufferIndex_].data(),\n>>> +\t linePointers[patternHeight] - lineBufferPadding_,\n>>> \t lineBufferLength_);\n>>> -\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n>>> +\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n>>> +\t\t\t\t + lineBufferPadding_;\n>>> \n>>> \tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n>>> }\n>>> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n>>> index 1dac64351f1d..8237a64be733 100644\n>>> --- a/src/libcamera/software_isp/debayer_cpu.h\n>>> +++ b/src/libcamera/software_isp/debayer_cpu.h\n>>> @@ -146,7 +146,7 @@ private:\n>>> \tDebayerInputConfig inputConfig_;\n>>> \tDebayerOutputConfig outputConfig_;\n>>> \tstd::unique_ptr stats_;\n>>> -\tuint8_t *lineBuffers_[kMaxLineBuffers];\n>>> +\tstd::vector lineBuffers_[kMaxLineBuffers];\n>>> \tunsigned int lineBufferLength_;\n>>> \tunsigned int lineBufferPadding_;\n>>> \tunsigned int lineBufferIndex_;\n>>>\n>>> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5\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 6C620C323E\n\tfor ;\n\tMon, 5 Aug 2024 10:56:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6E2563381;\n\tMon, 5 Aug 2024 12:56:14 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73EC46195D\n\tfor ;\n\tMon, 5 Aug 2024 12:56:12 +0200 (CEST)","from mail-lf1-f71.google.com (mail-lf1-f71.google.com\n\t[209.85.167.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-690-lzEUxq0iNqaUbXLM2Xov9Q-1; Mon, 05 Aug 2024 06:56:09 -0400","by mail-lf1-f71.google.com with SMTP id\n\t2adb3069b0e04-52f00bde29dso13564084e87.3\n\tfor ;\n\tMon, 05 Aug 2024 03:56:09 -0700 (PDT)","from [10.40.98.157] ([78.108.130.194])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a7dc9d45452sm437109566b.111.2024.08.05.03.56.06\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 05 Aug 2024 03:56:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"UltBCewc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722855371;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=XnxASbXvJP0eLN2TNowyHBiT+nFfNlPjXiv3Y9JOmgo=;\n\tb=UltBCewc2oi7OpyVF2UDgtTGNUfUvawsXo9Jz1NOoIIpZamE6LH8AS+1r4YcFx6xJjrSen\n\tzpxYJXkszx4aNHGLnkRyycAS401Avc1wm6oUbt5eXb/RhxDE3qATokrUKpnKOrpLFXYLld\n\tLF9dwkrGaPeIifHhHxo9FU7ioWfLdIg=","X-MC-Unique":"lzEUxq0iNqaUbXLM2Xov9Q-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722855368; x=1723460168;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=XnxASbXvJP0eLN2TNowyHBiT+nFfNlPjXiv3Y9JOmgo=;\n\tb=GZM0p3KZTgE+YeDu3lRVa7vUz5WGwn2uFdzLAV+BSKXpl/4k3pNEjirKr4v/+zHn/V\n\tUnphPocRI2uueVktH61TjUTmIxml7zeSlq0zfsqd7OPkhB16/kkKTa+NTxi3Sm08zKed\n\tMJXJWhKpJPGtIqn2zD9l90yN+P357E6Dw36tH7bvwLwtTsfXXWVZ7OO8e77pDIYlN40X\n\tYzx9jqr0R7QEG0hOsioRx4D7cWMHfveOH0PXhtLbzvtKJxsjV9QumSIZc9ebQp4HVNad\n\tSvlJL8Ix0fd7U3AHpJliGGH/crbv14XsqagP1FvQDiJwJeSvyzRNnSGdX2PGpkdkXGfA\n\tAGBg==","X-Gm-Message-State":"AOJu0YylDfADJzwUBCkH3CN3buUUUnYYrbVqSBdN7r6rMIA1iA+4R4wi\n\toL503R2czhr4It3Xb7sN7eBjQif4gOQ3hK2UpXebBKZIIiTMyFQ5++dr0WVnvSGLh3EO7tDhVxY\n\tuLOi5lljgCiW36NUSbfEflPHrTdsrfasHYQXCSgEIml7fC0R9DiXsz2yOVODPzdo0ZSoOx8+mVo\n\tnmoSI=","X-Received":["by 2002:a05:6512:6ce:b0:52c:8075:4f3 with SMTP id\n\t2adb3069b0e04-530bb39576emr9545661e87.36.1722855368091; \n\tMon, 05 Aug 2024 03:56:08 -0700 (PDT)","by 2002:a05:6512:6ce:b0:52c:8075:4f3 with SMTP id\n\t2adb3069b0e04-530bb39576emr9545629e87.36.1722855367387; \n\tMon, 05 Aug 2024 03:56:07 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHitEtXWwFAxYts/hbwfZObVQv6kjBh7QKcSb5BleBLb6m6l60PSLnhJ2b1RbGeK+jxKF22bQ==","Message-ID":"<917108fa-f502-4372-83b7-6f23d764d8ee@redhat.com>","Date":"Mon, 5 Aug 2024 12:56:06 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20240805103259.GB21286@pendragon.ideasonboard.com>","From":"Hans de Goede ","In-Reply-To":"<20240805103259.GB21286@pendragon.ideasonboard.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30600,"web_url":"https://patchwork.libcamera.org/comment/30600/","msgid":"<20240805162555.GB2753@pendragon.ideasonboard.com>","date":"2024-08-05T16:25:55","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Aug 05, 2024 at 12:37:50PM +0200, Milan Zamazal wrote:\n> Kieran Bingham writes:\n> \n> > Quoting Laurent Pinchart (2024-08-04 14:36:05)\n> >> libcamera is implemented in C++, use std::vector<> to manage the\n> >> dynamically allocated line buffers instead of malloc() and free(). This\n> >\n> >> simplifies the code and improves memory safety by ensuring no allocation\n> >> will be leaked.\n> >> \n> >> Signed-off-by: Laurent Pinchart \n> >> ---\n> >> Changes since v1:\n> >> \n> >> - Use std::vector instead of new[]() and delete[]()\n> >> ---\n> >> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n> >> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n> >> 2 files changed, 13 insertions(+), 20 deletions(-)\n> >> \n> >> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> >> index f8d2677d657a..077f7f4bc45b 100644\n> >> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> >> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> >> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> >> /* Initialize color lookup tables */\n> >> for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n> >> red_[i] = green_[i] = blue_[i] = i;\n> >> -\n> >> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> >> - lineBuffers_[i] = nullptr;\n> >> }\n> >> \n> >> -DebayerCpu::~DebayerCpu()\n> >> -{\n> >> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n> >> - free(lineBuffers_[i]);\n> >> -}\n> >> +DebayerCpu::~DebayerCpu() = default;\n> >> \n> >> #define DECLARE_SRC_POINTERS(pixel_t) \\\n> >> const pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n> >> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> >> lineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> >> lineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n> >> 2 * lineBufferPadding_;\n> >> - for (unsigned int i = 0;\n> >> - i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> >> - i++) {\n> >> - free(lineBuffers_[i]);\n> >> - lineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n> >> - if (!lineBuffers_[i])\n> >> - return -ENOMEM;\n> >> +\n> >> + if (enableInputMemcpy_) {\n> >> + for (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n> >> + lineBuffers_[i].resize(lineBufferLength_);\n> >\n> > that's easier.\n> >\n> > I don't think the previous implementation was doing it, but should any\n> > following lines be resized to 0 if the configuration shrinks?\n> >\n> > Worst case, this could grow to full allocation, then the later lines\n> > would remain allocated indefinitely on any system with a long running\n> > camera manager (i.e. pipewire). The memory wouldn't be lost, as it would\n> > be released when the camera manager is destroyed, but the memory\n> > wouldn't be usable...\n> \n> Can the Bayer pattern actually change when changing the configuration?\n\nIf you apply horizontal or vertical flipping, the pattern can change\nunless you adjust the crop rectangle by one pixel to keep the pattern\nidentical. Some sensors do so automatically at the hardware (or\nfirmware) level. We haven't really standardized on how it should be\nhandled in libcamera, but we should.\n\n> > Could be a patch on top as the previous implementation didn't do this\n> > and it's just a conversion patch here.\n\nProbably a good idea of a patch on top.\n\n> > So I think this patch still steps in a good way forwards:\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >> }\n> >> \n> >> measuredFrames_ = 0;\n> >> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n> >> return;\n> >> \n> >> for (unsigned int i = 0; i < patternHeight; i++) {\n> >> - memcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n> >> + memcpy(lineBuffers_[i].data(),\n> >> + linePointers[i + 1] - lineBufferPadding_,\n> >> lineBufferLength_);\n> >> - linePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n> >> + linePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n> >> }\n> >> \n> >> /* Point lineBufferIndex_ to first unused lineBuffer */\n> >> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n> >> if (!enableInputMemcpy_)\n> >> return;\n> >> \n> >> - memcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n> >> + memcpy(lineBuffers_[lineBufferIndex_].data(),\n> >> + linePointers[patternHeight] - lineBufferPadding_,\n> >> lineBufferLength_);\n> >> - linePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n> >> + linePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n> >> + + lineBufferPadding_;\n> >> \n> >> lineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> >> }\n> >> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> >> index 1dac64351f1d..8237a64be733 100644\n> >> --- a/src/libcamera/software_isp/debayer_cpu.h\n> >> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> >> @@ -146,7 +146,7 @@ private:\n> >> DebayerInputConfig inputConfig_;\n> >> DebayerOutputConfig outputConfig_;\n> >> std::unique_ptr stats_;\n> >> - uint8_t *lineBuffers_[kMaxLineBuffers];\n> >> + std::vector lineBuffers_[kMaxLineBuffers];\n> >> unsigned int lineBufferLength_;\n> >> unsigned int lineBufferPadding_;\n> >> unsigned int lineBufferIndex_;\n> >> \n> >> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5","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 526D7BE173\n\tfor ;\n\tMon, 5 Aug 2024 16:26:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 88BF063381;\n\tMon, 5 Aug 2024 18:26:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B2B46195D\n\tfor ;\n\tMon, 5 Aug 2024 18:26:18 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4033818D;\n\tMon, 5 Aug 2024 18:25:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vGF/WhcR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722875126;\n\tbh=sa9Oef6NFj7AYiJtV7sSpWN7Gnwjn4fdv2Fwt478TMg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vGF/WhcRSHS/LQak8Xd06ygEhi1/PW6nNQ5waWk07Poet0oiireRvC+3dpji/Zn2M\n\tbpRaX/Z267RoV/IVBLigVnKr8x+11x3LiMR6Tt/sEeN8m50TUbds1J7O7nnniYyU5w\n\tXf0OEmybdTldq2V1qGgk12IyMkEzuTGjOtqh/eaM=","Date":"Mon, 5 Aug 2024 19:25:55 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","Message-ID":"<20240805162555.GB2753@pendragon.ideasonboard.com>","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>\n\t<172284791414.2725865.10320885849259481853@ping.linuxembedded.co.uk>\n\t<8734njp85d.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<8734njp85d.fsf@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30603,"web_url":"https://patchwork.libcamera.org/comment/30603/","msgid":"<87o766ofmc.fsf@redhat.com>","date":"2024-08-05T20:54:03","subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> On Mon, Aug 05, 2024 at 12:37:50PM +0200, Milan Zamazal wrote:\n>> Kieran Bingham writes:\n>> \n>\n>> > Quoting Laurent Pinchart (2024-08-04 14:36:05)\n>> >> libcamera is implemented in C++, use std::vector<> to manage the\n>> >> dynamically allocated line buffers instead of malloc() and free(). This\n>> >\n>> >> simplifies the code and improves memory safety by ensuring no allocation\n>> >> will be leaked.\n>> >> \n>> >> Signed-off-by: Laurent Pinchart \n>> >> ---\n>> >> Changes since v1:\n>> >> \n>> >> - Use std::vector instead of new[]() and delete[]()\n>> >> ---\n>> >> src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------\n>> >> src/libcamera/software_isp/debayer_cpu.h | 2 +-\n>> >> 2 files changed, 13 insertions(+), 20 deletions(-)\n>> >> \n>> >> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>> >> index f8d2677d657a..077f7f4bc45b 100644\n>> >> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>> >> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>> >> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n>> >> /* Initialize color lookup tables */\n>> >> for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n>> >> red_[i] = green_[i] = blue_[i] = i;\n>> >> -\n>> >> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n>> >> - lineBuffers_[i] = nullptr;\n>> >> }\n>> >> \n>> >> -DebayerCpu::~DebayerCpu()\n>> >> -{\n>> >> - for (unsigned int i = 0; i < kMaxLineBuffers; i++)\n>> >> - free(lineBuffers_[i]);\n>> >> -}\n>> >> +DebayerCpu::~DebayerCpu() = default;\n>> >> \n>> >> #define DECLARE_SRC_POINTERS(pixel_t) \\\n>> >> const pixel_t *prev = (const pixel_t *)src[0] + xShift_; \\\n>> >> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n>> >> lineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n>> >> lineBufferLength_ = window_.width * inputConfig_.bpp / 8 +\n>> >> 2 * lineBufferPadding_;\n>> >> - for (unsigned int i = 0;\n>> >> - i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n>> >> - i++) {\n>> >> - free(lineBuffers_[i]);\n>> >> - lineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);\n>> >> - if (!lineBuffers_[i])\n>> >> - return -ENOMEM;\n>> >> +\n>> >> + if (enableInputMemcpy_) {\n>> >> + for (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)\n>> >> + lineBuffers_[i].resize(lineBufferLength_);\n>> >\n>> > that's easier.\n>> >\n>> > I don't think the previous implementation was doing it, but should any\n>> > following lines be resized to 0 if the configuration shrinks?\n>> >\n>> > Worst case, this could grow to full allocation, then the later lines\n>> > would remain allocated indefinitely on any system with a long running\n>> > camera manager (i.e. pipewire). The memory wouldn't be lost, as it would\n>> > be released when the camera manager is destroyed, but the memory\n>> > wouldn't be usable...\n>> \n>> Can the Bayer pattern actually change when changing the configuration?\n>\n> If you apply horizontal or vertical flipping, the pattern can change\n> unless you adjust the crop rectangle by one pixel to keep the pattern\n> identical. Some sensors do so automatically at the hardware (or\n> firmware) level. We haven't really standardized on how it should be\n> handled in libcamera, but we should.\n\nI see, OK. But I suppose the pattern size remains the same in those\ncases so we needn't worry much about shrinking the allocated line\nbuffers.\n\n>> > Could be a patch on top as the previous implementation didn't do this\n>> > and it's just a conversion patch here.\n>\n> Probably a good idea of a patch on top.\n>\n>> > So I think this patch still steps in a good way forwards:\n>> >\n>> > Reviewed-by: Kieran Bingham \n>> >\n>> >> }\n>> >> \n>> >> measuredFrames_ = 0;\n>> >> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])\n>> >> return;\n>> >> \n>> >> for (unsigned int i = 0; i < patternHeight; i++) {\n>> >> - memcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,\n>> >> + memcpy(lineBuffers_[i].data(),\n>> >> + linePointers[i + 1] - lineBufferPadding_,\n>> >> lineBufferLength_);\n>> >> - linePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;\n>> >> + linePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;\n>> >> }\n>> >> \n>> >> /* Point lineBufferIndex_ to first unused lineBuffer */\n>> >> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>> >> if (!enableInputMemcpy_)\n>> >> return;\n>> >> \n>> >> - memcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,\n>> >> + memcpy(lineBuffers_[lineBufferIndex_].data(),\n>> >> + linePointers[patternHeight] - lineBufferPadding_,\n>> >> lineBufferLength_);\n>> >> - linePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;\n>> >> + linePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()\n>> >> + + lineBufferPadding_;\n>> >> \n>> >> lineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n>> >> }\n>> >> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n>> >> index 1dac64351f1d..8237a64be733 100644\n>> >> --- a/src/libcamera/software_isp/debayer_cpu.h\n>> >> +++ b/src/libcamera/software_isp/debayer_cpu.h\n>> >> @@ -146,7 +146,7 @@ private:\n>> >> DebayerInputConfig inputConfig_;\n>> >> DebayerOutputConfig outputConfig_;\n>> >> std::unique_ptr stats_;\n>> >> - uint8_t *lineBuffers_[kMaxLineBuffers];\n>> >> + std::vector lineBuffers_[kMaxLineBuffers];\n>> >> unsigned int lineBufferLength_;\n>> >> unsigned int lineBufferPadding_;\n>> >> unsigned int lineBufferIndex_;\n>> >> \n>> >> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5","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 E9CA4BE173\n\tfor ;\n\tMon, 5 Aug 2024 20:54:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAA0E63381;\n\tMon, 5 Aug 2024 22:54:11 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D642C6195D\n\tfor ;\n\tMon, 5 Aug 2024 22:54:09 +0200 (CEST)","from mail-ej1-f72.google.com (mail-ej1-f72.google.com\n\t[209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-171-cLyT-WzQOX-b9na5L8qTIQ-1; Mon, 05 Aug 2024 16:54:06 -0400","by mail-ej1-f72.google.com with SMTP id\n\ta640c23a62f3a-a7d2d414949so857242166b.0\n\tfor ;\n\tMon, 05 Aug 2024 13:54:06 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a7dc9bcf0f5sm486605766b.3.2024.08.05.13.54.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 05 Aug 2024 13:54:03 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"A0327tfl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722891248;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=LLF8wUmmfDrDR2SCi0LI5UsCtIvk3/Y44wx2UYJXQlg=;\n\tb=A0327tflhL+ZHXKMzfH9CT4WNCGy9Nxzfd8Z0D5DRhO4nU6zi8Ac9nIVYiDGKoSdZhl+ay\n\t+cIKuFc1Yb2aNOaF5tMqbe3Mg+cui0xFZKyw87NQ3fWcLxy+VDTXFDEv7M4QevUzg+FMBY\n\tC8jiVVHa+fD8Z6lpW+wL181ueZirQpc=","X-MC-Unique":"cLyT-WzQOX-b9na5L8qTIQ-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722891245; x=1723496045;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=LLF8wUmmfDrDR2SCi0LI5UsCtIvk3/Y44wx2UYJXQlg=;\n\tb=CWzsqwRuTRy7s4jBAkxnrk3Xo/1e/Rx01+PQqzUbG+I1Ulg7maT1MHENB49WT0tpEA\n\tWAjUONyo0UDaS/p4rf/W562nw25wqDko4Cl27o2mkHzIXe+zYaCedNTn4SnBOrTzXFTP\n\tjimRrsFy+s/CLl4erVG2TVCQ61VMYV9YD+BznyuBE+zMBP87cWlay9hDyErovzaPeXpD\n\ti3mCMyjl3Dtsvz+aothyB/29vVYf2iKMn8JGyMDKC/1HOdtCkzCuPlGDBPp6dXz+K3eQ\n\tslwUhX5FZnym775Xbdn4U/oUKeRB7ngTx3lfy2TZR1dkUisAnNLZK/y/xHX6n1rI4GeN\n\t2jmQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVTlhXuFp0q3OyPo6+Qjuoq/w9/3lUItfbOXmr6o8ObvczaiYJ1OnURA8KZ3fiDKn9zQ9AEad+HUQc4vTFPvXa1dEfo0IBSwQ1dXzsgP2vvZI/oJQ==","X-Gm-Message-State":"AOJu0YzxfPV7fBwupRuG+a/zQdjLj0uwOq0FA8QOTMaXSiLz0BVgJ07D\n\tzhmNxADLBLY+HMlAkI4YZ+vudAHMuL6GOeW6GFt22zb46YF7OY8jw+OvGERkIkuMYRvnBZ5yzHE\n\t2UXaf0uFMa3os7g/o6C2pILF8X95iq5Y67ZILeEn+IWGxOVxtIBTFnOWqZPcSSxSjMFNjZBGSU0\n\teqPnup6jwA8ZrKhPPXLYEUAu/TsEnfRBcRNOlZyqtv5cRXK/08BOSvEfg=","X-Received":["by 2002:a17:907:8e07:b0:a7d:ea36:b294 with SMTP id\n\ta640c23a62f3a-a7dea36b555mr434261666b.26.1722891244978; \n\tMon, 05 Aug 2024 13:54:04 -0700 (PDT)","by 2002:a17:907:8e07:b0:a7d:ea36:b294 with SMTP id\n\ta640c23a62f3a-a7dea36b555mr434260066b.26.1722891244375; \n\tMon, 05 Aug 2024 13:54:04 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEou93MnhLtjvc7WyFQ2b3+w6mb9dpWsJteXX5MH6KUM/PehBBPcV1s5cS0YOfterbA8lE77Q==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: software_isp: Replace malloc() with\n\tstd::vector<>","In-Reply-To":"<20240805162555.GB2753@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 5 Aug 2024 19:25:55 +0300\")","References":"<20240804133605.8234-1-laurent.pinchart@ideasonboard.com>\n\t<172284791414.2725865.10320885849259481853@ping.linuxembedded.co.uk>\n\t<8734njp85d.fsf@redhat.com>\n\t<20240805162555.GB2753@pendragon.ideasonboard.com>","Date":"Mon, 05 Aug 2024 22:54:03 +0200","Message-ID":"<87o766ofmc.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]