[{"id":14773,"web_url":"https://patchwork.libcamera.org/comment/14773/","msgid":"","date":"2021-01-26T08:27:50","subject":"Re: [libcamera-devel] [PATCH v2 0/5] Remove Raspberry Pi logging","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Mon, Jan 25, 2021 at 06:48:53PM +0000, David Plowman wrote:\n> Hi again everyone\n> \n> Version 2 of this set again makes no functional changes beyond debug\n> logging. There are the following differences compared to v1:\n> \n> * I've removed some debug messages that were unnecessary (either just\n> tracing statements or duplicates).\n> \n> * A number of messages repeated the algorithm name which is\n> unnecessary with libcamera logging.\n> \n> * One or two that were actually warnings without using LOG(RPiXxx,\n> Warning) have been amended.\n> \n> * A bit more whitespace tidying.\n> \n> I've actually folded the changes into the original commits, I think\n> it's easy enough to follow like this.\n> \n> Most of Laurent's other suggestions I'd be fine with if we wanted to\n> pursue them (except the one about sharing frame counts between\n> algorithms - I'm actually really paranoid about that!).\n\nI'm not challenging your opinion, but for my information, could you\nbriefly explain why ?\n\n> David Plowman (5):\n> ipa: raspberrypi: controller: Replace Raspberry Pi debug with\n> libcamera debug\n> ipa: raspberrypi: alsc: Replace Raspberry Pi debug with libcamera\n> debug\n> ipa: raspberrypi: awb: Replace Raspberry Pi debug with libcamera debug\n> ipa: raspberrypi: Replace Raspberry Pi debug with libcamera debug\n> ipa: raspberrypi: Remove legacy Rasberry Pi logging\n> \n> src/ipa/raspberrypi/controller/algorithm.hpp | 1 -\n> src/ipa/raspberrypi/controller/controller.cpp | 19 ++--\n> src/ipa/raspberrypi/controller/logging.hpp | 30 ------\n> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 66 +++++++------\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 97 ++++++++++---------\n> .../controller/rpi/black_level.cpp | 11 ++-\n> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 26 +++--\n> .../raspberrypi/controller/rpi/contrast.cpp | 20 +++-\n> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 8 +-\n> src/ipa/raspberrypi/controller/rpi/geq.cpp | 20 ++--\n> src/ipa/raspberrypi/controller/rpi/lux.cpp | 11 ++-\n> src/ipa/raspberrypi/controller/rpi/noise.cpp | 14 ++-\n> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 21 ++--\n> .../raspberrypi/controller/rpi/sharpen.cpp | 11 ++-\n> 14 files changed, 192 insertions(+), 163 deletions(-)\n> delete mode 100644 src/ipa/raspberrypi/controller/logging.hpp","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 DA755BD808\n\tfor ;\n\tTue, 26 Jan 2021 08:28:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A8121682ED;\n\tTue, 26 Jan 2021 09:28:11 +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 EBAEB682E6\n\tfor ;\n\tTue, 26 Jan 2021 09:28:09 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7172B2C1;\n\tTue, 26 Jan 2021 09:28:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"enQg4Kfw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611649689;\n\tbh=IlKzsHduhDko6EqCw/ntpldCTiY88YLIKd0b98CblsY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=enQg4KfwThim3bR3X54gLIvV/MEX7DeChSKM7eAW/TGFD8Rj7TYUAKnrH5ABxcezZ\n\trBcLK3KgyE8Iqxf8cDtZzhEqOom5cw9884Dyl3at5hmJwmRS79hP+5T2/rztarWszh\n\tRYlGvAeY8i8Nd/cbLlL7B0IZSzqpk0K0UciSx4vc=","Date":"Tue, 26 Jan 2021 10:27:50 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20210125184858.16339-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210125184858.16339-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 0/5] Remove Raspberry Pi logging","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":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14776,"web_url":"https://patchwork.libcamera.org/comment/14776/","msgid":"","date":"2021-01-26T08:57:49","subject":"Re: [libcamera-devel] [PATCH v2 0/5] Remove Raspberry Pi logging","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nNo, that's fine, I'm quite happy to explain, just don't expect it to\nbe 100% rational!\n\nOn Tue, 26 Jan 2021 at 08:28, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> On Mon, Jan 25, 2021 at 06:48:53PM +0000, David Plowman wrote:\n> > Hi again everyone\n> >\n> > Version 2 of this set again makes no functional changes beyond debug\n> > logging. There are the following differences compared to v1:\n> >\n> > * I've removed some debug messages that were unnecessary (either just\n> > tracing statements or duplicates).\n> >\n> > * A number of messages repeated the algorithm name which is\n> > unnecessary with libcamera logging.\n> >\n> > * One or two that were actually warnings without using LOG(RPiXxx,\n> > Warning) have been amended.\n> >\n> > * A bit more whitespace tidying.\n> >\n> > I've actually folded the changes into the original commits, I think\n> > it's easy enough to follow like this.\n> >\n> > Most of Laurent's other suggestions I'd be fine with if we wanted to\n> > pursue them (except the one about sharing frame counts between\n> > algorithms - I'm actually really paranoid about that!).\n>\n> I'm not challenging your opinion, but for my information, could you\n> briefly explain why ?\n\nThe main thing I'm scared of is starting to develop \"tentacles\" that\nmake the different algorithms depend on each other, or on anything\nexternal (the only exception to this is that one algorithm is allowed\nto ask for another's \"status\" object from the metadata, but that's\nit). I'm desperately keen to try and keep them all as self-contained\nas I can. You can probably tell that I've been here before in a\nprevious life, and the mess that it grew into back then ended up very\ndifficult.\n\nThere are some other minor issues about the frame counts perhaps not\nalways being quite the same (some algorithms might be skipped just\nafter the camera is started), but the real reason is the one above.\nLike I said, it's not entirely rational!\n\nBest regards\nDavid\n\n>\n> > David Plowman (5):\n> > ipa: raspberrypi: controller: Replace Raspberry Pi debug with\n> > libcamera debug\n> > ipa: raspberrypi: alsc: Replace Raspberry Pi debug with libcamera\n> > debug\n> > ipa: raspberrypi: awb: Replace Raspberry Pi debug with libcamera debug\n> > ipa: raspberrypi: Replace Raspberry Pi debug with libcamera debug\n> > ipa: raspberrypi: Remove legacy Rasberry Pi logging\n> >\n> > src/ipa/raspberrypi/controller/algorithm.hpp | 1 -\n> > src/ipa/raspberrypi/controller/controller.cpp | 19 ++--\n> > src/ipa/raspberrypi/controller/logging.hpp | 30 ------\n> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 66 +++++++------\n> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 97 ++++++++++---------\n> > .../controller/rpi/black_level.cpp | 11 ++-\n> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 26 +++--\n> > .../raspberrypi/controller/rpi/contrast.cpp | 20 +++-\n> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 8 +-\n> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 20 ++--\n> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 11 ++-\n> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 14 ++-\n> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 21 ++--\n> > .../raspberrypi/controller/rpi/sharpen.cpp | 11 ++-\n> > 14 files changed, 192 insertions(+), 163 deletions(-)\n> > delete mode 100644 src/ipa/raspberrypi/controller/logging.hpp\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 CFB45BD808\n\tfor ;\n\tTue, 26 Jan 2021 08:58:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EA60682F5;\n\tTue, 26 Jan 2021 09:58:02 +0100 (CET)","from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com\n\t[IPv6:2607:f8b0:4864:20::22d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A0C9682E6\n\tfor ;\n\tTue, 26 Jan 2021 09:58:01 +0100 (CET)","by mail-oi1-x22d.google.com with SMTP id a77so435022oii.4\n\tfor ;\n\tTue, 26 Jan 2021 00:58:01 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"XQDhuZnO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=OuWm2IIJXEur/KcifJkGbcJNyNFboou73L3xI4MltYk=;\n\tb=XQDhuZnOkCetdcHdccGKB2YraMwS8iUZX4+CYdU6CqG+ybag5+RKXGJpc4CmU5imwc\n\tqZ3e2citsyOWKp+Lyhs8ye3uTdu7NVC88EMpiaFAU3v4tZ/ot6enX/4Aa2/TwNHTsYTS\n\th0rWv7/EqYtjk9N4SVtm7iyxivu6q4guvhJoatJ+sNpeBK3Rqq/qxG7Zzju30lfI8Gxd\n\tbnLi7iCCcBPT/Yyacb27aTBa4PmlDLSGVfm2h8fHdkI+npx+C9GFeevgO1+Zs393f90d\n\t63Q9Pdtun1tJyBcL42sXIoJyMJRDhheQlIp3FXZz811o8lIxYW49HbGWlNqzK5E0EuT7\n\tTeew==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=OuWm2IIJXEur/KcifJkGbcJNyNFboou73L3xI4MltYk=;\n\tb=UhGU3FoXh6lNr9jOGWFGO+FTVsxXqUVuGFZep+yeJ4NNiAqE80TtH8Mbaq72PYjmIe\n\tAjVVB/7y8tBKk5ul3tL+tDvi842RPsy1+3Yi/f5I90eYwivCuE8dB6Vnb+8TnpnsGo7U\n\tpn9lsNz7v8bPfne3U5JVrGY5MiqaXkd+ng20DBbo0JVWO8wIgwCKMFFIRC8dtquSRz7s\n\tp+2aRVBtxO6OwA1m569FEitVLsM+dLyNdTuiBCM/qSVmwBno6j9LgY3Z9hDJuV6iqgkq\n\tWYzK+pzC4WA+FyDmthxJHxQYUVfgvv7UzlTDjvrMpZudDUbvKK2QjxyRkHslmzIE4LJk\n\tzXGA==","X-Gm-Message-State":"AOAM531+IVe7Q/9LWV3pHVj8NocLoI3dtVzzO4bbM5sxBZ3Z4ZDRYUzn\n\tWK0AVtw3xhASVzDKHx4t4mYjHWQwcpvS+lJx5kUxsBLBinA9kA==","X-Google-Smtp-Source":"ABdhPJzkHf+9RL70AutF+/IPPb9D9OFgh4WCjt5nkj7jtXVslSEYe6ge7bTdYitq76WIlfu+H3qhTgC+HHHB06C1RQk=","X-Received":"by 2002:aca:4d93:: with SMTP id\n\ta141mr2497150oib.107.1611651479750; \n\tTue, 26 Jan 2021 00:57:59 -0800 (PST)","MIME-Version":"1.0","References":"<20210125184858.16339-1-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 26 Jan 2021 08:57:49 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v2 0/5] Remove Raspberry Pi logging","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":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14805,"web_url":"https://patchwork.libcamera.org/comment/14805/","msgid":"","date":"2021-01-26T23:25:56","subject":"Re: [libcamera-devel] [PATCH v2 0/5] Remove Raspberry Pi logging","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Tue, Jan 26, 2021 at 08:57:49AM +0000, David Plowman wrote:\n> Hi Laurent\n> \n> No, that's fine, I'm quite happy to explain, just don't expect it to\n> be 100% rational!\n> \n> On Tue, 26 Jan 2021 at 08:28, Laurent Pinchart wrote:\n> > On Mon, Jan 25, 2021 at 06:48:53PM +0000, David Plowman wrote:\n> > > Hi again everyone\n> > >\n> > > Version 2 of this set again makes no functional changes beyond debug\n> > > logging. There are the following differences compared to v1:\n> > >\n> > > * I've removed some debug messages that were unnecessary (either just\n> > > tracing statements or duplicates).\n> > >\n> > > * A number of messages repeated the algorithm name which is\n> > > unnecessary with libcamera logging.\n> > >\n> > > * One or two that were actually warnings without using LOG(RPiXxx,\n> > > Warning) have been amended.\n> > >\n> > > * A bit more whitespace tidying.\n> > >\n> > > I've actually folded the changes into the original commits, I think\n> > > it's easy enough to follow like this.\n> > >\n> > > Most of Laurent's other suggestions I'd be fine with if we wanted to\n> > > pursue them (except the one about sharing frame counts between\n> > > algorithms - I'm actually really paranoid about that!).\n> >\n> > I'm not challenging your opinion, but for my information, could you\n> > briefly explain why ?\n> \n> The main thing I'm scared of is starting to develop \"tentacles\" that\n> make the different algorithms depend on each other, or on anything\n> external (the only exception to this is that one algorithm is allowed\n> to ask for another's \"status\" object from the metadata, but that's\n> it). I'm desperately keen to try and keep them all as self-contained\n> as I can. You can probably tell that I've been here before in a\n> previous life, and the mess that it grew into back then ended up very\n> difficult.\n\nI can relate to that feeling, so this is perfectly understandable :-)\n\n> There are some other minor issues about the frame counts perhaps not\n> always being quite the same (some algorithms might be skipped just\n> after the camera is started), but the real reason is the one above.\n> Like I said, it's not entirely rational!\n\nI'm wondering if it would make sense for \"services\" such as frame count\nto be handled in the base Algorithm class, to avoid code duplication.\nAlgorithms would still be independent from each other (they would all\nhave their own frame count) but wouldn't be required to duplicate the\ncode.\n\nIt's really minor for the frame count, but could be useful for other\nfeatures. In any case it's a topic for later.\n\n> > > David Plowman (5):\n> > > ipa: raspberrypi: controller: Replace Raspberry Pi debug with\n> > > libcamera debug\n> > > ipa: raspberrypi: alsc: Replace Raspberry Pi debug with libcamera\n> > > debug\n> > > ipa: raspberrypi: awb: Replace Raspberry Pi debug with libcamera debug\n> > > ipa: raspberrypi: Replace Raspberry Pi debug with libcamera debug\n> > > ipa: raspberrypi: Remove legacy Rasberry Pi logging\n> > >\n> > > src/ipa/raspberrypi/controller/algorithm.hpp | 1 -\n> > > src/ipa/raspberrypi/controller/controller.cpp | 19 ++--\n> > > src/ipa/raspberrypi/controller/logging.hpp | 30 ------\n> > > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 66 +++++++------\n> > > src/ipa/raspberrypi/controller/rpi/awb.cpp | 97 ++++++++++---------\n> > > .../controller/rpi/black_level.cpp | 11 ++-\n> > > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 26 +++--\n> > > .../raspberrypi/controller/rpi/contrast.cpp | 20 +++-\n> > > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 8 +-\n> > > src/ipa/raspberrypi/controller/rpi/geq.cpp | 20 ++--\n> > > src/ipa/raspberrypi/controller/rpi/lux.cpp | 11 ++-\n> > > src/ipa/raspberrypi/controller/rpi/noise.cpp | 14 ++-\n> > > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 21 ++--\n> > > .../raspberrypi/controller/rpi/sharpen.cpp | 11 ++-\n> > > 14 files changed, 192 insertions(+), 163 deletions(-)\n> > > delete mode 100644 src/ipa/raspberrypi/controller/logging.hpp","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 E64D8BD808\n\tfor ;\n\tTue, 26 Jan 2021 23:26:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66BAF6833C;\n\tWed, 27 Jan 2021 00:26:18 +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 6482F682D2\n\tfor ;\n\tWed, 27 Jan 2021 00:26:16 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CACDC2C1;\n\tWed, 27 Jan 2021 00:26:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"et+v7VDX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611703576;\n\tbh=FGZCIz6i7uAZK2goUtG9hDSs//lAGpIO8ccQB2o6vDs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=et+v7VDXIdu4VhP0EU6CvVR0W1c8gu/J+qqLWwhPa8AJ2rXmIwyLrqy1stE0gZmsR\n\tMX77GXTRMyUD5dQQdXWXusRY9FIduvBwXxYqzpaUoNaBoTbXv1rtBHQ+bV3llMp6Jd\n\t5AChaRm9PqCOszdr4BTMXhDN5FbxjRcDrJ/URWUo=","Date":"Wed, 27 Jan 2021 01:25:56 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20210125184858.16339-1-david.plowman@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 0/5] Remove Raspberry Pi logging","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":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]