[{"id":29101,"web_url":"https://patchwork.libcamera.org/comment/29101/","msgid":"<20240327175728.GI4721@pendragon.ideasonboard.com>","date":"2024-03-27T17:57:28","subject":"Re: [PATCH v6 12/18] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan and Andrey,\n\nThank you for the patch.\n\nOn Tue, Mar 19, 2024 at 01:35:59PM +0100, Milan Zamazal wrote:\n> From: Andrey Konovalov \n> \n> To enable the Simple Soft ISP and Soft IPA for simple pipeline handler\n> configure the build with:\n> -Dpipelines=simple -Dipas=simple\n> \n> Also using the Soft ISP for the particular hardware platform must\n> be enabled in the supportedDevices[] table.\n\nI would mention here that the soft IPA is enabled for qcom-camss.\n\n> If the pipeline uses Converter, Soft ISP and Soft IPA aren't\n> available.\n> \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> Reviewed-by: Pavel Machek \n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 138 ++++++++++++++++++-----\n> 1 file changed, 109 insertions(+), 29 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 9c8f7ba3..ac796b9b 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -34,6 +34,7 @@\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/software_isp/software_isp.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> @@ -185,18 +186,23 @@ struct SimplePipelineInfo {\n> \t * and the number of streams it supports.\n> \t */\n> \tstd::vector> converters;\n> +\t/*\n> +\t * Using Software ISP is to be enabled per driver.\n> +\t * The Software ISP can't be used together with the converters.\n\n\t * Using Software ISP is to be enabled per driver. The Software ISP\n\t * can't be used together with the converters.\n\nOr if you want two paragraphs, you're missing a blank line.\n\n> +\t */\n> +\tbool swIspEnabled;\n> };\n> \n> namespace {\n> \n> static const SimplePipelineInfo supportedDevices[] = {\n> -\t{ \"dcmipp\", {} },\n> -\t{ \"imx7-csi\", { { \"pxp\", 1 } } },\n> -\t{ \"j721e-csi2rx\", {} },\n> -\t{ \"mtk-seninf\", { { \"mtk-mdp\", 3 } } },\n> -\t{ \"mxc-isi\", {} },\n> -\t{ \"qcom-camss\", {} },\n> -\t{ \"sun6i-csi\", {} },\n> +\t{ \"dcmipp\", {}, false },\n> +\t{ \"imx7-csi\", { { \"pxp\", 1 } }, false },\n> +\t{ \"j721e-csi2rx\", {}, false },\n> +\t{ \"mtk-seninf\", { { \"mtk-mdp\", 3 } }, false },\n> +\t{ \"mxc-isi\", {}, false },\n> +\t{ \"qcom-camss\", {}, true },\n> +\t{ \"sun6i-csi\", {}, false },\n> };\n> \n> } /* namespace */\n> @@ -275,6 +281,7 @@ public:\n> \tbool useConversion_;\n> \n> \tstd::unique_ptr converter_;\n> +\tstd::unique_ptr swIsp_;\n> \n> private:\n> \tvoid tryPipeline(unsigned int code, const Size &size);\n> @@ -282,6 +289,9 @@ private:\n> \n> \tvoid conversionInputDone(FrameBuffer *buffer);\n> \tvoid conversionOutputDone(FrameBuffer *buffer);\n> +\n> +\tvoid ispStatsReady(int dummy);\n> +\tvoid setSensorControls(const ControlList &sensorControls);\n> };\n> \n> class SimpleCameraConfiguration : public CameraConfiguration\n> @@ -333,6 +343,7 @@ public:\n> \tV4L2VideoDevice *video(const MediaEntity *entity);\n> \tV4L2Subdevice *subdev(const MediaEntity *entity);\n> \tMediaDevice *converter() { return converter_; }\n> +\tbool swIspEnabled() { return swIspEnabled_; }\n\n\tbool swIspEnabled() const { return swIspEnabled_; }\n\n> \n> protected:\n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -361,6 +372,7 @@ private:\n> \tstd::map entities_;\n> \n> \tMediaDevice *converter_;\n> +\tbool swIspEnabled_;\n> };\n> \n> /* -----------------------------------------------------------------------------\n> @@ -510,6 +522,29 @@ int SimpleCameraData::init()\n> \t\t}\n> \t}\n> \n> +\t/*\n> +\t * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.\n> +\t */\n> +\tif (!converter_ && pipe->swIspEnabled()) {\n> +\t\tswIsp_ = std::make_unique(pipe, sensor_->controls());\n> +\t\tif (!swIsp_->isValid()) {\n> +\t\t\tLOG(SimplePipeline, Warning)\n> +\t\t\t\t<< \"Failed to create software ISP, disabling software debayering\";\n> +\t\t\tswIsp_.reset();\n> +\t\t} else {\n> +\t\t\t/*\n> +\t\t\t * \\todo explain why SimpleCameraData::conversionInputDone() can't be directly\n> +\t\t\t * connected to inputBufferReady signal.\n\nPlease explain it already :-) Nobody will remember otherwise.\n\n\t\t\t/*\n\t\t\t * The inputBufferReady signal is emitted from the soft\n\t\t\t * ISP thread, and needs to be handled in the pipeline\n\t\t\t * handler thread. Signals implement queued delivery,\n\t\t\t * but this works transparently only if the receiver is\n\t\t\t * bound to the target thread. As the SimpleCameraData\n\t\t\t * class doesn't inherit from the Object class, it is\n\t\t\t * not bound to any thread, and the signal would be\n\t\t\t * delivered synchronously. Instead, connect the signal\n\t\t\t * to a lambda function bound explicitly to the pipe,\n\t\t\t * which is bound to the pipeline handler thread. The\n\t\t\t * function then simply forwards the call to\n\t\t\t * conversionInputDone().\n\t\t\t */\n\n> +\t\t\t */\n> +\t\t\tswIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> +\t\t\t\tthis->conversionInputDone(buffer);\n> +\t\t\t});\n> +\t\t\tswIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n> +\t\t\tswIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n> +\t\t\tswIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);\n> +\t\t}\n> +\t}\n> +\n> \tvideo_ = pipe->video(entities_.back().entity);\n> \tASSERT(video_);\n> \n> @@ -600,12 +635,20 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n> \t\tconfig.captureFormat = pixelFormat;\n> \t\tconfig.captureSize = format.size;\n> \n> -\t\tif (!converter_) {\n> -\t\t\tconfig.outputFormats = { pixelFormat };\n> -\t\t\tconfig.outputSizes = config.captureSize;\n> -\t\t} else {\n> +\t\tif (converter_) {\n> \t\t\tconfig.outputFormats = converter_->formats(pixelFormat);\n> \t\t\tconfig.outputSizes = converter_->sizes(format.size);\n> +\t\t} else if (swIsp_) {\n> +\t\t\tconfig.outputFormats = swIsp_->formats(pixelFormat);\n> +\t\t\tconfig.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n> +\t\t\tif (config.outputFormats.empty()) {\n> +\t\t\t\t/* Do not use swIsp for unsupported pixelFormat's: */\n\ns/:/./\n\n> +\t\t\t\tconfig.outputFormats = { pixelFormat };\n> +\t\t\t\tconfig.outputSizes = config.captureSize;\n> +\t\t\t}\n> +\t\t} else {\n> +\t\t\tconfig.outputFormats = { pixelFormat };\n> +\t\t\tconfig.outputSizes = config.captureSize;\n> \t\t}\n> \n> \t\tconfigs_.push_back(config);\n> @@ -751,9 +794,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> \t\t}\n> \n> \t\t/*\n> -\t\t * The converter is in use. Requeue the internal buffer for\n> -\t\t * capture (unless the stream is being stopped), and complete\n> -\t\t * the request with all the user-facing buffers.\n> +\t\t * The converter or Software ISP is in use. Requeue the internal\n> +\t\t * buffer for capture (unless the stream is being stopped), and\n> +\t\t * complete the request with all the user-facing buffers.\n> \t\t */\n> \t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n> \t\t\tvideo_->queueBuffer(buffer);\n> @@ -799,9 +842,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> \t\t\t\t\tbuffer->metadata().timestamp);\n> \n> \t/*\n> -\t * Queue the captured and the request buffer to the converter if format\n> -\t * conversion is needed. If there's no queued request, just requeue the\n> -\t * captured buffer for capture.\n> +\t * Queue the captured and the request buffer to the converter or Software\n> +\t * ISP if format conversion is needed. If there's no queued request, just\n> +\t * requeue the captured buffer for capture.\n> \t */\n> \tif (useConversion_) {\n> \t\tif (conversionQueue_.empty()) {\n> @@ -809,7 +852,11 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> \t\t\treturn;\n> \t\t}\n> \n> -\t\tconverter_->queueBuffers(buffer, conversionQueue_.front());\n> +\t\tif (converter_)\n> +\t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front());\n> +\t\telse\n> +\t\t\tswIsp_->queueBuffers(buffer, conversionQueue_.front());\n> +\n> \t\tconversionQueue_.pop();\n> \t\treturn;\n> \t}\n> @@ -835,6 +882,18 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> \t\tpipe->completeRequest(request);\n> }\n> \n> +void SimpleCameraData::ispStatsReady([[maybe_unused]] int dummy)\n> +{\n> +\tswIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n> +\t\t\t\t\t\t V4L2_CID_EXPOSURE }));\n\nYou should use the DelayedControls class (can be done later, but please\nadd a \\todo comment).\n\n> +}\n> +\n> +void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> +{\n> +\tControlList ctrls(sensorControls);\n> +\tsensor_->setControls(&ctrls);\n> +}\n> +\n> /* Retrieve all source pads connected to a sink pad through active routes. */\n> std::vector SimpleCameraData::routedSourcePads(MediaPad *sink)\n> {\n> @@ -1047,8 +1106,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\t/* Set the stride, frameSize and bufferCount. */\n> \t\tif (needConversion_) {\n> \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n> -\t\t\t\tdata_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> -\t\t\t\t\t\t\t\t cfg.size);\n> +\t\t\t\t(data_->converter_) ? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n\nNo need for parentheses.\n\n> +\t\t\t\t\t\t\t\t\t\t\t cfg.size)\n> +\t\t\t\t\t\t : data_->swIsp_->strideAndFrameSize(cfg.pixelFormat,\n> +\t\t\t\t\t\t\t\t\t\t\tcfg.size);\n> \t\t\tif (cfg.stride == 0)\n> \t\t\t\treturn Invalid;\n> \t\t} else {\n> @@ -1211,7 +1272,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tinputCfg.stride = captureFormat.planes[0].bpl;\n> \tinputCfg.bufferCount = kNumInternalBuffers;\n> \n> -\treturn data->converter_->configure(inputCfg, outputCfgs);\n> +\treturn (data->converter_) ? data->converter_->configure(inputCfg, outputCfgs)\n\nDitto.\n\n> +\t\t\t\t : data->swIsp_->configure(inputCfg, outputCfgs,\n> +\t\t\t\t\t\t\t data->sensor_->controls());\n> }\n> \n> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -1225,8 +1288,10 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t * whether the converter is used or not.\n> \t */\n> \tif (data->useConversion_)\n> -\t\treturn data->converter_->exportBuffers(data->streamIndex(stream),\n> -\t\t\t\t\t\t count, buffers);\n> +\t\treturn (data->converter_) ? data->converter_->exportBuffers(data->streamIndex(stream),\n\nHere too.\n\n> +\t\t\t\t\t\t\t\t\t count, buffers)\n> +\t\t\t\t\t : data->swIsp_->exportBuffers(data->streamIndex(stream),\n> +\t\t\t\t\t\t\t\t\tcount, buffers);\n> \telse\n> \t\treturn data->video_->exportBuffers(count, buffers);\n> }\n> @@ -1271,10 +1336,18 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t}\n> \n> \tif (data->useConversion_) {\n> -\t\tret = data->converter_->start();\n> -\t\tif (ret < 0) {\n> -\t\t\tstop(camera);\n> -\t\t\treturn ret;\n> +\t\tif (data->converter_) {\n> +\t\t\tret = data->converter_->start();\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tstop(camera);\n> +\t\t\t\treturn ret;\n> +\t\t\t}\n> +\t\t} else if (data->swIsp_) {\n> +\t\t\tret = data->swIsp_->start();\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tstop(camera);\n> +\t\t\t\treturn ret;\n> +\t\t\t}\n> \t\t}\n\n\n\t\tif (data->converter_)\n\t\t\tret = data->converter_->start();\n\t\telse if (data->swIsp_)\n\t\t\tret = data->swIsp_->start();\n\t\telse\n\t\t\tret = 0;\n\n\t\tif (ret < 0) {\n\t\t\tstop(camera);\n\t\t\treturn ret;\n\t\t}\n\nMaybe the else if could be turned into an else, as if useConversion_ is\ntrue you should be guaranteed to have either a converter or a software\nISP.\n\n> \n> \t\t/* Queue all internal buffers for capture. */\n> @@ -1290,8 +1363,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> \tSimpleCameraData *data = cameraData(camera);\n> \tV4L2VideoDevice *video = data->video_;\n> \n> -\tif (data->useConversion_)\n> -\t\tdata->converter_->stop();\n> +\tif (data->useConversion_) {\n> +\t\tif (data->converter_)\n> +\t\t\tdata->converter_->stop();\n> +\t\telse if (data->swIsp_) {\n> +\t\t\tdata->swIsp_->stop();\n> +\t\t}\n\nNo need for curly braces, and same comment regarding the else if.\n\n> +\t}\n> \n> \tvideo->streamOff();\n> \tvideo->releaseBuffers();\n> @@ -1453,6 +1531,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t\t}\n> \t}\n> \n> +\tswIspEnabled_ = info->swIspEnabled;\n> +\n> \t/* Locate the sensors. */\n> \tstd::vector sensors = locateSensors();\n> \tif (sensors.empty()) {","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 59501C0DA4\n\tfor ;\n\tWed, 27 Mar 2024 17:57:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 859DF63331;\n\tWed, 27 Mar 2024 18:57:38 +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 E792261C35\n\tfor ;\n\tWed, 27 Mar 2024 18:57:36 +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 0CBA51571;\n\tWed, 27 Mar 2024 18:57:03 +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=\"dbiTWXiH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711562224;\n\tbh=zo7wUIZOjurVXq0ycqNsbAM9n93H1ZDHG+7mSVgr1NQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dbiTWXiHGHEOSjqZdt/jnivP8KRGeKUbenglvPMyvr/1ActpxhflnIBvAh05YrrpU\n\tB9hUWNZE/t1aEvbU8oXdorEtt97p/AdMSmJ01GVT75h5eHVyLvO4ixN50AronRb64J\n\tS1KnAAXk/sF/6BkPB81Y9u9ZuM6RGa61yAzO4WWA=","Date":"Wed, 27 Mar 2024 19:57:28 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tAndrey Konovalov ,\n\tBryan O'Donoghue ,\n\tMaxime Ripard , Pavel Machek ,\n\tHans de Goede ,\n\tKieran Bingham ","Subject":"Re: [PATCH v6 12/18] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","Message-ID":"<20240327175728.GI4721@pendragon.ideasonboard.com>","References":"<20240319123622.675599-1-mzamazal@redhat.com>\n\t<20240319123622.675599-13-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240319123622.675599-13-mzamazal@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":29129,"web_url":"https://patchwork.libcamera.org/comment/29129/","msgid":"<87o7arj1n2.fsf@redhat.com>","date":"2024-04-02T14:10:57","subject":"Re: [PATCH v6 12/18] libcamera: pipeline: simple: enable use of\n\tSoft ISP and Soft IPA","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n>> @@ -1290,8 +1363,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n>> \tSimpleCameraData *data = cameraData(camera);\n>> \tV4L2VideoDevice *video = data->video_;\n>> \n>> -\tif (data->useConversion_)\n>> -\t\tdata->converter_->stop();\n>> +\tif (data->useConversion_) {\n>> +\t\tif (data->converter_)\n>> +\t\t\tdata->converter_->stop();\n>> +\t\telse if (data->swIsp_) {\n>> +\t\t\tdata->swIsp_->stop();\n>> +\t\t}\n>\n> No need for curly braces, and same comment regarding the else if.\n\nAs for the outer ones, my compiler requires them to prevent nested if-else\nconfusion.\n\nRegards,\nMilan","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 F334BC0DA4\n\tfor ;\n\tTue, 2 Apr 2024 14:11:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 074136308D;\n\tTue, 2 Apr 2024 16:11:08 +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 4E00861C33\n\tfor ;\n\tTue, 2 Apr 2024 16:11:03 +0200 (CEST)","from mail-ej1-f70.google.com (mail-ej1-f70.google.com\n\t[209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-106-OleV58hpPEqBKyFy881biA-1; Tue, 02 Apr 2024 10:11:00 -0400","by mail-ej1-f70.google.com with SMTP id\n\ta640c23a62f3a-a46bae02169so530927266b.1\n\tfor ;\n\tTue, 02 Apr 2024 07:11:00 -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\td22-20020a170906175600b00a4e583adcc7sm3618159eje.99.2024.04.02.07.10.58\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 02 Apr 2024 07:10:58 -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=\"YUgdxBep\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1712067062;\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=8dndCS9TpIpuWa1BvldAsskkOZlAY7m63gZwmPjgOzY=;\n\tb=YUgdxBepawr1JTCBeGalrdzAZ21YJJZW4GjuasyY7oDwHulOpV5CzdgUpQdIHbnp1gjWIP\n\tVgBvgWR9DPzSbgcfBuH9dzR5DkdJKoR2Tys/1CeOi036Pnp01Zb+sSSwYAH+VYtp3yQzoV\n\tc7mTG+Nd6Rc+HtFS7fAXaKMcJDrJA3w=","X-MC-Unique":"OleV58hpPEqBKyFy881biA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1712067059; x=1712671859;\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=8dndCS9TpIpuWa1BvldAsskkOZlAY7m63gZwmPjgOzY=;\n\tb=Izr7knWjNqGpAjYRv798htDmwyW4bRt21bRkukflDqyiyfmcFOdtq5kr1V94tGYLBc\n\t8VTHC7lm0e5cuGtzRZdkDj/qunmZq6i6DHKIGp0xTtht85NhYgBXtgT7BmWq0rfVIXnQ\n\tOl7oAPX15hIAKbSxj1dWQ49XFtPtJPlZIB2aQsAW5GtWnPyIluCfLKJJtPQ+xQ+ovaTM\n\t0fu1vNEZ7JFIqNW5Uqeahnd1WEO4xOzvoyZgfUHlFqSP9WH83xQ4PCMHDAEYv+Lsmryg\n\trlN4LZPP1dwV4zMnvralNeL/ZH1cjh+VsA3FwrnemwsuhYBxTLV+JNULnWti1nfYI1rm\n\tYU4g==","X-Gm-Message-State":"AOJu0Ywf6mWcS6oqWXHvd8Ip92jM+kc5Diiuj1oi3pryVVvcrQNj50cN\n\tHot3TeoKd1XNlQYpDbegVguKG0jBnS5QkSCHLEMwMwu544nuaDLClH1vJ6W9226mQmUJWYhe4q+\n\tqcA00ht7bEEXTE+At6cHtW7BdOpraqLd0owkaO+vwT67r+v1Ea17nfS6YRY8LgsxG3HTl7Kg=","X-Received":["by 2002:a17:906:370e:b0:a4e:6601:2968 with SMTP id\n\td14-20020a170906370e00b00a4e66012968mr5201915ejc.30.1712067059510; \n\tTue, 02 Apr 2024 07:10:59 -0700 (PDT)","by 2002:a17:906:370e:b0:a4e:6601:2968 with SMTP id\n\td14-20020a170906370e00b00a4e66012968mr5201906ejc.30.1712067059182; \n\tTue, 02 Apr 2024 07:10:59 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHnl4C1FLHs3HCJigxdyiSqCfD1lSiwEjiM5cHXBqJltSQAliZpbiQkPttNLuBodPn/niicag==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Andrey Konovalov\n\t, Bryan O'Donoghue\n\t, Maxime Ripard , Pavel\n\tMachek , Hans de Goede , Kieran\n\tBingham ","Subject":"Re: [PATCH v6 12/18] libcamera: pipeline: simple: enable use of\n\tSoft ISP and Soft IPA","In-Reply-To":"<20240327175728.GI4721@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Wed, 27 Mar 2024 19:57:28 +0200\")","References":"<20240319123622.675599-1-mzamazal@redhat.com>\n\t<20240319123622.675599-13-mzamazal@redhat.com>\n\t<20240327175728.GI4721@pendragon.ideasonboard.com>","Date":"Tue, 02 Apr 2024 16:10:57 +0200","Message-ID":"<87o7arj1n2.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":29130,"web_url":"https://patchwork.libcamera.org/comment/29130/","msgid":"<20240402150214.GA20073@pendragon.ideasonboard.com>","date":"2024-04-02T15:02:14","subject":"Re: [PATCH v6 12/18] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 02, 2024 at 04:10:57PM +0200, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> \n> >> @@ -1290,8 +1363,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> >> \tSimpleCameraData *data = cameraData(camera);\n> >> \tV4L2VideoDevice *video = data->video_;\n> >> \n> >> -\tif (data->useConversion_)\n> >> -\t\tdata->converter_->stop();\n> >> +\tif (data->useConversion_) {\n> >> +\t\tif (data->converter_)\n> >> +\t\t\tdata->converter_->stop();\n> >> +\t\telse if (data->swIsp_) {\n> >> +\t\t\tdata->swIsp_->stop();\n> >> +\t\t}\n> >\n> > No need for curly braces, and same comment regarding the else if.\n> \n> As for the outer ones, my compiler requires them to prevent nested if-else\n> confusion.\n\nYes, I meant the inner ones only, I have probably misread the code\ninitially. Sorry about that.","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 95B3BBD16B\n\tfor ;\n\tTue, 2 Apr 2024 15:02:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7FE166334D;\n\tTue, 2 Apr 2024 17:02:28 +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 EDB4E61C33\n\tfor ;\n\tTue, 2 Apr 2024 17:02:24 +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 CD9D5564;\n\tTue, 2 Apr 2024 17:01:47 +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=\"IZeaN4C0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712070108;\n\tbh=naifRjSy8h9ONoZBP5d6oJPusdSKiHj8efFid+b6K88=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IZeaN4C068rfXusRKeKSW33M9cQgrZbqIluUxpTSzeiHp4R5qmERHCrV2UVWYkhDB\n\thvai2faCDJnyGrCOB8zAbwzlTnStNW9j6hHuDUxsWbVwy58m0IdIJPuNm5L4vmiRFL\n\tEcOwJB+YfW9dwV4gwp6LLEYwNkjGfqHcsbsN/jCQ=","Date":"Tue, 2 Apr 2024 18:02:14 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tAndrey Konovalov ,\n\tBryan O'Donoghue ,\n\tMaxime Ripard , Pavel Machek ,\n\tHans de Goede ,\n\tKieran Bingham ","Subject":"Re: [PATCH v6 12/18] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","Message-ID":"<20240402150214.GA20073@pendragon.ideasonboard.com>","References":"<20240319123622.675599-1-mzamazal@redhat.com>\n\t<20240319123622.675599-13-mzamazal@redhat.com>\n\t<20240327175728.GI4721@pendragon.ideasonboard.com>\n\t<87o7arj1n2.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87o7arj1n2.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\" "}}]