[{"id":33301,"web_url":"https://patchwork.libcamera.org/comment/33301/","msgid":"<173884161508.1773152.10432086726470923220@ping.linuxembedded.co.uk>","date":"2025-02-06T11:33:35","subject":"Re: [PATCH v2] ipa: rpi: Apply default ControlInfo values for sensor\n\tcontrols","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2025-02-06 09:24:36)\n> The existing IPA initialisation code did not set default values for\n> some sensor related controls. This caused a crash using libcamerify\n> when the it was trying to access the default value for\n> controls::FrameDurationLimits as part of a recent change.\n> \n> Ensure controls::FrameDurationLimits, controls::AnalogueGain and\n> controls::ExposureTime advertise default values along with the existing\n> min/max values. The default is set to the defaults defined in the IPA\n> set during initialisation.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=253\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/rpi/common/ipa_base.cpp | 9 ++++++---\n> 1 file changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index bd3c22000df5..3c77665e6ab4 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -259,15 +259,18 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa\n> ControlInfoMap::Map ctrlMap = ipaControls;\n> ctrlMap[&controls::FrameDurationLimits] =\n> ControlInfo(static_cast(mode_.minFrameDuration.get()),\n> - static_cast(mode_.maxFrameDuration.get()));\n> + static_cast(mode_.maxFrameDuration.get()),\n> + static_cast(defaultMinFrameDuration.get()));\n> \n\nIs there any risk or concern now that the default could be reported\noutside of the min/max values when configured?\n\nBut reading the if (firstStart_) {} code block, I think these are the\nvalues that will be used on start so I expect they are at least the\ncurrent representation of the defaults.\n\n\nReviewed-by: Kieran Bingham \n\n> ctrlMap[&controls::AnalogueGain] =\n> ControlInfo(static_cast(mode_.minAnalogueGain),\n> - static_cast(mode_.maxAnalogueGain));\n> + static_cast(mode_.maxAnalogueGain),\n> + static_cast(defaultAnalogueGain));\n> \n> ctrlMap[&controls::ExposureTime] =\n> ControlInfo(static_cast(mode_.minExposureTime.get()),\n> - static_cast(mode_.maxExposureTime.get()));\n> + static_cast(mode_.maxExposureTime.get()),\n> + static_cast(defaultExposureTime.get()));\n> \n> /* Declare colour processing related controls for non-mono sensors. */\n> if (!monoSensor_)\n> -- \n> 2.43.0\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 A3697C32EA\n\tfor ;\n\tThu, 6 Feb 2025 11:33:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5FBB685D5;\n\tThu, 6 Feb 2025 12:33:39 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D2056053F\n\tfor ;\n\tThu, 6 Feb 2025 12:33:38 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AFB022BA;\n\tThu, 6 Feb 2025 12:32:24 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ScmZv1ka\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738841544;\n\tbh=zDrG2OiRDGSOnt5Wp6mXWXnD+tGET7w60SNH7uxdVJk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ScmZv1kazed0MlHTs0MJ4+UyfaYITY8k4OmjK/2FEsJOhVvHrDtOo2OYtWl2Z9omE\n\tW601r1XK/5y3syR4DNdp0bxHJAfTwQOQ9655ydCsae356CVE/VFSQXpgGcCCpZeq2t\n\tQomtbkfSCFGwuK2nRqIyd+6ufcUk+/Et8enYTQhA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250206092446.36237-1-naush@raspberrypi.com>","References":"<20250206092446.36237-1-naush@raspberrypi.com>","Subject":"Re: [PATCH v2] ipa: rpi: Apply default ControlInfo values for sensor\n\tcontrols","From":"Kieran Bingham ","Cc":"Naushir Patuck ","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 06 Feb 2025 11:33:35 +0000","Message-ID":"<173884161508.1773152.10432086726470923220@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":33305,"web_url":"https://patchwork.libcamera.org/comment/33305/","msgid":"","date":"2025-02-06T12:55:55","subject":"Re: [PATCH v2] ipa: rpi: Apply default ControlInfo values for sensor\n\tcontrols","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 the revised version.\n\nOn Thu, 6 Feb 2025 at 11:33, Kieran Bingham\n wrote:\n>\n> Quoting Naushir Patuck (2025-02-06 09:24:36)\n> > The existing IPA initialisation code did not set default values for\n> > some sensor related controls. This caused a crash using libcamerify\n> > when the it was trying to access the default value for\n> > controls::FrameDurationLimits as part of a recent change.\n> >\n> > Ensure controls::FrameDurationLimits, controls::AnalogueGain and\n> > controls::ExposureTime advertise default values along with the existing\n> > min/max values. The default is set to the defaults defined in the IPA\n> > set during initialisation.\n> >\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=253\n> > Signed-off-by: Naushir Patuck \n\nReviewed-by: David Plowman \n\nDavid\n\n> > ---\n> > src/ipa/rpi/common/ipa_base.cpp | 9 ++++++---\n> > 1 file changed, 6 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index bd3c22000df5..3c77665e6ab4 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -259,15 +259,18 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa\n> > ControlInfoMap::Map ctrlMap = ipaControls;\n> > ctrlMap[&controls::FrameDurationLimits] =\n> > ControlInfo(static_cast(mode_.minFrameDuration.get()),\n> > - static_cast(mode_.maxFrameDuration.get()));\n> > + static_cast(mode_.maxFrameDuration.get()),\n> > + static_cast(defaultMinFrameDuration.get()));\n> >\n>\n> Is there any risk or concern now that the default could be reported\n> outside of the min/max values when configured?\n>\n> But reading the if (firstStart_) {} code block, I think these are the\n> values that will be used on start so I expect they are at least the\n> current representation of the defaults.\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n> > ctrlMap[&controls::AnalogueGain] =\n> > ControlInfo(static_cast(mode_.minAnalogueGain),\n> > - static_cast(mode_.maxAnalogueGain));\n> > + static_cast(mode_.maxAnalogueGain),\n> > + static_cast(defaultAnalogueGain));\n> >\n> > ctrlMap[&controls::ExposureTime] =\n> > ControlInfo(static_cast(mode_.minExposureTime.get()),\n> > - static_cast(mode_.maxExposureTime.get()));\n> > + static_cast(mode_.maxExposureTime.get()),\n> > + static_cast(defaultExposureTime.get()));\n> >\n> > /* Declare colour processing related controls for non-mono sensors. */\n> > if (!monoSensor_)\n> > --\n> > 2.43.0\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 16EAEC32EA\n\tfor ;\n\tThu, 6 Feb 2025 12:56:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F0AC5685E3;\n\tThu, 6 Feb 2025 13:56:08 +0100 (CET)","from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com\n\t[IPv6:2607:f8b0:4864:20::72f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 141A96053F\n\tfor ;\n\tThu, 6 Feb 2025 13:56:07 +0100 (CET)","by mail-qk1-x72f.google.com with SMTP id\n\taf79cd13be357-7be6fdeee35so142457285a.1\n\tfor ;\n\tThu, 06 Feb 2025 04:56:06 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"sRdUvj2n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1738846566; x=1739451366;\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=VGlDP9hsiFdfOCmkxkA1rRMIDGv0/DqcY2r+2pCbUQo=;\n\tb=sRdUvj2nkFkKbWw7Dp9E8uFd66J6msk2Pi2tNZruT+yeNAHTOcOLSwuyqmcRAMFiyF\n\tHLsqDqNvSMrQYgD9KfPF31kmIV5wyBciM3hriSuipz+TaFXHZHq+sfKN0GL8e0ZjAyK3\n\tqHkaPrU6mil/nYiT9Q9zCrqYDm8J08I1vi3pJEeSEb59BTUXYOb+S30K3zPvIaXr3Oy7\n\t50yotPPI1x3PQ2W6VTRsAQlrwpmv9JzL5JlDeFsU7guM2p1e+wUrD/WVHX9BtVj9QtF6\n\tLfRDRnZLfRLoyHeObTW05B7bYZ13mSxvuoWHjfb+MldJos8YVp0BCMwK2W2fpvsGnv89\n\tpgRw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1738846566; x=1739451366;\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=VGlDP9hsiFdfOCmkxkA1rRMIDGv0/DqcY2r+2pCbUQo=;\n\tb=t7yKCzlZ0W73rKxxyl2aP7+AuDc4pH5pT3FnwNzStDnT4nLrZRQRfoZ0kCFzvzhU3E\n\t8M0iyf0ixmQFAaZRTXkQuyyZNqJDnZCAacWt+hZR9IksqxF7LpgbYvs2pGs9FWRD8TQT\n\tAxCKwzP1eHJlFMyL51kdo64PFqkcW0H8r4o5mGB2Hplo6+i9En/faZqmP9LbXLNZDc+y\n\t9/oHkiZYlPAI3JTbQ84Rax1EGrmFnL8N3JQqucfEkqVvag4A+Cp47DsHDk0bTSEXHQfU\n\tSaN7s60cL0WA3Fp+b+Wu5oOeJlN9ypUE3WTuZU3EbTAzM9gIGu91kHIem3h0hmh/5UbY\n\tGSRg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVB0T+WM+q75Zq6cBL1DsmhKqHk/HVyHpwlYN0nk+MUg9rwpOH/XiiRC2XOVAUKurmx9BLOlt6bXURrTEu9bFc=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Ywi5IVl1Mv1ingo0LbUbbzadHGNfQJUsTebDlGsqn4huHY6+DEA\n\tkx83LM0P+V0aEzN4tsRJFkhsuX93VcD0M2kSlNiXF1sq2h5PvUaJ5RnOviiCLd3pqXNIMKPXABA\n\t3yP4E5ZCoXh8McaMC+SPQSpeMyTBHanc+QyGVWLVPzJKv8pNl","X-Gm-Gg":"ASbGncuRgsCcgq0u73xnF7E/tIo3ITc9ek6NECmTSZLpA7tz4j3kPuFiXPdCAAnhFHi\n\t7eFYT8CsExWau6cngN4JMNxL7MFVKUDG6l4baJ+posvtp+3sXfzhk16KN5aCT0nC778LPenrAmr\n\t9BIzTDirVWsTOewkQ//SyBunL+j6g=","X-Google-Smtp-Source":"AGHT+IGBhkMCDUjsQxjSVW0vyDxuTCSNaqbe1IIyw+M6LYp32tlQGtYQUNoFkLxliXpqDgcROx/tHzWUMKQy2c1SSKk=","X-Received":"by 2002:a05:620a:2b93:b0:7a9:d14f:2374 with SMTP id\n\taf79cd13be357-7c03a0165ccmr949647385a.44.1738846566070;\n\tThu, 06 Feb 2025 04:56:06 -0800 (PST)","MIME-Version":"1.0","References":"<20250206092446.36237-1-naush@raspberrypi.com>\n\t<173884161508.1773152.10432086726470923220@ping.linuxembedded.co.uk>","In-Reply-To":"<173884161508.1773152.10432086726470923220@ping.linuxembedded.co.uk>","From":"David Plowman ","Date":"Thu, 6 Feb 2025 12:55:55 +0000","X-Gm-Features":"AWEUYZlcLOGjuG2OBPTvxvTJOizyYEyJLaRY_d2J_I2701ZMLzl_5HvHduUwlDo","Message-ID":"","Subject":"Re: [PATCH v2] ipa: rpi: Apply default ControlInfo values for sensor\n\tcontrols","To":"Kieran Bingham ","Cc":"Naushir Patuck ,\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\" "}}]