[{"id":20387,"web_url":"https://patchwork.libcamera.org/comment/20387/","msgid":"","date":"2021-10-22T13:18:31","subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this patch.\n\nOn Fri, 22 Oct 2021 at 12:55, Naushir Patuck wrote:\n>\n> The unicam driver no longer regesters an embedded data node if the sensor does\n> not provide this stream. Account for this in the pipeline handler match routine\n> by not assuming it is always present.\n>\n> Add a warning if Unicam and the CamHelper do not agree on the presense of sensor\n> embedded data, and disable its useage in these cases.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 16 +++++++++++++---\n> 1 file changed, 13 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index fc190e39732a..5aaf24436f27 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -994,7 +994,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> DeviceMatch unicam(\"unicam\");\n> DeviceMatch isp(\"bcm2835-isp\");\n>\n> - unicam.add(\"unicam-embedded\");\n> unicam.add(\"unicam-image\");\n>\n> isp.add(\"bcm2835-isp0-output0\"); /* Input */\n> @@ -1015,9 +1014,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> return false;\n>\n> /* Locate and open the unicam video streams. */\n> - data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> data->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n>\n> + /* An embedded data node will not be present if the sensor does not support it. */\n> + MediaEntity *embeddedEntity = unicam_->getEntityByName(\"unicam-embedded\");\n> + if (embeddedEntity) {\n> + data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\", embeddedEntity);\n> + data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n> + &RPiCameraData::unicamBufferDequeue);\n> + }\n> +\n> /* Tag the ISP input stream as an import stream. */\n> data->isp_[Isp::Input] = RPi::Stream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> data->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> @@ -1027,7 +1033,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> /* Wire up all the buffer connections. */\n> data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(), &RPiCameraData::frameStarted);\n> data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);\n> - data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);\n> data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispInputDequeue);\n> data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);\n> data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);\n> @@ -1055,6 +1060,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> return false;\n> }\n>\n> + if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {\n> + LOG(RPI, Warning) << \"Mismatch between Unicam and CamHelper for embedded data usage!\";\n> + sensorConfig.sensorMetadata = false;\n> + }\n> +\n> /*\n> * Open all Unicam and ISP streams. The exception is the embedded data\n> * stream, which only gets opened below if the IPA reports that the sensor\n> --\n> 2.25.1\n>\n\nI was just wondering, is there any implication for our \"embedded data\npresent\" CamHelper functions? Can we now just tell, or do we still\nneed them?\n\nOther than this:\n\nReviewed-by: David Plowman \n\nThanks!\n\nDavid","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 AB338BDB1C\n\tfor ;\n\tFri, 22 Oct 2021 13:18:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C86068F59;\n\tFri, 22 Oct 2021 15:18:44 +0200 (CEST)","from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com\n\t[IPv6:2a00:1450:4864:20::32f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 226EE6012A\n\tfor ;\n\tFri, 22 Oct 2021 15:18:42 +0200 (CEST)","by mail-wm1-x32f.google.com with SMTP id\n\tr17-20020a7bc091000000b0032ca1d30b30so884128wmh.0\n\tfor ;\n\tFri, 22 Oct 2021 06:18:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"e7G9QdXR\"; 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=jA98RSRbLJ8OC/1i3SRXzRXf7dqJouB+B36GdCVlB/Y=;\n\tb=e7G9QdXRCN/d3H1lnY0ZuyNGWNfXrMP8UXjzp+qYh03qe9or4+UJ7bITm+XhmFKf56\n\tWUN7quSuI5EC5HVKzZHx8NPFSVSa/BRDmlHFUqoeS1uu9o0vVsGxtbbtU50CXtbZrK3g\n\tEei01VCqcXGLJeMWWK7HOapQQPHonRi+A/U4/2qyu5eJwOBRtpmoQT1DMmV2Mo3EXXfg\n\t/COIKua/76JLxeOmDHKfAwSyyVJqz9zyj/lYkTj0EHjkSm/cmYs+Owhh4RYA5iLrFh6+\n\tS+zivVApJ8p2IYFODUoYQO6Quq0uhmqsTTwwS0STHgRQpya/LGyzF7j/SPbK7l7KmLCN\n\t0Xfw==","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=jA98RSRbLJ8OC/1i3SRXzRXf7dqJouB+B36GdCVlB/Y=;\n\tb=06LjsT2bGtS1oYCthEYTZZfNaOQbvouaGCn7QUzpdlI2m4jLq+C9fUOFWC9M/FWKS2\n\teB+qHkywlOkGD/vbPb66493Lo0xbDYCL/AHP9LhsY946tEp1ii8F7duKDZ33JQS41XHE\n\ti9JMjdhwQyLTBWIbTKWErMcpveCHRcmpR2tJrZmVEduKYF4Z/BwFo66m1rahudJtpkZs\n\tJS5P62+EbG9lSf2sqSC4ihBG+UJCyjJS5Gup/DUmRrMKWJ9BplD1A04lUi6JjHli2akD\n\tl1qeGxBRuwgJ1xMVl+gHfXrdBwYyK8tqLq3+sC3Ows+xfYwbBKWQeJuQEW/D21DQrhRT\n\tdMlQ==","X-Gm-Message-State":"AOAM531SIMyqUDQOaO29hH6/223bicTw+sTtYPTtR75a7lK4K4K8ATt0\n\toSmfc2jdg+vZXVmQbD0ubbEbyXZG+TCC05SaYHnnnPB4Tio=","X-Google-Smtp-Source":"ABdhPJw2aBXbk3VkJ7/N4xqlkpSlzAKPlQdOu9OD9C9NCkfpzlrIfpuOTPNOxSgzzYnNgB9Dya1hmG1d+kEjDNeITFk=","X-Received":"by 2002:a05:600c:1c21:: with SMTP id\n\tj33mr28905253wms.163.1634908721770; \n\tFri, 22 Oct 2021 06:18:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022115537.2964533-1-naush@raspberrypi.com>\n\t<20211022115537.2964533-6-naush@raspberrypi.com>","In-Reply-To":"<20211022115537.2964533-6-naush@raspberrypi.com>","From":"David Plowman ","Date":"Fri, 22 Oct 2021 14:18:31 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20391,"web_url":"https://patchwork.libcamera.org/comment/20391/","msgid":"","date":"2021-10-22T13:31:38","subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your feedback.\n\nOn Fri, 22 Oct 2021 at 14:18, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for this patch.\n>\n> On Fri, 22 Oct 2021 at 12:55, Naushir Patuck \n> wrote:\n> >\n> > The unicam driver no longer regesters an embedded data node if the\n> sensor does\n> > not provide this stream. Account for this in the pipeline handler match\n> routine\n> > by not assuming it is always present.\n> >\n> > Add a warning if Unicam and the CamHelper do not agree on the presense\n> of sensor\n> > embedded data, and disable its useage in these cases.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 16 +++++++++++++---\n> > 1 file changed, 13 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index fc190e39732a..5aaf24436f27 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -994,7 +994,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> > DeviceMatch unicam(\"unicam\");\n> > DeviceMatch isp(\"bcm2835-isp\");\n> >\n> > - unicam.add(\"unicam-embedded\");\n> > unicam.add(\"unicam-image\");\n> >\n> > isp.add(\"bcm2835-isp0-output0\"); /* Input */\n> > @@ -1015,9 +1014,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> > return false;\n> >\n> > /* Locate and open the unicam video streams. */\n> > - data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\",\n> unicam_->getEntityByName(\"unicam-embedded\"));\n> > data->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\",\n> unicam_->getEntityByName(\"unicam-image\"));\n> >\n> > + /* An embedded data node will not be present if the sensor does\n> not support it. */\n> > + MediaEntity *embeddedEntity =\n> unicam_->getEntityByName(\"unicam-embedded\");\n> > + if (embeddedEntity) {\n> > + data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam\n> Embedded\", embeddedEntity);\n> > +\n> data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n> > +\n> &RPiCameraData::unicamBufferDequeue);\n> > + }\n> > +\n> > /* Tag the ISP input stream as an import stream. */\n> > data->isp_[Isp::Input] = RPi::Stream(\"ISP Input\",\n> isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> > data->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> > @@ -1027,7 +1033,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> > /* Wire up all the buffer connections. */\n> >\n> data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(),\n> &RPiCameraData::frameStarted);\n> >\n> data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::unicamBufferDequeue);\n> > -\n> data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::unicamBufferDequeue);\n> > data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::ispInputDequeue);\n> > data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::ispOutputDequeue);\n> > data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::ispOutputDequeue);\n> > @@ -1055,6 +1060,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> > return false;\n> > }\n> >\n> > + if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {\n> > + LOG(RPI, Warning) << \"Mismatch between Unicam and\n> CamHelper for embedded data usage!\";\n> > + sensorConfig.sensorMetadata = false;\n> > + }\n> > +\n> > /*\n> > * Open all Unicam and ISP streams. The exception is the\n> embedded data\n> > * stream, which only gets opened below if the IPA reports that\n> the sensor\n> > --\n> > 2.25.1\n> >\n>\n> I was just wondering, is there any implication for our \"embedded data\n> present\" CamHelper functions? Can we now just tell, or do we still\n> need them?\n>\n\nIt does make CamHelper::SensorEmbeddedDataPresent() a bit redundant.\nI currently just use it to verify that both kernel and userland agree, but\nperhaps\nI should remove that member function entirely?\n\nRegards,\nNaush\n\n\n>\n> Other than this:\n>\n> Reviewed-by: David Plowman \n>\n> Thanks!\n>\n> David\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 51787BDB1C\n\tfor ;\n\tFri, 22 Oct 2021 13:31:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 108D868F59;\n\tFri, 22 Oct 2021 15:31:57 +0200 (CEST)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DDD256012A\n\tfor ;\n\tFri, 22 Oct 2021 15:31:54 +0200 (CEST)","by mail-lf1-x135.google.com with SMTP id p16so3378100lfa.2\n\tfor ;\n\tFri, 22 Oct 2021 06:31:54 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"aPTh9IUm\"; 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=TCAdQ0+VZb+tAPCn5t3kGzOevypu9GqPBaa4e1l65UA=;\n\tb=aPTh9IUmxkLRfGClkJ4xrbU7xHeeCQCNQ+S+l0lZ3wfZVquSFnT8jHJZPVCIa/OfF3\n\tb+yCqgoEzFhj/WrRYiHawuk5Q1s+sefTOu1/9yo5X3cuGLLkqpkchHnlzcB/YTRhQbRz\n\t8WGJOiJ3H2cNxRvopJg8iYnHG+tAaIy4OXFp1znCjHA2IzpbxKf9opXSQf0VNn+Xyz0X\n\tJfhNXQ9CdTO8DW+YVkYwIKbrwG2mhgWYa9gEIbHynx65a6Ql+mdLSN7b7sm9pvycIazb\n\tWiJS1iPKSXzi3j5incV1oy7wXuTCU39GgO+xaU8LMEVQgIZPzVQSB3iEQjSy31H4aFuv\n\tEqCg==","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=TCAdQ0+VZb+tAPCn5t3kGzOevypu9GqPBaa4e1l65UA=;\n\tb=P7AJIcXPZ0M5azRbelpYkI+kfG79CosOUkIA9D7ZnRDlRIvYclOww3De7V23Y9zgAN\n\t3iE0niIQaTZ4BCbTSyRUujRaH9OgmvydjCqRxU+1605L1G0e0sGbNB+bWNT68v3nnTCs\n\tjsqpPpXU3lHRVdY9SMBPGD2QRzugTjgFCAdGm5p9onsdRSeMNs37hpmO2UHKvdZSeho9\n\t3rmhLpA1/s1yASPMXFBlF5zst6JoWU8+hwqPrXefjWDiXg0Qq9lVz0Mr7BWc5Vvn3Fs6\n\ttU2Hwo78nJOIpWPS3ssoaQa9+jd7WefHBVHYtLSjhmg9Ut1Bec8mVACeU787sVjXfH9h\n\ttAkA==","X-Gm-Message-State":"AOAM532IRDgT1BTQgU3YA3HIMssffGJZorzQb1xpJQvlZHC4h319fonZ\n\t3gIg6zJH3nQIVix7MPsmgbQr8RWxO7+vp9Wfw4XZlA==","X-Google-Smtp-Source":"ABdhPJzL4VKOoIVgEsQIe8o58v8M5jlcw9JD+N5XIGoC8tHSk2nL0kSxGPBy/tjmpHZ3yjpschZ6G/U70Ln/yQhvD8s=","X-Received":"by 2002:a19:ad0d:: with SMTP id\n\tt13mr11620604lfc.161.1634909514316; \n\tFri, 22 Oct 2021 06:31:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022115537.2964533-1-naush@raspberrypi.com>\n\t<20211022115537.2964533-6-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 22 Oct 2021 14:31:38 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000d759e905cef108ee\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20393,"web_url":"https://patchwork.libcamera.org/comment/20393/","msgid":"","date":"2021-10-22T13:49:36","subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nOn Fri, 22 Oct 2021 at 14:31, Naushir Patuck wrote:\n>\n> Hi David,\n>\n> Thank you for your feedback.\n>\n> On Fri, 22 Oct 2021 at 14:18, David Plowman wrote:\n>>\n>> Hi Naush\n>>\n>> Thanks for this patch.\n>>\n>> On Fri, 22 Oct 2021 at 12:55, Naushir Patuck wrote:\n>> >\n>> > The unicam driver no longer regesters an embedded data node if the sensor does\n>> > not provide this stream. Account for this in the pipeline handler match routine\n>> > by not assuming it is always present.\n>> >\n>> > Add a warning if Unicam and the CamHelper do not agree on the presense of sensor\n>> > embedded data, and disable its useage in these cases.\n>> >\n>> > Signed-off-by: Naushir Patuck \n>> > ---\n>> > .../pipeline/raspberrypi/raspberrypi.cpp | 16 +++++++++++++---\n>> > 1 file changed, 13 insertions(+), 3 deletions(-)\n>> >\n>> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > index fc190e39732a..5aaf24436f27 100644\n>> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > @@ -994,7 +994,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>> > DeviceMatch unicam(\"unicam\");\n>> > DeviceMatch isp(\"bcm2835-isp\");\n>> >\n>> > - unicam.add(\"unicam-embedded\");\n>> > unicam.add(\"unicam-image\");\n>> >\n>> > isp.add(\"bcm2835-isp0-output0\"); /* Input */\n>> > @@ -1015,9 +1014,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>> > return false;\n>> >\n>> > /* Locate and open the unicam video streams. */\n>> > - data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n>> > data->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n>> >\n>> > + /* An embedded data node will not be present if the sensor does not support it. */\n>> > + MediaEntity *embeddedEntity = unicam_->getEntityByName(\"unicam-embedded\");\n>> > + if (embeddedEntity) {\n>> > + data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\", embeddedEntity);\n>> > + data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n>> > + &RPiCameraData::unicamBufferDequeue);\n>> > + }\n>> > +\n>> > /* Tag the ISP input stream as an import stream. */\n>> > data->isp_[Isp::Input] = RPi::Stream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n>> > data->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n>> > @@ -1027,7 +1033,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>> > /* Wire up all the buffer connections. */\n>> > data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(), &RPiCameraData::frameStarted);\n>> > data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);\n>> > - data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue);\n>> > data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispInputDequeue);\n>> > data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);\n>> > data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);\n>> > @@ -1055,6 +1060,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>> > return false;\n>> > }\n>> >\n>> > + if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {\n>> > + LOG(RPI, Warning) << \"Mismatch between Unicam and CamHelper for embedded data usage!\";\n>> > + sensorConfig.sensorMetadata = false;\n>> > + }\n>> > +\n>> > /*\n>> > * Open all Unicam and ISP streams. The exception is the embedded data\n>> > * stream, which only gets opened below if the IPA reports that the sensor\n>> > --\n>> > 2.25.1\n>> >\n>>\n>> I was just wondering, is there any implication for our \"embedded data\n>> present\" CamHelper functions? Can we now just tell, or do we still\n>> need them?\n>\n>\n> It does make CamHelper::SensorEmbeddedDataPresent() a bit redundant.\n> I currently just use it to verify that both kernel and userland agree, but perhaps\n> I should remove that member function entirely?\n\nPersonally I'd vote for removal, though I don't feel very strongly if\nanyone disagrees. The less \"boilerplate\" in there that you can get\nwrong, the better.\n\nBut we do just need to consider the CamHelperImx219 where the embedded\ndata is technically available but we choose to ignore it. Will that\nstill work?\n\nDavid\n\n>\n> Regards,\n> Naush\n>\n>>\n>>\n>> Other than this:\n>>\n>> Reviewed-by: David Plowman \n>>\n>> Thanks!\n>>\n>> David","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 8380EBDB1C\n\tfor ;\n\tFri, 22 Oct 2021 13:49:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB08568F5B;\n\tFri, 22 Oct 2021 15:49:48 +0200 (CEST)","from mail-wr1-x436.google.com (mail-wr1-x436.google.com\n\t[IPv6:2a00:1450:4864:20::436])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 864F56012A\n\tfor ;\n\tFri, 22 Oct 2021 15:49:47 +0200 (CEST)","by mail-wr1-x436.google.com with SMTP id e12so4124193wra.4\n\tfor ;\n\tFri, 22 Oct 2021 06:49:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"l7wWJ9Zb\"; 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=NiyGL16IsP58qfF3/X2UJs25/KkdzrOHCSls+a5qNU4=;\n\tb=l7wWJ9Zb9ETdKxjgcMtvGYWAH9zKnVf2auhP4A1XzFmqOepooCl9AtGb3r+R3fOG9H\n\tnzsNT9eBuF/ah3QYuCPbQEwbY5md4MnOQjINUQDiTXXbIV6imKxEPJnsP6HkA9Lmz+i3\n\tjFXpqdkMFMuiyknPdcGlBxUX/xesP/Tq861ZMFtTdAFbyxQxIsVJEtcr1myNU6aynD+6\n\t7NyHYiB/QbSZ0T2cHf4H8PI46YcR9KbgnI/9kJHG7M8KpCZwXGP0I/4rUpUN8mhJMkDM\n\tETkcWwOR+FUJWgVw/O8PxSwXsZSLfNajmGr6Cbp8rVgqmwbqPH6QgZHEF/kvA55Sr5no\n\ttYhA==","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=NiyGL16IsP58qfF3/X2UJs25/KkdzrOHCSls+a5qNU4=;\n\tb=PohjD00tlZlvKTKXll/h1XVpCmXd48VW2tn9uDVqZ+zntjYzlJIK68JNwixrKBrF4f\n\tpy8ftAWplXzekyB4zSAtQSN5shq+fj2RdLO3Tw1eDD76z5HW2ybtrIjaK7YSyaGbrMMe\n\tggw6laJ3Aor3VXLIYt/d1PWfBcXOFp9t5l8royvRIwa6rhmylWVGRa4qsV9c15AH55f4\n\t3HsK8++LAFJ9cXeqFdXPtwannQkJmLjc/4oBLbuv0k+fa7h+EmJRmipL4VKiFoXrnzbX\n\tlLMMLe1WsGK+YjWADL2Q2zmssgVlPWD3g6TuyQXEae4tAJTtkphAA6SKWIDKpo+3ctwM\n\tqCIQ==","X-Gm-Message-State":"AOAM533wKAwPXuXIfuN2nHlmc3KPaI6gZW/Niv5PANyBax7pSVex6Id8\n\tMCdOWQG0odTDoYNHLHy38XqkkcAEFV2c+EE75ltMwvVyV+k=","X-Google-Smtp-Source":"ABdhPJzG041qV77QfUxNbbRkRmhaJ6rSJJDZNx3bg3CoV422PBVpung3gjIuSAfwjBYAMXbFfx+ynvRSQm1vcfagMgE=","X-Received":"by 2002:adf:9b8e:: with SMTP id\n\td14mr16049376wrc.288.1634910587079; \n\tFri, 22 Oct 2021 06:49:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022115537.2964533-1-naush@raspberrypi.com>\n\t<20211022115537.2964533-6-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Fri, 22 Oct 2021 14:49:36 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20394,"web_url":"https://patchwork.libcamera.org/comment/20394/","msgid":"","date":"2021-10-22T14:05:02","subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Fri, 22 Oct 2021 at 14:49, David Plowman \nwrote:\n\n> Hi Naush\n>\n> On Fri, 22 Oct 2021 at 14:31, Naushir Patuck \n> wrote:\n> >\n> > Hi David,\n> >\n> > Thank you for your feedback.\n> >\n> > On Fri, 22 Oct 2021 at 14:18, David Plowman <\n> david.plowman@raspberrypi.com> wrote:\n> >>\n> >> Hi Naush\n> >>\n> >> Thanks for this patch.\n> >>\n> >> On Fri, 22 Oct 2021 at 12:55, Naushir Patuck \n> wrote:\n> >> >\n> >> > The unicam driver no longer regesters an embedded data node if the\n> sensor does\n> >> > not provide this stream. Account for this in the pipeline handler\n> match routine\n> >> > by not assuming it is always present.\n> >> >\n> >> > Add a warning if Unicam and the CamHelper do not agree on the\n> presense of sensor\n> >> > embedded data, and disable its useage in these cases.\n> >> >\n> >> > Signed-off-by: Naushir Patuck \n> >> > ---\n> >> > .../pipeline/raspberrypi/raspberrypi.cpp | 16\n> +++++++++++++---\n> >> > 1 file changed, 13 insertions(+), 3 deletions(-)\n> >> >\n> >> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > index fc190e39732a..5aaf24436f27 100644\n> >> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > @@ -994,7 +994,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> >> > DeviceMatch unicam(\"unicam\");\n> >> > DeviceMatch isp(\"bcm2835-isp\");\n> >> >\n> >> > - unicam.add(\"unicam-embedded\");\n> >> > unicam.add(\"unicam-image\");\n> >> >\n> >> > isp.add(\"bcm2835-isp0-output0\"); /* Input */\n> >> > @@ -1015,9 +1014,16 @@ bool\n> PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> >> > return false;\n> >> >\n> >> > /* Locate and open the unicam video streams. */\n> >> > - data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam\n> Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> >> > data->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\",\n> unicam_->getEntityByName(\"unicam-image\"));\n> >> >\n> >> > + /* An embedded data node will not be present if the sensor\n> does not support it. */\n> >> > + MediaEntity *embeddedEntity =\n> unicam_->getEntityByName(\"unicam-embedded\");\n> >> > + if (embeddedEntity) {\n> >> > + data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam\n> Embedded\", embeddedEntity);\n> >> > +\n> data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n> >> > +\n> &RPiCameraData::unicamBufferDequeue);\n> >> > + }\n> >> > +\n> >> > /* Tag the ISP input stream as an import stream. */\n> >> > data->isp_[Isp::Input] = RPi::Stream(\"ISP Input\",\n> isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> >> > data->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> >> > @@ -1027,7 +1033,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> >> > /* Wire up all the buffer connections. */\n> >> >\n> data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(),\n> &RPiCameraData::frameStarted);\n> >> >\n> data->unicam_[Unicam::Image].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::unicamBufferDequeue);\n> >> > -\n> data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::unicamBufferDequeue);\n> >> > data->isp_[Isp::Input].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::ispInputDequeue);\n> >> >\n> data->isp_[Isp::Output0].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::ispOutputDequeue);\n> >> >\n> data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(),\n> &RPiCameraData::ispOutputDequeue);\n> >> > @@ -1055,6 +1060,11 @@ bool\n> PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> >> > return false;\n> >> > }\n> >> >\n> >> > + if (sensorConfig.sensorMetadata ^ !!embeddedEntity) {\n> >> > + LOG(RPI, Warning) << \"Mismatch between Unicam and\n> CamHelper for embedded data usage!\";\n> >> > + sensorConfig.sensorMetadata = false;\n> >> > + }\n> >> > +\n> >> > /*\n> >> > * Open all Unicam and ISP streams. The exception is the\n> embedded data\n> >> > * stream, which only gets opened below if the IPA reports\n> that the sensor\n> >> > --\n> >> > 2.25.1\n> >> >\n> >>\n> >> I was just wondering, is there any implication for our \"embedded data\n> >> present\" CamHelper functions? Can we now just tell, or do we still\n> >> need them?\n> >\n> >\n> > It does make CamHelper::SensorEmbeddedDataPresent() a bit redundant.\n> > I currently just use it to verify that both kernel and userland agree,\n> but perhaps\n> > I should remove that member function entirely?\n>\n> Personally I'd vote for removal, though I don't feel very strongly if\n> anyone disagrees. The less \"boilerplate\" in there that you can get\n> wrong, the better.\n>\n> But we do just need to consider the CamHelperImx219 where the embedded\n> data is technically available but we choose to ignore it. Will that\n> still work?\n>\n\nGood point. With this change as-is, we will safely ignore the imx219\nembedded\ndata. If we were to remove CamHelper::SensorEmbeddedDataPresent(), the\nonly way to avoid using it would be to report correctly through the sensor\ndriver\nthat embedded data is missing.\n\nPerhaps we are not ready to remove it just yet...\n\nNaush\n\n\n>\n> David\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >>\n> >>\n> >> Other than this:\n> >>\n> >> Reviewed-by: David Plowman \n> >>\n> >> Thanks!\n> >>\n> >> David\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 A8EDBBF415\n\tfor ;\n\tFri, 22 Oct 2021 14:05:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C5CF68F59;\n\tFri, 22 Oct 2021 16:05:20 +0200 (CEST)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06DE86012A\n\tfor ;\n\tFri, 22 Oct 2021 16:05:19 +0200 (CEST)","by mail-lf1-x131.google.com with SMTP id j21so9548778lfe.0\n\tfor ;\n\tFri, 22 Oct 2021 07:05:18 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"f79ivuxP\"; 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=2QmBaV62F3APvuJWP6xqpWaUzEmFmG6pd7oF2zR4mGM=;\n\tb=f79ivuxPkCgT4SxRnqbLumaNDW1RHIB8IikaJiuns95qmafEZxRp/eZXbMnBoSWU+p\n\tScAUFhk5QpecrcEppXXDdNJoLvYjoREXDs0Kfc+cSOi8HbRIi4bQvjBPpQgvttDk3a0a\n\t/QPxp2LWSvpD9QWLS603/2uoLhlBgSucD6OkgEYafdQko1/fGRb/GG3ldWDcDpVvLqco\n\tZPl3Nx4R6iRE1m6501YSPUS63JNK6ioj8Y+SENx+JsWAzovOnTZePByhXHng+IU9wOXJ\n\t+Ts+am1az6V2rNfgGq+xNzzy3Ufp9Jwp6gGiX+qq3g68g/I2CJTsoihVki+Nfr67lJsk\n\taXdQ==","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=2QmBaV62F3APvuJWP6xqpWaUzEmFmG6pd7oF2zR4mGM=;\n\tb=KesaekaICBfNAk24qdBj+i4wZ4M1V4DuhTLAHjRJJ3H48ipY0yJ0EyEvN9ZaQKImw1\n\t6+KF0ffTHgng5AXZnJM0J/Sy+b1pFNFehw/6V2yKe5+Za29P5XUL0DB4gkDFc/ioIh6+\n\t5ISp4lNGrXD4H/U79FRvGLVGJq4tHHYy8sIFibJQ9kVPcJdb0qNGQHHh50lDt8y7hIRv\n\tE7XVVD64ZG5EqUqeHPz8032pnypuLOqWPOcsI3LQi/7QaDw3y+V59KUJLcHQOYZ/yzST\n\tAeV1WDAPndQQZz6vzEBswP0MAqXlPdyvgh5R0/bQsBx1ZvujQCB6ryQ1VPUNoGiDi3fJ\n\tyjDA==","X-Gm-Message-State":"AOAM532Y5fDj6Y2FSZLRWT6yXSLjwU6njyqgoCI9PowYZjz8M/JGmxvV\n\tfS6++5g0SUkVqRbrcvkyLphNnoDo/ljNvl+4XwGPrkOTI0pblwPd","X-Google-Smtp-Source":"ABdhPJwknGKUGAL4qBk5+6Na3d47FVr3PTDS7F3prcssPsh1O5k2RhIHea62hXITV1lpl2K8KFFLqUPnApBFojpoRHE=","X-Received":"by 2002:ac2:5dfc:: with SMTP id\n\tz28mr11838154lfq.79.1634911518196; \n\tFri, 22 Oct 2021 07:05:18 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022115537.2964533-1-naush@raspberrypi.com>\n\t<20211022115537.2964533-6-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 22 Oct 2021 15:05:02 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000481b6605cef180d4\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] pipeline: raspberrypi: Account\n\tfor a missing Unicam embedded data node","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]