[{"id":28676,"web_url":"https://patchwork.libcamera.org/comment/28676/","msgid":"<871q9dd685.fsf@redhat.com>","date":"2024-02-15T16:47:54","subject":"Re: [PATCH v3 12/16] 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":"Hans de Goede writes:\n\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\nIt would be nice if it was possible to use/test software ISP without specific\npatches to the array. Do I get it right that the idea is to enable softisp for\ncertain devices where it can be used & there is no better pipeline handler to\nuse? And there could be a runtime option for devices where it can be used &\nthere is a better pipeline handler to use? Not a blocker but it should be\nclarified in the commit message.\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 | 136 ++++++++++++++++++-----\n> 1 file changed, 108 insertions(+), 28 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 78854ef8..eab5b56e 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,17 +186,22 @@ 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> +\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{ \"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{ \"mxc-isi\", {}, false },\n> +\t{ \"qcom-camss\", {}, true },\n> +\t{ \"sun6i-csi\", {}, false },\n> };\n> \n> } /* namespace */\n> @@ -274,6 +280,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> @@ -281,6 +288,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> @@ -332,6 +342,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> protected:\n> \tint queueRequestDevice(Camera *camera, Request *request) override;\n> @@ -360,6 +371,7 @@ private:\n> \tstd::map entities_;\n> \n> \tMediaDevice *converter_;\n> +\tbool swIspEnabled_;\n> };\n> \n> /* -----------------------------------------------------------------------------\n> @@ -509,6 +521,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> +\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> @@ -599,12 +634,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> +\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> @@ -750,9 +793,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> @@ -798,9 +841,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> @@ -808,7 +851,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> @@ -834,6 +881,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> +}\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> @@ -1046,8 +1105,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> +\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> @@ -1210,7 +1271,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> +\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> @@ -1224,8 +1287,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> +\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> @@ -1270,10 +1335,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> \t\t/* Queue all internal buffers for capture. */\n> @@ -1289,8 +1362,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> +\t}\n> \n> \tvideo->streamOff();\n> \tvideo->releaseBuffers();\n> @@ -1452,6 +1530,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 66405C3257\n\tfor ;\n\tThu, 15 Feb 2024 16:48:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 936FB62805;\n\tThu, 15 Feb 2024 17:48:02 +0100 (CET)","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 40FA761CB0\n\tfor ;\n\tThu, 15 Feb 2024 17:48:00 +0100 (CET)","from mail-lf1-f70.google.com (mail-lf1-f70.google.com\n\t[209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-648-k16QpJhDN8GHqj63DKBrkg-1; Thu, 15 Feb 2024 11:47:58 -0500","by mail-lf1-f70.google.com with SMTP id\n\t2adb3069b0e04-511a4a286f2so996050e87.3\n\tfor ;\n\tThu, 15 Feb 2024 08:47:57 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\t9-20020a05600c228900b004101543e843sm5576098wmf.10.2024.02.15.08.47.55\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 15 Feb 2024 08:47:55 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"FhlKy8y4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1708015679;\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=E4Gne5iFn+K8HzHuu6s8PN7C1Xi6l+/PWP73vYzMIbo=;\n\tb=FhlKy8y4lJqMNNKbKSEBz74ds9Ck4E4nyB1+Mkvv/LlD3ANXRixnxkk/fk7JplGMoYYyHQ\n\tMxETFiUSemIAVP3OSNg5w4GeWeXkQOPMMTaxdOu3DjEBDCyS7bk+E+sBkHvCgkg5qYcTKc\n\tIsIULaPPLfnJAzuw8CMVj+9Yl++8Hak=","X-MC-Unique":"k16QpJhDN8GHqj63DKBrkg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1708015676; x=1708620476;\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=E4Gne5iFn+K8HzHuu6s8PN7C1Xi6l+/PWP73vYzMIbo=;\n\tb=e6qK0Dh0ectktAbuQmafVJ4fr/bXUmXOz3S9lnckwcuNpO/mNv4Y9jEkl/Fv/U1V83\n\tyTGV8Ikaefgno/XFn9jSgG8+cRJVMPC2f2cF4JX7WUCmgQN20ZZXpadS5gmnB3e3APVH\n\tEXjDV0MpwmDzWH2bAxsYpbIelZHU7gan3EQj8olMScgMlRXapocKbRO8y2bkbePIqWf5\n\twTdh08PuU3a/annTUZTQMlv2+zduqIbBi8QiStM2xZX+sxy+4zePh8jZQaRSkQGmSd0n\n\tov9ru0WVMHI4l4cJouDgacwbv089uN4935xgBh5ZBg7+dqkXc2LkRx4vysRrMHaoLOAw\n\tJkdg==","X-Gm-Message-State":"AOJu0YzLhr6957235jkv68ztQtxbjreb2/RZyA8F4XMOczbVaDwIdPaw\n\tx0q6kbUQvFY7aV5p1g0UAogvgOx1VSAN8fX3F2IMoXEzpEuzUA6BhxW/kgKUeo0Ks1aHcp/Vpkt\n\tO3lvRdrxWpRz9iv7+ps6NFrMwbCzNC5S55oQ0mOelED0LEZgNmrtnfbFhL77HqjgbV7Vw83I=","X-Received":["by 2002:ac2:4189:0:b0:511:3460:925c with SMTP id\n\tz9-20020ac24189000000b005113460925cmr1699623lfh.65.1708015676376; \n\tThu, 15 Feb 2024 08:47:56 -0800 (PST)","by 2002:ac2:4189:0:b0:511:3460:925c with SMTP id\n\tz9-20020ac24189000000b005113460925cmr1699608lfh.65.1708015675965; \n\tThu, 15 Feb 2024 08:47:55 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IGO1RzQpjNrygLAZMZOF4gAnBwL+KEO0vHSvqOBBtePCE1A4WCLP4iUIeNzAWoma/ajnChUPA==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of\n\tSoft ISP and Soft IPA","In-Reply-To":"<20240214170122.60754-13-hdegoede@redhat.com> (Hans de Goede's\n\tmessage of \"Wed, 14 Feb 2024 18:01:16 +0100\")","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-13-hdegoede@redhat.com>","Date":"Thu, 15 Feb 2024 17:47:54 +0100","Message-ID":"<871q9dd685.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","Cc":"Bryan O'Donoghue ,\n\tlibcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28686,"web_url":"https://patchwork.libcamera.org/comment/28686/","msgid":"<170809713694.2629073.10070034761974663420@ping.linuxembedded.co.uk>","date":"2024-02-16T15:25:36","subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2024-02-15 16:47:54)\n> Hans de Goede writes:\n> \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> \n> It would be nice if it was possible to use/test software ISP without specific\n> patches to the array. Do I get it right that the idea is to enable softisp for\n> certain devices where it can be used & there is no better pipeline handler to\n> use? And there could be a runtime option for devices where it can be used &\n> there is a better pipeline handler to use? Not a blocker but it should be\n> clarified in the commit message.\n\nI think for now - this is compile time code configuration, which I preferred\nover having a compile meson option that would have been harder to\nsupport to distributions to convey the diffence between \"This pipeline\nreally needs the SoftISP\" vs \"This pipeline can optionally use the\nSoftISP\".\n\nHaving runtime configuration with a pipeline configuration file may be\nappropriate in the near future, so it doesn't have to stay fixed like\nthis.\n\nBut we probably need to handle things like raw stream passthrough before\nwe 'enable' the SoftISP more generically.\n\n--\nKieran\n\n\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 | 136 ++++++++++++++++++-----\n> > 1 file changed, 108 insertions(+), 28 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 78854ef8..eab5b56e 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,17 +186,22 @@ struct SimplePipelineInfo {\n> > * and the number of streams it supports.\n> > */\n> > std::vector> converters;\n> > + /*\n> > + * Using Software ISP is to be enabled per driver.\n> > + * The Software ISP can't be used together with the converters.\n> > + */\n> > + bool swIspEnabled;\n> > };\n> > \n> > namespace {\n> > \n> > static const SimplePipelineInfo supportedDevices[] = {\n> > - { \"dcmipp\", {} },\n> > - { \"imx7-csi\", { { \"pxp\", 1 } } },\n> > - { \"j721e-csi2rx\", {} },\n> > - { \"mxc-isi\", {} },\n> > - { \"qcom-camss\", {} },\n> > - { \"sun6i-csi\", {} },\n> > + { \"dcmipp\", {}, false },\n> > + { \"imx7-csi\", { { \"pxp\", 1 } }, false },\n> > + { \"j721e-csi2rx\", {}, false },\n> > + { \"mxc-isi\", {}, false },\n> > + { \"qcom-camss\", {}, true },\n> > + { \"sun6i-csi\", {}, false },\n> > };\n> > \n> > } /* namespace */\n> > @@ -274,6 +280,7 @@ public:\n> > bool useConversion_;\n> > \n> > std::unique_ptr converter_;\n> > + std::unique_ptr swIsp_;\n> > \n> > private:\n> > void tryPipeline(unsigned int code, const Size &size);\n> > @@ -281,6 +288,9 @@ private:\n> > \n> > void conversionInputDone(FrameBuffer *buffer);\n> > void conversionOutputDone(FrameBuffer *buffer);\n> > +\n> > + void ispStatsReady(int dummy);\n> > + void setSensorControls(const ControlList &sensorControls);\n> > };\n> > \n> > class SimpleCameraConfiguration : public CameraConfiguration\n> > @@ -332,6 +342,7 @@ public:\n> > V4L2VideoDevice *video(const MediaEntity *entity);\n> > V4L2Subdevice *subdev(const MediaEntity *entity);\n> > MediaDevice *converter() { return converter_; }\n> > + bool swIspEnabled() { return swIspEnabled_; }\n> > \n> > protected:\n> > int queueRequestDevice(Camera *camera, Request *request) override;\n> > @@ -360,6 +371,7 @@ private:\n> > std::map entities_;\n> > \n> > MediaDevice *converter_;\n> > + bool swIspEnabled_;\n> > };\n> > \n> > /* -----------------------------------------------------------------------------\n> > @@ -509,6 +521,29 @@ int SimpleCameraData::init()\n> > }\n> > }\n> > \n> > + /*\n> > + * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.\n> > + */\n> > + if (!converter_ && pipe->swIspEnabled()) {\n> > + swIsp_ = std::make_unique(pipe, sensor_->controls());\n> > + if (!swIsp_->isValid()) {\n> > + LOG(SimplePipeline, Warning)\n> > + << \"Failed to create software ISP, disabling software debayering\";\n> > + swIsp_.reset();\n> > + } else {\n> > + /*\n> > + * \\todo explain why SimpleCameraData::conversionInputDone() can't be directly\n> > + * connected to inputBufferReady signal.\n> > + */\n> > + swIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> > + this->conversionInputDone(buffer);\n> > + });\n> > + swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n> > + swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n> > + swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);\n> > + }\n> > + }\n> > +\n> > video_ = pipe->video(entities_.back().entity);\n> > ASSERT(video_);\n> > \n> > @@ -599,12 +634,20 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n> > config.captureFormat = pixelFormat;\n> > config.captureSize = format.size;\n> > \n> > - if (!converter_) {\n> > - config.outputFormats = { pixelFormat };\n> > - config.outputSizes = config.captureSize;\n> > - } else {\n> > + if (converter_) {\n> > config.outputFormats = converter_->formats(pixelFormat);\n> > config.outputSizes = converter_->sizes(format.size);\n> > + } else if (swIsp_) {\n> > + config.outputFormats = swIsp_->formats(pixelFormat);\n> > + config.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n> > + if (config.outputFormats.empty()) {\n> > + /* Do not use swIsp for unsupported pixelFormat's: */\n> > + config.outputFormats = { pixelFormat };\n> > + config.outputSizes = config.captureSize;\n> > + }\n> > + } else {\n> > + config.outputFormats = { pixelFormat };\n> > + config.outputSizes = config.captureSize;\n> > }\n> > \n> > configs_.push_back(config);\n> > @@ -750,9 +793,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> > }\n> > \n> > /*\n> > - * The converter is in use. Requeue the internal buffer for\n> > - * capture (unless the stream is being stopped), and complete\n> > - * the request with all the user-facing buffers.\n> > + * The converter or Software ISP is in use. Requeue the internal\n> > + * buffer for capture (unless the stream is being stopped), and\n> > + * complete the request with all the user-facing buffers.\n> > */\n> > if (buffer->metadata().status != FrameMetadata::FrameCancelled)\n> > video_->queueBuffer(buffer);\n> > @@ -798,9 +841,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> > buffer->metadata().timestamp);\n> > \n> > /*\n> > - * Queue the captured and the request buffer to the converter if format\n> > - * conversion is needed. If there's no queued request, just requeue the\n> > - * captured buffer for capture.\n> > + * Queue the captured and the request buffer to the converter or Software\n> > + * ISP if format conversion is needed. If there's no queued request, just\n> > + * requeue the captured buffer for capture.\n> > */\n> > if (useConversion_) {\n> > if (conversionQueue_.empty()) {\n> > @@ -808,7 +851,11 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> > return;\n> > }\n> > \n> > - converter_->queueBuffers(buffer, conversionQueue_.front());\n> > + if (converter_)\n> > + converter_->queueBuffers(buffer, conversionQueue_.front());\n> > + else\n> > + swIsp_->queueBuffers(buffer, conversionQueue_.front());\n> > +\n> > conversionQueue_.pop();\n> > return;\n> > }\n> > @@ -834,6 +881,18 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> > pipe->completeRequest(request);\n> > }\n> > \n> > +void SimpleCameraData::ispStatsReady([[maybe_unused]] int dummy)\n> > +{\n> > + swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n> > + V4L2_CID_EXPOSURE }));\n> > +}\n> > +\n> > +void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> > +{\n> > + ControlList ctrls(sensorControls);\n> > + sensor_->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> > @@ -1046,8 +1105,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > /* Set the stride, frameSize and bufferCount. */\n> > if (needConversion_) {\n> > std::tie(cfg.stride, cfg.frameSize) =\n> > - data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> > - cfg.size);\n> > + (data_->converter_) ? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> > + cfg.size)\n> > + : data_->swIsp_->strideAndFrameSize(cfg.pixelFormat,\n> > + cfg.size);\n> > if (cfg.stride == 0)\n> > return Invalid;\n> > } else {\n> > @@ -1210,7 +1271,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > inputCfg.stride = captureFormat.planes[0].bpl;\n> > inputCfg.bufferCount = kNumInternalBuffers;\n> > \n> > - return data->converter_->configure(inputCfg, outputCfgs);\n> > + return (data->converter_) ? data->converter_->configure(inputCfg, outputCfgs)\n> > + : data->swIsp_->configure(inputCfg, outputCfgs,\n> > + data->sensor_->controls());\n> > }\n> > \n> > int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> > @@ -1224,8 +1287,10 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> > * whether the converter is used or not.\n> > */\n> > if (data->useConversion_)\n> > - return data->converter_->exportBuffers(data->streamIndex(stream),\n> > - count, buffers);\n> > + return (data->converter_) ? data->converter_->exportBuffers(data->streamIndex(stream),\n> > + count, buffers)\n> > + : data->swIsp_->exportBuffers(data->streamIndex(stream),\n> > + count, buffers);\n> > else\n> > return data->video_->exportBuffers(count, buffers);\n> > }\n> > @@ -1270,10 +1335,18 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> > }\n> > \n> > if (data->useConversion_) {\n> > - ret = data->converter_->start();\n> > - if (ret < 0) {\n> > - stop(camera);\n> > - return ret;\n> > + if (data->converter_) {\n> > + ret = data->converter_->start();\n> > + if (ret < 0) {\n> > + stop(camera);\n> > + return ret;\n> > + }\n> > + } else if (data->swIsp_) {\n> > + ret = data->swIsp_->start();\n> > + if (ret < 0) {\n> > + stop(camera);\n> > + return ret;\n> > + }\n> > }\n> > \n> > /* Queue all internal buffers for capture. */\n> > @@ -1289,8 +1362,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> > SimpleCameraData *data = cameraData(camera);\n> > V4L2VideoDevice *video = data->video_;\n> > \n> > - if (data->useConversion_)\n> > - data->converter_->stop();\n> > + if (data->useConversion_) {\n> > + if (data->converter_)\n> > + data->converter_->stop();\n> > + else if (data->swIsp_) {\n> > + data->swIsp_->stop();\n> > + }\n> > + }\n> > \n> > video->streamOff();\n> > video->releaseBuffers();\n> > @@ -1452,6 +1530,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > }\n> > }\n> > \n> > + swIspEnabled_ = info->swIspEnabled;\n> > +\n> > /* Locate the sensors. */\n> > std::vector sensors = locateSensors();\n> > if (sensors.empty()) {\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 53F9DBDE17\n\tfor ;\n\tFri, 16 Feb 2024 15:25:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B61B62805;\n\tFri, 16 Feb 2024 16:25:41 +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 250B661CAE\n\tfor ;\n\tFri, 16 Feb 2024 16:25:40 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 84C596B3;\n\tFri, 16 Feb 2024 16:25:35 +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=\"EOv85h0b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708097135;\n\tbh=D9TvbtpElI9b/VOCVgvn6xtx7kk6ghkU/LmNlwWZWwo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=EOv85h0bV/LAJdHyJZAOAyUczCUDOR3IOdwMtLcQjTMTfaqbhX//CQJmsfLLMiO2A\n\t7xSsgp/7xMXVhYjj3ZTdSzsOFacPFOD15sF0DBOAHUN4dTbZmMF7WHKM2PeBHA6DYY\n\toHC3oIOf0AXXw4iJoSoZSy6smxdxf7et9CUUmxFg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<871q9dd685.fsf@redhat.com>","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-13-hdegoede@redhat.com>\n\t<871q9dd685.fsf@redhat.com>","Subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","From":"Kieran Bingham ","To":"Hans de Goede , Milan Zamazal ","Date":"Fri, 16 Feb 2024 15:25:36 +0000","Message-ID":"<170809713694.2629073.10070034761974663420@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","Cc":"Bryan O'Donoghue ,\n\tlibcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28687,"web_url":"https://patchwork.libcamera.org/comment/28687/","msgid":"<0b940292-aa73-44d2-b4e9-568e2820149e@gmail.com>","date":"2024-02-16T16:19:25","subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","submitter":{"id":179,"url":"https://patchwork.libcamera.org/api/people/179/","name":"Andrei Konovalov","email":"andrey.konovalov.ynk@gmail.com"},"content":"Hi Kieran,\n\nOn 16.02.2024 18:25, Kieran Bingham wrote:\n> Quoting Milan Zamazal (2024-02-15 16:47:54)\n>> Hans de Goede writes:\n>>\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>>\n>> It would be nice if it was possible to use/test software ISP without specific\n>> patches to the array. Do I get it right that the idea is to enable softisp for\n>> certain devices where it can be used & there is no better pipeline handler to\n>> use? And there could be a runtime option for devices where it can be used &\n>> there is a better pipeline handler to use? Not a blocker but it should be\n>> clarified in the commit message.\n> \n> I think for now - this is compile time code configuration, which I preferred\n> over having a compile meson option that would have been harder to\n> support to distributions to convey the diffence between \"This pipeline\n> really needs the SoftISP\" vs \"This pipeline can optionally use the\n> SoftISP\".\n> \n> Having runtime configuration with a pipeline configuration file may be\n> appropriate in the near future, so it doesn't have to stay fixed like\n> this.\n> \n> But we probably need to handle things like raw stream passthrough before\n> we 'enable' the SoftISP more generically.\n\nGetting the raw bayer frames with \"--stream pixelformat=$RAW_BAYER_FMT\" vs\nthe processed by SoftISP ones with e.g. \"--stream pixelformat=RGB888\" - would it\ncount as raw stream passthrough?\n\nThis doesn't currently work with simple pipeline handler, but I have two\npatches to fix this. (They conflict with the SoftISP patch set, so I am keeping\nthem locally not to mess with the current work.)\n\nThanks,\nAndrei\n\n> --\n> Kieran\n> \n> \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 | 136 ++++++++++++++++++-----\n>>> 1 file changed, 108 insertions(+), 28 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 78854ef8..eab5b56e 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,17 +186,22 @@ struct SimplePipelineInfo {\n>>> * and the number of streams it supports.\n>>> */\n>>> std::vector> converters;\n>>> + /*\n>>> + * Using Software ISP is to be enabled per driver.\n>>> + * The Software ISP can't be used together with the converters.\n>>> + */\n>>> + bool swIspEnabled;\n>>> };\n>>> \n>>> namespace {\n>>> \n>>> static const SimplePipelineInfo supportedDevices[] = {\n>>> - { \"dcmipp\", {} },\n>>> - { \"imx7-csi\", { { \"pxp\", 1 } } },\n>>> - { \"j721e-csi2rx\", {} },\n>>> - { \"mxc-isi\", {} },\n>>> - { \"qcom-camss\", {} },\n>>> - { \"sun6i-csi\", {} },\n>>> + { \"dcmipp\", {}, false },\n>>> + { \"imx7-csi\", { { \"pxp\", 1 } }, false },\n>>> + { \"j721e-csi2rx\", {}, false },\n>>> + { \"mxc-isi\", {}, false },\n>>> + { \"qcom-camss\", {}, true },\n>>> + { \"sun6i-csi\", {}, false },\n>>> };\n>>> \n>>> } /* namespace */\n>>> @@ -274,6 +280,7 @@ public:\n>>> bool useConversion_;\n>>> \n>>> std::unique_ptr converter_;\n>>> + std::unique_ptr swIsp_;\n>>> \n>>> private:\n>>> void tryPipeline(unsigned int code, const Size &size);\n>>> @@ -281,6 +288,9 @@ private:\n>>> \n>>> void conversionInputDone(FrameBuffer *buffer);\n>>> void conversionOutputDone(FrameBuffer *buffer);\n>>> +\n>>> + void ispStatsReady(int dummy);\n>>> + void setSensorControls(const ControlList &sensorControls);\n>>> };\n>>> \n>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>> @@ -332,6 +342,7 @@ public:\n>>> V4L2VideoDevice *video(const MediaEntity *entity);\n>>> V4L2Subdevice *subdev(const MediaEntity *entity);\n>>> MediaDevice *converter() { return converter_; }\n>>> + bool swIspEnabled() { return swIspEnabled_; }\n>>> \n>>> protected:\n>>> int queueRequestDevice(Camera *camera, Request *request) override;\n>>> @@ -360,6 +371,7 @@ private:\n>>> std::map entities_;\n>>> \n>>> MediaDevice *converter_;\n>>> + bool swIspEnabled_;\n>>> };\n>>> \n>>> /* -----------------------------------------------------------------------------\n>>> @@ -509,6 +521,29 @@ int SimpleCameraData::init()\n>>> }\n>>> }\n>>> \n>>> + /*\n>>> + * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.\n>>> + */\n>>> + if (!converter_ && pipe->swIspEnabled()) {\n>>> + swIsp_ = std::make_unique(pipe, sensor_->controls());\n>>> + if (!swIsp_->isValid()) {\n>>> + LOG(SimplePipeline, Warning)\n>>> + << \"Failed to create software ISP, disabling software debayering\";\n>>> + swIsp_.reset();\n>>> + } else {\n>>> + /*\n>>> + * \\todo explain why SimpleCameraData::conversionInputDone() can't be directly\n>>> + * connected to inputBufferReady signal.\n>>> + */\n>>> + swIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n>>> + this->conversionInputDone(buffer);\n>>> + });\n>>> + swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n>>> + swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n>>> + swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);\n>>> + }\n>>> + }\n>>> +\n>>> video_ = pipe->video(entities_.back().entity);\n>>> ASSERT(video_);\n>>> \n>>> @@ -599,12 +634,20 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n>>> config.captureFormat = pixelFormat;\n>>> config.captureSize = format.size;\n>>> \n>>> - if (!converter_) {\n>>> - config.outputFormats = { pixelFormat };\n>>> - config.outputSizes = config.captureSize;\n>>> - } else {\n>>> + if (converter_) {\n>>> config.outputFormats = converter_->formats(pixelFormat);\n>>> config.outputSizes = converter_->sizes(format.size);\n>>> + } else if (swIsp_) {\n>>> + config.outputFormats = swIsp_->formats(pixelFormat);\n>>> + config.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n>>> + if (config.outputFormats.empty()) {\n>>> + /* Do not use swIsp for unsupported pixelFormat's: */\n>>> + config.outputFormats = { pixelFormat };\n>>> + config.outputSizes = config.captureSize;\n>>> + }\n>>> + } else {\n>>> + config.outputFormats = { pixelFormat };\n>>> + config.outputSizes = config.captureSize;\n>>> }\n>>> \n>>> configs_.push_back(config);\n>>> @@ -750,9 +793,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n>>> }\n>>> \n>>> /*\n>>> - * The converter is in use. Requeue the internal buffer for\n>>> - * capture (unless the stream is being stopped), and complete\n>>> - * the request with all the user-facing buffers.\n>>> + * The converter or Software ISP is in use. Requeue the internal\n>>> + * buffer for capture (unless the stream is being stopped), and\n>>> + * complete the request with all the user-facing buffers.\n>>> */\n>>> if (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>> video_->queueBuffer(buffer);\n>>> @@ -798,9 +841,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n>>> buffer->metadata().timestamp);\n>>> \n>>> /*\n>>> - * Queue the captured and the request buffer to the converter if format\n>>> - * conversion is needed. If there's no queued request, just requeue the\n>>> - * captured buffer for capture.\n>>> + * Queue the captured and the request buffer to the converter or Software\n>>> + * ISP if format conversion is needed. If there's no queued request, just\n>>> + * requeue the captured buffer for capture.\n>>> */\n>>> if (useConversion_) {\n>>> if (conversionQueue_.empty()) {\n>>> @@ -808,7 +851,11 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n>>> return;\n>>> }\n>>> \n>>> - converter_->queueBuffers(buffer, conversionQueue_.front());\n>>> + if (converter_)\n>>> + converter_->queueBuffers(buffer, conversionQueue_.front());\n>>> + else\n>>> + swIsp_->queueBuffers(buffer, conversionQueue_.front());\n>>> +\n>>> conversionQueue_.pop();\n>>> return;\n>>> }\n>>> @@ -834,6 +881,18 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>> pipe->completeRequest(request);\n>>> }\n>>> \n>>> +void SimpleCameraData::ispStatsReady([[maybe_unused]] int dummy)\n>>> +{\n>>> + swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n>>> + V4L2_CID_EXPOSURE }));\n>>> +}\n>>> +\n>>> +void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n>>> +{\n>>> + ControlList ctrls(sensorControls);\n>>> + sensor_->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>>> @@ -1046,8 +1105,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>> /* Set the stride, frameSize and bufferCount. */\n>>> if (needConversion_) {\n>>> std::tie(cfg.stride, cfg.frameSize) =\n>>> - data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>>> - cfg.size);\n>>> + (data_->converter_) ? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>>> + cfg.size)\n>>> + : data_->swIsp_->strideAndFrameSize(cfg.pixelFormat,\n>>> + cfg.size);\n>>> if (cfg.stride == 0)\n>>> return Invalid;\n>>> } else {\n>>> @@ -1210,7 +1271,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>> inputCfg.stride = captureFormat.planes[0].bpl;\n>>> inputCfg.bufferCount = kNumInternalBuffers;\n>>> \n>>> - return data->converter_->configure(inputCfg, outputCfgs);\n>>> + return (data->converter_) ? data->converter_->configure(inputCfg, outputCfgs)\n>>> + : data->swIsp_->configure(inputCfg, outputCfgs,\n>>> + data->sensor_->controls());\n>>> }\n>>> \n>>> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>> @@ -1224,8 +1287,10 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>> * whether the converter is used or not.\n>>> */\n>>> if (data->useConversion_)\n>>> - return data->converter_->exportBuffers(data->streamIndex(stream),\n>>> - count, buffers);\n>>> + return (data->converter_) ? data->converter_->exportBuffers(data->streamIndex(stream),\n>>> + count, buffers)\n>>> + : data->swIsp_->exportBuffers(data->streamIndex(stream),\n>>> + count, buffers);\n>>> else\n>>> return data->video_->exportBuffers(count, buffers);\n>>> }\n>>> @@ -1270,10 +1335,18 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> }\n>>> \n>>> if (data->useConversion_) {\n>>> - ret = data->converter_->start();\n>>> - if (ret < 0) {\n>>> - stop(camera);\n>>> - return ret;\n>>> + if (data->converter_) {\n>>> + ret = data->converter_->start();\n>>> + if (ret < 0) {\n>>> + stop(camera);\n>>> + return ret;\n>>> + }\n>>> + } else if (data->swIsp_) {\n>>> + ret = data->swIsp_->start();\n>>> + if (ret < 0) {\n>>> + stop(camera);\n>>> + return ret;\n>>> + }\n>>> }\n>>> \n>>> /* Queue all internal buffers for capture. */\n>>> @@ -1289,8 +1362,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n>>> SimpleCameraData *data = cameraData(camera);\n>>> V4L2VideoDevice *video = data->video_;\n>>> \n>>> - if (data->useConversion_)\n>>> - data->converter_->stop();\n>>> + if (data->useConversion_) {\n>>> + if (data->converter_)\n>>> + data->converter_->stop();\n>>> + else if (data->swIsp_) {\n>>> + data->swIsp_->stop();\n>>> + }\n>>> + }\n>>> \n>>> video->streamOff();\n>>> video->releaseBuffers();\n>>> @@ -1452,6 +1530,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n>>> }\n>>> }\n>>> \n>>> + swIspEnabled_ = info->swIspEnabled;\n>>> +\n>>> /* Locate the sensors. */\n>>> std::vector sensors = locateSensors();\n>>> if (sensors.empty()) {\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 96552C3257\n\tfor ;\n\tFri, 16 Feb 2024 16:19:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 11EAE62805;\n\tFri, 16 Feb 2024 17:19:29 +0100 (CET)","from mail-ed1-x530.google.com (mail-ed1-x530.google.com\n\t[IPv6:2a00:1450:4864:20::530])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 75FF061CAE\n\tfor ;\n\tFri, 16 Feb 2024 17:19:27 +0100 (CET)","by mail-ed1-x530.google.com with SMTP id\n\t4fb4d7f45d1cf-563ed3d221aso1209213a12.0\n\tfor ;\n\tFri, 16 Feb 2024 08:19:27 -0800 (PST)","from [192.168.118.26] ([87.116.166.222])\n\tby smtp.gmail.com with ESMTPSA id\n\ts26-20020a05640217da00b0056387e8b63csm112220edy.90.2024.02.16.08.19.25\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tFri, 16 Feb 2024 08:19:26 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"d4MQgGt1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1708100367; x=1708705167;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=TMUJIinQa1dYd8QLcbmiWKsNg/sAPjR7HGQJxfCmnK8=;\n\tb=d4MQgGt1R+PA75vLu5uXUcHk+Bx7pDHqnA7lyGidl6GwrH/uBy0AjjFNz4Uvm4teWz\n\t8mNP9VUvKV8K784w2RdExh6Tg4rhT2oNkB/xMUZ8nAP338ZSNcdrgmXtbVodA0paw+jD\n\tp9nmcBjVaWYQui80ywd1JwYLprxhRioSjeRh3uMi4XoYW+2ESc4wuJpYaFRwGwqpFmu8\n\tzNrE7vKZi8jOoKSGj+mGQ7voSoPbjVKlOFd8qGkWOvp+Oi2KHL+EL7U3IAZg9ROe4Sl1\n\tSbXsTvPcRLlPpPT/VB4KPCxwriTUCt6W0sderabOOunh41JxP08zzeCssNkbdDzdocaV\n\tTOMw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1708100367; x=1708705167;\n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language: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=TMUJIinQa1dYd8QLcbmiWKsNg/sAPjR7HGQJxfCmnK8=;\n\tb=hdFSw1UNcvg2S61VloKGyFn0Pf84ArE7HieEfyFufIO0IltbwpgPgRH4uSHCl6FBVX\n\tsnRWRpzYZ8do+8aYyVO3yS65/9JsNXK9C8+rv8dXIAZtFPvkOdg/l0+7eSRywq/lajWY\n\tw1zP4LlW9J5cBEf9nfofn4sCsdWBdrsJSijjohQ5wU3a3WS6+TfTfI80bA05agsKN7AC\n\tXTy/KHQRPb3Pi3zGs1tNBbjpKMuNo0ChZy4sG2089QBBMAkVLbxxp9alnso41MINHmCc\n\tYk0T+d3PWMe9S0pcwIasaozgmYZC5OFgMnAiY9gCJAKn8VZhv7qCSglsKBHBhfoU1lA5\n\tW2mg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUBTFN5M9r5OGFQwquZr4BJ86M/WL+y0JFJk49bI4o6NH1CJ0DJFE3nI64VjgMPzX5/nrV/6llJhgJvKm39KHnYyA7f0F3fTjGJrGjht8pxoC6ZGg==","X-Gm-Message-State":"AOJu0YwanCwg1766b41qvrR5PA0iuCW/w5qcB/37EyGVM9pt2y1+6C/D\n\tHsqyCgdCtRJ8SQ5ozPkXk5HbG28hA5qn3WczEKeJtR7cNYzJ7Mxo","X-Google-Smtp-Source":"AGHT+IGVomBQt0lGp+MqNaajpp+71MGCpE3xlnGp5TMxvloRRsk5B2mPvfWs2kPFSFSJL9zoxj06dw==","X-Received":"by 2002:aa7:d4c4:0:b0:564:152b:f7ae with SMTP id\n\tt4-20020aa7d4c4000000b00564152bf7aemr20614edr.9.1708100366576; \n\tFri, 16 Feb 2024 08:19:26 -0800 (PST)","Message-ID":"<0b940292-aa73-44d2-b4e9-568e2820149e@gmail.com>","Date":"Fri, 16 Feb 2024 19:19:25 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","Content-Language":"en-US","To":"Kieran Bingham ,\n\tHans de Goede , Milan Zamazal ","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-13-hdegoede@redhat.com>\n\t<871q9dd685.fsf@redhat.com>\n\t<170809713694.2629073.10070034761974663420@ping.linuxembedded.co.uk>","From":"Andrei Konovalov ","In-Reply-To":"<170809713694.2629073.10070034761974663420@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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","Cc":"Bryan O'Donoghue ,\n\tlibcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28689,"web_url":"https://patchwork.libcamera.org/comment/28689/","msgid":"<170810365510.1011926.5084923081917308215@ping.linuxembedded.co.uk>","date":"2024-02-16T17:14:15","subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Andrei Konovalov (2024-02-16 16:19:25)\n> Hi Kieran,\n> \n> On 16.02.2024 18:25, Kieran Bingham wrote:\n> > Quoting Milan Zamazal (2024-02-15 16:47:54)\n> >> Hans de Goede writes:\n> >>\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> >>\n> >> It would be nice if it was possible to use/test software ISP without specific\n> >> patches to the array. Do I get it right that the idea is to enable softisp for\n> >> certain devices where it can be used & there is no better pipeline handler to\n> >> use? And there could be a runtime option for devices where it can be used &\n> >> there is a better pipeline handler to use? Not a blocker but it should be\n> >> clarified in the commit message.\n> > \n> > I think for now - this is compile time code configuration, which I preferred\n> > over having a compile meson option that would have been harder to\n> > support to distributions to convey the diffence between \"This pipeline\n> > really needs the SoftISP\" vs \"This pipeline can optionally use the\n> > SoftISP\".\n> > \n> > Having runtime configuration with a pipeline configuration file may be\n> > appropriate in the near future, so it doesn't have to stay fixed like\n> > this.\n> > \n> > But we probably need to handle things like raw stream passthrough before\n> > we 'enable' the SoftISP more generically.\n> \n> Getting the raw bayer frames with \"--stream pixelformat=$RAW_BAYER_FMT\" vs\n> the processed by SoftISP ones with e.g. \"--stream pixelformat=RGB888\" - would it\n> count as raw stream passthrough?\n\nYes.\n\n> This doesn't currently work with simple pipeline handler, but I have two\n> patches to fix this. (They conflict with the SoftISP patch set, so I am keeping\n> them locally not to mess with the current work.)\n\nThat's fine for now I think. But the 'big picture' goal here for simple\npipeline handler is that Raw streams should still be accessible. That\ncould be initially that you can only support a single stream (either RAW\nor SoftProcessed) ... but I could imagine later (particurly when the ISP\ncan be GPU accellerated) that there might also be the RAW+Processed\nstreams desired.\n\n\nMaybe we should really rename 'simple' pipeline handler sometime ...\n\n--\nKieran\n\n> \n> Thanks,\n> Andrei\n> \n> > --\n> > Kieran\n> > \n> > \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 | 136 ++++++++++++++++++-----\n> >>> 1 file changed, 108 insertions(+), 28 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >>> index 78854ef8..eab5b56e 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,17 +186,22 @@ struct SimplePipelineInfo {\n> >>> * and the number of streams it supports.\n> >>> */\n> >>> std::vector> converters;\n> >>> + /*\n> >>> + * Using Software ISP is to be enabled per driver.\n> >>> + * The Software ISP can't be used together with the converters.\n> >>> + */\n> >>> + bool swIspEnabled;\n> >>> };\n> >>> \n> >>> namespace {\n> >>> \n> >>> static const SimplePipelineInfo supportedDevices[] = {\n> >>> - { \"dcmipp\", {} },\n> >>> - { \"imx7-csi\", { { \"pxp\", 1 } } },\n> >>> - { \"j721e-csi2rx\", {} },\n> >>> - { \"mxc-isi\", {} },\n> >>> - { \"qcom-camss\", {} },\n> >>> - { \"sun6i-csi\", {} },\n> >>> + { \"dcmipp\", {}, false },\n> >>> + { \"imx7-csi\", { { \"pxp\", 1 } }, false },\n> >>> + { \"j721e-csi2rx\", {}, false },\n> >>> + { \"mxc-isi\", {}, false },\n> >>> + { \"qcom-camss\", {}, true },\n> >>> + { \"sun6i-csi\", {}, false },\n> >>> };\n> >>> \n> >>> } /* namespace */\n> >>> @@ -274,6 +280,7 @@ public:\n> >>> bool useConversion_;\n> >>> \n> >>> std::unique_ptr converter_;\n> >>> + std::unique_ptr swIsp_;\n> >>> \n> >>> private:\n> >>> void tryPipeline(unsigned int code, const Size &size);\n> >>> @@ -281,6 +288,9 @@ private:\n> >>> \n> >>> void conversionInputDone(FrameBuffer *buffer);\n> >>> void conversionOutputDone(FrameBuffer *buffer);\n> >>> +\n> >>> + void ispStatsReady(int dummy);\n> >>> + void setSensorControls(const ControlList &sensorControls);\n> >>> };\n> >>> \n> >>> class SimpleCameraConfiguration : public CameraConfiguration\n> >>> @@ -332,6 +342,7 @@ public:\n> >>> V4L2VideoDevice *video(const MediaEntity *entity);\n> >>> V4L2Subdevice *subdev(const MediaEntity *entity);\n> >>> MediaDevice *converter() { return converter_; }\n> >>> + bool swIspEnabled() { return swIspEnabled_; }\n> >>> \n> >>> protected:\n> >>> int queueRequestDevice(Camera *camera, Request *request) override;\n> >>> @@ -360,6 +371,7 @@ private:\n> >>> std::map entities_;\n> >>> \n> >>> MediaDevice *converter_;\n> >>> + bool swIspEnabled_;\n> >>> };\n> >>> \n> >>> /* -----------------------------------------------------------------------------\n> >>> @@ -509,6 +521,29 @@ int SimpleCameraData::init()\n> >>> }\n> >>> }\n> >>> \n> >>> + /*\n> >>> + * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.\n> >>> + */\n> >>> + if (!converter_ && pipe->swIspEnabled()) {\n> >>> + swIsp_ = std::make_unique(pipe, sensor_->controls());\n> >>> + if (!swIsp_->isValid()) {\n> >>> + LOG(SimplePipeline, Warning)\n> >>> + << \"Failed to create software ISP, disabling software debayering\";\n> >>> + swIsp_.reset();\n> >>> + } else {\n> >>> + /*\n> >>> + * \\todo explain why SimpleCameraData::conversionInputDone() can't be directly\n> >>> + * connected to inputBufferReady signal.\n> >>> + */\n> >>> + swIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n> >>> + this->conversionInputDone(buffer);\n> >>> + });\n> >>> + swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n> >>> + swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n> >>> + swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);\n> >>> + }\n> >>> + }\n> >>> +\n> >>> video_ = pipe->video(entities_.back().entity);\n> >>> ASSERT(video_);\n> >>> \n> >>> @@ -599,12 +634,20 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n> >>> config.captureFormat = pixelFormat;\n> >>> config.captureSize = format.size;\n> >>> \n> >>> - if (!converter_) {\n> >>> - config.outputFormats = { pixelFormat };\n> >>> - config.outputSizes = config.captureSize;\n> >>> - } else {\n> >>> + if (converter_) {\n> >>> config.outputFormats = converter_->formats(pixelFormat);\n> >>> config.outputSizes = converter_->sizes(format.size);\n> >>> + } else if (swIsp_) {\n> >>> + config.outputFormats = swIsp_->formats(pixelFormat);\n> >>> + config.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n> >>> + if (config.outputFormats.empty()) {\n> >>> + /* Do not use swIsp for unsupported pixelFormat's: */\n> >>> + config.outputFormats = { pixelFormat };\n> >>> + config.outputSizes = config.captureSize;\n> >>> + }\n> >>> + } else {\n> >>> + config.outputFormats = { pixelFormat };\n> >>> + config.outputSizes = config.captureSize;\n> >>> }\n> >>> \n> >>> configs_.push_back(config);\n> >>> @@ -750,9 +793,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> >>> }\n> >>> \n> >>> /*\n> >>> - * The converter is in use. Requeue the internal buffer for\n> >>> - * capture (unless the stream is being stopped), and complete\n> >>> - * the request with all the user-facing buffers.\n> >>> + * The converter or Software ISP is in use. Requeue the internal\n> >>> + * buffer for capture (unless the stream is being stopped), and\n> >>> + * complete the request with all the user-facing buffers.\n> >>> */\n> >>> if (buffer->metadata().status != FrameMetadata::FrameCancelled)\n> >>> video_->queueBuffer(buffer);\n> >>> @@ -798,9 +841,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> >>> buffer->metadata().timestamp);\n> >>> \n> >>> /*\n> >>> - * Queue the captured and the request buffer to the converter if format\n> >>> - * conversion is needed. If there's no queued request, just requeue the\n> >>> - * captured buffer for capture.\n> >>> + * Queue the captured and the request buffer to the converter or Software\n> >>> + * ISP if format conversion is needed. If there's no queued request, just\n> >>> + * requeue the captured buffer for capture.\n> >>> */\n> >>> if (useConversion_) {\n> >>> if (conversionQueue_.empty()) {\n> >>> @@ -808,7 +851,11 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n> >>> return;\n> >>> }\n> >>> \n> >>> - converter_->queueBuffers(buffer, conversionQueue_.front());\n> >>> + if (converter_)\n> >>> + converter_->queueBuffers(buffer, conversionQueue_.front());\n> >>> + else\n> >>> + swIsp_->queueBuffers(buffer, conversionQueue_.front());\n> >>> +\n> >>> conversionQueue_.pop();\n> >>> return;\n> >>> }\n> >>> @@ -834,6 +881,18 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> >>> pipe->completeRequest(request);\n> >>> }\n> >>> \n> >>> +void SimpleCameraData::ispStatsReady([[maybe_unused]] int dummy)\n> >>> +{\n> >>> + swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n> >>> + V4L2_CID_EXPOSURE }));\n> >>> +}\n> >>> +\n> >>> +void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n> >>> +{\n> >>> + ControlList ctrls(sensorControls);\n> >>> + sensor_->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> >>> @@ -1046,8 +1105,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >>> /* Set the stride, frameSize and bufferCount. */\n> >>> if (needConversion_) {\n> >>> std::tie(cfg.stride, cfg.frameSize) =\n> >>> - data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> >>> - cfg.size);\n> >>> + (data_->converter_) ? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> >>> + cfg.size)\n> >>> + : data_->swIsp_->strideAndFrameSize(cfg.pixelFormat,\n> >>> + cfg.size);\n> >>> if (cfg.stride == 0)\n> >>> return Invalid;\n> >>> } else {\n> >>> @@ -1210,7 +1271,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> >>> inputCfg.stride = captureFormat.planes[0].bpl;\n> >>> inputCfg.bufferCount = kNumInternalBuffers;\n> >>> \n> >>> - return data->converter_->configure(inputCfg, outputCfgs);\n> >>> + return (data->converter_) ? data->converter_->configure(inputCfg, outputCfgs)\n> >>> + : data->swIsp_->configure(inputCfg, outputCfgs,\n> >>> + data->sensor_->controls());\n> >>> }\n> >>> \n> >>> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> >>> @@ -1224,8 +1287,10 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> >>> * whether the converter is used or not.\n> >>> */\n> >>> if (data->useConversion_)\n> >>> - return data->converter_->exportBuffers(data->streamIndex(stream),\n> >>> - count, buffers);\n> >>> + return (data->converter_) ? data->converter_->exportBuffers(data->streamIndex(stream),\n> >>> + count, buffers)\n> >>> + : data->swIsp_->exportBuffers(data->streamIndex(stream),\n> >>> + count, buffers);\n> >>> else\n> >>> return data->video_->exportBuffers(count, buffers);\n> >>> }\n> >>> @@ -1270,10 +1335,18 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> >>> }\n> >>> \n> >>> if (data->useConversion_) {\n> >>> - ret = data->converter_->start();\n> >>> - if (ret < 0) {\n> >>> - stop(camera);\n> >>> - return ret;\n> >>> + if (data->converter_) {\n> >>> + ret = data->converter_->start();\n> >>> + if (ret < 0) {\n> >>> + stop(camera);\n> >>> + return ret;\n> >>> + }\n> >>> + } else if (data->swIsp_) {\n> >>> + ret = data->swIsp_->start();\n> >>> + if (ret < 0) {\n> >>> + stop(camera);\n> >>> + return ret;\n> >>> + }\n> >>> }\n> >>> \n> >>> /* Queue all internal buffers for capture. */\n> >>> @@ -1289,8 +1362,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> >>> SimpleCameraData *data = cameraData(camera);\n> >>> V4L2VideoDevice *video = data->video_;\n> >>> \n> >>> - if (data->useConversion_)\n> >>> - data->converter_->stop();\n> >>> + if (data->useConversion_) {\n> >>> + if (data->converter_)\n> >>> + data->converter_->stop();\n> >>> + else if (data->swIsp_) {\n> >>> + data->swIsp_->stop();\n> >>> + }\n> >>> + }\n> >>> \n> >>> video->streamOff();\n> >>> video->releaseBuffers();\n> >>> @@ -1452,6 +1530,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> >>> }\n> >>> }\n> >>> \n> >>> + swIspEnabled_ = info->swIspEnabled;\n> >>> +\n> >>> /* Locate the sensors. */\n> >>> std::vector sensors = locateSensors();\n> >>> if (sensors.empty()) {\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 B2178C3257\n\tfor ;\n\tFri, 16 Feb 2024 17:14:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1270562805;\n\tFri, 16 Feb 2024 18:14:19 +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 CA25A61CAE\n\tfor ;\n\tFri, 16 Feb 2024 18:14:17 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 193076B3;\n\tFri, 16 Feb 2024 18:14:13 +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=\"FwsLrxVw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708103653;\n\tbh=MV1hs663MB1qPBSJZn+gVo1/kEq2cq2WH4wk+kDuVxY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=FwsLrxVw2r+1qRhqfqh/kO9+tg28Ds0V6tQt/Wga4+7GoC5U02AERXPwN7zMA154+\n\t7FyG6Og286Fq8Nr/T2qd4bTKbXfkL5BCni6GSpzn2diqOPWIEDAyOmFU8vUTyChbVN\n\twtFZBttcxkWw7vEhrIUhas/is/0DmO/BdAkdRUlI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<0b940292-aa73-44d2-b4e9-568e2820149e@gmail.com>","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-13-hdegoede@redhat.com>\n\t<871q9dd685.fsf@redhat.com>\n\t<170809713694.2629073.10070034761974663420@ping.linuxembedded.co.uk>\n\t<0b940292-aa73-44d2-b4e9-568e2820149e@gmail.com>","Subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","From":"Kieran Bingham ","To":"Andrei Konovalov ,\n\tHans de Goede , Milan Zamazal ","Date":"Fri, 16 Feb 2024 17:14:15 +0000","Message-ID":"<170810365510.1011926.5084923081917308215@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","Cc":"Bryan O'Donoghue ,\n\tlibcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28690,"web_url":"https://patchwork.libcamera.org/comment/28690/","msgid":"<49878a00-b95f-4e2e-884d-22b506886214@gmail.com>","date":"2024-02-16T17:23:13","subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","submitter":{"id":179,"url":"https://patchwork.libcamera.org/api/people/179/","name":"Andrei Konovalov","email":"andrey.konovalov.ynk@gmail.com"},"content":"On 16.02.2024 20:14, Kieran Bingham wrote:\n> Quoting Andrei Konovalov (2024-02-16 16:19:25)\n>> Hi Kieran,\n>>\n>> On 16.02.2024 18:25, Kieran Bingham wrote:\n>>> Quoting Milan Zamazal (2024-02-15 16:47:54)\n>>>> Hans de Goede writes:\n>>>>\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>>>>\n>>>> It would be nice if it was possible to use/test software ISP without specific\n>>>> patches to the array. Do I get it right that the idea is to enable softisp for\n>>>> certain devices where it can be used & there is no better pipeline handler to\n>>>> use? And there could be a runtime option for devices where it can be used &\n>>>> there is a better pipeline handler to use? Not a blocker but it should be\n>>>> clarified in the commit message.\n>>>\n>>> I think for now - this is compile time code configuration, which I preferred\n>>> over having a compile meson option that would have been harder to\n>>> support to distributions to convey the diffence between \"This pipeline\n>>> really needs the SoftISP\" vs \"This pipeline can optionally use the\n>>> SoftISP\".\n>>>\n>>> Having runtime configuration with a pipeline configuration file may be\n>>> appropriate in the near future, so it doesn't have to stay fixed like\n>>> this.\n>>>\n>>> But we probably need to handle things like raw stream passthrough before\n>>> we 'enable' the SoftISP more generically.\n>>\n>> Getting the raw bayer frames with \"--stream pixelformat=$RAW_BAYER_FMT\" vs\n>> the processed by SoftISP ones with e.g. \"--stream pixelformat=RGB888\" - would it\n>> count as raw stream passthrough?\n> \n> Yes.\n> \n>> This doesn't currently work with simple pipeline handler, but I have two\n>> patches to fix this. (They conflict with the SoftISP patch set, so I am keeping\n>> them locally not to mess with the current work.)\n> \n> That's fine for now I think. But the 'big picture' goal here for simple\n> pipeline handler is that Raw streams should still be accessible. That\n> could be initially that you can only support a single stream (either RAW\n> or SoftProcessed) ... but I could imagine later (particurly when the ISP\n> can be GPU accellerated) that there might also be the RAW+Processed\n> streams desired.\n\nI see. Multiple streams do make sense in longer term.\n\n> Maybe we should really rename 'simple' pipeline handler sometime ...\n\nMaybe. Next time one should think twice before calling a piece of software\n'simple', as this is hard to predict what the things end up with :)\n\nThanks,\nAndrei\n\n> --\n> Kieran\n> \n>>\n>> Thanks,\n>> Andrei\n>>\n>>> --\n>>> Kieran\n>>>\n>>>\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 | 136 ++++++++++++++++++-----\n>>>>> 1 file changed, 108 insertions(+), 28 deletions(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>>> index 78854ef8..eab5b56e 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,17 +186,22 @@ struct SimplePipelineInfo {\n>>>>> * and the number of streams it supports.\n>>>>> */\n>>>>> std::vector> converters;\n>>>>> + /*\n>>>>> + * Using Software ISP is to be enabled per driver.\n>>>>> + * The Software ISP can't be used together with the converters.\n>>>>> + */\n>>>>> + bool swIspEnabled;\n>>>>> };\n>>>>> \n>>>>> namespace {\n>>>>> \n>>>>> static const SimplePipelineInfo supportedDevices[] = {\n>>>>> - { \"dcmipp\", {} },\n>>>>> - { \"imx7-csi\", { { \"pxp\", 1 } } },\n>>>>> - { \"j721e-csi2rx\", {} },\n>>>>> - { \"mxc-isi\", {} },\n>>>>> - { \"qcom-camss\", {} },\n>>>>> - { \"sun6i-csi\", {} },\n>>>>> + { \"dcmipp\", {}, false },\n>>>>> + { \"imx7-csi\", { { \"pxp\", 1 } }, false },\n>>>>> + { \"j721e-csi2rx\", {}, false },\n>>>>> + { \"mxc-isi\", {}, false },\n>>>>> + { \"qcom-camss\", {}, true },\n>>>>> + { \"sun6i-csi\", {}, false },\n>>>>> };\n>>>>> \n>>>>> } /* namespace */\n>>>>> @@ -274,6 +280,7 @@ public:\n>>>>> bool useConversion_;\n>>>>> \n>>>>> std::unique_ptr converter_;\n>>>>> + std::unique_ptr swIsp_;\n>>>>> \n>>>>> private:\n>>>>> void tryPipeline(unsigned int code, const Size &size);\n>>>>> @@ -281,6 +288,9 @@ private:\n>>>>> \n>>>>> void conversionInputDone(FrameBuffer *buffer);\n>>>>> void conversionOutputDone(FrameBuffer *buffer);\n>>>>> +\n>>>>> + void ispStatsReady(int dummy);\n>>>>> + void setSensorControls(const ControlList &sensorControls);\n>>>>> };\n>>>>> \n>>>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>>>> @@ -332,6 +342,7 @@ public:\n>>>>> V4L2VideoDevice *video(const MediaEntity *entity);\n>>>>> V4L2Subdevice *subdev(const MediaEntity *entity);\n>>>>> MediaDevice *converter() { return converter_; }\n>>>>> + bool swIspEnabled() { return swIspEnabled_; }\n>>>>> \n>>>>> protected:\n>>>>> int queueRequestDevice(Camera *camera, Request *request) override;\n>>>>> @@ -360,6 +371,7 @@ private:\n>>>>> std::map entities_;\n>>>>> \n>>>>> MediaDevice *converter_;\n>>>>> + bool swIspEnabled_;\n>>>>> };\n>>>>> \n>>>>> /* -----------------------------------------------------------------------------\n>>>>> @@ -509,6 +521,29 @@ int SimpleCameraData::init()\n>>>>> }\n>>>>> }\n>>>>> \n>>>>> + /*\n>>>>> + * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.\n>>>>> + */\n>>>>> + if (!converter_ && pipe->swIspEnabled()) {\n>>>>> + swIsp_ = std::make_unique(pipe, sensor_->controls());\n>>>>> + if (!swIsp_->isValid()) {\n>>>>> + LOG(SimplePipeline, Warning)\n>>>>> + << \"Failed to create software ISP, disabling software debayering\";\n>>>>> + swIsp_.reset();\n>>>>> + } else {\n>>>>> + /*\n>>>>> + * \\todo explain why SimpleCameraData::conversionInputDone() can't be directly\n>>>>> + * connected to inputBufferReady signal.\n>>>>> + */\n>>>>> + swIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {\n>>>>> + this->conversionInputDone(buffer);\n>>>>> + });\n>>>>> + swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);\n>>>>> + swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);\n>>>>> + swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);\n>>>>> + }\n>>>>> + }\n>>>>> +\n>>>>> video_ = pipe->video(entities_.back().entity);\n>>>>> ASSERT(video_);\n>>>>> \n>>>>> @@ -599,12 +634,20 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n>>>>> config.captureFormat = pixelFormat;\n>>>>> config.captureSize = format.size;\n>>>>> \n>>>>> - if (!converter_) {\n>>>>> - config.outputFormats = { pixelFormat };\n>>>>> - config.outputSizes = config.captureSize;\n>>>>> - } else {\n>>>>> + if (converter_) {\n>>>>> config.outputFormats = converter_->formats(pixelFormat);\n>>>>> config.outputSizes = converter_->sizes(format.size);\n>>>>> + } else if (swIsp_) {\n>>>>> + config.outputFormats = swIsp_->formats(pixelFormat);\n>>>>> + config.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n>>>>> + if (config.outputFormats.empty()) {\n>>>>> + /* Do not use swIsp for unsupported pixelFormat's: */\n>>>>> + config.outputFormats = { pixelFormat };\n>>>>> + config.outputSizes = config.captureSize;\n>>>>> + }\n>>>>> + } else {\n>>>>> + config.outputFormats = { pixelFormat };\n>>>>> + config.outputSizes = config.captureSize;\n>>>>> }\n>>>>> \n>>>>> configs_.push_back(config);\n>>>>> @@ -750,9 +793,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n>>>>> }\n>>>>> \n>>>>> /*\n>>>>> - * The converter is in use. Requeue the internal buffer for\n>>>>> - * capture (unless the stream is being stopped), and complete\n>>>>> - * the request with all the user-facing buffers.\n>>>>> + * The converter or Software ISP is in use. Requeue the internal\n>>>>> + * buffer for capture (unless the stream is being stopped), and\n>>>>> + * complete the request with all the user-facing buffers.\n>>>>> */\n>>>>> if (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>>>> video_->queueBuffer(buffer);\n>>>>> @@ -798,9 +841,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n>>>>> buffer->metadata().timestamp);\n>>>>> \n>>>>> /*\n>>>>> - * Queue the captured and the request buffer to the converter if format\n>>>>> - * conversion is needed. If there's no queued request, just requeue the\n>>>>> - * captured buffer for capture.\n>>>>> + * Queue the captured and the request buffer to the converter or Software\n>>>>> + * ISP if format conversion is needed. If there's no queued request, just\n>>>>> + * requeue the captured buffer for capture.\n>>>>> */\n>>>>> if (useConversion_) {\n>>>>> if (conversionQueue_.empty()) {\n>>>>> @@ -808,7 +851,11 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)\n>>>>> return;\n>>>>> }\n>>>>> \n>>>>> - converter_->queueBuffers(buffer, conversionQueue_.front());\n>>>>> + if (converter_)\n>>>>> + converter_->queueBuffers(buffer, conversionQueue_.front());\n>>>>> + else\n>>>>> + swIsp_->queueBuffers(buffer, conversionQueue_.front());\n>>>>> +\n>>>>> conversionQueue_.pop();\n>>>>> return;\n>>>>> }\n>>>>> @@ -834,6 +881,18 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>>>> pipe->completeRequest(request);\n>>>>> }\n>>>>> \n>>>>> +void SimpleCameraData::ispStatsReady([[maybe_unused]] int dummy)\n>>>>> +{\n>>>>> + swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n>>>>> + V4L2_CID_EXPOSURE }));\n>>>>> +}\n>>>>> +\n>>>>> +void SimpleCameraData::setSensorControls(const ControlList &sensorControls)\n>>>>> +{\n>>>>> + ControlList ctrls(sensorControls);\n>>>>> + sensor_->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>>>>> @@ -1046,8 +1105,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>>> /* Set the stride, frameSize and bufferCount. */\n>>>>> if (needConversion_) {\n>>>>> std::tie(cfg.stride, cfg.frameSize) =\n>>>>> - data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>>>>> - cfg.size);\n>>>>> + (data_->converter_) ? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>>>>> + cfg.size)\n>>>>> + : data_->swIsp_->strideAndFrameSize(cfg.pixelFormat,\n>>>>> + cfg.size);\n>>>>> if (cfg.stride == 0)\n>>>>> return Invalid;\n>>>>> } else {\n>>>>> @@ -1210,7 +1271,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>>>> inputCfg.stride = captureFormat.planes[0].bpl;\n>>>>> inputCfg.bufferCount = kNumInternalBuffers;\n>>>>> \n>>>>> - return data->converter_->configure(inputCfg, outputCfgs);\n>>>>> + return (data->converter_) ? data->converter_->configure(inputCfg, outputCfgs)\n>>>>> + : data->swIsp_->configure(inputCfg, outputCfgs,\n>>>>> + data->sensor_->controls());\n>>>>> }\n>>>>> \n>>>>> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>>>> @@ -1224,8 +1287,10 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>>>> * whether the converter is used or not.\n>>>>> */\n>>>>> if (data->useConversion_)\n>>>>> - return data->converter_->exportBuffers(data->streamIndex(stream),\n>>>>> - count, buffers);\n>>>>> + return (data->converter_) ? data->converter_->exportBuffers(data->streamIndex(stream),\n>>>>> + count, buffers)\n>>>>> + : data->swIsp_->exportBuffers(data->streamIndex(stream),\n>>>>> + count, buffers);\n>>>>> else\n>>>>> return data->video_->exportBuffers(count, buffers);\n>>>>> }\n>>>>> @@ -1270,10 +1335,18 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>>> }\n>>>>> \n>>>>> if (data->useConversion_) {\n>>>>> - ret = data->converter_->start();\n>>>>> - if (ret < 0) {\n>>>>> - stop(camera);\n>>>>> - return ret;\n>>>>> + if (data->converter_) {\n>>>>> + ret = data->converter_->start();\n>>>>> + if (ret < 0) {\n>>>>> + stop(camera);\n>>>>> + return ret;\n>>>>> + }\n>>>>> + } else if (data->swIsp_) {\n>>>>> + ret = data->swIsp_->start();\n>>>>> + if (ret < 0) {\n>>>>> + stop(camera);\n>>>>> + return ret;\n>>>>> + }\n>>>>> }\n>>>>> \n>>>>> /* Queue all internal buffers for capture. */\n>>>>> @@ -1289,8 +1362,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n>>>>> SimpleCameraData *data = cameraData(camera);\n>>>>> V4L2VideoDevice *video = data->video_;\n>>>>> \n>>>>> - if (data->useConversion_)\n>>>>> - data->converter_->stop();\n>>>>> + if (data->useConversion_) {\n>>>>> + if (data->converter_)\n>>>>> + data->converter_->stop();\n>>>>> + else if (data->swIsp_) {\n>>>>> + data->swIsp_->stop();\n>>>>> + }\n>>>>> + }\n>>>>> \n>>>>> video->streamOff();\n>>>>> video->releaseBuffers();\n>>>>> @@ -1452,6 +1530,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n>>>>> }\n>>>>> }\n>>>>> \n>>>>> + swIspEnabled_ = info->swIspEnabled;\n>>>>> +\n>>>>> /* Locate the sensors. */\n>>>>> std::vector sensors = locateSensors();\n>>>>> if (sensors.empty()) {\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 F3F53BDE17\n\tfor ;\n\tFri, 16 Feb 2024 17:23:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 625CC62805;\n\tFri, 16 Feb 2024 18:23:18 +0100 (CET)","from mail-ej1-x631.google.com (mail-ej1-x631.google.com\n\t[IPv6:2a00:1450:4864:20::631])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B89B61CAE\n\tfor ;\n\tFri, 16 Feb 2024 18:23:16 +0100 (CET)","by mail-ej1-x631.google.com with SMTP id\n\ta640c23a62f3a-a2a17f3217aso296034566b.2\n\tfor ;\n\tFri, 16 Feb 2024 09:23:16 -0800 (PST)","from [192.168.118.26] ([87.116.166.222])\n\tby smtp.gmail.com with ESMTPSA id\n\ts11-20020a1709062ecb00b00a3ce60b003asm148521eji.176.2024.02.16.09.23.14\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tFri, 16 Feb 2024 09:23:14 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"h/xE2hrK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1708104195; x=1708708995;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=5xS3tWw8h6GFNRJOENcrWX/J2THwPh8KEPbyT1W6R1g=;\n\tb=h/xE2hrKDoZxvj3M5NRav4+j1Lndwo0M+AdYpoa2tvGBejVY2xmrgAJP9N8I3u4Ryq\n\tBK/oPRl+eN1AeNL7KYG9uB1PAF62eQuqbyutdDrBwEWfKerbzJEZsjloRB9WO45oHTn2\n\tKeD+ON+glRsL8ieBhNM4bFvbsc9X/QNNh1Om3tE6JILvpZc1Q7AbABBv/BJqktUzuty9\n\td6AAiev4RAXUDvykdjENTTUf6WBTWtQNfr5WfKQVrY+5tQxlgkppkZgZDv9V9w4TiSBM\n\t7NahI8HlYhXo8iEAb4FDNXRfkg3gMR7JZGBShlLuBOiGtvYXFXSLuf8BTuCbV6HFyvUO\n\tHs0g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1708104195; x=1708708995;\n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language: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=5xS3tWw8h6GFNRJOENcrWX/J2THwPh8KEPbyT1W6R1g=;\n\tb=AIfQnjqQ1Swg/JH/w9uhe/NMvTFvnRVW2HMPh2ZbAXZBiTjFMYENfodeQVa9k6F6N4\n\teIM4TkyGCVUM4bXU5pWhrkJaIBWC0/k/1yzsXDA9CxOLIl4gPVfFYHTwq6v9vyPAhgL4\n\tl9AQHjY+n7pVSOLuzLNkDrxYt0W8HU5iSVsLHPkccjmHg10/bbZJkL7Etwx2vC2ZRtmD\n\tLxu6YWeo+INuohCrA5YcWGixp6jzMo5dNnSNg1nJGBTEqWm68PAvggiR+5quX0ws9iIT\n\tftDJu4/juQSn/C006hWkHYP6mdk9yQARLZg/IsAat0+c8Gda1MdFHX81xAbCS60qDDI7\n\tGFkA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWLuFw/eVpQA+xgX1PkoWD3uciT4gj3CPj2lhNWJV+exDEOCwVL69BvX/a0U7jJD7wDL9CsCLo59uHdMFryi0mKSjzTp9EYPwkSttdKtMdsTHgCRA==","X-Gm-Message-State":"AOJu0Ywcg57PeGGhP/sRNAZ6HwLEdQ/pzdtOt7iQH3eK8kZvH9HwFYWG\n\tIGmIYK4sirm9T2jALdMOH/kefE44ZLyqOOwHK4i866d8kMnv/Wli","X-Google-Smtp-Source":"AGHT+IHV+WPGvApCjlmDNqX0Hjc1Nwpjt2ey/m94yZU/je970Fw6W85uvLsxp6psqNWJDA8Q5uVWDQ==","X-Received":"by 2002:a17:906:261a:b0:a3d:4e12:c0c1 with SMTP id\n\th26-20020a170906261a00b00a3d4e12c0c1mr3544705ejc.39.1708104195200; \n\tFri, 16 Feb 2024 09:23:15 -0800 (PST)","Message-ID":"<49878a00-b95f-4e2e-884d-22b506886214@gmail.com>","Date":"Fri, 16 Feb 2024 20:23:13 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft\n\tISP and Soft IPA","Content-Language":"en-US","To":"Kieran Bingham ,\n\tHans de Goede , Milan Zamazal ","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-13-hdegoede@redhat.com>\n\t<871q9dd685.fsf@redhat.com>\n\t<170809713694.2629073.10070034761974663420@ping.linuxembedded.co.uk>\n\t<0b940292-aa73-44d2-b4e9-568e2820149e@gmail.com>\n\t<170810365510.1011926.5084923081917308215@ping.linuxembedded.co.uk>","From":"Andrei Konovalov ","In-Reply-To":"<170810365510.1011926.5084923081917308215@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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","Cc":"Bryan O'Donoghue ,\n\tlibcamera-devel@lists.libcamera.org, Maxime Ripard , \n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]