[{"id":27946,"web_url":"https://patchwork.libcamera.org/comment/27946/","msgid":"<ci7gidfai3jebks5kw5u4ksbvdbuvg7vxgrxjnyrp6re7ttm2z@wy43hihlkoyu>","date":"2023-10-12T08:30:00","subject":"Re: [libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to\n\tthe Controller hardware description","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Fri, Oct 06, 2023 at 02:19:47PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Add a new boolean field (statsInline) to Controller::HardwareConfigMap.\n> This field indicates where the statistics are generated in the hardware\n> ISP pipeline. For statsInline == true, statistics are generated before\n> the frame is processed (e.g. the PiSP case), and statsInline == false\n> indicates statistics are generated after the frame is processed (e.g.\n> the VC4 case).\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/ipa/rpi/common/ipa_base.cpp       | 19 ++++++++++++++-----\n>  src/ipa/rpi/controller/controller.cpp |  3 ++-\n>  src/ipa/rpi/controller/controller.h   |  1 +\n>  3 files changed, 17 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index 97f647a9e53e..f28eb36b7a44 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -429,11 +429,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n>  \t}\n>\n>  \t/*\n> -\t * If a statistics buffer has been passed in, call processStats\n> -\t * directly now before prepare() since the statistics are available in-line\n> -\t * with the Bayer frame.\n> +\t * If the statistics are inline (i.e. already available with the Bayer\n> +\t * frame), call processStats() now before prepare().\n>  \t */\n> -\tif (params.buffers.stats)\n> +\tif (controller_.getHardwareConfig().statsInline)\n>  \t\tprocessStats({ params.buffers, params.ipaContext });\n>\n>  \t/* Do we need/want to call prepare? */\n> @@ -445,6 +444,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n>\n>  \tframeCount_++;\n>\n> +\t/* If the statistics are inline the metadata can be returned early. */\n> +\tif (controller_.getHardwareConfig().statsInline)\n> +\t\treportMetadata(ipaContext);\n> +\n\nCan't you drop this one and unconditionally call reportMetadata() in\nprocessStats() ?\n\n>  \t/* Ready to push the input buffer into the ISP. */\n>  \tprepareIspComplete.emit(params.buffers, false);\n>  }\n> @@ -479,7 +482,13 @@ void IpaBase::processStats(const ProcessParams &params)\n>  \t\t}\n>  \t}\n>\n> -\treportMetadata(ipaContext);\n> +\t/*\n> +\t * If the statistics are not inline the metadata must be returned now,\n> +\t * before the processStatsComplete signal.\n> +\t */\n> +\tif (!controller_.getHardwareConfig().statsInline)\n> +\t\treportMetadata(ipaContext);\n> +\n>  \tprocessStatsComplete.emit(params.buffers);\n>  }\n>\n> diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp\n> index 14d245da2ce7..4b6f82b41916 100644\n> --- a/src/ipa/rpi/controller/controller.cpp\n> +++ b/src/ipa/rpi/controller/controller.cpp\n> @@ -34,7 +34,8 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap\n>  \t\t\t.focusRegions = { 4, 3 },\n>  \t\t\t.numHistogramBins = 128,\n>  \t\t\t.numGammaPoints = 33,\n> -\t\t\t.pipelineWidth = 13\n> +\t\t\t.pipelineWidth = 13,\n> +\t\t\t.statsInline = false,\n>  \t\t}\n>  \t},\n>  };\n> diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h\n> index c6af5cd6c7d4..a8bc61880ab4 100644\n> --- a/src/ipa/rpi/controller/controller.h\n> +++ b/src/ipa/rpi/controller/controller.h\n> @@ -45,6 +45,7 @@ public:\n>  \t\tunsigned int numHistogramBins;\n>  \t\tunsigned int numGammaPoints;\n>  \t\tunsigned int pipelineWidth;\n> +\t\tbool statsInline;\n>  \t};\n>\n>  \tController();\n> --\n> 2.34.1\n>","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 0F489C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 08:30:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B3CC66297F;\n\tThu, 12 Oct 2023 10:30:05 +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 535B06297C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 10:30:03 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 48154583;\n\tThu, 12 Oct 2023 10:30:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697099405;\n\tbh=BsFzBxL8tIYySYijgZP1Xg8m9GiBpqR5a8MBoxBr7/0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1IEW/9j9WMN3o/VsKp5u6jJ7JJ8yDPk0vubDj6hJg6EfuZRkw5BPNs2peKWYqYoSE\n\tVsPopNZqQLPTYV0vZr21VQbzKHH9I9E33kabKJvcca7R90ObRDIMNinwZsc0AEruJf\n\tj8pm5GA2OeubDQSapXxxgRShXebZ+kBjHJT1ACBH7JpyMWvZpSqRd0S82rxwCZDRHy\n\tOAeXaJcYsRb+ERVWn/AJwD2+6VX8hCzeq+2J/ky13hVg+YC+joYcx4yBmfN0vPz0k8\n\tdAUO8sOL2+h/XVk+WNL/nV9lN5Zlo0ExfFCNN3WWvF5VPjPsZ/MoYrR3QFhFj1FRoX\n\tI13/iY2Pg5vyg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697099400;\n\tbh=BsFzBxL8tIYySYijgZP1Xg8m9GiBpqR5a8MBoxBr7/0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rkx0nEWrITZZFoE8VX88QDLK1O1uRODhJzTASAuX292pKoCtxdNmb2uGMtaCvblY4\n\tFUSFuz4PGZTkqzI/kiicXATB/AUe85SfS7IRT7jomZNqVmkn9gDtKJLa1uS3janDyy\n\tb6uNuY1UxIxIZMl+WU169YZIF6sZ3mbomgCENn1Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rkx0nEWr\"; dkim-atps=neutral","Date":"Thu, 12 Oct 2023 10:30:00 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<ci7gidfai3jebks5kw5u4ksbvdbuvg7vxgrxjnyrp6re7ttm2z@wy43hihlkoyu>","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-8-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231006132000.23504-8-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to\n\tthe Controller hardware description","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27948,"web_url":"https://patchwork.libcamera.org/comment/27948/","msgid":"<CAEmqJPpvT+7yBQih1T18rnnu=BkM-dSQTsTmQViR1u2JZ4TnSw@mail.gmail.com>","date":"2023-10-12T08:37:43","subject":"Re: [libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to\n\tthe Controller hardware description","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Thu, 12 Oct 2023 at 09:30, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n>\n> On Fri, Oct 06, 2023 at 02:19:47PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > Add a new boolean field (statsInline) to Controller::HardwareConfigMap.\n> > This field indicates where the statistics are generated in the hardware\n> > ISP pipeline. For statsInline == true, statistics are generated before\n> > the frame is processed (e.g. the PiSP case), and statsInline == false\n> > indicates statistics are generated after the frame is processed (e.g.\n> > the VC4 case).\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  src/ipa/rpi/common/ipa_base.cpp       | 19 ++++++++++++++-----\n> >  src/ipa/rpi/controller/controller.cpp |  3 ++-\n> >  src/ipa/rpi/controller/controller.h   |  1 +\n> >  3 files changed, 17 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index 97f647a9e53e..f28eb36b7a44 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -429,11 +429,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> >       }\n> >\n> >       /*\n> > -      * If a statistics buffer has been passed in, call processStats\n> > -      * directly now before prepare() since the statistics are available in-line\n> > -      * with the Bayer frame.\n> > +      * If the statistics are inline (i.e. already available with the Bayer\n> > +      * frame), call processStats() now before prepare().\n> >        */\n> > -     if (params.buffers.stats)\n> > +     if (controller_.getHardwareConfig().statsInline)\n> >               processStats({ params.buffers, params.ipaContext });\n> >\n> >       /* Do we need/want to call prepare? */\n> > @@ -445,6 +444,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> >\n> >       frameCount_++;\n> >\n> > +     /* If the statistics are inline the metadata can be returned early. */\n> > +     if (controller_.getHardwareConfig().statsInline)\n> > +             reportMetadata(ipaContext);\n> > +\n>\n> Can't you drop this one and unconditionally call reportMetadata() in\n> processStats() ?\n\nSadly not (well, not without a massive amount of rework).  The\ncontents of the metadata get collated in platformPrepareIsp(), and\nthis run *after* processStats() in statsInline (pisp) case.  So I have\nto do this conditionally as above.\n\nThe rework would be to update all the algorithms to report metadata in\nthe processStats() phase, but I don't have the stomach for that big\nchange :)\n\nRegards,\nNaush\n\n\n>\n> >       /* Ready to push the input buffer into the ISP. */\n> >       prepareIspComplete.emit(params.buffers, false);\n> >  }\n> > @@ -479,7 +482,13 @@ void IpaBase::processStats(const ProcessParams &params)\n> >               }\n> >       }\n> >\n> > -     reportMetadata(ipaContext);\n> > +     /*\n> > +      * If the statistics are not inline the metadata must be returned now,\n> > +      * before the processStatsComplete signal.\n> > +      */\n> > +     if (!controller_.getHardwareConfig().statsInline)\n> > +             reportMetadata(ipaContext);\n> > +\n> >       processStatsComplete.emit(params.buffers);\n> >  }\n> >\n> > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp\n> > index 14d245da2ce7..4b6f82b41916 100644\n> > --- a/src/ipa/rpi/controller/controller.cpp\n> > +++ b/src/ipa/rpi/controller/controller.cpp\n> > @@ -34,7 +34,8 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap\n> >                       .focusRegions = { 4, 3 },\n> >                       .numHistogramBins = 128,\n> >                       .numGammaPoints = 33,\n> > -                     .pipelineWidth = 13\n> > +                     .pipelineWidth = 13,\n> > +                     .statsInline = false,\n> >               }\n> >       },\n> >  };\n> > diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h\n> > index c6af5cd6c7d4..a8bc61880ab4 100644\n> > --- a/src/ipa/rpi/controller/controller.h\n> > +++ b/src/ipa/rpi/controller/controller.h\n> > @@ -45,6 +45,7 @@ public:\n> >               unsigned int numHistogramBins;\n> >               unsigned int numGammaPoints;\n> >               unsigned int pipelineWidth;\n> > +             bool statsInline;\n> >       };\n> >\n> >       Controller();\n> > --\n> > 2.34.1\n> >","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 F40CFC3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 08:38:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6594C62962;\n\tThu, 12 Oct 2023 10:38:13 +0200 (CEST)","from mail-yw1-x1136.google.com (mail-yw1-x1136.google.com\n\t[IPv6:2607:f8b0:4864:20::1136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2CE2662962\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 10:38:12 +0200 (CEST)","by mail-yw1-x1136.google.com with SMTP id\n\t00721157ae682-5a7af20c488so8717477b3.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 01:38:12 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697099893;\n\tbh=xKHR1OwRbFrhJK8EQaBn5rJ+uGa2l75rQG7cb74/YBk=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tuY0ZDvVI5I5bM4I35s9Yh7qA41hjivrLbU++gmxThtIeHEwV/H+KmbIHORQlIbg2\n\t9x5zcHVfmZHAFL9LJgmkYQWrQQnmehrGLbRGDr5CJhUjZv4mQr3LlVSAJ7AeVcxh40\n\t3yjM6FAyzfbtWsM+r0hKuECPpWIc1RpNl6HBfwl+mz2DyGwojXr7h5BhojkhdwUZSM\n\tB7ty1WM/thXX6poSa97kfvofWIpuTF871KkpVIXzJ2rmgg2ut7B4Df23ZQmb9pYLqs\n\tQV43xCN9MojCQnvvhqmtYU/cH5CI9ILMgm4oJ+j09kHR/HqPbs9mm98mu9PREsLuEB\n\t9BnpijaNZNdzA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1697099891; x=1697704691;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=9PhqBjkYjFyGB5lpRMTSDje6hChIHn48rWFhy0pGigc=;\n\tb=sLogNOMSKZ4yJNwf8N3kwkZwFqjuCaUGhocDcUSQfEDEbYgxwewO7KfoCRU6rhf12H\n\t6v3OL0H7F/a0SVRKlffAvur57OwrkYW3X60BQoAvgE6LqKZyYDUopxcL/UVaKflP/1mz\n\tIHOcmrZ19kxs033XG12fjN8H9yZm6vUCIf5FVsKuq2uI69uwjeQ2bdXej8eNCdGx2k8m\n\teRwNqx2tGNx10S6e7mfGTCJbfjzxzCok7JE39EaX6gxvj2+K+Puq8GUmzvfoXQQQCxd6\n\tD7nbuwmTvTRXsPgIjG69kkDiuOYu9Di0EaBd2DuTp+nfoX6qIoitKXg7h3FiTMateHYN\n\tteWQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"sLogNOMS\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1697099891; x=1697704691;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=9PhqBjkYjFyGB5lpRMTSDje6hChIHn48rWFhy0pGigc=;\n\tb=tN527mjxVQR3/RhwvCSfU87swZJPNLW1dNZNX80UP4EFuI1PWgrSQo6isWqqlNOF0c\n\tiIDNkwhag/3CkYWNvNf4As+X763u6Zw29+kE9jJK99T4WuNrIZhJpwcCrvQWYH1fNb3h\n\tECnRRJWTQwAUBnOXJsL8gt+U8Qn0N13gsmFZyDip2sWhpHE/K6J2HTvueqosbkIGN//8\n\t5XiXDFTnuckhTp8fJb8gzXoYR9EBthqe9F64bPFUCNb8ZfqwFmcy4SeFI3l5IUI3e9h+\n\thGicGZE95M8WnFwkWPkocD1MDx4emBpFAyE8S/owJUDSEF4ra02s+wDWx65mEaTNibdA\n\tKO8Q==","X-Gm-Message-State":"AOJu0Yy+cxiLJyiLlRbVMHnAi8UHm7eLgMx58kYyf5RAcH4gQhnvwQqe\n\tRB18EhZYPSRBv0DhT9fpNV/E02+1RVCSoXXZS2P9TA==","X-Google-Smtp-Source":"AGHT+IHhQRErbpCz3Tmk2awYjRwYcAWUNp8mo5KTwAu4k1XO1aTsrzIRT9tNIm6F4weN9KL9hhvNXCGv8OMBDgeQFRc=","X-Received":"by 2002:a81:ad5f:0:b0:5a7:ab31:f4e2 with SMTP id\n\tl31-20020a81ad5f000000b005a7ab31f4e2mr9922731ywk.13.1697099890812;\n\tThu, 12 Oct 2023 01:38:10 -0700 (PDT)","MIME-Version":"1.0","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-8-naush@raspberrypi.com>\n\t<ci7gidfai3jebks5kw5u4ksbvdbuvg7vxgrxjnyrp6re7ttm2z@wy43hihlkoyu>","In-Reply-To":"<ci7gidfai3jebks5kw5u4ksbvdbuvg7vxgrxjnyrp6re7ttm2z@wy43hihlkoyu>","Date":"Thu, 12 Oct 2023 09:37:43 +0100","Message-ID":"<CAEmqJPpvT+7yBQih1T18rnnu=BkM-dSQTsTmQViR1u2JZ4TnSw@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to\n\tthe Controller hardware description","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27949,"web_url":"https://patchwork.libcamera.org/comment/27949/","msgid":"<7wl4mzzips4cdp53b7utwffe7bvbszmy533pwizn6su6vga6lj@ymocssdlagdu>","date":"2023-10-12T08:51:34","subject":"Re: [libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to\n\tthe Controller hardware description","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Thu, Oct 12, 2023 at 09:37:43AM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> On Thu, 12 Oct 2023 at 09:30, Jacopo Mondi\n> <jacopo.mondi@ideasonboard.com> wrote:\n> >\n> >\n> > On Fri, Oct 06, 2023 at 02:19:47PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > Add a new boolean field (statsInline) to Controller::HardwareConfigMap.\n> > > This field indicates where the statistics are generated in the hardware\n> > > ISP pipeline. For statsInline == true, statistics are generated before\n> > > the frame is processed (e.g. the PiSP case), and statsInline == false\n> > > indicates statistics are generated after the frame is processed (e.g.\n> > > the VC4 case).\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > ---\n> > >  src/ipa/rpi/common/ipa_base.cpp       | 19 ++++++++++++++-----\n> > >  src/ipa/rpi/controller/controller.cpp |  3 ++-\n> > >  src/ipa/rpi/controller/controller.h   |  1 +\n> > >  3 files changed, 17 insertions(+), 6 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > index 97f647a9e53e..f28eb36b7a44 100644\n> > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > @@ -429,11 +429,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> > >       }\n> > >\n> > >       /*\n> > > -      * If a statistics buffer has been passed in, call processStats\n> > > -      * directly now before prepare() since the statistics are available in-line\n> > > -      * with the Bayer frame.\n> > > +      * If the statistics are inline (i.e. already available with the Bayer\n> > > +      * frame), call processStats() now before prepare().\n> > >        */\n> > > -     if (params.buffers.stats)\n> > > +     if (controller_.getHardwareConfig().statsInline)\n> > >               processStats({ params.buffers, params.ipaContext });\n> > >\n> > >       /* Do we need/want to call prepare? */\n> > > @@ -445,6 +444,10 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> > >\n> > >       frameCount_++;\n> > >\n> > > +     /* If the statistics are inline the metadata can be returned early. */\n> > > +     if (controller_.getHardwareConfig().statsInline)\n> > > +             reportMetadata(ipaContext);\n> > > +\n> >\n> > Can't you drop this one and unconditionally call reportMetadata() in\n> > processStats() ?\n>\n> Sadly not (well, not without a massive amount of rework).  The\n> contents of the metadata get collated in platformPrepareIsp(), and\n> this run *after* processStats() in statsInline (pisp) case.  So I have\n> to do this conditionally as above.\n>\n> The rework would be to update all the algorithms to report metadata in\n> the processStats() phase, but I don't have the stomach for that big\n> change :)\n>\n\nI see! Thanks for explaining!\n\nSo this looks good indeed!\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n> Regards,\n> Naush\n>\n>\n> >\n> > >       /* Ready to push the input buffer into the ISP. */\n> > >       prepareIspComplete.emit(params.buffers, false);\n> > >  }\n> > > @@ -479,7 +482,13 @@ void IpaBase::processStats(const ProcessParams &params)\n> > >               }\n> > >       }\n> > >\n> > > -     reportMetadata(ipaContext);\n> > > +     /*\n> > > +      * If the statistics are not inline the metadata must be returned now,\n> > > +      * before the processStatsComplete signal.\n> > > +      */\n> > > +     if (!controller_.getHardwareConfig().statsInline)\n> > > +             reportMetadata(ipaContext);\n> > > +\n> > >       processStatsComplete.emit(params.buffers);\n> > >  }\n> > >\n> > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp\n> > > index 14d245da2ce7..4b6f82b41916 100644\n> > > --- a/src/ipa/rpi/controller/controller.cpp\n> > > +++ b/src/ipa/rpi/controller/controller.cpp\n> > > @@ -34,7 +34,8 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap\n> > >                       .focusRegions = { 4, 3 },\n> > >                       .numHistogramBins = 128,\n> > >                       .numGammaPoints = 33,\n> > > -                     .pipelineWidth = 13\n> > > +                     .pipelineWidth = 13,\n> > > +                     .statsInline = false,\n> > >               }\n> > >       },\n> > >  };\n> > > diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h\n> > > index c6af5cd6c7d4..a8bc61880ab4 100644\n> > > --- a/src/ipa/rpi/controller/controller.h\n> > > +++ b/src/ipa/rpi/controller/controller.h\n> > > @@ -45,6 +45,7 @@ public:\n> > >               unsigned int numHistogramBins;\n> > >               unsigned int numGammaPoints;\n> > >               unsigned int pipelineWidth;\n> > > +             bool statsInline;\n> > >       };\n> > >\n> > >       Controller();\n> > > --\n> > > 2.34.1\n> > >","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 B1F73BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 08:51:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0024A6297F;\n\tThu, 12 Oct 2023 10:51:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 528C762962\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 10:51:38 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 278397FC;\n\tThu, 12 Oct 2023 10:51:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697100700;\n\tbh=JrV4HPF4EfmzpeJtOS/RRQHfdn5oz+PzpwlXy7VAiFE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=uouhT11O6zo9HdChvWF0yg3BxxjopHtBFsNPIvCK5+97oM/sy2UYqSEaDfSnQFXYw\n\tufsM8Bmo2QK2o2r3870YjxO0V/6kp7EV0Olj6QPFLQ82VcppOTO2Wx9ejKM9tNs3+4\n\tS5Tn0yTwqjnh7JPgXDlEYNiMQT+snECs9qSweZHbcGGVUWD8L4dF2CVhqiHEtDKD3q\n\tZgWFf633ZzXH7DtwykgcYpmG/+BfcVZAxlx+uI4uLxzXHVuOkzFfJtC1Pk+wsMMKvJ\n\tcgRIxLIRxCbeLH/h1m9K6+YrywoOj4jcSvgLGsbdteRvxiD7OrRzn5mrBfbiHfyJdU\n\tqWgfTbFGGzskg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697100695;\n\tbh=JrV4HPF4EfmzpeJtOS/RRQHfdn5oz+PzpwlXy7VAiFE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DpSzNZZVgSu7C4iy6tSVWgcfAvnO8VcbUijx6oITDfpSeESYwTrh1ZlkZTraJ6xSw\n\tUy6VJs7251wvPxJHlhm4BTPgusfVbiR5yDZKCRfRSrX7Ma5tb+Bbuw4YLZfr9pSqLx\n\tol+rXKNHW0dITjANG18dtvuDXRuFE5QLMrl49PIo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DpSzNZZV\"; dkim-atps=neutral","Date":"Thu, 12 Oct 2023 10:51:34 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<7wl4mzzips4cdp53b7utwffe7bvbszmy533pwizn6su6vga6lj@ymocssdlagdu>","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-8-naush@raspberrypi.com>\n\t<ci7gidfai3jebks5kw5u4ksbvdbuvg7vxgrxjnyrp6re7ttm2z@wy43hihlkoyu>\n\t<CAEmqJPpvT+7yBQih1T18rnnu=BkM-dSQTsTmQViR1u2JZ4TnSw@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPpvT+7yBQih1T18rnnu=BkM-dSQTsTmQViR1u2JZ4TnSw@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 07/20] ipa: rpi: Add statsInline to\n\tthe Controller hardware description","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]