Patch Detail
Show a patch.
GET /api/1.1/patches/19933/?format=api
{ "id": 19933, "url": "https://patchwork.libcamera.org/api/1.1/patches/19933/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19933/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20240423182000.1527425-5-mzamazal@redhat.com>", "date": "2024-04-23T18:19:59", "name": "[4/5] libcamera: software_isp: Remove TODO #13", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "d87a6321b102cafe8d84e39cedeafccf05610d57", "submitter": { "id": 177, "url": "https://patchwork.libcamera.org/api/1.1/people/177/?format=api", "name": "Milan Zamazal", "email": "mzamazal@redhat.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/19933/mbox/", "series": [ { "id": 4268, "url": "https://patchwork.libcamera.org/api/1.1/series/4268/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4268", "date": "2024-04-23T18:19:55", "name": "Software ISP levels cleanup", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4268/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19933/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19933/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0BE60C328D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Apr 2024 18:20:29 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 51107633FE;\n\tTue, 23 Apr 2024 20:20:28 +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 BB5F2633EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Apr 2024 20:20:25 +0200 (CEST)", "from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-18-UUBtCbdGM8eGlgW3UzAiBA-1;\n\tTue, 23 Apr 2024 14:20:22 -0400", "from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mimecast-mx02.redhat.com (Postfix) with ESMTPS id 87FBA29AC00A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Apr 2024 18:20:22 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.225.63])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id AE81649102;\n\tTue, 23 Apr 2024 18:20:21 +0000 (UTC)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"QlJtMV4m\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1713896424;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=9iPHvaerPCvm1J8fb11HBL7RWIiLE6dX/qwpyR82Kfw=;\n\tb=QlJtMV4mBAF2/z98RZ1JeV6bW4DAhm7mZFwrN/UQjNcqHbnG384W4c6ytirG6p5W5Ynwau\n\t3YhehECfyqHtfN4CfoEAmdHyZeI/n09U6eJvV8quGYyB59P8wpNOGdFfb3Lz06Topfx6JW\n\t+ZQP4yzl6ZMYDO+lLlPeTBo5oLe/1gU=", "X-MC-Unique": "UUBtCbdGM8eGlgW3UzAiBA-1", "From": "Milan Zamazal <mzamazal@redhat.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>", "Subject": "[PATCH 4/5] libcamera: software_isp: Remove TODO #13", "Date": "Tue, 23 Apr 2024 20:19:59 +0200", "Message-ID": "<20240423182000.1527425-5-mzamazal@redhat.com>", "In-Reply-To": "<20240423182000.1527425-1-mzamazal@redhat.com>", "References": "<20240423182000.1527425-1-mzamazal@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.4.1 on 10.11.54.1", "X-Mimecast-Spam-Score": "0", "X-Mimecast-Originator": "redhat.com", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain; charset=\"US-ASCII\"; x-default=true", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The current software ISP debayering implementation uses color lookup\ntables that apply black level subtraction, color gain and gamma\ncorrection in a single step, while performing debayering. In theory,\nblack level subtraction and color gains should be applied before\ndebayering and gamma correction after debayering. But because black\nlevel subtraction and color gain corrections are currently linear and we\naverage only same-color pixels in debayering, we can apply all the\noperations after debayering. The only difference is with clipping where\nout-of-range pixels contribute more than they deserve but this is not\ngenerally significant.\n\nCombining all the operations at once is conceptually not ideal but it is\nnot incorrect in this case and saves CPU cycles, which is critical for\nsoftware ISP CPU implementation (it may be less important with future\nGPU implementation). This means we need the current implementation. It\nmay get replaced or become optional (configurable) in future, for\nexample if the separation is needed due to introducing other image\nprocessing operations.\n\nUnder these circumstances and considering that the lookup tables\nconstruction has been moved out of the debayering code in the preceding\npatch, it makes no sense to keep software ISP TODO #13. Let's remove it\nin favor of a clarifying code comment.\n\nSigned-off-by: Milan Zamazal <mzamazal@redhat.com>\n---\n src/ipa/simple/soft_simple.cpp | 10 ++++++++++\n src/libcamera/software_isp/TODO | 10 ----------\n 2 files changed, 10 insertions(+), 10 deletions(-)", "diff": "diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex c5085f71..b2f4d308 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -247,6 +247,16 @@ void IPASoftSimple::stop()\n \n void IPASoftSimple::processStats(const ControlList &sensorControls)\n {\n+\t/*\n+\t * Here black level subtraction, color gains and gamma correction are\n+\t * combined into a single operation (table lookup) to save CPU cycles.\n+\t * This works because black level subtraction and color gains are currently\n+\t * linear operations and we average only same-color pixels in debayering.\n+\t * If we change anything on this or introduce other operations in between,\n+\t * it may be needed to split the operations and perform black and gain\n+\t * corrections before debayering and gamma correction after debayering.\n+\t */\n+\n \tSwIspStats::Histogram histogram = stats_->yHistogram;\n \tif (ignoreUpdates_ > 0)\n \t\tblackLevel_.update(histogram);\ndiff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO\nindex 4fcee39b..fcb02588 100644\n--- a/src/libcamera/software_isp/TODO\n+++ b/src/libcamera/software_isp/TODO\n@@ -267,13 +267,3 @@ This could be handled better with DelayedControls.\n > \t\t\t\t\t\t V4L2_CID_EXPOSURE }));\n \n You should use the DelayedControls class.\n-\n----\n-\n-13. Improve black level and colour gains application\n-\n-I think the black level should eventually be moved before debayering, and\n-ideally the colour gains as well. I understand the need for optimizations to\n-lower the CPU consumption, but at the same time I don't feel comfortable\n-building up on top of an implementation that may work a bit more by chance than\n-by correctness, as that's not very maintainable.\n", "prefixes": [ "4/5" ] }