[{"id":33177,"web_url":"https://patchwork.libcamera.org/comment/33177/","msgid":"<20250126210234.GB1133@pendragon.ideasonboard.com>","date":"2025-01-26T21:02:34","subject":"Re: [RFC PATCH v2 03/13] libcamera: simple: Don't use raw output\n\tformats with conversions","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Fri, Jan 24, 2025 at 10:57:54PM +0100, Milan Zamazal wrote:\n> In order to support raw streams, we need to add raw formats to software\n> ISP configurations.  In this preparatory patch, the raw formats are\n> excluded from software ISP output configurations.\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 10 +++++++++-\n>  1 file changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 6e9bc630..06df909b 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -26,6 +26,7 @@\n>  \n>  #include <libcamera/camera.h>\n>  #include <libcamera/control_ids.h>\n> +#include <libcamera/pixel_format.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n>  \n> @@ -209,6 +210,12 @@ static const SimplePipelineInfo supportedDevices[] = {\n>  \t{ \"sun6i-csi\", {}, false },\n>  };\n>  \n> +bool isRawFormat(const PixelFormat &format)\n> +{\n> +\treturn libcamera::PixelFormatInfo::info(format).colourEncoding ==\n> +\t       libcamera::PixelFormatInfo::ColourEncodingRAW;\n> +}\n> +\n>  } /* namespace */\n>  \n>  class SimpleCameraData : public Camera::Private\n> @@ -1284,7 +1291,8 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>  \n>  \t\tcfg.setStream(&data->streams_[i]);\n>  \n> -\t\tif (data->useConversion_)\n> +\t\tif (data->useConversion_ &&\n> +\t\t    (!data->swIsp_ || !isRawFormat(cfg.pixelFormat)))\n\nThat's getting confusing. As the code is hard enough to follow, I'm not\nkeen on making it even more obfuscated.\n\nAt the moment, useConversion_ and swIsp_ are mutually exclusive. I\nunderstand this won't be the case anymore with this series, and\nuseConversion_ will be true when using the software ISP and capturing\nboth the raw and processed streams.\n\nIf you want to reuse some of the converter infrastructure (including\nvariables), I would like to see clear naming rules. We could for\ninstance use convert* for data related to the hardware converters,\nswisp* for data related to the software ISP, and another prefix (name\nTBD) for data common to both. Ideally, of course, we should abstract\nboth hardware converters and the software ISP with the same interface.\n\n>  \t\t\toutputCfgs.push_back(cfg);\n>  \t}\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D7A53BE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 26 Jan 2025 21:02:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D17226855D;\n\tSun, 26 Jan 2025 22:02:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22AAE68506\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 26 Jan 2025 22:02:46 +0100 (CET)","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 0ADA51874;\n\tSun, 26 Jan 2025 22:01:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oIdGNFWx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737925300;\n\tbh=Vlyg2jv6TWkBmac6FzUU4WrFPjuANK8WIV6bwoeK+vs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oIdGNFWxOIKn0GaGhFFcq3m/u3SLSyMzCI29DOctal60maPDm6VudUdIin8EjSYxp\n\tacMcnkJtNjHm/lSnZOYrUAusJXdUY/64gXnrnCvmi+AjI6OywOezYJprS1Sen/wKsr\n\tDHQ7UXa4MBodKMxz0qXejYWmsCWUV5r2qEEJKmqk=","Date":"Sun, 26 Jan 2025 23:02:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 03/13] libcamera: simple: Don't use raw output\n\tformats with conversions","Message-ID":"<20250126210234.GB1133@pendragon.ideasonboard.com>","References":"<20250124215806.158024-1-mzamazal@redhat.com>\n\t<20250124215806.158024-4-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250124215806.158024-4-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]