[{"id":33841,"web_url":"https://patchwork.libcamera.org/comment/33841/","msgid":"<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-04-01T10:17:56","subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Stanislaw,\n\nthank you for the patch.\n\nStanislaw Gruszka writes:\n\n> ccmEnabled variable is not initialized by default, what result of\n> usage of CCM when the algorithm itself is not enabled and configured.\n\nInteresting, I thought the default boolean value in C++ is false.\nIf true or not, it doesn't harm to set it explicitly.\n\n> The bug manifest itself as seldom reproducible corrupted video stream.\n> Fix by initialize the variable in IPAContext class initialization list.\n>\n> Fixes: ac3068655643 (\"libcamera: software_isp: Track whether CCM is enabled\")\n> Signed-off-by: Stanislaw Gruszka \n> ---\n> src/ipa/simple/ipa_context.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 7dc2cd7ae828..afccf557121e 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -82,7 +82,7 @@ struct IPAFrameContext : public FrameContext {\n> \n> struct IPAContext {\n> \tIPAContext(unsigned int frameContextSize)\n> -\t\t: frameContexts(frameContextSize)\n> +\t\t: frameContexts(frameContextSize), ccmEnabled(false)\n\nOr setting it\n\n bool ccmEnabled = false;\n\nin IPAContext? Up to the maintainers, what they prefer.\n\nFrom my side, in either case:\n\nReviewed-by: Milan Zamazal \n\n> \t{\n> \t}","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 D0A5FC3213\n\tfor ;\n\tTue, 1 Apr 2025 10:18:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7576D68981;\n\tTue, 1 Apr 2025 12:18:05 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6752D68947\n\tfor ;\n\tTue, 1 Apr 2025 12:18:03 +0200 (CEST)","from mail-wr1-f69.google.com (mail-wr1-f69.google.com\n\t[209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-569-y18cOHBQPnCeiiQ52_pLXg-1; Tue, 01 Apr 2025 06:18:01 -0400","by mail-wr1-f69.google.com with SMTP id\n\tffacd0b85a97d-3912e4e2033so2295178f8f.0\n\tfor ;\n\tTue, 01 Apr 2025 03:18:01 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-39c0b6656afsm13555751f8f.40.2025.04.01.03.17.57\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 01 Apr 2025 03:17:57 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"hmJ9nPQa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1743502682;\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=N+4hWA/bHimkIQ3iE5sVlk+NcyRSYUallArocpNuUh8=;\n\tb=hmJ9nPQabgocd1yK6v3ZFGx6EVUK5jue2LbkhiX9Wf45FeNy9u1vjjR0+0SHPSk+trZk8P\n\t9mrDv3UzUkJSxwYUzn/QwDD/PYnwAEZpTjcKmyoFMffTP35arQbi/uyPMrtIFiATB9NoJR\n\toZEUCB0cquKxW14Sd+aSAukzObGEsvk=","X-MC-Unique":"y18cOHBQPnCeiiQ52_pLXg-1","X-Mimecast-MFC-AGG-ID":"y18cOHBQPnCeiiQ52_pLXg_1743502680","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1743502679; x=1744107479;\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=N+4hWA/bHimkIQ3iE5sVlk+NcyRSYUallArocpNuUh8=;\n\tb=NFjt6si5xCf7LcVH/eZbkba1s3mFqL8skyLh+i9htUwUGBeyyxqgWIY2S5aIqtBrK+\n\tpTJQwrnyCxQER6iL3DrvkhZjUhTnLzm+h68nlalEczDGneV8sm68DEZ4J7FYWLN2pvbx\n\t57sEyqXTRhO3D80LO/DZc93U3F0LdZmH5rbYy0R+7caPw9t6UUANCHaGyiZOyWRF0jNx\n\t3xf00xdnmOIF9hWxgXSqlIT62h2sBJSruCAHj4el7p7AmyIVFrZGN4nCtQUJQitoZqtc\n\tZMTPMG19C6Ymg3QeC8mbMXxHYME1oxvcl+qGV6aDK2yAvA1DSxMhxO135AU1ZFFFKAqF\n\tHrGw==","X-Gm-Message-State":"AOJu0YzZj2HKbBt5yyPG4OP6OmtkyhwdZw/Vb3gFwvW8k4qzX5IkOcUu\n\tB6ry0gBV6K9UFyE5Bqi4AARychpVOXS+4zNKlDw+8VAMJ4mlAzy2l8qILfLfu1A7XYMy4q+zkVf\n\tTDCcZcax4QG16KchpW2AmN3xnZ8xdW97myQgKnplp2H6/Owrj1vxe7YeD+JU/+T0C3U7dUKk=","X-Gm-Gg":"ASbGncuwYZaWFF+VPVY4LYG+DIPqZRl1qJp3U/ajCeR3X4e6wr8KjY0UkZeFjU/zqoO\n\tQPml2Frs+FJk3cXRbgljFbxg/lC7UrUPFLMpteXmpxzxk6hvlpidJ+T8Lkgq8NFvji+04nN8G3i\n\tlxXkg5vQYplx/K31OQXg/SNmtkTwiYSSlrdXiWnxHtw9F4y4cWEMgV9x1dejeQ/caDM+Qss7nr9\n\tg5/ul8A3FJayIk1afeVPv8jK8kisLnxpru9QLHlq6stT1j/yaU1KWU0k7akY88+dm9mKu9Oj3Wg\n\tMqaWfDL5gko3oXwb9FE4ILVH+OFMwNiGaCAkEt3lBUToj/JQW79KlmGyIsoYf+JCVVId","X-Received":["by 2002:a05:6000:1a89:b0:391:4189:d28d with SMTP id\n\tffacd0b85a97d-39c121139a1mr8854383f8f.34.1743502678786; \n\tTue, 01 Apr 2025 03:17:58 -0700 (PDT)","by 2002:a05:6000:1a89:b0:391:4189:d28d with SMTP id\n\tffacd0b85a97d-39c121139a1mr8854362f8f.34.1743502678460; \n\tTue, 01 Apr 2025 03:17:58 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEMSS5y6yw4J61n6dw9n2ow4nw5oCiorU3t98N8o85v9fEpb0jaY1mNMW1VcbjpDZWI4Jhvtg==","From":"Milan Zamazal ","To":"Stanislaw Gruszka ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Kieran Bingham\n\t, Naushir Patuck\n\t, Sakari Ailus , \n\tHans de Goede ","Subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","In-Reply-To":"<20250401093308.475810-1-stanislaw.gruszka@linux.intel.com>\n\t(Stanislaw Gruszka's message of \"Tue, 1 Apr 2025 11:33:08 +0200\")","References":"<20250401093308.475810-1-stanislaw.gruszka@linux.intel.com>","Date":"Tue, 01 Apr 2025 12:17:56 +0200","Message-ID":"<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"cvm_567dGq0MC4tCOnoJ7tSODYPuubOnPXwATj2mW-o_1743502680","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33842,"web_url":"https://patchwork.libcamera.org/comment/33842/","msgid":"<00a52c82-635a-48f1-999f-e050c52d8c97@ideasonboard.com>","date":"2025-04-01T10:34:14","subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 04. 01. 12:17 keltezéssel, Milan Zamazal írta:\n> Hi Stanislaw,\n> \n> thank you for the patch.\n> \n> Stanislaw Gruszka writes:\n> \n>> ccmEnabled variable is not initialized by default, what result of\n>> usage of CCM when the algorithm itself is not enabled and configured.\n> \n> Interesting, I thought the default boolean value in C++ is false.\n> If true or not, it doesn't harm to set it explicitly.\n\nSadly a default-initialized `bool` like `bool x;` has indeterminate value:\nhttps://en.cppreference.com/w/cpp/language/default_initialization\n\n\"\"\"\nThe effects of default-initialization are:\n * if T is a (possibly cv-qualified) class type, [...]\n * if T is an array type, [...]\n * otherwise, no initialization is performed [...]\n\n[...]\n\nIf no initialization is performed for an object, that object retains an\nindeterminate value until that value is replaced.\n\"\"\"\n\nOne needs something like `bool x{}, y = {}, z = bool(), w = false;` at least.\n\n> \n>> The bug manifest itself as seldom reproducible corrupted video stream.\n>> Fix by initialize the variable in IPAContext class initialization list.\n>>\n>> Fixes: ac3068655643 (\"libcamera: software_isp: Track whether CCM is enabled\")\n>> Signed-off-by: Stanislaw Gruszka \n>> ---\n>> src/ipa/simple/ipa_context.h | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> index 7dc2cd7ae828..afccf557121e 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -82,7 +82,7 @@ struct IPAFrameContext : public FrameContext {\n>> \n>> struct IPAContext {\n>> \tIPAContext(unsigned int frameContextSize)\n>> -\t\t: frameContexts(frameContextSize)\n>> +\t\t: frameContexts(frameContextSize), ccmEnabled(false)\n> \n> Or setting it\n> \n> bool ccmEnabled = false;\n> \n> in IPAContext? Up to the maintainers, what they prefer.\n\nI would generally prefer initializing right where the member is declared\nas it is clearer than member init lists in my opinion, and is not affected\nnegatively when additional constructors are introduced.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> From my side, in either case:\n> \n> Reviewed-by: Milan Zamazal \n> \n>> \t{\n>> \t}\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 B177BC323E\n\tfor ;\n\tTue, 1 Apr 2025 10:34:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E1636894F;\n\tTue, 1 Apr 2025 12:34:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A576568947\n\tfor ;\n\tTue, 1 Apr 2025 12:34:18 +0200 (CEST)","from [192.168.33.14] (185.221.143.221.nat.pool.zt.hu\n\t[185.221.143.221])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EB2876F9;\n\tTue, 1 Apr 2025 12:32:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MkmIRSm2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743503546;\n\tbh=hoe4r+YKs0HLzkgJpHxOcJdJySH9u6X9wGQkc/PN+rE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=MkmIRSm2NhSXmgaH2RAqoSKDglFXiZGA+j1I5saYxxeNF9FVK9d73a7zEm7gBuyLh\n\tuK8JS+rFVUZS+035SjWSsjdFiWpgEHpzmUQRpyj1lA4Ris6Bxi8ZhEfYBTNvypLU9w\n\t9wF8m8YpmVURXraJvTxai/Sw2u4M/oskMXhUw+ac=","Message-ID":"<00a52c82-635a-48f1-999f-e050c52d8c97@ideasonboard.com>","Date":"Tue, 1 Apr 2025 12:34:14 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","To":"Milan Zamazal ,\n\tStanislaw Gruszka ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ,\n\tKieran Bingham ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","References":"<20250401093308.475810-1-stanislaw.gruszka@linux.intel.com>\n\t<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33844,"web_url":"https://patchwork.libcamera.org/comment/33844/","msgid":"","date":"2025-04-01T10:45:07","subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"Hi Milan,\n\nOn Tue, Apr 01, 2025 at 12:17:56PM +0200, Milan Zamazal wrote:\n> Hi Stanislaw,\n> \n> thank you for the patch.\n> \n> Stanislaw Gruszka writes:\n> \n> > ccmEnabled variable is not initialized by default, what result of\n> > usage of CCM when the algorithm itself is not enabled and configured.\n> \n> Interesting, I thought the default boolean value in C++ is false.\n> If true or not, it doesn't harm to set it explicitly.\n\nI think class members are not initialized if not explicitly done so.\nAnd can have random values unfortunately.\n\nI can not really confirm the fix because, it happened randomly and very\nseldom. But I initialized ccmEnabled to true and got the same effect\nas when the bug happened: first collorfull images with only coutures\nrecognizable and then all black.\n\n> > struct IPAContext {\n> > \tIPAContext(unsigned int frameContextSize)\n> > -\t\t: frameContexts(frameContextSize)\n> > +\t\t: frameContexts(frameContextSize), ccmEnabled(false)\n> \n> Or setting it\n> \n> bool ccmEnabled = false;\n> \n> in IPAContext? Up to the maintainers, what they prefer.\n> \n> From my side, in either case:\n> \n> Reviewed-by: Milan Zamazal \n\nThanks!\n\nRegards\nStanislaw","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 EA65CC323E\n\tfor ;\n\tTue, 1 Apr 2025 10:45:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CCF568985;\n\tTue, 1 Apr 2025 12:45:17 +0200 (CEST)","from mgamail.intel.com (mgamail.intel.com [192.198.163.11])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B89868947\n\tfor ;\n\tTue, 1 Apr 2025 12:45:13 +0200 (CEST)","from orviesa003.jf.intel.com ([10.64.159.143])\n\tby fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t01 Apr 2025 03:45:12 -0700","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.245.112.43]) by ORVIESA003-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Apr 2025 03:45:10 -0700"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"ip+d99t3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1743504314; x=1775040314;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=LunlxkmGEmbo+q5OzSn+cDpQNPm2YPDDhKl42veT7/o=;\n\tb=ip+d99t3bJfkn5xZhwgbjiakWBzLQJ/owwu3g/wj2UO6P9xIJWqUfKFp\n\tbtFHFksWpvavV+5iC4nsHJdU1D4UqR/yh7dj5yl/cuH4kNgDur37Ao+4V\n\teBG/3iLHMZcs5C88p7LmArt0n2bh/mbxu0km2SD7GqXALgX85lcuuNqjl\n\tTmWs90nLrZgxqH0S1lB9qkENDiK6ZNdcEbFbb5R2QH2FvBhTu1LbOGllb\n\tdqt32Smk0s6T6s/RYCLoQvGQ5d+lLFA1xf322Xhd+VmkJn5XPVhVpk43g\n\trRbRSatBtdE63JLLKvQk5xzbc6TF9CmqPpR3h1Y2q4EcfawBuKLNA5mkK A==;","X-CSE-ConnectionGUID":["JA4P1kfZS/mBVWZK8SRkqg==","4XUSEKqRTiKcf9dSyiu2IQ=="],"X-CSE-MsgGUID":["SMZe3xfjRqe4KpBaR4jDeg==","lISFgb3iTsa9g9b7ASfNuA=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11390\"; a=\"55481192\"","E=Sophos;i=\"6.14,293,1736841600\"; d=\"scan'208\";a=\"55481192\"","E=Sophos;i=\"6.14,293,1736841600\"; d=\"scan'208\";a=\"131209186\""],"X-ExtLoop1":"1","Date":"Tue, 1 Apr 2025 12:45:07 +0200","From":"Stanislaw Gruszka ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ,\n\tKieran Bingham ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","Message-ID":"","References":"<20250401093308.475810-1-stanislaw.gruszka@linux.intel.com>\n\t<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33845,"web_url":"https://patchwork.libcamera.org/comment/33845/","msgid":"<20250401111150.GB11469@pendragon.ideasonboard.com>","date":"2025-04-01T11:11:50","subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 01, 2025 at 12:34:14PM +0200, Barnabás Pőcze wrote:\n> Hi\n> \n> \n> 2025. 04. 01. 12:17 keltezéssel, Milan Zamazal írta:\n> > Hi Stanislaw,\n> > \n> > thank you for the patch.\n> > \n> > Stanislaw Gruszka writes:\n> > \n> >> ccmEnabled variable is not initialized by default, what result of\n\ns/what result of/which results in/\n\n> >> usage of CCM when the algorithm itself is not enabled and configured.\n> > \n> > Interesting, I thought the default boolean value in C++ is false.\n> > If true or not, it doesn't harm to set it explicitly.\n> \n> Sadly a default-initialized `bool` like `bool x;` has indeterminate value:\n> https://en.cppreference.com/w/cpp/language/default_initialization\n> \n> \"\"\"\n> The effects of default-initialization are:\n> * if T is a (possibly cv-qualified) class type, [...]\n> * if T is an array type, [...]\n> * otherwise, no initialization is performed [...]\n> \n> [...]\n> \n> If no initialization is performed for an object, that object retains an\n> indeterminate value until that value is replaced.\n> \"\"\"\n> \n> One needs something like `bool x{}, y = {}, z = bool(), w = false;` at least.\n> \n> >> The bug manifest itself as seldom reproducible corrupted video stream.\n\ns/manifest/manifests/\n\n> >> Fix by initialize the variable in IPAContext class initialization list.\n> >>\n> >> Fixes: ac3068655643 (\"libcamera: software_isp: Track whether CCM is enabled\")\n> >> Signed-off-by: Stanislaw Gruszka \n> >> ---\n> >> src/ipa/simple/ipa_context.h | 2 +-\n> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>\n> >> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> >> index 7dc2cd7ae828..afccf557121e 100644\n> >> --- a/src/ipa/simple/ipa_context.h\n> >> +++ b/src/ipa/simple/ipa_context.h\n> >> @@ -82,7 +82,7 @@ struct IPAFrameContext : public FrameContext {\n> >> \n> >> struct IPAContext {\n> >> \tIPAContext(unsigned int frameContextSize)\n> >> -\t\t: frameContexts(frameContextSize)\n> >> +\t\t: frameContexts(frameContextSize), ccmEnabled(false)\n> > \n> > Or setting it\n> > \n> > bool ccmEnabled = false;\n> > \n> > in IPAContext? Up to the maintainers, what they prefer.\n> \n> I would generally prefer initializing right where the member is declared\n> as it is clearer than member init lists in my opinion, and is not affected\n> negatively when additional constructors are introduced.\n\nWe've initially used initializer lists in constructors, probably because\nI didn't know member variable initialization in the class definition was\npossible :-) I agree with Barnabás that it would be better. With that,\n\nReviewed-by: Laurent Pinchart \n\nStanislaw, could you test a v2 with this change ? We can then merge it\nin time for v0.5.\n\n> > From my side, in either case:\n> > \n> > Reviewed-by: Milan Zamazal \n> > \n> >> \t{\n> >> \t}","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 1BF9CC3213\n\tfor ;\n\tTue, 1 Apr 2025 11:12:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FD8F6894F;\n\tTue, 1 Apr 2025 13:12:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2616868947\n\tfor ;\n\tTue, 1 Apr 2025 13:12:15 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A2F1C741;\n\tTue, 1 Apr 2025 13:10:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"keE1T4K8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743505822;\n\tbh=KHnLi8d5S7O3QJy0MHHLDyfalRD2V8BMKQAq3M7A5fU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=keE1T4K8mdvDsfYEaL2OR9kiaUncc3/LTM48quow5dwPOpPTwHrqy8lyjt1Hjb4Rv\n\tJxg6YcP6S7nNgabuNPUyt+0CcwVrOYE0InW1RDcZUmsI659sS9ujd1otEGztPqlc03\n\tSNu2WZtA4lmiFQrSBbpY1YxPVkdrzjmpsUgQ3bTY=","Date":"Tue, 1 Apr 2025 14:11:50 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Milan Zamazal ,\n\tStanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH] ipa: simple: Initialize ccmEnabled to false","Message-ID":"<20250401111150.GB11469@pendragon.ideasonboard.com>","References":"<20250401093308.475810-1-stanislaw.gruszka@linux.intel.com>\n\t<85zfh0kwx7.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<00a52c82-635a-48f1-999f-e050c52d8c97@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<00a52c82-635a-48f1-999f-e050c52d8c97@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]