[{"id":20612,"web_url":"https://patchwork.libcamera.org/comment/20612/","msgid":"<163542808789.2498920.18393009648624395797@Monstersaurus>","date":"2021-10-28T13:34:47","subject":"Re: [libcamera-devel] [PATCH 2/4] ipu3: ipa: Report effective\n\tsensor controls with stastistics to IPA","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Han-Lin Chen (2021-10-28 11:03:17)\n> The Intel close sourced IPA requires the effective controls applied to\n> the sensor when the stastistics are generated. Report effective sensor controls\n> with the stastistics to IPA.\n> \n> Signed-off-by: Han-Lin Chen \n> ---\n> src/libcamera/pipeline/ipu3/frames.h | 3 +++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++\n> 2 files changed, 8 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\n> index 3ef7e445..a897e307 100644\n> --- a/src/libcamera/pipeline/ipu3/frames.h\n> +++ b/src/libcamera/pipeline/ipu3/frames.h\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -34,6 +35,8 @@ public:\n> FrameBuffer *paramBuffer;\n> FrameBuffer *statBuffer;\n> \n> + ControlList effectiveSensorControls;\n> +\n> bool paramDequeued;\n> bool metadataProcessed;\n> };\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 8816efc5..6a7f5b9a 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> return ret;\n> }\n> \n> + data->delayedCtrls_->reset();\n> +\n> return updateControls(data);\n> }\n> \n> @@ -1363,6 +1365,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> request->metadata().set(controls::SensorTimestamp,\n> buffer->metadata().timestamp);\n> \n> + info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n> +\n\nAre the delayedControls and the metadata sequence numbers directly\ncorrelated? Or are they both just sequentially increasing sequences...\n\nI see we're resetting the delayed controls above, and that makes me\nworry that this 'works' by resetting the delayed controls, and assumeing\nthat the capture device will always give the same sequence numbers. But\nI'm not sure they always do that. .. I'm sure I've seen some continue\nsequences after start stops.\n\nHopefully the V4L2 API will specify what is correct here.\n\n\n> if (request->findBuffer(&rawStream_))\n> pipe()->completeBuffer(request, buffer);\n> \n> @@ -1419,6 +1423,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> ev.frame = info->id;\n> ev.bufferId = info->statBuffer->cookie();\n> ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);\n> + ev.sensorControls = info->effectiveSensorControls;\n> ipa_->processEvent(ev);\n> }\n> \n> -- \n> 2.33.1.1089.g2158813163f-goog\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 469E3BF415\n\tfor ;\n\tThu, 28 Oct 2021 13:34:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0330C600BA;\n\tThu, 28 Oct 2021 15:34:52 +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 575BF600B5\n\tfor ;\n\tThu, 28 Oct 2021 15:34:50 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E082513;\n\tThu, 28 Oct 2021 15:34:50 +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=\"YtdS3EMt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635428090;\n\tbh=/NdmX7ORd5boZOQ1J5uV74SWG+2+iuMJD7y9wxMEZeA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YtdS3EMtDxuwTyiNNxA/HQ0GdnQ4BWw2sqDnImOJGjVGOpVqPOQ91vS8WlX0BGMjm\n\taIv4mWIgNin9cvhW2BiGf+wuEED6Z3knffg9/+FCs8q9Y9kqe5WXdNQX0QluSLlGxx\n\twLnIrMstQ7onaf0Y3F7QLZKIgU07J7aHfsY+vTkk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211028100319.1097720-2-hanlinchen@chromium.org>","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>\n\t<20211028100319.1097720-2-hanlinchen@chromium.org>","From":"Kieran Bingham ","To":"Han-Lin Chen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 28 Oct 2021 14:34:47 +0100","Message-ID":"<163542808789.2498920.18393009648624395797@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 2/4] ipu3: ipa: Report effective\n\tsensor controls with stastistics to IPA","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":"Han-Lin Chen ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20632,"web_url":"https://patchwork.libcamera.org/comment/20632/","msgid":"","date":"2021-10-29T09:26:44","subject":"Re: [libcamera-devel] [PATCH 2/4] ipu3: ipa: Report effective\n\tsensor controls with stastistics to IPA","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Kieran,\nThanks for the review.\n\nOn Thu, Oct 28, 2021 at 9:34 PM Kieran Bingham\n wrote:\n>\n> Quoting Han-Lin Chen (2021-10-28 11:03:17)\n> > The Intel close sourced IPA requires the effective controls applied to\n> > the sensor when the stastistics are generated. Report effective sensor controls\n> > with the stastistics to IPA.\n> >\n> > Signed-off-by: Han-Lin Chen \n> > ---\n> > src/libcamera/pipeline/ipu3/frames.h | 3 +++\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++\n> > 2 files changed, 8 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\n> > index 3ef7e445..a897e307 100644\n> > --- a/src/libcamera/pipeline/ipu3/frames.h\n> > +++ b/src/libcamera/pipeline/ipu3/frames.h\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > namespace libcamera {\n> > @@ -34,6 +35,8 @@ public:\n> > FrameBuffer *paramBuffer;\n> > FrameBuffer *statBuffer;\n> >\n> > + ControlList effectiveSensorControls;\n> > +\n> > bool paramDequeued;\n> > bool metadataProcessed;\n> > };\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 8816efc5..6a7f5b9a 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > return ret;\n> > }\n> >\n> > + data->delayedCtrls_->reset();\n> > +\n> > return updateControls(data);\n> > }\n> >\n> > @@ -1363,6 +1365,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> > request->metadata().set(controls::SensorTimestamp,\n> > buffer->metadata().timestamp);\n> >\n> > + info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n> > +\n>\n> Are the delayedControls and the metadata sequence numbers directly\n> correlated? Or are they both just sequentially increasing sequences...\n>\n> I see we're resetting the delayed controls above, and that makes me\n> worry that this 'works' by resetting the delayed controls, and assumeing\n> that the capture device will always give the same sequence numbers. But\n> I'm not sure they always do that. .. I'm sure I've seen some continue\n> sequences after start stops.\n\nThe DelayedControl uses the first sequence after reset() as the\n\"first_sequence\" and uses the diff between it and the current sequence\nas\nindex to it's internal ring buffer. I suppose it's designed this way,\nso it works as long as the following sequence is increasing.\nThe V4L2 API seems not to specify whether the sequence should be reset\nafter stops, but it should be increasing after start.\nThis is why I suppose resetting it before start is a safer bet.\nAnother reason is to clear the internal ring buffer in the\nDelayedControls.\n>\n> Hopefully the V4L2 API will specify what is correct here.\n>\n>\n> > if (request->findBuffer(&rawStream_))\n> > pipe()->completeBuffer(request, buffer);\n> >\n> > @@ -1419,6 +1423,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> > ev.frame = info->id;\n> > ev.bufferId = info->statBuffer->cookie();\n> > ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);\n> > + ev.sensorControls = info->effectiveSensorControls;\n> > ipa_->processEvent(ev);\n> > }\n> >\n> > --\n> > 2.33.1.1089.g2158813163f-goog\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 44AADBF415\n\tfor ;\n\tFri, 29 Oct 2021 09:27:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3C82600B7;\n\tFri, 29 Oct 2021 11:26:59 +0200 (CEST)","from mail-oo1-xc2c.google.com (mail-oo1-xc2c.google.com\n\t[IPv6:2607:f8b0:4864:20::c2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27052600B4\n\tfor ;\n\tFri, 29 Oct 2021 11:26:58 +0200 (CEST)","by mail-oo1-xc2c.google.com with SMTP id\n\ty133-20020a4a458b000000b002bb71084420so1582093ooa.6\n\tfor ;\n\tFri, 29 Oct 2021 02:26:58 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"G91zEWI0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=X3HdGR3xhOon/CW1WjXdFoTawE0IHMr5CFVb43+X9Qo=;\n\tb=G91zEWI03sA+IfYcPZf3ktOh7+YJUqARH/88odANPkWDUHhyiT1GCsqnXQeUuFE3Bv\n\tLB25EZGzRQAXde+3s/9/irm9xFl/LSWKVQgBjHTkNwuG6mc/fai+aFbre3/8g2vzsWm2\n\tFb5k5hGGGUVwlIOUWIP+XheLDeH6DMcZD0n18=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=X3HdGR3xhOon/CW1WjXdFoTawE0IHMr5CFVb43+X9Qo=;\n\tb=6w3AsLgvtRU6leeoDciD9EFoT+YoScCr6eJPD2OHl6Qo+H5ISkN35KSWPDcpUpwM66\n\t3Kvn+2leqMdzHZiGu0z4lSGPWXlq2XzdMROPC4zquEN1T/JB9RqT+r6e1yF8tzb3Gi9X\n\tloell76Z2KKNWGocl5bJjTKKlVUSUgBlJiDM1tpzSCTXOH3iuqPMKUvMI5v2R1D6cwKL\n\tdLMeobcWLL9/G0OLm5dNzVTTBUGAT8bggPI4Df9hD6oqPDFQjgM0gMgaX27V/iYoAtzy\n\tyuBnbaCnr4xIIycpqSF78nCK9OegVhWWMJ6x68xhrmEMly/V9l5UJKsTffbgX3P/R4tj\n\tfrsA==","X-Gm-Message-State":"AOAM532rhWGzNoWoKA3pbD+MFALCM8bZb5JPTYCjdAi0bRj7wifNCJZq\n\t2E90BOmJGlnj29brDrW0dZ2j2qeUMPaSn2C+lo4OJ+U3fQRyqHqt","X-Google-Smtp-Source":"ABdhPJzqx0F+oA8cmzYZ/JBv0eSvOX+UyNwUbnDCcK3PuOIcHChtdqx2Vm2vvzv4+tTdUXoafxknx6a4KKahEH4IkMQ=","X-Received":"by 2002:a4a:e292:: with SMTP id k18mr6745592oot.80.1635499615452;\n\tFri, 29 Oct 2021 02:26:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>\n\t<20211028100319.1097720-2-hanlinchen@chromium.org>\n\t<163542808789.2498920.18393009648624395797@Monstersaurus>","In-Reply-To":"<163542808789.2498920.18393009648624395797@Monstersaurus>","From":"Hanlin Chen ","Date":"Fri, 29 Oct 2021 17:26:44 +0800","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 2/4] ipu3: ipa: Report effective\n\tsensor controls with stastistics to IPA","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]