[{"id":33815,"web_url":"https://patchwork.libcamera.org/comment/33815/","msgid":"<20250331213004.GI14432@pendragon.ideasonboard.com>","date":"2025-03-31T21:30:04","subject":"Re: [PATCH 0/5] Fix histogram for some (corner) cases","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Mar 24, 2025 at 06:07:35PM +0100, Stefan Klug wrote:\n> Hi all,\n> \n> during my work on WDR I stumbled over a few non-linearities when dealing\n> with small inter quantile mean ranges. These led to hard to handle\n> oscillations. This series adds a testcase for the histogram class and\n> fixes the related issues.\n> \n> Patch 1 adds the basic tests that pass with the current implementation.\n> \n> Patch 2-3 fix issues in the quantile() implementation and add\n> corresponding tests.\n> \n> Patch 4-5 fix issues in the interQuantileMean() implementation and also\n> add tests.\n> \n> I added the tests after the fixes, so that I don't have to toggle should_fail\n> in the meson.build file all the time (And CI doesn't test every commit anyways).\n\nDoesn't it ? The build-history test is supposed to do just that.\n\n> To test that in review, I believe the easiest is to temporarily revert the\n> corresponding fix and see the test fail.\n\nshould_fail is meant exactly for this :-)\n\n> Stefan Klug (5):\n> test: ipa: libipa: Add histogram tests\n> libipa: histogram: Fix quantile() calculation for fractional results\n> test: ipa: libipa: histogram: Add tests for quantile() returning a\n> fraction\n> libipa: histogram: Fix interQuantileMean() for small ranges\n> test: ipa: libipa: histogram: Add tests for small inter quantile mean\n> ranges\n> \n> src/ipa/libipa/histogram.cpp | 23 ++++++------\n> test/ipa/libipa/histogram.cpp | 66 +++++++++++++++++++++++++++++++++++\n> test/ipa/libipa/meson.build | 1 +\n> 3 files changed, 80 insertions(+), 10 deletions(-)\n> create mode 100644 test/ipa/libipa/histogram.cpp","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 1544CC3213\n\tfor ;\n\tMon, 31 Mar 2025 21:30:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CD1968981;\n\tMon, 31 Mar 2025 23:30:31 +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 20EF668967\n\tfor ;\n\tMon, 31 Mar 2025 23:30:29 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-147-224-nat.elisa-mobile.fi\n\t[85.76.147.224])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 20FBF703;\n\tMon, 31 Mar 2025 23:28:37 +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=\"vXZ1mB/j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743456517;\n\tbh=LO5SAsjF6bUWK9EMnf+1tGA4JdJX85n2LG9/80PuB98=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vXZ1mB/jMkWHXypDOSxJAXtdbfj1GLlxY6NTUXL/zj8JscymXUsIjSu2LvzPutbkA\n\tBt80TmXLwCXKz7Au3WduBmK0NqLmgrrhlend8lwaRZIPs1mfEHcH9FxgsQK6CEd5hW\n\tn0KixDn0Z09wg0i3kPbVRO1IWAvzOpaPJfnGFWQg=","Date":"Tue, 1 Apr 2025 00:30:04 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/5] Fix histogram for some (corner) cases","Message-ID":"<20250331213004.GI14432@pendragon.ideasonboard.com>","References":"<20250324170803.103296-1-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250324170803.103296-1-stefan.klug@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\" "}},{"id":33817,"web_url":"https://patchwork.libcamera.org/comment/33817/","msgid":"<20250331215448.GA28057@pendragon.ideasonboard.com>","date":"2025-03-31T21:54:48","subject":"Re: [PATCH 0/5] Fix histogram for some (corner) cases","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:30:04AM +0300, Laurent Pinchart wrote:\n> On Mon, Mar 24, 2025 at 06:07:35PM +0100, Stefan Klug wrote:\n> > Hi all,\n> > \n> > during my work on WDR I stumbled over a few non-linearities when dealing\n> > with small inter quantile mean ranges. These led to hard to handle\n> > oscillations. This series adds a testcase for the histogram class and\n> > fixes the related issues.\n> > \n> > Patch 1 adds the basic tests that pass with the current implementation.\n> > \n> > Patch 2-3 fix issues in the quantile() implementation and add\n> > corresponding tests.\n> > \n> > Patch 4-5 fix issues in the interQuantileMean() implementation and also\n> > add tests.\n> > \n> > I added the tests after the fixes, so that I don't have to toggle should_fail\n> > in the meson.build file all the time (And CI doesn't test every commit anyways).\n> \n> Doesn't it ? The build-history test is supposed to do just that.\n\nAh, you meant that CI doesn't run unit tests for every commit. That's\ntrue.\n\n> > To test that in review, I believe the easiest is to temporarily revert the\n> > corresponding fix and see the test fail.\n> \n> should_fail is meant exactly for this :-)\n\nshould_fail isn't meant for CI. Adding tests first ensures that the test\nfails. This is meant to avoid adding a buggy test that doesn't actually\ncatch the issue it's supposed to catch. I would much prefer having the\ntests added first.\n\n> > Stefan Klug (5):\n> > test: ipa: libipa: Add histogram tests\n> > libipa: histogram: Fix quantile() calculation for fractional results\n> > test: ipa: libipa: histogram: Add tests for quantile() returning a\n> > fraction\n> > libipa: histogram: Fix interQuantileMean() for small ranges\n> > test: ipa: libipa: histogram: Add tests for small inter quantile mean\n> > ranges\n> > \n> > src/ipa/libipa/histogram.cpp | 23 ++++++------\n> > test/ipa/libipa/histogram.cpp | 66 +++++++++++++++++++++++++++++++++++\n> > test/ipa/libipa/meson.build | 1 +\n> > 3 files changed, 80 insertions(+), 10 deletions(-)\n> > create mode 100644 test/ipa/libipa/histogram.cpp","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 5173FC3213\n\tfor ;\n\tMon, 31 Mar 2025 21:55:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7843468985;\n\tMon, 31 Mar 2025 23:55:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BAD7C68967\n\tfor ;\n\tMon, 31 Mar 2025 23:55:13 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-147-224-nat.elisa-mobile.fi\n\t[85.76.147.224])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5F81703;\n\tMon, 31 Mar 2025 23:53:21 +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=\"m65i2JcY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743458002;\n\tbh=zoUHR5eJ+Bp9VjeAv89yXY4edVUgKYS7Nr2dpxfNslw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=m65i2JcYjl3NtlWLHDgldmb4GFY3I7TIZTZu32xxO4RdV1TtiriZLrPjqdVZhFVAv\n\tbbeR2n/1s5MhnhpBuAo14hhIjAt1s2v916qzWgkMdniyReicXASFY4gW1Q0f0KyXEV\n\tW4pHTkZI284q6+ft588s4h3hNusipGyjLLfv2rcI=","Date":"Tue, 1 Apr 2025 00:54:48 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/5] Fix histogram for some (corner) cases","Message-ID":"<20250331215448.GA28057@pendragon.ideasonboard.com>","References":"<20250324170803.103296-1-stefan.klug@ideasonboard.com>\n\t<20250331213004.GI14432@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250331213004.GI14432@pendragon.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\" "}},{"id":33847,"web_url":"https://patchwork.libcamera.org/comment/33847/","msgid":"<4avlriruyv7aoaknywxpfzrc32mvqjmke52hpfb4zn7ocounrb@vlhbmipxhrw4>","date":"2025-04-01T13:07:35","subject":"Re: [PATCH 0/5] Fix histogram for some (corner) cases","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Tue, Apr 01, 2025 at 12:54:48AM +0300, Laurent Pinchart wrote:\n> On Tue, Apr 01, 2025 at 12:30:04AM +0300, Laurent Pinchart wrote:\n> > On Mon, Mar 24, 2025 at 06:07:35PM +0100, Stefan Klug wrote:\n> > > Hi all,\n> > > \n> > > during my work on WDR I stumbled over a few non-linearities when dealing\n> > > with small inter quantile mean ranges. These led to hard to handle\n> > > oscillations. This series adds a testcase for the histogram class and\n> > > fixes the related issues.\n> > > \n> > > Patch 1 adds the basic tests that pass with the current implementation.\n> > > \n> > > Patch 2-3 fix issues in the quantile() implementation and add\n> > > corresponding tests.\n> > > \n> > > Patch 4-5 fix issues in the interQuantileMean() implementation and also\n> > > add tests.\n> > > \n> > > I added the tests after the fixes, so that I don't have to toggle should_fail\n> > > in the meson.build file all the time (And CI doesn't test every commit anyways).\n> > \n> > Doesn't it ? The build-history test is supposed to do just that.\n> \n> Ah, you meant that CI doesn't run unit tests for every commit. That's\n> true.\n> \n> > > To test that in review, I believe the easiest is to temporarily revert the\n> > > corresponding fix and see the test fail.\n> > \n> > should_fail is meant exactly for this :-)\n> \n> should_fail isn't meant for CI. Adding tests first ensures that the test\n> fails. This is meant to avoid adding a buggy test that doesn't actually\n> catch the issue it's supposed to catch. I would much prefer having the\n> tests added first.\n\nI somehow missed \"meson test\". That actually makes this quite useful and\nnow that I changed the order of patches I actually like it :-)\n\nIt would be great though to see all the tests that fail because the\ncheck you mention above actually only works until the first failing\ntest. But that is for another day :-)\n\nRegards,\nStefan\n\n> \n> > > Stefan Klug (5):\n> > > test: ipa: libipa: Add histogram tests\n> > > libipa: histogram: Fix quantile() calculation for fractional results\n> > > test: ipa: libipa: histogram: Add tests for quantile() returning a\n> > > fraction\n> > > libipa: histogram: Fix interQuantileMean() for small ranges\n> > > test: ipa: libipa: histogram: Add tests for small inter quantile mean\n> > > ranges\n> > > \n> > > src/ipa/libipa/histogram.cpp | 23 ++++++------\n> > > test/ipa/libipa/histogram.cpp | 66 +++++++++++++++++++++++++++++++++++\n> > > test/ipa/libipa/meson.build | 1 +\n> > > 3 files changed, 80 insertions(+), 10 deletions(-)\n> > > create mode 100644 test/ipa/libipa/histogram.cpp\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 69180C3213\n\tfor ;\n\tTue, 1 Apr 2025 13:07:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B018E68947;\n\tTue, 1 Apr 2025 15:07:38 +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 00E7468947\n\tfor ;\n\tTue, 1 Apr 2025 15:07:37 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:14c7:4fcc:495b:719f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A4540741;\n\tTue, 1 Apr 2025 15:05:45 +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=\"XK4NUAfm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743512745;\n\tbh=C237dZjiKRM1tJDvxpakmrk3K5Hfs+nJRgrKyGp+xbQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XK4NUAfmi2XBqNm0Ycr52u/mBHan/l0G1ZxuvYNX1ohSVyT1ve+UbKexrFyMt0ZUl\n\tlpaorjegYGAmgyoj8CmwzwCJ09+CcTrv/UWZrj3BOKpxrXl0teXcH1KEkwzmCNjPkE\n\taaRU57nlp/r7XykXah9eibKyij9Vi8Su9hY9Rdvk=","Date":"Tue, 1 Apr 2025 15:07:35 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/5] Fix histogram for some (corner) cases","Message-ID":"<4avlriruyv7aoaknywxpfzrc32mvqjmke52hpfb4zn7ocounrb@vlhbmipxhrw4>","References":"<20250324170803.103296-1-stefan.klug@ideasonboard.com>\n\t<20250331213004.GI14432@pendragon.ideasonboard.com>\n\t<20250331215448.GA28057@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250331215448.GA28057@pendragon.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\" "}}]