[{"id":30938,"web_url":"https://patchwork.libcamera.org/comment/30938/","msgid":"","date":"2024-08-28T09:09:55","subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","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, Aug 08, 2024 at 11:23:40AM GMT, Naushir Patuck wrote:\n> Add a vendor control rpi::ScalerCrops that is analogous to the current\n> core::ScalerCrop, but can apply a different crop to each configured\n> stream.\n>\n> This control takes a span of Rectangle structures - the order of\n> rectangles must match the order of streams configured by the application.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/rpi/common/ipa_base.cpp | 2 ++\n> src/libcamera/control_ids_rpi.yaml | 15 +++++++++++++++\n> 2 files changed, 17 insertions(+)\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index ee3848b54f21..463f6d384c9e 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -71,6 +71,7 @@ const ControlInfoMap::Map ipaControls{\n> \t{ &controls::HdrMode, ControlInfo(controls::HdrModeValues) },\n> \t{ &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> \t{ &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> +\t{ &controls::rpi::ScalerCrops, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n\nCan this be registered only in the pisp IPA ? In this way you won't\nsee the control on vc4 platforms, and you can safely keep the code\nhandling it in pipeline_base, but it will only be exercized on pi5\n\n> \t{ &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },\n> \t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> \t{ &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },\n> @@ -1070,6 +1071,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> \t\t\tbreak;\n> \t\t}\n>\n> +\t\tcase controls::rpi::SCALER_CROPS:\n> \t\tcase controls::SCALER_CROP: {\n> \t\t\t/* We do nothing with this, but should avoid the warning below. */\n> \t\t\tbreak;\n> diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> index cb097f887e16..3d4a305dd8f5 100644\n> --- a/src/libcamera/control_ids_rpi.yaml\n> +++ b/src/libcamera/control_ids_rpi.yaml\n> @@ -26,4 +26,19 @@ controls:\n>\n> \\sa StatsOutputEnable\n>\n> + - ScalerCrops:\n> + type: Rectangle\n> + size: [n]\n> + description: |\n> + An array of rectangles, where each singular value has identical functionality\n> + to the ScalerCrop control. This control allows the Raspberry Pi pipeline\n> + handler to control individual scaler crops per output stream.\n> +\n> + The order of rectangles passed into the control must match the order of\n> + streams configured by the application.\n> +\n> + Note that using different crop rectangles for each output stream is only\n> + applicable on the Pi5/PiSP platform.\n> +\n> + \\sa ScalerCrop\n> ...\n> --\n> 2.34.1\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 E35FBC324C\n\tfor ;\n\tWed, 28 Aug 2024 09:10:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C22F763460;\n\tWed, 28 Aug 2024 11:10:00 +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 C55CE61903\n\tfor ;\n\tWed, 28 Aug 2024 11:09:59 +0200 (CEST)","from ideasonboard.com (mob-5-90-141-165.net.vodafone.it\n\t[5.90.141.165])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 187702C5;\n\tWed, 28 Aug 2024 11:08:52 +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=\"f62nkBDu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1724836132;\n\tbh=2DBQxXfOYgdMARXI8D3UCNs1y0U85UiSdbv+ExV7u24=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=f62nkBDuoMGAOOCPp63E70aCWIY/FHggWs6Yfs0U8bOauQSGVIAuYm2AAP/TVmXY9\n\tYt0muBWUpgwLSuVZK6+JzCzZY6CTwMZeigHuMSoZncicEXnPKX/HZtBIMKprvN5v6n\n\tBGSRciso/jipBB33TB6egbVStNKfl/NwERThXOVo=","Date":"Wed, 28 Aug 2024 11:09:55 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","Message-ID":"","References":"<20240808102346.13065-1-naush@raspberrypi.com>\n\t<20240808102346.13065-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240808102346.13065-2-naush@raspberrypi.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":30941,"web_url":"https://patchwork.libcamera.org/comment/30941/","msgid":"","date":"2024-08-28T09:58:55","subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for the feedback.\n\n\nOn Wed, 28 Aug 2024 at 10:10, Jacopo Mondi \nwrote:\n\n> Hi Naush\n>\n> On Thu, Aug 08, 2024 at 11:23:40AM GMT, Naushir Patuck wrote:\n> > Add a vendor control rpi::ScalerCrops that is analogous to the current\n> > core::ScalerCrop, but can apply a different crop to each configured\n> > stream.\n> >\n> > This control takes a span of Rectangle structures - the order of\n> > rectangles must match the order of streams configured by the application.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/rpi/common/ipa_base.cpp | 2 ++\n> > src/libcamera/control_ids_rpi.yaml | 15 +++++++++++++++\n> > 2 files changed, 17 insertions(+)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp\n> b/src/ipa/rpi/common/ipa_base.cpp\n> > index ee3848b54f21..463f6d384c9e 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -71,6 +71,7 @@ const ControlInfoMap::Map ipaControls{\n> > { &controls::HdrMode, ControlInfo(controls::HdrModeValues) },\n> > { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535,\n> 65535, 65535, 65535), Rectangle{}) },\n> > + { &controls::rpi::ScalerCrops, ControlInfo(Rectangle{},\n> Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n>\n> Can this be registered only in the pisp IPA ? In this way you won't\n> see the control on vc4 platforms, and you can safely keep the code\n> handling it in pipeline_base, but it will only be exercized on pi5\n>\n\nYes this is possible. I can rework it for v2.\n\nRegards,\nNaush\n\n\n>\n> > { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333),\n> INT64_C(120000)) },\n> > { &controls::draft::NoiseReductionMode,\n> ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > { &controls::rpi::StatsOutputEnable, ControlInfo(false, true,\n> false) },\n> > @@ -1070,6 +1071,7 @@ void IpaBase::applyControls(const ControlList\n> &controls)\n> > break;\n> > }\n> >\n> > + case controls::rpi::SCALER_CROPS:\n> > case controls::SCALER_CROP: {\n> > /* We do nothing with this, but should avoid the\n> warning below. */\n> > break;\n> > diff --git a/src/libcamera/control_ids_rpi.yaml\n> b/src/libcamera/control_ids_rpi.yaml\n> > index cb097f887e16..3d4a305dd8f5 100644\n> > --- a/src/libcamera/control_ids_rpi.yaml\n> > +++ b/src/libcamera/control_ids_rpi.yaml\n> > @@ -26,4 +26,19 @@ controls:\n> >\n> > \\sa StatsOutputEnable\n> >\n> > + - ScalerCrops:\n> > + type: Rectangle\n> > + size: [n]\n> > + description: |\n> > + An array of rectangles, where each singular value has identical\n> functionality\n> > + to the ScalerCrop control. This control allows the Raspberry Pi\n> pipeline\n> > + handler to control individual scaler crops per output stream.\n> > +\n> > + The order of rectangles passed into the control must match the\n> order of\n> > + streams configured by the application.\n> > +\n> > + Note that using different crop rectangles for each output\n> stream is only\n> > + applicable on the Pi5/PiSP platform.\n> > +\n> > + \\sa ScalerCrop\n> > ...\n> > --\n> > 2.34.1\n> >\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 6E515C323E\n\tfor ;\n\tWed, 28 Aug 2024 09:59:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D100263460;\n\tWed, 28 Aug 2024 11:59:24 +0200 (CEST)","from mail-yw1-x1131.google.com (mail-yw1-x1131.google.com\n\t[IPv6:2607:f8b0:4864:20::1131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CEAE861903\n\tfor ;\n\tWed, 28 Aug 2024 11:59:22 +0200 (CEST)","by mail-yw1-x1131.google.com with SMTP id\n\t00721157ae682-6bcde7538a2so5664507b3.3\n\tfor ;\n\tWed, 28 Aug 2024 02:59:22 -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=\"fPr0nD1J\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1724839161; x=1725443961;\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=bNLa4SiuLruee5lbR0Lu23gmqAn+NZ0ClHGkjrERecQ=;\n\tb=fPr0nD1JrhY0FZeeiXKDbrHYSLQkTEGJHe4k21H98woeLYWQQ17R6bgZDG8NawjlSO\n\t8BNANSXrrHfRfxsdyZ715W2Hx2frrWt4FNuWb9cs+aT8H3LArFcdgnOEaFE2vPPNcOtq\n\t4Q3mMZeGBaK4dIcGBX2YLrpE2YhSY7apkh8rj137I+pWbTvKgZYRlolKLKm0KTB2iE/G\n\tTk2jDewoFQrjuK1zXIZuvF2T2UOKkeda4EhZb2WhIyKC2liUv+4RgTmKVdlWiH6BlSn0\n\tI5m+csqy2ILfIF3jh8KEN5KdsWkrHmR64j+AQKdjTMPPanJODg8KQCYuiLRt4ctpVuDD\n\tCVyg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1724839161; x=1725443961;\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=bNLa4SiuLruee5lbR0Lu23gmqAn+NZ0ClHGkjrERecQ=;\n\tb=j7RGIqPmaxDzgDmGDfx5ZxU9egSA5YhdH0kyEMQy09ED07mpn6U96Ic6ldFFZW7wHI\n\tSixk3T4srSKOl4ZDiTPlaC9TDzhJwZktkiJfVLK+CPokGLiPjaz8rD1S4aOZI1TRUspW\n\tJ94SkaswtxH6wGniImCH4P22cdDc5e7NkGNGt/J7CU1Dd3oaGz5GFUeAyGra9H1YDri5\n\tqSsa5yaUjhf/pUeq0zS5JfdCQvVwL02vLNLbASZiSddnH/JUkq8E5Z9qnYVw6aB9bx8e\n\tdOXWxjbNOsl16bcpRBQkAUbaarIwQKgGeQfWC94PMVcK9wh0i8DLQz12LbzkzxHjKTMw\n\tHiww==","X-Gm-Message-State":"AOJu0Yz9KKsIotIG+IsGPmghXwvJABeyJLEJRt3IhN4OaOzQcQrKt5Iy\n\tbgstZgqlSBBM4FXg1MAoRsqt7Y5z/viyXivZWxggxc7LmDb/PoRTMznUJCqbisTEkhkFLhnChai\n\tVq8GKcvmXmBJMaNZrKzQzL0kAUAGNzVpIhz8+VGRLQF7ATXzh","X-Google-Smtp-Source":"AGHT+IFKg+I4ad7z2Bza7vUp1H7uR9a/gkVLK6whH7AcmPPJnpmIywDG+DBua335/MiJ4Nh5tcGVDgclPt0MmEeOniQ=","X-Received":"by 2002:a05:690c:102:b0:695:e4b0:10ce with SMTP id\n\t00721157ae682-6c629dda7bfmr74059417b3.7.1724839161490;\n\tWed, 28 Aug 2024 02:59:21 -0700 (PDT)","MIME-Version":"1.0","References":"<20240808102346.13065-1-naush@raspberrypi.com>\n\t<20240808102346.13065-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 28 Aug 2024 10:58:55 +0100","Message-ID":"","Subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"000000000000840a360620bb6993\"","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":30949,"web_url":"https://patchwork.libcamera.org/comment/30949/","msgid":"<20240828161733.GF27131@pendragon.ideasonboard.com>","date":"2024-08-28T16:17:33","subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Wed, Aug 28, 2024 at 10:58:55AM +0100, Naushir Patuck wrote:\n> On Wed, 28 Aug 2024 at 10:10, Jacopo Mondi wrote:\n> > On Thu, Aug 08, 2024 at 11:23:40AM GMT, Naushir Patuck wrote:\n> > > Add a vendor control rpi::ScalerCrops that is analogous to the current\n> > > core::ScalerCrop, but can apply a different crop to each configured\n> > > stream.\n> > >\n> > > This control takes a span of Rectangle structures - the order of\n> > > rectangles must match the order of streams configured by the application.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/ipa/rpi/common/ipa_base.cpp | 2 ++\n> > > src/libcamera/control_ids_rpi.yaml | 15 +++++++++++++++\n> > > 2 files changed, 17 insertions(+)\n> > >\n> > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > index ee3848b54f21..463f6d384c9e 100644\n> > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > @@ -71,6 +71,7 @@ const ControlInfoMap::Map ipaControls{\n> > > { &controls::HdrMode, ControlInfo(controls::HdrModeValues) },\n> > > { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> > > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > + { &controls::rpi::ScalerCrops, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> >\n> > Can this be registered only in the pisp IPA ? In this way you won't\n> > see the control on vc4 platforms, and you can safely keep the code\n> > handling it in pipeline_base, but it will only be exercized on pi5\n> \n> Yes this is possible. I can rework it for v2.\n\nI would prefer that too.\n\n> > > { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },\n> > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },\n> > > @@ -1070,6 +1071,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > break;\n> > > }\n> > >\n> > > + case controls::rpi::SCALER_CROPS:\n> > > case controls::SCALER_CROP: {\n> > > /* We do nothing with this, but should avoid the warning below. */\n> > > break;\n> > > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> > > index cb097f887e16..3d4a305dd8f5 100644\n> > > --- a/src/libcamera/control_ids_rpi.yaml\n> > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > @@ -26,4 +26,19 @@ controls:\n> > >\n> > > \\sa StatsOutputEnable\n> > >\n> > > + - ScalerCrops:\n> > > + type: Rectangle\n> > > + size: [n]\n> > > + description: |\n> > > + An array of rectangles, where each singular value has identical functionality\n> > > + to the ScalerCrop control. This control allows the Raspberry Pi pipeline\n> > > + handler to control individual scaler crops per output stream.\n> > > +\n> > > + The order of rectangles passed into the control must match the order of\n> > > + streams configured by the application.\n\nHow about the number of rectangles ? Can you specify less rectangles\nthan the number of streams ? Or more ? What happens if you do, is the\ncontrol completely ignored, or do part of the rectangles get applied ?\n\nIs there also a need to allow applications to change the scaler crop for\na subset of the streams, without affecting other ones ?\n\nSomething that isn't explained in this, and barely mentioned in the\ncover letter (it just says \"preliminary\" without elaborating) is that\nthis vendor control is a work around for the lack of a per-stream\ncontrols API. Do we agree that this control will be replaced by\nper-stream controls in the future, and that it will then be dropped and\nnot kept for backward compatibility ? If so, could you please record\nthis here, with a \\todo note ?\n\nCan I also volunteer you for a per-stream control RFC ? :-)\n\n> > > +\n> > > + Note that using different crop rectangles for each output stream is only\n> > > + applicable on the Pi5/PiSP platform.\n> > > +\n> > > + \\sa ScalerCrop\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 90345C324C\n\tfor ;\n\tWed, 28 Aug 2024 16:17:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82DFA633CD;\n\tWed, 28 Aug 2024 18:17:40 +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 6BC9B61903\n\tfor ;\n\tWed, 28 Aug 2024 18:17:38 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BDFCD220;\n\tWed, 28 Aug 2024 18:16:29 +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=\"rqL5G8Z0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1724861790;\n\tbh=48i46qrI6MZdUSJ0r77l0XDHQ8ilOiBkRuwodlQ7i9E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rqL5G8Z0yDZE8oJwnjEylTWlxLQ4HtHHC2sMXcJ5eyi2OLZx92MW0H+Tu3Yet3cV1\n\te//um+90mCJm8blh0KBxGfRW7/o07gD+4SuTSLtdD8yL5s9jfWm1mWjnHDHBbYFrrQ\n\tc7/XUo1AGUfmUeU0ZVBDngjctg7DtbH26/lIOfAw=","Date":"Wed, 28 Aug 2024 19:17:33 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","Message-ID":"<20240828161733.GF27131@pendragon.ideasonboard.com>","References":"<20240808102346.13065-1-naush@raspberrypi.com>\n\t<20240808102346.13065-2-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":30986,"web_url":"https://patchwork.libcamera.org/comment/30986/","msgid":"","date":"2024-08-30T10:05:28","subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Wed, 28 Aug 2024 at 17:17, Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Wed, Aug 28, 2024 at 10:58:55AM +0100, Naushir Patuck wrote:\n> > On Wed, 28 Aug 2024 at 10:10, Jacopo Mondi wrote:\n> > > On Thu, Aug 08, 2024 at 11:23:40AM GMT, Naushir Patuck wrote:\n> > > > Add a vendor control rpi::ScalerCrops that is analogous to the current\n> > > > core::ScalerCrop, but can apply a different crop to each configured\n> > > > stream.\n> > > >\n> > > > This control takes a span of Rectangle structures - the order of\n> > > > rectangles must match the order of streams configured by the application.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > src/ipa/rpi/common/ipa_base.cpp | 2 ++\n> > > > src/libcamera/control_ids_rpi.yaml | 15 +++++++++++++++\n> > > > 2 files changed, 17 insertions(+)\n> > > >\n> > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > > index ee3848b54f21..463f6d384c9e 100644\n> > > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > > @@ -71,6 +71,7 @@ const ControlInfoMap::Map ipaControls{\n> > > > { &controls::HdrMode, ControlInfo(controls::HdrModeValues) },\n> > > > { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> > > > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > + { &controls::rpi::ScalerCrops, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > >\n> > > Can this be registered only in the pisp IPA ? In this way you won't\n> > > see the control on vc4 platforms, and you can safely keep the code\n> > > handling it in pipeline_base, but it will only be exercized on pi5\n> >\n> > Yes this is possible. I can rework it for v2.\n>\n> I would prefer that too.\n\nAck\n\n>\n> > > > { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },\n> > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },\n> > > > @@ -1070,6 +1071,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > > break;\n> > > > }\n> > > >\n> > > > + case controls::rpi::SCALER_CROPS:\n> > > > case controls::SCALER_CROP: {\n> > > > /* We do nothing with this, but should avoid the warning below. */\n> > > > break;\n> > > > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> > > > index cb097f887e16..3d4a305dd8f5 100644\n> > > > --- a/src/libcamera/control_ids_rpi.yaml\n> > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > @@ -26,4 +26,19 @@ controls:\n> > > >\n> > > > \\sa StatsOutputEnable\n> > > >\n> > > > + - ScalerCrops:\n> > > > + type: Rectangle\n> > > > + size: [n]\n> > > > + description: |\n> > > > + An array of rectangles, where each singular value has identical functionality\n> > > > + to the ScalerCrop control. This control allows the Raspberry Pi pipeline\n> > > > + handler to control individual scaler crops per output stream.\n> > > > +\n> > > > + The order of rectangles passed into the control must match the order of\n> > > > + streams configured by the application.\n>\n> How about the number of rectangles ? Can you specify less rectangles\n> than the number of streams ? Or more ? What happens if you do, is the\n> control completely ignored, or do part of the rectangles get applied ?\n>\n> Is there also a need to allow applications to change the scaler crop for\n> a subset of the streams, without affecting other ones ?\n\nSadly this mechanism is not flexible at all. So the application must\nspecify the correct number of scalar crop rectangles based on the\nnumber of configured streams unconditionally. A new control API will\ndefinitely need such flexibility.\n\n\n>\n> Something that isn't explained in this, and barely mentioned in the\n> cover letter (it just says \"preliminary\" without elaborating) is that\n> this vendor control is a work around for the lack of a per-stream\n> controls API. Do we agree that this control will be replaced by\n> per-stream controls in the future, and that it will then be dropped and\n> not kept for backward compatibility ? If so, could you please record\n> this here, with a \\todo note ?\n>\n> Can I also volunteer you for a per-stream control RFC ? :-)\n\nThis change is really a stop-gap until we have an official API for\nper-stream controls. I'll add a todo to note this. I will also make\na RFC proposal for the changes and perhaps we can spend some time\ntalking about it in the Vienna F2F.\n\nRegards,\nNaush\n\n>\n> > > > +\n> > > > + Note that using different crop rectangles for each output stream is only\n> > > > + applicable on the Pi5/PiSP platform.\n> > > > +\n> > > > + \\sa ScalerCrop\n> > > > ...\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 8DD09C323E\n\tfor ;\n\tFri, 30 Aug 2024 10:05:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 796A463469;\n\tFri, 30 Aug 2024 12:05:58 +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 2817F6341E\n\tfor ;\n\tFri, 30 Aug 2024 12:05:56 +0200 (CEST)","by mail-yw1-x1136.google.com with SMTP id\n\t00721157ae682-6b565f00111so1731347b3.2\n\tfor ;\n\tFri, 30 Aug 2024 03:05:56 -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=\"AtVjbAX6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1725012355; x=1725617155;\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=FWypzjiOyx4mODm2SWyFQ/8BCErGankapoPtbirDJec=;\n\tb=AtVjbAX6Q1O5sVe25RufZyKbztkXEyAdzShG/bpVhJUxzyFnQhSqbTDbMriIflB2Xu\n\tIO/myKucSVkFxmDBzxvHXA+WUUQD4AHv0tsA1rLbhdt/2EozmoxT76qEHBYBR5/Bc9EQ\n\tyh0muFZSibs5dKWaP74ukOFv6+1xQ9a8uBGe3MIXCcipDT5aI+SHaHhAH1hE7j6ntOxE\n\tle0z9np8kaGFP/jiO0uan9buvZgcbaT/2IvqDpQxHFEdZ6BkPA30VgB5Brqm6G383odj\n\tG+isrt7FOsLNv6wwJPG1iKFLUN5lIVCkq6QgV+5d6QYn2nwX1g1Nw1GbDyojSHbz9tgf\n\t+x4Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725012355; x=1725617155;\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=FWypzjiOyx4mODm2SWyFQ/8BCErGankapoPtbirDJec=;\n\tb=xD90wsYxXy6ErPla8F7yjxEaf/xUYjJDgUZq8gyc3kVC82XrFgYikkruwwS8HKxEV5\n\t8DlfuHj/bSfl5XCpwQFAbIcOsyNKT75Ou6FMDktTpgbqyRvKu7G26BWjRvfSrBCteNgp\n\tMz8uIKAQdg4wtMFLrpuBF9SFkdPPazWhJ+3Tnr/F9DP6uB2qpDWQsy97VKcuxfUhpBmd\n\tyUdggj8ItPnQ5mL/d4biD+qWBTJrpSta5VOy6b9ZD7RayxbKiQ96A67cHBnz+l7YO6sW\n\t7MMY9aNfJW0OXXR/aaR7+JNgzi1igsA3rQtuFDFTFjyY6UcHtyhVisIqAd6eg1VjaekE\n\t31lg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVKJ5WGtlotHgy4ih1tCSUJcVzrGF0y8ppI9Kn/EMLVEOs3BOmL+RfW/q632xB+4mnpEgrqZdU2EK9HYZ9i8WU=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxAMeWP8LfsJu3f9ZGHBHqy+FpsVzcL3CVnOKexsrXm6cFebFaQ\n\tOToXc+JpulcHcfGLgfmtyZZJXB+5cqlSMVcMr5DAGhc5//82V2Lng6r+rNNA4EafPot7VG88vua\n\tCXmVGXajjtI0LhMencxY+vuhcrozA2ha6lQDLFb+aHM1/f6Q+","X-Google-Smtp-Source":"AGHT+IGhMbd6WEy6JaviyzJO5wrToA8GNB06uesn6C9M+P2IZH/RRBeuLqGUH/owoKM6msvokPV2b5rp9k1p0CX4u+Q=","X-Received":"by 2002:a05:690c:83:b0:6b2:7494:455a with SMTP id\n\t00721157ae682-6d40d9869d0mr8635497b3.2.1725012354911; Fri, 30 Aug 2024\n\t03:05:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20240808102346.13065-1-naush@raspberrypi.com>\n\t<20240808102346.13065-2-naush@raspberrypi.com>\n\t\n\t\n\t<20240828161733.GF27131@pendragon.ideasonboard.com>","In-Reply-To":"<20240828161733.GF27131@pendragon.ideasonboard.com>","From":"Naushir Patuck ","Date":"Fri, 30 Aug 2024 11:05:28 +0100","Message-ID":"","Subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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":30988,"web_url":"https://patchwork.libcamera.org/comment/30988/","msgid":"<20240830102454.GH25163@pendragon.ideasonboard.com>","date":"2024-08-30T10:24:54","subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Aug 30, 2024 at 11:05:28AM +0100, Naushir Patuck wrote:\n> On Wed, 28 Aug 2024 at 17:17, Laurent Pinchart wrote:\n> > On Wed, Aug 28, 2024 at 10:58:55AM +0100, Naushir Patuck wrote:\n> > > On Wed, 28 Aug 2024 at 10:10, Jacopo Mondi wrote:\n> > > > On Thu, Aug 08, 2024 at 11:23:40AM GMT, Naushir Patuck wrote:\n> > > > > Add a vendor control rpi::ScalerCrops that is analogous to the current\n> > > > > core::ScalerCrop, but can apply a different crop to each configured\n> > > > > stream.\n> > > > >\n> > > > > This control takes a span of Rectangle structures - the order of\n> > > > > rectangles must match the order of streams configured by the application.\n> > > > >\n> > > > > Signed-off-by: Naushir Patuck \n> > > > > ---\n> > > > > src/ipa/rpi/common/ipa_base.cpp | 2 ++\n> > > > > src/libcamera/control_ids_rpi.yaml | 15 +++++++++++++++\n> > > > > 2 files changed, 17 insertions(+)\n> > > > >\n> > > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > > > index ee3848b54f21..463f6d384c9e 100644\n> > > > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > > > @@ -71,6 +71,7 @@ const ControlInfoMap::Map ipaControls{\n> > > > > { &controls::HdrMode, ControlInfo(controls::HdrModeValues) },\n> > > > > { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> > > > > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > > + { &controls::rpi::ScalerCrops, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > >\n> > > > Can this be registered only in the pisp IPA ? In this way you won't\n> > > > see the control on vc4 platforms, and you can safely keep the code\n> > > > handling it in pipeline_base, but it will only be exercized on pi5\n> > >\n> > > Yes this is possible. I can rework it for v2.\n> >\n> > I would prefer that too.\n> \n> Ack\n> \n> > > > > { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },\n> > > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > > { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },\n> > > > > @@ -1070,6 +1071,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > > > break;\n> > > > > }\n> > > > >\n> > > > > + case controls::rpi::SCALER_CROPS:\n> > > > > case controls::SCALER_CROP: {\n> > > > > /* We do nothing with this, but should avoid the warning below. */\n> > > > > break;\n> > > > > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> > > > > index cb097f887e16..3d4a305dd8f5 100644\n> > > > > --- a/src/libcamera/control_ids_rpi.yaml\n> > > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > > @@ -26,4 +26,19 @@ controls:\n> > > > >\n> > > > > \\sa StatsOutputEnable\n> > > > >\n> > > > > + - ScalerCrops:\n> > > > > + type: Rectangle\n> > > > > + size: [n]\n> > > > > + description: |\n> > > > > + An array of rectangles, where each singular value has identical functionality\n> > > > > + to the ScalerCrop control. This control allows the Raspberry Pi pipeline\n> > > > > + handler to control individual scaler crops per output stream.\n> > > > > +\n> > > > > + The order of rectangles passed into the control must match the order of\n> > > > > + streams configured by the application.\n> >\n> > How about the number of rectangles ? Can you specify less rectangles\n> > than the number of streams ? Or more ? What happens if you do, is the\n> > control completely ignored, or do part of the rectangles get applied ?\n> >\n> > Is there also a need to allow applications to change the scaler crop for\n> > a subset of the streams, without affecting other ones ?\n> \n> Sadly this mechanism is not flexible at all. So the application must\n> specify the correct number of scalar crop rectangles based on the\n> number of configured streams unconditionally. A new control API will\n> definitely need such flexibility.\n\nFine with me. Could you please document the requirement here ?\n\n> > Something that isn't explained in this, and barely mentioned in the\n> > cover letter (it just says \"preliminary\" without elaborating) is that\n> > this vendor control is a work around for the lack of a per-stream\n> > controls API. Do we agree that this control will be replaced by\n> > per-stream controls in the future, and that it will then be dropped and\n> > not kept for backward compatibility ? If so, could you please record\n> > this here, with a \\todo note ?\n> >\n> > Can I also volunteer you for a per-stream control RFC ? :-)\n> \n> This change is really a stop-gap until we have an official API for\n> per-stream controls. I'll add a todo to note this. I will also make\n> a RFC proposal for the changes and perhaps we can spend some time\n> talking about it in the Vienna F2F.\n\nGreat, thank you !\n\n> > > > > +\n> > > > > + Note that using different crop rectangles for each output stream is only\n> > > > > + applicable on the Pi5/PiSP platform.\n> > > > > +\n> > > > > + \\sa ScalerCrop\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 25B88C323E\n\tfor ;\n\tFri, 30 Aug 2024 10:25:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8C9E63469;\n\tFri, 30 Aug 2024 12:25:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 511C96341E\n\tfor ;\n\tFri, 30 Aug 2024 12:25:25 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FE94227;\n\tFri, 30 Aug 2024 12:24:16 +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=\"F8ooYeN0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725013456;\n\tbh=eGsNIXkDj460AN5A7y++GFY8FHhhq3LYk35Nl8QnIck=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=F8ooYeN0MjOI4GE+1b4LMe20a7bPdQZGypqkl++EoIGvk548lHAB7W6GUgNMBC36w\n\t7tUzzGzUxHNBh9/MGTTunhvmA6BbZHskSTTshcHSx9bjPnzKpAMpCgh+6XYqMsiP4f\n\tYQcc31sEL+lUPZ8usYmyPRKZi8lNAkydTzjdlE2M=","Date":"Fri, 30 Aug 2024 13:24:54 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/7] controls: rpi: Add a vendor rpi::ScalerCrops\n\tcontrol","Message-ID":"<20240830102454.GH25163@pendragon.ideasonboard.com>","References":"<20240808102346.13065-1-naush@raspberrypi.com>\n\t<20240808102346.13065-2-naush@raspberrypi.com>\n\t\n\t\n\t<20240828161733.GF27131@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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\" "}}]