[{"id":36053,"web_url":"https://patchwork.libcamera.org/comment/36053/","msgid":"<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>","date":"2025-09-30T11:43:36","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n> At the moment all enumerators have a common prefix, in many cases\n> the name of the control, but not always. This is reasonable for\n> C++ because currently non-scoped enumerations are used, so some\n> kind of prefix is needed to differentiate common names like `Auto`,\n> `Manual`, `On`, `Off`, etc.\n\nHow does this patch prevent those collisions ?\n\n> However, for e.g. language bindings, it might be more desirable to\n> have access to the the unprefixed name. (This is even the case for\n> C++ scoped enumerations.)\n> \n> Currently, both the gstreamer and python bindings have extra code\n> to strip the common prefix. So instead of doing that separately in\n> every binding, etc. store the unprefixed name in the source of truth,\n> the control/property definition yaml files. This affect all C++, python,\n> gstreamer generated code.\n> \n> This is an API break, but it only affects C++ because gst and py already\n> strip the common prefix (TODO: recheck again). And in case of C++ it is\n> only an API break for enum controls where the common prefix is not the\n> same as the control name:\n\nThis sounds reasonable to me.\n\n> * properties::Location\n> * properties::draft::ColorFilterArrangement\n> * controls::AwbMode\n> * controls::AeConstraintMode\n> * controls::AeFlickerMode\n> * controls::AeMeteringMode\n> * controls::AeExposureMode\n> * controls::draft::ColorCorrectionAberrationMode\n> * controls::WdrMode\n> * TODO: check again\n> \n> Additionally, in some cases the corresponding `*NameValueMap` objects\n> are used to parse configuration files, these are also affected:\n> \n> * ipa::AgcMeanLuminance::parseConstraintModes\n> * ipa::AwbAlgorithm::parseModeConfigs\n> * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n> * ConfigParser::parseLocation\n> \n> Signed-off-by: Barnabás Pőcze \n> Reviewed-by: Paul Elder \n> ---\n> More testing and review would be beneficial still.\n> \n...\n\n> @@ -1170,13 +1170,13 @@ controls:\n> \\sa HdrChannel\n> \n> enum:\n> - - name: HdrModeOff\n> + - name: \"Off\"\n\nWhat happens here ? Why is this one \"quoted\". \n\nShould all name: entries be quoted ?\n\n> value: 0\n> description: |\n> HDR is disabled.\n> \n> Metadata for this frame will not include the HdrChannel control.\n> - - name: HdrModeMultiExposureUnmerged\n> + - name: MultiExposureUnmerged\n> value: 1\n> description: |\n> Multiple exposures will be generated in an alternating fashion.\n> @@ -1188,7 +1188,7 @@ controls:\n> \n> The expectation is that an application using this mode would merge\n> the frames to create HDR images for itself if it requires them.\n> - - name: HdrModeMultiExposure\n> + - name: MultiExposure\n> value: 2\n> description: |\n> Multiple exposures will be generated and merged to create HDR\n> @@ -1201,7 +1201,7 @@ controls:\n> alternately as the short and long channel. Systems that use three\n> channels for HDR will cycle through the short, medium and long\n> channel before repeating.\n> - - name: HdrModeSingleExposure\n> + - name: SingleExposure\n> value: 3\n> description: |\n> Multiple frames all at a single exposure will be used to create HDR\n> @@ -1209,7 +1209,7 @@ controls:\n> \n> These images should be reported as all corresponding to the HDR\n> short channel.\n> - - name: HdrModeNight\n> + - name: Night\n> value: 4\n> description: |\n> Multiple frames will be combined to produce \"night mode\" images.\n> @@ -1235,20 +1235,20 @@ controls:\n> \\sa HdrMode\n> \n> enum:\n> - - name: HdrChannelNone\n> + - name: None\n> value: 0\n> description: |\n> This image does not correspond to any of the captures used to create\n> an HDR image.\n> - - name: HdrChannelShort\n> + - name: Short\n> value: 1\n> description: |\n> This is a short exposure image.\n> - - name: HdrChannelMedium\n> + - name: Medium\n> value: 2\n> description: |\n> This is a medium exposure image.\n> - - name: HdrChannelLong\n> + - name: Long\n> value: 3\n> description: |\n> This is a long exposure image.\n> @@ -1295,17 +1295,17 @@ controls:\n> The algorithm then compensates for the loss of brightness by applying a\n> global tone mapping curve to the image.\n> enum:\n> - - name: WdrOff\n> + - name: \"Off\"\n\nDo they 'collide' otherwise ?\n\n--\nKieran","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 820F6C328C\n\tfor ;\n\tTue, 30 Sep 2025 11:43:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D7CC6B5F9;\n\tTue, 30 Sep 2025 13:43:41 +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 B175F6936E\n\tfor ;\n\tTue, 30 Sep 2025 13:43:39 +0200 (CEST)","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 3339442B;\n\tTue, 30 Sep 2025 13:42:11 +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=\"OdOM4MN6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759232531;\n\tbh=8GemMVn2Dep94KYPzSzDOQdFhHWJSmNQ3LgM9GUsO8I=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=OdOM4MN6h4EzjG+HmOZvMC7fb4dz5mlNXJ+CkPa2A1FzQME7Sx40KJw55lNsPj2pi\n\txWS2x9R5lz9GkAqWhOAi6mL20cOeHCaUfeWtNlN4Es0r4qFd9835D9ui5o4ssFiibH\n\t9lz6aLCXz1yF6/Yg/UjLVqdGAawn5F/9zGuKdcTs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","From":"Kieran Bingham ","Cc":"Paul Elder ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 30 Sep 2025 12:43:36 +0100","Message-ID":"<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":36054,"web_url":"https://patchwork.libcamera.org/comment/36054/","msgid":"","date":"2025-09-30T11:58:45","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n> Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n>> At the moment all enumerators have a common prefix, in many cases\n>> the name of the control, but not always. This is reasonable for\n>> C++ because currently non-scoped enumerations are used, so some\n>> kind of prefix is needed to differentiate common names like `Auto`,\n>> `Manual`, `On`, `Off`, etc.\n> \n> How does this patch prevent those collisions ?\n\nSorry, maybe I wasn't clear. I meant that we cannot do\n\nenum SomeFeature { On, Off, Auto };\nenum OtherFeature { On, Off, Auto };\n\nbecause the names would conflict. And this is why the enumerators are prefixed\nwith the name of the enumeration. But this only really makes sense for something\nlike C++ and not python or gstreamer, where things would not conflict. However,\nthe prefixed name is what is in the \"source of truth\" yaml files, which necessitates\nworkarounds like stripping the common prefix in e.g. the python parts. Having the\n\"unprefixed\" name in the source yaml files would address this, and also give a\nconsistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n\n> \n>> However, for e.g. language bindings, it might be more desirable to\n>> have access to the the unprefixed name. (This is even the case for\n>> C++ scoped enumerations.)\n>>\n>> Currently, both the gstreamer and python bindings have extra code\n>> to strip the common prefix. So instead of doing that separately in\n>> every binding, etc. store the unprefixed name in the source of truth,\n>> the control/property definition yaml files. This affect all C++, python,\n>> gstreamer generated code.\n>>\n>> This is an API break, but it only affects C++ because gst and py already\n>> strip the common prefix (TODO: recheck again). And in case of C++ it is\n>> only an API break for enum controls where the common prefix is not the\n>> same as the control name:\n> \n> This sounds reasonable to me.\n> \n>> * properties::Location\n>> * properties::draft::ColorFilterArrangement\n>> * controls::AwbMode\n>> * controls::AeConstraintMode\n>> * controls::AeFlickerMode\n>> * controls::AeMeteringMode\n>> * controls::AeExposureMode\n>> * controls::draft::ColorCorrectionAberrationMode\n>> * controls::WdrMode\n>> * TODO: check again\n>>\n>> Additionally, in some cases the corresponding `*NameValueMap` objects\n>> are used to parse configuration files, these are also affected:\n>>\n>> * ipa::AgcMeanLuminance::parseConstraintModes\n>> * ipa::AwbAlgorithm::parseModeConfigs\n>> * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n>> * ConfigParser::parseLocation\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> Reviewed-by: Paul Elder \n>> ---\n>> More testing and review would be beneficial still.\n>>\n> ...\n> \n>> @@ -1170,13 +1170,13 @@ controls:\n>> \\sa HdrChannel\n>>\n>> enum:\n>> - - name: HdrModeOff\n>> + - name: \"Off\"\n> \n> What happens here ? Why is this one \"quoted\".\n\nhttps://hitchdev.com/strictyaml/why/implicit-typing-removed/\n\nThe plain word `Off` is considered to be the boolean value false:\n\n$ python\nPython 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n>>> import yaml\n>>> yaml.safe_load(\"name: Off\")\n{'name': False}\n\n\n> \n> Should all name: entries be quoted ?\n> \n>> value: 0\n>> description: |\n>> HDR is disabled.\n>>\n>> Metadata for this frame will not include the HdrChannel control.\n>> - - name: HdrModeMultiExposureUnmerged\n>> + - name: MultiExposureUnmerged\n>> value: 1\n>> description: |\n>> Multiple exposures will be generated in an alternating fashion.\n>> @@ -1188,7 +1188,7 @@ controls:\n>>\n>> The expectation is that an application using this mode would merge\n>> the frames to create HDR images for itself if it requires them.\n>> - - name: HdrModeMultiExposure\n>> + - name: MultiExposure\n>> value: 2\n>> description: |\n>> Multiple exposures will be generated and merged to create HDR\n>> @@ -1201,7 +1201,7 @@ controls:\n>> alternately as the short and long channel. Systems that use three\n>> channels for HDR will cycle through the short, medium and long\n>> channel before repeating.\n>> - - name: HdrModeSingleExposure\n>> + - name: SingleExposure\n>> value: 3\n>> description: |\n>> Multiple frames all at a single exposure will be used to create HDR\n>> @@ -1209,7 +1209,7 @@ controls:\n>>\n>> These images should be reported as all corresponding to the HDR\n>> short channel.\n>> - - name: HdrModeNight\n>> + - name: Night\n>> value: 4\n>> description: |\n>> Multiple frames will be combined to produce \"night mode\" images.\n>> @@ -1235,20 +1235,20 @@ controls:\n>> \\sa HdrMode\n>>\n>> enum:\n>> - - name: HdrChannelNone\n>> + - name: None\n>> value: 0\n>> description: |\n>> This image does not correspond to any of the captures used to create\n>> an HDR image.\n>> - - name: HdrChannelShort\n>> + - name: Short\n>> value: 1\n>> description: |\n>> This is a short exposure image.\n>> - - name: HdrChannelMedium\n>> + - name: Medium\n>> value: 2\n>> description: |\n>> This is a medium exposure image.\n>> - - name: HdrChannelLong\n>> + - name: Long\n>> value: 3\n>> description: |\n>> This is a long exposure image.\n>> @@ -1295,17 +1295,17 @@ controls:\n>> The algorithm then compensates for the loss of brightness by applying a\n>> global tone mapping curve to the image.\n>> enum:\n>> - - name: WdrOff\n>> + - name: \"Off\"\n> \n> Do they 'collide' otherwise ?\n> \n> --\n> Kieran","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 D0475C324C\n\tfor ;\n\tTue, 30 Sep 2025 11:58:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A86236B5F3;\n\tTue, 30 Sep 2025 13:58:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 953BB6936E\n\tfor ;\n\tTue, 30 Sep 2025 13:58:48 +0200 (CEST)","from [192.168.33.11] (185.221.142.146.nat.pool.zt.hu\n\t[185.221.142.146])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 209B2B3;\n\tTue, 30 Sep 2025 13:57:20 +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=\"p1EaDp4F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759233440;\n\tbh=c/npOc0z2JOkYlEji4NY2p+fg3GohyRKBjJOnplYvzM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=p1EaDp4Fl9+7sfHWz+2Ken2ZRP6a/HKmrall2HqRbS77QQmzp2oNoK28HD0j14k0Y\n\trx7GYVOa88Vj8ArXJz5tB7Zla8gS3589I+WSeSifET0HstVGomk6YSmfKhkYdn9x8i\n\t3/c2+H9vey+fePgga31rC7KAb958XABlwT+aeOZI=","Message-ID":"","Date":"Tue, 30 Sep 2025 13:58:45 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Paul Elder ","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36589,"web_url":"https://patchwork.libcamera.org/comment/36589/","msgid":"","date":"2025-11-01T16:17:53","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabas\n\nOn Tue, Sep 30, 2025 at 01:58:45PM +0200, Barnabás Pőcze wrote:\n> 2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n> > Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n> > > At the moment all enumerators have a common prefix, in many cases\n> > > the name of the control, but not always. This is reasonable for\n> > > C++ because currently non-scoped enumerations are used, so some\n> > > kind of prefix is needed to differentiate common names like `Auto`,\n> > > `Manual`, `On`, `Off`, etc.\n> >\n> > How does this patch prevent those collisions ?\n>\n> Sorry, maybe I wasn't clear. I meant that we cannot do\n>\n> enum SomeFeature { On, Off, Auto };\n> enum OtherFeature { On, Off, Auto };\n>\n\nIs the whole purpose of the patch to remove the \"strip common prefix\"\nfrom the Python and gstreamer bindings ? If that was just that I\nwouldn't say it's worth it the longer names in C++, but I presume\nwe'll have more bindings so this becomes more critical.\n\nThe thing I'm not sure about is that if it's desirable to have even\nlonger names when referring to the enumerated value from C++ code\n\n- case controls::FlickerOff:\n+ case controls::AeFlickerModeOff:\n\nas an example result of this change\n\nMaybe I'm missing something, but wouldn't scoped enum solve the\nproblem in a better way ? I presume if we didn't use them, there might\nbe a reason I'm now missing.\n\nWouldn't\n case controls:AeFlickerMode::Off\n\nbe nicer ?\n\nYes, it requires you to specify the whole namespace, but that's almost\nthe case already with 'controls::AeFlickerModeOff'\n\nBut I understand that:\n\n enum class AeFlickerMode {\n Off,\n On,\n };\n\n\nWould indeed conflic with the control name ('AeFlickerMode' in this\ncase)\n\nOne possibly naive way of avoiding this would be to make the scoped\nenum names as \"Modes\" by appending an 's' to the enumeration type\nname:\n\n enum class AeFlickerModes {\n Off,\n On,\n };\n\nNot sure it's much nicer\n\n> because the names would conflict. And this is why the enumerators are prefixed\n> with the name of the enumeration. But this only really makes sense for something\n> like C++ and not python or gstreamer, where things would not conflict. However,\n> the prefixed name is what is in the \"source of truth\" yaml files, which necessitates\n> workarounds like stripping the common prefix in e.g. the python parts. Having the\n> \"unprefixed\" name in the source yaml files would address this, and also give a\n> consistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n>\n> >\n> > > However, for e.g. language bindings, it might be more desirable to\n> > > have access to the the unprefixed name. (This is even the case for\n> > > C++ scoped enumerations.)\n> > >\n> > > Currently, both the gstreamer and python bindings have extra code\n> > > to strip the common prefix. So instead of doing that separately in\n> > > every binding, etc. store the unprefixed name in the source of truth,\n> > > the control/property definition yaml files. This affect all C++, python,\n> > > gstreamer generated code.\n> > >\n> > > This is an API break, but it only affects C++ because gst and py already\n> > > strip the common prefix (TODO: recheck again). And in case of C++ it is\n\nHow should we re-check ?\n\n> > > only an API break for enum controls where the common prefix is not the\n> > > same as the control name:\n> >\n> > This sounds reasonable to me.\n> >\n> > > * properties::Location\n> > > * properties::draft::ColorFilterArrangement\n> > > * controls::AwbMode\n> > > * controls::AeConstraintMode\n> > > * controls::AeFlickerMode\n> > > * controls::AeMeteringMode\n> > > * controls::AeExposureMode\n> > > * controls::draft::ColorCorrectionAberrationMode\n> > > * controls::WdrMode\n};\n> > > * TODO: check again\n\nI see AwbState too:\n\nenum AwbStateEnum {\n\tAwbStateInactive = 0,\n\tAwbStateSearching = 1,\n-\tAwbConverged = 2,\n-\tAwbLocked = 3,\n+\tAwbStateConverged = 2,\n+\tAwbStateLocked = 3,\n};\n\n\n> > >\n> > > Additionally, in some cases the corresponding `*NameValueMap` objects\n> > > are used to parse configuration files, these are also affected:\n> > >\n> > > * ipa::AgcMeanLuminance::parseConstraintModes\n> > > * ipa::AwbAlgorithm::parseModeConfigs\n> > > * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n> > > * ConfigParser::parseLocation\n> > >\n> > > Signed-off-by: Barnabás Pőcze \n> > > Reviewed-by: Paul Elder \n> > > ---\n> > > More testing and review would be beneficial still.\n> > >\n> > ...\n> >\n> > > @@ -1170,13 +1170,13 @@ controls:\n> > > \\sa HdrChannel\n> > >\n> > > enum:\n> > > - - name: HdrModeOff\n> > > + - name: \"Off\"\n> >\n> > What happens here ? Why is this one \"quoted\".\n>\n> https://hitchdev.com/strictyaml/why/implicit-typing-removed/\n>\n> The plain word `Off` is considered to be the boolean value false:\n>\n> $ python\n> Python 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\n> Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n> > > > import yaml\n> > > > yaml.safe_load(\"name: Off\")\n> {'name': False}\n>\n\nScaryyyy\n\n>\n> >\n> > Should all name: entries be quoted ?\n> >\n> > > value: 0\n> > > description: |\n> > > HDR is disabled.\n> > >\n> > > Metadata for this frame will not include the HdrChannel control.\n> > > - - name: HdrModeMultiExposureUnmerged\n> > > + - name: MultiExposureUnmerged\n> > > value: 1\n> > > description: |\n> > > Multiple exposures will be generated in an alternating fashion.\n> > > @@ -1188,7 +1188,7 @@ controls:\n> > >\n> > > The expectation is that an application using this mode would merge\n> > > the frames to create HDR images for itself if it requires them.\n> > > - - name: HdrModeMultiExposure\n> > > + - name: MultiExposure\n> > > value: 2\n> > > description: |\n> > > Multiple exposures will be generated and merged to create HDR\n> > > @@ -1201,7 +1201,7 @@ controls:\n> > > alternately as the short and long channel. Systems that use three\n> > > channels for HDR will cycle through the short, medium and long\n> > > channel before repeating.\n> > > - - name: HdrModeSingleExposure\n> > > + - name: SingleExposure\n> > > value: 3\n> > > description: |\n> > > Multiple frames all at a single exposure will be used to create HDR\n> > > @@ -1209,7 +1209,7 @@ controls:\n> > >\n> > > These images should be reported as all corresponding to the HDR\n> > > short channel.\n> > > - - name: HdrModeNight\n> > > + - name: Night\n> > > value: 4\n> > > description: |\n> > > Multiple frames will be combined to produce \"night mode\" images.\n> > > @@ -1235,20 +1235,20 @@ controls:\n> > > \\sa HdrMode\n> > >\n> > > enum:\n> > > - - name: HdrChannelNone\n> > > + - name: None\n> > > value: 0\n> > > description: |\n> > > This image does not correspond to any of the captures used to create\n> > > an HDR image.\n> > > - - name: HdrChannelShort\n> > > + - name: Short\n> > > value: 1\n> > > description: |\n> > > This is a short exposure image.\n> > > - - name: HdrChannelMedium\n> > > + - name: Medium\n> > > value: 2\n> > > description: |\n> > > This is a medium exposure image.\n> > > - - name: HdrChannelLong\n> > > + - name: Long\n> > > value: 3\n> > > description: |\n> > > This is a long exposure image.\n> > > @@ -1295,17 +1295,17 @@ controls:\n> > > The algorithm then compensates for the loss of brightness by applying a\n> > > global tone mapping curve to the image.\n> > > enum:\n> > > - - name: WdrOff\n> > > + - name: \"Off\"\n> >\n> > Do they 'collide' otherwise ?\n> >\n> > --\n> > Kieran\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 0ADA9BDE4C\n\tfor ;\n\tSat, 1 Nov 2025 16:18:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 415A66086F;\n\tSat, 1 Nov 2025 17:17:59 +0100 (CET)","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 80533606D5\n\tfor ;\n\tSat, 1 Nov 2025 17:17:57 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:6462:5de2:153:f9b8:5024:faa2])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E868666;\n\tSat, 1 Nov 2025 17:16:05 +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=\"niv89noJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762013765;\n\tbh=KlWxdMd3+s0c8iL3bPdTx0hetafK1Kxd2UFIa+7b6wU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=niv89noJkHayYvF6zn/S3p0MaNMPwhYLSFxV/M75cGEguYlvjNB+fDtj935pvMCTE\n\tbQK8rUjst+8c7n+AE6LHSToJe6VB8ogJjnGMlxNK4ygrsax8qmg7Dv64SW1uzs/Ufn\n\tNxBOiutQAUSwYnOrx91StAbCeweRt13ksE9Nf0d0=","Date":"Sat, 1 Nov 2025 17:17:53 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham , \n\tlibcamera-devel@lists.libcamera.org,\n\tPaul Elder ","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","Message-ID":"","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","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":36621,"web_url":"https://patchwork.libcamera.org/comment/36621/","msgid":"<20251102161251.GC27255@pendragon.ideasonboard.com>","date":"2025-11-02T16:12:51","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sat, Nov 01, 2025 at 05:17:53PM +0100, Jacopo Mondi wrote:\n> On Tue, Sep 30, 2025 at 01:58:45PM +0200, Barnabás Pőcze wrote:\n> > 2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n> > > Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n> > > > At the moment all enumerators have a common prefix, in many cases\n> > > > the name of the control, but not always. This is reasonable for\n> > > > C++ because currently non-scoped enumerations are used, so some\n> > > > kind of prefix is needed to differentiate common names like `Auto`,\n> > > > `Manual`, `On`, `Off`, etc.\n> > >\n> > > How does this patch prevent those collisions ?\n> >\n> > Sorry, maybe I wasn't clear. I meant that we cannot do\n> >\n> > enum SomeFeature { On, Off, Auto };\n\nThis reminds me of https://thedailywtf.com/articles/What_Is_Truth_0x3f_\n\n> > enum OtherFeature { On, Off, Auto };\n> \n> Is the whole purpose of the patch to remove the \"strip common prefix\"\n> from the Python and gstreamer bindings ? If that was just that I\n> wouldn't say it's worth it the longer names in C++, but I presume\n> we'll have more bindings so this becomes more critical.\n> \n> The thing I'm not sure about is that if it's desirable to have even\n> longer names when referring to the enumerated value from C++ code\n> \n> - case controls::FlickerOff:\n> + case controls::AeFlickerModeOff:\n> \n> as an example result of this change\n> \n> Maybe I'm missing something, but wouldn't scoped enum solve the\n> problem in a better way ? I presume if we didn't use them, there might\n> be a reason I'm now missing.\n> \n> Wouldn't\n> case controls:AeFlickerMode::Off\n> \n> be nicer ?\n\nI quite like that.\n\n> Yes, it requires you to specify the whole namespace, but that's almost\n> the case already with 'controls::AeFlickerModeOff'\n> \n> But I understand that:\n> \n> enum class AeFlickerMode {\n> Off,\n> On,\n> };\n> \n> \n> Would indeed conflic with the control name ('AeFlickerMode' in this\n> case)\n\nThe following code compiles:\n\n----\nenum class Foo {\n\tOn,\n\tOff,\n\tAuto,\n};\n\nstatic int Foo = 42;\n\nint main()\n{\n\tFoo = static_cast(Foo::On);\n\n\treturn Foo;\n}\n----\n\nBarnabás, are you aware of issues this could cause ?\n\n> One possibly naive way of avoiding this would be to make the scoped\n> enum names as \"Modes\" by appending an 's' to the enumeration type\n> name:\n> \n> enum class AeFlickerModes {\n> Off,\n> On,\n> };\n> \n> Not sure it's much nicer\n> \n> > because the names would conflict. And this is why the enumerators are prefixed\n> > with the name of the enumeration. But this only really makes sense for something\n> > like C++ and not python or gstreamer, where things would not conflict. However,\n> > the prefixed name is what is in the \"source of truth\" yaml files, which necessitates\n> > workarounds like stripping the common prefix in e.g. the python parts. Having the\n> > \"unprefixed\" name in the source yaml files would address this, and also give a\n> > consistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n> >\n> > > > However, for e.g. language bindings, it might be more desirable to\n> > > > have access to the the unprefixed name. (This is even the case for\n> > > > C++ scoped enumerations.)\n> > > >\n> > > > Currently, both the gstreamer and python bindings have extra code\n> > > > to strip the common prefix. So instead of doing that separately in\n> > > > every binding, etc. store the unprefixed name in the source of truth,\n> > > > the control/property definition yaml files. This affect all C++, python,\n> > > > gstreamer generated code.\n> > > >\n> > > > This is an API break, but it only affects C++ because gst and py already\n> > > > strip the common prefix (TODO: recheck again). And in case of C++ it is\n> \n> How should we re-check ?\n> \n> > > > only an API break for enum controls where the common prefix is not the\n> > > > same as the control name:\n> > >\n> > > This sounds reasonable to me.\n> > >\n> > > > * properties::Location\n> > > > * properties::draft::ColorFilterArrangement\n> > > > * controls::AwbMode\n> > > > * controls::AeConstraintMode\n> > > > * controls::AeFlickerMode\n> > > > * controls::AeMeteringMode\n> > > > * controls::AeExposureMode\n> > > > * controls::draft::ColorCorrectionAberrationMode\n> > > > * controls::WdrMode\n> };\n> > > > * TODO: check again\n> \n> I see AwbState too:\n> \n> enum AwbStateEnum {\n> \tAwbStateInactive = 0,\n> \tAwbStateSearching = 1,\n> -\tAwbConverged = 2,\n> -\tAwbLocked = 3,\n> +\tAwbStateConverged = 2,\n> +\tAwbStateLocked = 3,\n> };\n> \n> > > >\n> > > > Additionally, in some cases the corresponding `*NameValueMap` objects\n> > > > are used to parse configuration files, these are also affected:\n> > > >\n> > > > * ipa::AgcMeanLuminance::parseConstraintModes\n> > > > * ipa::AwbAlgorithm::parseModeConfigs\n> > > > * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n> > > > * ConfigParser::parseLocation\n> > > >\n> > > > Signed-off-by: Barnabás Pőcze \n> > > > Reviewed-by: Paul Elder \n> > > > ---\n> > > > More testing and review would be beneficial still.\n> > > >\n> > > ...\n> > >\n> > > > @@ -1170,13 +1170,13 @@ controls:\n> > > > \\sa HdrChannel\n> > > >\n> > > > enum:\n> > > > - - name: HdrModeOff\n> > > > + - name: \"Off\"\n> > >\n> > > What happens here ? Why is this one \"quoted\".\n> >\n> > https://hitchdev.com/strictyaml/why/implicit-typing-removed/\n> >\n> > The plain word `Off` is considered to be the boolean value false:\n> >\n> > $ python\n> > Python 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\n> > Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n> > > > > import yaml\n> > > > > yaml.safe_load(\"name: Off\")\n> > {'name': False}\n> \n> Scaryyyy\n\nYAML is full of \"interesting\" design decisions.\n\n> > > Should all name: entries be quoted ?\n> > >\n> > > > value: 0\n> > > > description: |\n> > > > HDR is disabled.\n> > > >\n> > > > Metadata for this frame will not include the HdrChannel control.\n> > > > - - name: HdrModeMultiExposureUnmerged\n> > > > + - name: MultiExposureUnmerged\n> > > > value: 1\n> > > > description: |\n> > > > Multiple exposures will be generated in an alternating fashion.\n> > > > @@ -1188,7 +1188,7 @@ controls:\n> > > >\n> > > > The expectation is that an application using this mode would merge\n> > > > the frames to create HDR images for itself if it requires them.\n> > > > - - name: HdrModeMultiExposure\n> > > > + - name: MultiExposure\n> > > > value: 2\n> > > > description: |\n> > > > Multiple exposures will be generated and merged to create HDR\n> > > > @@ -1201,7 +1201,7 @@ controls:\n> > > > alternately as the short and long channel. Systems that use three\n> > > > channels for HDR will cycle through the short, medium and long\n> > > > channel before repeating.\n> > > > - - name: HdrModeSingleExposure\n> > > > + - name: SingleExposure\n> > > > value: 3\n> > > > description: |\n> > > > Multiple frames all at a single exposure will be used to create HDR\n> > > > @@ -1209,7 +1209,7 @@ controls:\n> > > >\n> > > > These images should be reported as all corresponding to the HDR\n> > > > short channel.\n> > > > - - name: HdrModeNight\n> > > > + - name: Night\n> > > > value: 4\n> > > > description: |\n> > > > Multiple frames will be combined to produce \"night mode\" images.\n> > > > @@ -1235,20 +1235,20 @@ controls:\n> > > > \\sa HdrMode\n> > > >\n> > > > enum:\n> > > > - - name: HdrChannelNone\n> > > > + - name: None\n> > > > value: 0\n> > > > description: |\n> > > > This image does not correspond to any of the captures used to create\n> > > > an HDR image.\n> > > > - - name: HdrChannelShort\n> > > > + - name: Short\n> > > > value: 1\n> > > > description: |\n> > > > This is a short exposure image.\n> > > > - - name: HdrChannelMedium\n> > > > + - name: Medium\n> > > > value: 2\n> > > > description: |\n> > > > This is a medium exposure image.\n> > > > - - name: HdrChannelLong\n> > > > + - name: Long\n> > > > value: 3\n> > > > description: |\n> > > > This is a long exposure image.\n> > > > @@ -1295,17 +1295,17 @@ controls:\n> > > > The algorithm then compensates for the loss of brightness by applying a\n> > > > global tone mapping curve to the image.\n> > > > enum:\n> > > > - - name: WdrOff\n> > > > + - name: \"Off\"\n> > >\n> > > Do they 'collide' otherwise ?","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 889EABDE4C\n\tfor ;\n\tSun, 2 Nov 2025 16:13:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B5AE9606E6;\n\tSun, 2 Nov 2025 17:13:06 +0100 (CET)","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 BFCCF606E6\n\tfor ;\n\tSun, 2 Nov 2025 17:13:05 +0100 (CET)","from pendragon.ideasonboard.com (82-203-160-149.bb.dnainternet.fi\n\t[82.203.160.149])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id E908CA98;\n\tSun, 2 Nov 2025 17:11:12 +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=\"g35Np9R0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762099873;\n\tbh=jsSMWKgOyRPONGSTZ3UfraMtIyuM8HOpdQAsNxXnvQA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=g35Np9R0NWJmwPo6zkXqNyDDlEBZVBF/AWuHExXSVOcJk8RlFTMjA1JfAZlCmqbx4\n\ti8Dnu2tjWYaVpWUifQtJU544lfIH593bp0Q/do3LkPqUmy4Pb2QTtT/EW4EZowLLmC\n\tID5Rc3zPeScNTSepYfCElSBD/6VvClHGaTUEQeUQ=","Date":"Sun, 2 Nov 2025 18:12:51 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tKieran Bingham , \n\tlibcamera-devel@lists.libcamera.org, Paul Elder\n\t","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","Message-ID":"<20251102161251.GC27255@pendragon.ideasonboard.com>","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","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":36645,"web_url":"https://patchwork.libcamera.org/comment/36645/","msgid":"<01597fab-88fb-4d22-a4de-9c4cf635a65c@ideasonboard.com>","date":"2025-11-03T11:11:59","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 02. 17:12 keltezéssel, Laurent Pinchart írta:\n> On Sat, Nov 01, 2025 at 05:17:53PM +0100, Jacopo Mondi wrote:\n>> On Tue, Sep 30, 2025 at 01:58:45PM +0200, Barnabás Pőcze wrote:\n>>> 2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n>>>> Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n>>>>> At the moment all enumerators have a common prefix, in many cases\n>>>>> the name of the control, but not always. This is reasonable for\n>>>>> C++ because currently non-scoped enumerations are used, so some\n>>>>> kind of prefix is needed to differentiate common names like `Auto`,\n>>>>> `Manual`, `On`, `Off`, etc.\n>>>>\n>>>> How does this patch prevent those collisions ?\n>>>\n>>> Sorry, maybe I wasn't clear. I meant that we cannot do\n>>>\n>>> enum SomeFeature { On, Off, Auto };\n> \n> This reminds me of https://thedailywtf.com/articles/What_Is_Truth_0x3f_\n> \n>>> enum OtherFeature { On, Off, Auto };\n>>\n>> Is the whole purpose of the patch to remove the \"strip common prefix\"\n>> from the Python and gstreamer bindings ? If that was just that I\n\nYes. Not only are they differently implemented at the moment, they also need\nspecial hacks to strip the common prefix: e.g. https://patchwork.libcamera.org/patch/24251/\n\n\n>> wouldn't say it's worth it the longer names in C++, but I presume\n>> we'll have more bindings so this becomes more critical.\n\nI think the consistency is worth - in my opinion - the minor inconvenience\nof longer names in C++.\n\n\n>>\n>> The thing I'm not sure about is that if it's desirable to have even\n>> longer names when referring to the enumerated value from C++ code\n>>\n>> - case controls::FlickerOff:\n>> + case controls::AeFlickerModeOff:\n>>\n>> as an example result of this change\n>>\n>> Maybe I'm missing something, but wouldn't scoped enum solve the\n>> problem in a better way ? I presume if we didn't use them, there might\n>> be a reason I'm now missing.\n>>\n>> Wouldn't\n>> case controls:AeFlickerMode::Off\n>>\n>> be nicer ?\n> \n> I quite like that.\n> \n>> Yes, it requires you to specify the whole namespace, but that's almost\n>> the case already with 'controls::AeFlickerModeOff'\n>>\n>> But I understand that:\n>>\n>> enum class AeFlickerMode {\n>> Off,\n>> On,\n>> };\n>>\n>>\n>> Would indeed conflic with the control name ('AeFlickerMode' in this\n>> case)\n> \n> The following code compiles:\n> \n> ----\n> enum class Foo {\n> \tOn,\n> \tOff,\n> \tAuto,\n> };\n> \n> static int Foo = 42;\n> \n> int main()\n> {\n> \tFoo = static_cast(Foo::On);\n> \n> \treturn Foo;\n> }\n> ----\n> \n> Barnabás, are you aware of issues this could cause ?\n\nNot an issue per se, but one will have to use elaborated type specifiers\nin some cases, e.g. to declare a variable of the type. Same situation as\nwith `struct stat` vs `stat()`.\n\nhttps://en.cppreference.com/w/cpp/language/elaborated_type_specifier.html\n\n\n> \n>> One possibly naive way of avoiding this would be to make the scoped\n>> enum names as \"Modes\" by appending an 's' to the enumeration type\n>> name:\n>>\n>> enum class AeFlickerModes {\n>> Off,\n>> On,\n>> };\n>>\n>> Not sure it's much nicer\n\nThat is also an option, yes. My main concern wrt. scoped enumerations is the\ninteraction with controls. Enums are saved as `int32_t`, which is not an issue,\nbut they are also returned as such. This is not an issue for unscoped enumerations\nbecause of implicit casting. But for scoped enumerations, that would make things\nquite a bit more inconvenient.\n\nSo I think I would only want to go with scoped enums if the control parts are\nextended to directly support them. E.g. something like\n\n Control AeFlickerMode;\n // instead of Control AeFlickerMode\n\nbut I am not yet sure about the implications of such change.\n\n>>\n>>> because the names would conflict. And this is why the enumerators are prefixed\n>>> with the name of the enumeration. But this only really makes sense for something\n>>> like C++ and not python or gstreamer, where things would not conflict. However,\n>>> the prefixed name is what is in the \"source of truth\" yaml files, which necessitates\n>>> workarounds like stripping the common prefix in e.g. the python parts. Having the\n>>> \"unprefixed\" name in the source yaml files would address this, and also give a\n>>> consistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n>>>\n>>>>> However, for e.g. language bindings, it might be more desirable to\n>>>>> have access to the the unprefixed name. (This is even the case for\n>>>>> C++ scoped enumerations.)\n>>>>>\n>>>>> Currently, both the gstreamer and python bindings have extra code\n>>>>> to strip the common prefix. So instead of doing that separately in\n>>>>> every binding, etc. store the unprefixed name in the source of truth,\n>>>>> the control/property definition yaml files. This affect all C++, python,\n>>>>> gstreamer generated code.\n>>>>>\n>>>>> This is an API break, but it only affects C++ because gst and py already\n>>>>> strip the common prefix (TODO: recheck again). And in case of C++ it is\n>>\n>> How should we re-check ?\n\nUnfortunately I don't remember what I had in mind. I think what I meant is going\nthrough the generated gst and py code and checking enumerator names to see if\nthe prefix is indeed removed.\n\n\n>>\n>>>>> only an API break for enum controls where the common prefix is not the\n>>>>> same as the control name:\n>>>>\n>>>> This sounds reasonable to me.\n>>>>\n>>>>> * properties::Location\n>>>>> * properties::draft::ColorFilterArrangement\n>>>>> * controls::AwbMode\n>>>>> * controls::AeConstraintMode\n>>>>> * controls::AeFlickerMode\n>>>>> * controls::AeMeteringMode\n>>>>> * controls::AeExposureMode\n>>>>> * controls::draft::ColorCorrectionAberrationMode\n>>>>> * controls::WdrMode\n>> };\n>>>>> * TODO: check again\n>>\n>> I see AwbState too:\n>>\n>> enum AwbStateEnum {\n>> \tAwbStateInactive = 0,\n>> \tAwbStateSearching = 1,\n>> -\tAwbConverged = 2,\n>> -\tAwbLocked = 3,\n>> +\tAwbStateConverged = 2,\n>> +\tAwbStateLocked = 3,\n>> };\n\nOops, you're right, there is that as well. In the end I made a script to check:\n\n```py\nimport argparse\nimport yaml\n\ndef main():\n\tparser = argparse.ArgumentParser()\n\tparser.add_argument(\"--old\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n\tparser.add_argument(\"--new\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n\targs = parser.parse_args()\n\n\told_enums = {}\n\n\tfor f in args.old:\n\t\tc = yaml.safe_load(f)\n\t\tv = c.get(\"vendor\")\n\t\tfor x in c.get(\"controls\"):\n\t\t\tfor (y, z) in x.items():\n\t\t\t\tes = z.get(\"enum\")\n\t\t\t\tif not es:\n\t\t\t\t\tcontinue\n\n\t\t\t\tfor e in es:\n\t\t\t\t\tvals = old_enums.setdefault((v, y), {})\n\t\t\t\t\tvals[e[\"value\"]] = e[\"name\"]\n\n\tchanges = {}\n\n\tfor f in args.new:\n\t\tc = yaml.safe_load(f)\n\t\tv = c.get(\"vendor\")\n\t\tfor x in c.get(\"controls\"):\n\t\t\tfor (y, z) in x.items():\n\t\t\t\tes = z.get(\"enum\")\n\t\t\t\tif not es:\n\t\t\t\t\tcontinue\n\n\t\t\t\tvals = old_enums[(v, y)]\n\t\t\t\tassert len(vals) == len(es)\n\n\t\t\t\tfor e in es:\n\t\t\t\t\told_name = vals[e[\"value\"]]\n\t\t\t\t\tif old_name != y + e[\"name\"]:\n\t\t\t\t\t\tchanges.setdefault(v, {}).setdefault(y, []).append((old_name, e[\"name\"]))\n\n\tfor (v, cs) in changes.items():\n\t\tprint(v)\n\t\tfor (c, vs) in cs.items():\n\t\t\tprint(\"\\t\", c, sep=\"\")\n\t\t\tfor (o, n) in vs:\n\t\t\t\tprint(\"\\t\\t\", o, \" -> \", c + n, sep=\"\")\n\nif __name__ == \"__main__\":\n\tmain()\n```\n\nwhich shows:\n\n```\n$ python asd.py --old src/libcamera/control_ids_core.yaml --old src/libcamera/control_ids_draft.yaml --old src/libcamera/control_ids_rpi.yaml --new ../libcamera/src/libcamera/control_ids_core.yaml --new ../libcamera/src/libcamera/control_ids_draft.yaml --new ../libcamera/src/libcamera/control_ids_rpi.yaml\nlibcamera\n\t AeMeteringMode\n\t\t MeteringCentreWeighted -> AeMeteringModeCentreWeighted\n\t\t MeteringSpot -> AeMeteringModeSpot\n\t\t MeteringMatrix -> AeMeteringModeMatrix\n\t\t MeteringCustom -> AeMeteringModeCustom\n\t AeConstraintMode\n\t\t ConstraintNormal -> AeConstraintModeNormal\n\t\t ConstraintHighlight -> AeConstraintModeHighlight\n\t\t ConstraintShadows -> AeConstraintModeShadows\n\t\t ConstraintCustom -> AeConstraintModeCustom\n\t AeExposureMode\n\t\t ExposureNormal -> AeExposureModeNormal\n\t\t ExposureShort -> AeExposureModeShort\n\t\t ExposureLong -> AeExposureModeLong\n\t\t ExposureCustom -> AeExposureModeCustom\n\t AeFlickerMode\n\t\t FlickerOff -> AeFlickerModeOff\n\t\t FlickerManual -> AeFlickerModeManual\n\t\t FlickerAuto -> AeFlickerModeAuto\n\t AwbMode\n\t\t AwbAuto -> AwbModeAuto\n\t\t AwbIncandescent -> AwbModeIncandescent\n\t\t AwbTungsten -> AwbModeTungsten\n\t\t AwbFluorescent -> AwbModeFluorescent\n\t\t AwbIndoor -> AwbModeIndoor\n\t\t AwbDaylight -> AwbModeDaylight\n\t\t AwbCloudy -> AwbModeCloudy\n\t\t AwbCustom -> AwbModeCustom\n\t WdrMode\n\t\t WdrOff -> WdrModeOff\n\t\t WdrLinear -> WdrModeLinear\n\t\t WdrPower -> WdrModePower\n\t\t WdrExponential -> WdrModeExponential\n\t\t WdrHistogramEqualization -> WdrModeHistogramEqualization\ndraft\n\t ColorCorrectionAberrationMode\n\t\t ColorCorrectionAberrationOff -> ColorCorrectionAberrationModeOff\n\t\t ColorCorrectionAberrationFast -> ColorCorrectionAberrationModeFast\n\t\t ColorCorrectionAberrationHighQuality -> ColorCorrectionAberrationModeHighQuality\n\t AwbState\n\t\t AwbConverged -> AwbStateConverged\n\t\t AwbLocked -> AwbStateLocked\n```\n\nand\n\n```\n$ asd.py --old src/libcamera/property_ids_core.yaml --old src/libcamera/property_ids_draft.yaml --new ../libcamera/src/libcamera/property_ids_core.yaml --new ../libcamera/src/libcamera/property_ids_draft.yaml\nlibcamera\n\t Location\n\t\t CameraLocationFront -> LocationFront\n\t\t CameraLocationBack -> LocationBack\n\t\t CameraLocationExternal -> LocationExternal\ndraft\n\t ColorFilterArrangement\n\t\t RGGB -> ColorFilterArrangementRGGB\n\t\t GRBG -> ColorFilterArrangementGRBG\n\t\t GBRG -> ColorFilterArrangementGBRG\n\t\t BGGR -> ColorFilterArrangementBGGR\n\t\t RGB -> ColorFilterArrangementRGB\n\t\t MONO -> ColorFilterArrangementMONO\n```\n\nso I think it was only `controls::draft::AwbState` that I missed.\n\n\nRegards,\nBarnabás Pőcze\n\n>>\n>>>>>\n>>>>> Additionally, in some cases the corresponding `*NameValueMap` objects\n>>>>> are used to parse configuration files, these are also affected:\n>>>>>\n>>>>> * ipa::AgcMeanLuminance::parseConstraintModes\n>>>>> * ipa::AwbAlgorithm::parseModeConfigs\n>>>>> * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n>>>>> * ConfigParser::parseLocation\n>>>>>\n>>>>> Signed-off-by: Barnabás Pőcze \n>>>>> Reviewed-by: Paul Elder \n>>>>> ---\n>>>>> More testing and review would be beneficial still.\n>>>>>\n>>>> ...\n>>>>\n>>>>> @@ -1170,13 +1170,13 @@ controls:\n>>>>> \\sa HdrChannel\n>>>>>\n>>>>> enum:\n>>>>> - - name: HdrModeOff\n>>>>> + - name: \"Off\"\n>>>>\n>>>> What happens here ? Why is this one \"quoted\".\n>>>\n>>> https://hitchdev.com/strictyaml/why/implicit-typing-removed/\n>>>\n>>> The plain word `Off` is considered to be the boolean value false:\n>>>\n>>> $ python\n>>> Python 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\n>>> Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n>>>>>> import yaml\n>>>>>> yaml.safe_load(\"name: Off\")\n>>> {'name': False}\n>>\n>> Scaryyyy\n> \n> YAML is full of \"interesting\" design decisions.\n> \n>>>> Should all name: entries be quoted ?\n>>>>\n>>>>> value: 0\n>>>>> description: |\n>>>>> HDR is disabled.\n>>>>>\n>>>>> Metadata for this frame will not include the HdrChannel control.\n>>>>> - - name: HdrModeMultiExposureUnmerged\n>>>>> + - name: MultiExposureUnmerged\n>>>>> value: 1\n>>>>> description: |\n>>>>> Multiple exposures will be generated in an alternating fashion.\n>>>>> @@ -1188,7 +1188,7 @@ controls:\n>>>>>\n>>>>> The expectation is that an application using this mode would merge\n>>>>> the frames to create HDR images for itself if it requires them.\n>>>>> - - name: HdrModeMultiExposure\n>>>>> + - name: MultiExposure\n>>>>> value: 2\n>>>>> description: |\n>>>>> Multiple exposures will be generated and merged to create HDR\n>>>>> @@ -1201,7 +1201,7 @@ controls:\n>>>>> alternately as the short and long channel. Systems that use three\n>>>>> channels for HDR will cycle through the short, medium and long\n>>>>> channel before repeating.\n>>>>> - - name: HdrModeSingleExposure\n>>>>> + - name: SingleExposure\n>>>>> value: 3\n>>>>> description: |\n>>>>> Multiple frames all at a single exposure will be used to create HDR\n>>>>> @@ -1209,7 +1209,7 @@ controls:\n>>>>>\n>>>>> These images should be reported as all corresponding to the HDR\n>>>>> short channel.\n>>>>> - - name: HdrModeNight\n>>>>> + - name: Night\n>>>>> value: 4\n>>>>> description: |\n>>>>> Multiple frames will be combined to produce \"night mode\" images.\n>>>>> @@ -1235,20 +1235,20 @@ controls:\n>>>>> \\sa HdrMode\n>>>>>\n>>>>> enum:\n>>>>> - - name: HdrChannelNone\n>>>>> + - name: None\n>>>>> value: 0\n>>>>> description: |\n>>>>> This image does not correspond to any of the captures used to create\n>>>>> an HDR image.\n>>>>> - - name: HdrChannelShort\n>>>>> + - name: Short\n>>>>> value: 1\n>>>>> description: |\n>>>>> This is a short exposure image.\n>>>>> - - name: HdrChannelMedium\n>>>>> + - name: Medium\n>>>>> value: 2\n>>>>> description: |\n>>>>> This is a medium exposure image.\n>>>>> - - name: HdrChannelLong\n>>>>> + - name: Long\n>>>>> value: 3\n>>>>> description: |\n>>>>> This is a long exposure image.\n>>>>> @@ -1295,17 +1295,17 @@ controls:\n>>>>> The algorithm then compensates for the loss of brightness by applying a\n>>>>> global tone mapping curve to the image.\n>>>>> enum:\n>>>>> - - name: WdrOff\n>>>>> + - name: \"Off\"\n>>>>\n>>>> Do they 'collide' otherwise ?\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 D30B7BDE4C\n\tfor ;\n\tMon, 3 Nov 2025 11:12:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2719C609D8;\n\tMon, 3 Nov 2025 12:12:05 +0100 (CET)","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 C550B606A0\n\tfor ;\n\tMon, 3 Nov 2025 12:12:02 +0100 (CET)","from [192.168.33.39] (185.221.140.239.nat.pool.zt.hu\n\t[185.221.140.239])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C5F3499F;\n\tMon, 3 Nov 2025 12:10:09 +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=\"MIkp9u7q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762168210;\n\tbh=3A+JqQpnYIArGFnFrYEzn5sCP9iKaFYAwVIdKa2dLVk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=MIkp9u7qsEsA+9JntJkg2IGP9xreISjLFJ4yfFASnc6++mKVfVmDL7WOmQXfpIOHS\n\tUbIq/2nvUCoZxqSRNBMtYQ6qxhuFL8i5zVjVqd2per/wdehppUP1uPNP6pSt7qXu0R\n\tHx85yArH/LahMoyDo3NLVhA5+3tgjga9eNb8aHZc=","Message-ID":"<01597fab-88fb-4d22-a4de-9c4cf635a65c@ideasonboard.com>","Date":"Mon, 3 Nov 2025 12:11:59 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","To":"Laurent Pinchart ,\n\tJacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org,\n\tPaul Elder ","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>\n\t\n\t\n\t<20251102161251.GC27255@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251102161251.GC27255@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36651,"web_url":"https://patchwork.libcamera.org/comment/36651/","msgid":"<20251103123130.GU27255@pendragon.ideasonboard.com>","date":"2025-11-03T12:31:30","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Nov 03, 2025 at 12:11:59PM +0100, Barnabás Pőcze wrote:\n> 2025. 11. 02. 17:12 keltezéssel, Laurent Pinchart írta:\n> > On Sat, Nov 01, 2025 at 05:17:53PM +0100, Jacopo Mondi wrote:\n> >> On Tue, Sep 30, 2025 at 01:58:45PM +0200, Barnabás Pőcze wrote:\n> >>> 2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n> >>>> Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n> >>>>> At the moment all enumerators have a common prefix, in many cases\n> >>>>> the name of the control, but not always. This is reasonable for\n> >>>>> C++ because currently non-scoped enumerations are used, so some\n> >>>>> kind of prefix is needed to differentiate common names like `Auto`,\n> >>>>> `Manual`, `On`, `Off`, etc.\n> >>>>\n> >>>> How does this patch prevent those collisions ?\n> >>>\n> >>> Sorry, maybe I wasn't clear. I meant that we cannot do\n> >>>\n> >>> enum SomeFeature { On, Off, Auto };\n> > \n> > This reminds me of https://thedailywtf.com/articles/What_Is_Truth_0x3f_\n> > \n> >>> enum OtherFeature { On, Off, Auto };\n> >>\n> >> Is the whole purpose of the patch to remove the \"strip common prefix\"\n> >> from the Python and gstreamer bindings ? If that was just that I\n> \n> Yes. Not only are they differently implemented at the moment, they also need\n> special hacks to strip the common prefix: e.g. https://patchwork.libcamera.org/patch/24251/\n> \n> >> wouldn't say it's worth it the longer names in C++, but I presume\n> >> we'll have more bindings so this becomes more critical.\n> \n> I think the consistency is worth - in my opinion - the minor inconvenience\n> of longer names in C++.\n> \n> >>\n> >> The thing I'm not sure about is that if it's desirable to have even\n> >> longer names when referring to the enumerated value from C++ code\n> >>\n> >> - case controls::FlickerOff:\n> >> + case controls::AeFlickerModeOff:\n> >>\n> >> as an example result of this change\n> >>\n> >> Maybe I'm missing something, but wouldn't scoped enum solve the\n> >> problem in a better way ? I presume if we didn't use them, there might\n> >> be a reason I'm now missing.\n> >>\n> >> Wouldn't\n> >> case controls:AeFlickerMode::Off\n> >>\n> >> be nicer ?\n> > \n> > I quite like that.\n> > \n> >> Yes, it requires you to specify the whole namespace, but that's almost\n> >> the case already with 'controls::AeFlickerModeOff'\n> >>\n> >> But I understand that:\n> >>\n> >> enum class AeFlickerMode {\n> >> Off,\n> >> On,\n> >> };\n> >>\n> >>\n> >> Would indeed conflic with the control name ('AeFlickerMode' in this\n> >> case)\n> > \n> > The following code compiles:\n> > \n> > ----\n> > enum class Foo {\n> > \tOn,\n> > \tOff,\n> > \tAuto,\n> > };\n> > \n> > static int Foo = 42;\n> > \n> > int main()\n> > {\n> > \tFoo = static_cast(Foo::On);\n> > \n> > \treturn Foo;\n> > }\n> > ----\n> > \n> > Barnabás, are you aware of issues this could cause ?\n> \n> Not an issue per se, but one will have to use elaborated type specifiers\n> in some cases, e.g. to declare a variable of the type. Same situation as\n> with `struct stat` vs `stat()`.\n> \n> https://en.cppreference.com/w/cpp/language/elaborated_type_specifier.html\n\nA bit inconvenient, but maybe not the end of the world.\n\n> >> One possibly naive way of avoiding this would be to make the scoped\n> >> enum names as \"Modes\" by appending an 's' to the enumeration type\n> >> name:\n> >>\n> >> enum class AeFlickerModes {\n> >> Off,\n> >> On,\n> >> };\n> >>\n> >> Not sure it's much nicer\n> \n> That is also an option, yes. My main concern wrt. scoped enumerations is the\n> interaction with controls. Enums are saved as `int32_t`, which is not an issue,\n> but they are also returned as such. This is not an issue for unscoped enumerations\n> because of implicit casting. But for scoped enumerations, that would make things\n> quite a bit more inconvenient.\n> \n> So I think I would only want to go with scoped enums if the control parts are\n> extended to directly support them. E.g. something like\n> \n> Control AeFlickerMode;\n> // instead of Control AeFlickerMode\n> \n> but I am not yet sure about the implications of such change.\n\nThis may be more problematic indeed. Would you give it a try to find the\nimplications ? I feel it will be either relative simple to handle, or a\ncomplete nightmare.\n\n> >>> because the names would conflict. And this is why the enumerators are prefixed\n> >>> with the name of the enumeration. But this only really makes sense for something\n> >>> like C++ and not python or gstreamer, where things would not conflict. However,\n> >>> the prefixed name is what is in the \"source of truth\" yaml files, which necessitates\n> >>> workarounds like stripping the common prefix in e.g. the python parts. Having the\n> >>> \"unprefixed\" name in the source yaml files would address this, and also give a\n> >>> consistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n> >>>\n> >>>>> However, for e.g. language bindings, it might be more desirable to\n> >>>>> have access to the the unprefixed name. (This is even the case for\n> >>>>> C++ scoped enumerations.)\n> >>>>>\n> >>>>> Currently, both the gstreamer and python bindings have extra code\n> >>>>> to strip the common prefix. So instead of doing that separately in\n> >>>>> every binding, etc. store the unprefixed name in the source of truth,\n> >>>>> the control/property definition yaml files. This affect all C++, python,\n> >>>>> gstreamer generated code.\n> >>>>>\n> >>>>> This is an API break, but it only affects C++ because gst and py already\n> >>>>> strip the common prefix (TODO: recheck again). And in case of C++ it is\n> >>\n> >> How should we re-check ?\n> \n> Unfortunately I don't remember what I had in mind. I think what I meant is going\n> through the generated gst and py code and checking enumerator names to see if\n> the prefix is indeed removed.\n> \n> >>>>> only an API break for enum controls where the common prefix is not the\n> >>>>> same as the control name:\n> >>>>\n> >>>> This sounds reasonable to me.\n> >>>>\n> >>>>> * properties::Location\n> >>>>> * properties::draft::ColorFilterArrangement\n> >>>>> * controls::AwbMode\n> >>>>> * controls::AeConstraintMode\n> >>>>> * controls::AeFlickerMode\n> >>>>> * controls::AeMeteringMode\n> >>>>> * controls::AeExposureMode\n> >>>>> * controls::draft::ColorCorrectionAberrationMode\n> >>>>> * controls::WdrMode\n> >> };\n> >>>>> * TODO: check again\n> >>\n> >> I see AwbState too:\n> >>\n> >> enum AwbStateEnum {\n> >> \tAwbStateInactive = 0,\n> >> \tAwbStateSearching = 1,\n> >> -\tAwbConverged = 2,\n> >> -\tAwbLocked = 3,\n> >> +\tAwbStateConverged = 2,\n> >> +\tAwbStateLocked = 3,\n> >> };\n> \n> Oops, you're right, there is that as well. In the end I made a script to check:\n> \n> ```py\n> import argparse\n> import yaml\n> \n> def main():\n> \tparser = argparse.ArgumentParser()\n> \tparser.add_argument(\"--old\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n> \tparser.add_argument(\"--new\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n> \targs = parser.parse_args()\n> \n> \told_enums = {}\n> \n> \tfor f in args.old:\n> \t\tc = yaml.safe_load(f)\n> \t\tv = c.get(\"vendor\")\n> \t\tfor x in c.get(\"controls\"):\n> \t\t\tfor (y, z) in x.items():\n> \t\t\t\tes = z.get(\"enum\")\n> \t\t\t\tif not es:\n> \t\t\t\t\tcontinue\n> \n> \t\t\t\tfor e in es:\n> \t\t\t\t\tvals = old_enums.setdefault((v, y), {})\n> \t\t\t\t\tvals[e[\"value\"]] = e[\"name\"]\n> \n> \tchanges = {}\n> \n> \tfor f in args.new:\n> \t\tc = yaml.safe_load(f)\n> \t\tv = c.get(\"vendor\")\n> \t\tfor x in c.get(\"controls\"):\n> \t\t\tfor (y, z) in x.items():\n> \t\t\t\tes = z.get(\"enum\")\n> \t\t\t\tif not es:\n> \t\t\t\t\tcontinue\n> \n> \t\t\t\tvals = old_enums[(v, y)]\n> \t\t\t\tassert len(vals) == len(es)\n> \n> \t\t\t\tfor e in es:\n> \t\t\t\t\told_name = vals[e[\"value\"]]\n> \t\t\t\t\tif old_name != y + e[\"name\"]:\n> \t\t\t\t\t\tchanges.setdefault(v, {}).setdefault(y, []).append((old_name, e[\"name\"]))\n> \n> \tfor (v, cs) in changes.items():\n> \t\tprint(v)\n> \t\tfor (c, vs) in cs.items():\n> \t\t\tprint(\"\\t\", c, sep=\"\")\n> \t\t\tfor (o, n) in vs:\n> \t\t\t\tprint(\"\\t\\t\", o, \" -> \", c + n, sep=\"\")\n> \n> if __name__ == \"__main__\":\n> \tmain()\n> ```\n> \n> which shows:\n> \n> ```\n> $ python asd.py --old src/libcamera/control_ids_core.yaml --old src/libcamera/control_ids_draft.yaml --old src/libcamera/control_ids_rpi.yaml --new ../libcamera/src/libcamera/control_ids_core.yaml --new ../libcamera/src/libcamera/control_ids_draft.yaml --new ../libcamera/src/libcamera/control_ids_rpi.yaml\n> libcamera\n> \t AeMeteringMode\n> \t\t MeteringCentreWeighted -> AeMeteringModeCentreWeighted\n> \t\t MeteringSpot -> AeMeteringModeSpot\n> \t\t MeteringMatrix -> AeMeteringModeMatrix\n> \t\t MeteringCustom -> AeMeteringModeCustom\n> \t AeConstraintMode\n> \t\t ConstraintNormal -> AeConstraintModeNormal\n> \t\t ConstraintHighlight -> AeConstraintModeHighlight\n> \t\t ConstraintShadows -> AeConstraintModeShadows\n> \t\t ConstraintCustom -> AeConstraintModeCustom\n> \t AeExposureMode\n> \t\t ExposureNormal -> AeExposureModeNormal\n> \t\t ExposureShort -> AeExposureModeShort\n> \t\t ExposureLong -> AeExposureModeLong\n> \t\t ExposureCustom -> AeExposureModeCustom\n> \t AeFlickerMode\n> \t\t FlickerOff -> AeFlickerModeOff\n> \t\t FlickerManual -> AeFlickerModeManual\n> \t\t FlickerAuto -> AeFlickerModeAuto\n> \t AwbMode\n> \t\t AwbAuto -> AwbModeAuto\n> \t\t AwbIncandescent -> AwbModeIncandescent\n> \t\t AwbTungsten -> AwbModeTungsten\n> \t\t AwbFluorescent -> AwbModeFluorescent\n> \t\t AwbIndoor -> AwbModeIndoor\n> \t\t AwbDaylight -> AwbModeDaylight\n> \t\t AwbCloudy -> AwbModeCloudy\n> \t\t AwbCustom -> AwbModeCustom\n> \t WdrMode\n> \t\t WdrOff -> WdrModeOff\n> \t\t WdrLinear -> WdrModeLinear\n> \t\t WdrPower -> WdrModePower\n> \t\t WdrExponential -> WdrModeExponential\n> \t\t WdrHistogramEqualization -> WdrModeHistogramEqualization\n> draft\n> \t ColorCorrectionAberrationMode\n> \t\t ColorCorrectionAberrationOff -> ColorCorrectionAberrationModeOff\n> \t\t ColorCorrectionAberrationFast -> ColorCorrectionAberrationModeFast\n> \t\t ColorCorrectionAberrationHighQuality -> ColorCorrectionAberrationModeHighQuality\n> \t AwbState\n> \t\t AwbConverged -> AwbStateConverged\n> \t\t AwbLocked -> AwbStateLocked\n> ```\n> \n> and\n> \n> ```\n> $ asd.py --old src/libcamera/property_ids_core.yaml --old src/libcamera/property_ids_draft.yaml --new ../libcamera/src/libcamera/property_ids_core.yaml --new ../libcamera/src/libcamera/property_ids_draft.yaml\n> libcamera\n> \t Location\n> \t\t CameraLocationFront -> LocationFront\n> \t\t CameraLocationBack -> LocationBack\n> \t\t CameraLocationExternal -> LocationExternal\n> draft\n> \t ColorFilterArrangement\n> \t\t RGGB -> ColorFilterArrangementRGGB\n> \t\t GRBG -> ColorFilterArrangementGRBG\n> \t\t GBRG -> ColorFilterArrangementGBRG\n> \t\t BGGR -> ColorFilterArrangementBGGR\n> \t\t RGB -> ColorFilterArrangementRGB\n> \t\t MONO -> ColorFilterArrangementMONO\n> ```\n> \n> so I think it was only `controls::draft::AwbState` that I missed.\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> >>\n> >>>>>\n> >>>>> Additionally, in some cases the corresponding `*NameValueMap` objects\n> >>>>> are used to parse configuration files, these are also affected:\n> >>>>>\n> >>>>> * ipa::AgcMeanLuminance::parseConstraintModes\n> >>>>> * ipa::AwbAlgorithm::parseModeConfigs\n> >>>>> * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n> >>>>> * ConfigParser::parseLocation\n> >>>>>\n> >>>>> Signed-off-by: Barnabás Pőcze \n> >>>>> Reviewed-by: Paul Elder \n> >>>>> ---\n> >>>>> More testing and review would be beneficial still.\n> >>>>>\n> >>>> ...\n> >>>>\n> >>>>> @@ -1170,13 +1170,13 @@ controls:\n> >>>>> \\sa HdrChannel\n> >>>>>\n> >>>>> enum:\n> >>>>> - - name: HdrModeOff\n> >>>>> + - name: \"Off\"\n> >>>>\n> >>>> What happens here ? Why is this one \"quoted\".\n> >>>\n> >>> https://hitchdev.com/strictyaml/why/implicit-typing-removed/\n> >>>\n> >>> The plain word `Off` is considered to be the boolean value false:\n> >>>\n> >>> $ python\n> >>> Python 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\n> >>> Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n> >>>>>> import yaml\n> >>>>>> yaml.safe_load(\"name: Off\")\n> >>> {'name': False}\n> >>\n> >> Scaryyyy\n> > \n> > YAML is full of \"interesting\" design decisions.\n> > \n> >>>> Should all name: entries be quoted ?\n> >>>>\n> >>>>> value: 0\n> >>>>> description: |\n> >>>>> HDR is disabled.\n> >>>>>\n> >>>>> Metadata for this frame will not include the HdrChannel control.\n> >>>>> - - name: HdrModeMultiExposureUnmerged\n> >>>>> + - name: MultiExposureUnmerged\n> >>>>> value: 1\n> >>>>> description: |\n> >>>>> Multiple exposures will be generated in an alternating fashion.\n> >>>>> @@ -1188,7 +1188,7 @@ controls:\n> >>>>>\n> >>>>> The expectation is that an application using this mode would merge\n> >>>>> the frames to create HDR images for itself if it requires them.\n> >>>>> - - name: HdrModeMultiExposure\n> >>>>> + - name: MultiExposure\n> >>>>> value: 2\n> >>>>> description: |\n> >>>>> Multiple exposures will be generated and merged to create HDR\n> >>>>> @@ -1201,7 +1201,7 @@ controls:\n> >>>>> alternately as the short and long channel. Systems that use three\n> >>>>> channels for HDR will cycle through the short, medium and long\n> >>>>> channel before repeating.\n> >>>>> - - name: HdrModeSingleExposure\n> >>>>> + - name: SingleExposure\n> >>>>> value: 3\n> >>>>> description: |\n> >>>>> Multiple frames all at a single exposure will be used to create HDR\n> >>>>> @@ -1209,7 +1209,7 @@ controls:\n> >>>>>\n> >>>>> These images should be reported as all corresponding to the HDR\n> >>>>> short channel.\n> >>>>> - - name: HdrModeNight\n> >>>>> + - name: Night\n> >>>>> value: 4\n> >>>>> description: |\n> >>>>> Multiple frames will be combined to produce \"night mode\" images.\n> >>>>> @@ -1235,20 +1235,20 @@ controls:\n> >>>>> \\sa HdrMode\n> >>>>>\n> >>>>> enum:\n> >>>>> - - name: HdrChannelNone\n> >>>>> + - name: None\n> >>>>> value: 0\n> >>>>> description: |\n> >>>>> This image does not correspond to any of the captures used to create\n> >>>>> an HDR image.\n> >>>>> - - name: HdrChannelShort\n> >>>>> + - name: Short\n> >>>>> value: 1\n> >>>>> description: |\n> >>>>> This is a short exposure image.\n> >>>>> - - name: HdrChannelMedium\n> >>>>> + - name: Medium\n> >>>>> value: 2\n> >>>>> description: |\n> >>>>> This is a medium exposure image.\n> >>>>> - - name: HdrChannelLong\n> >>>>> + - name: Long\n> >>>>> value: 3\n> >>>>> description: |\n> >>>>> This is a long exposure image.\n> >>>>> @@ -1295,17 +1295,17 @@ controls:\n> >>>>> The algorithm then compensates for the loss of brightness by applying a\n> >>>>> global tone mapping curve to the image.\n> >>>>> enum:\n> >>>>> - - name: WdrOff\n> >>>>> + - name: \"Off\"\n> >>>>\n> >>>> Do they 'collide' otherwise ?\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 65656C3241\n\tfor ;\n\tMon, 3 Nov 2025 12:31:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 903AC60A80;\n\tMon, 3 Nov 2025 13:31:46 +0100 (CET)","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 D9243606A0\n\tfor ;\n\tMon, 3 Nov 2025 13:31:44 +0100 (CET)","from pendragon.ideasonboard.com (82-203-160-149.bb.dnainternet.fi\n\t[82.203.160.149])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 8ACB499F;\n\tMon, 3 Nov 2025 13:29:51 +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=\"h+Mi9JNu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762172991;\n\tbh=xHu8KI212dqD/xIxAe8O27Sy0a7P4Yp2YPA7LbGcPhQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=h+Mi9JNuV6/nN47VRvp/6zVMvEWr3wizgA1IHhsMFHTWncfnBuh0Vt9QjJTm8btma\n\tSr+3u8Zvr1litZDd3V28YhJb1BIvvx2g6J1mIypKA7xw6wlilTbpwErLucY4TYgQuE\n\tpy+PS4EaujeBDEQo54HsOqdvtRaH9xWTYDOKAr2w=","Date":"Mon, 3 Nov 2025 14:31:30 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi ,\n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org,\n\tPaul Elder ","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","Message-ID":"<20251103123130.GU27255@pendragon.ideasonboard.com>","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>\n\t\n\t\n\t<20251102161251.GC27255@pendragon.ideasonboard.com>\n\t<01597fab-88fb-4d22-a4de-9c4cf635a65c@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<01597fab-88fb-4d22-a4de-9c4cf635a65c@ideasonboard.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":36664,"web_url":"https://patchwork.libcamera.org/comment/36664/","msgid":"","date":"2025-11-03T14:52:45","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 03. 13:31 keltezéssel, Laurent Pinchart írta:\n> On Mon, Nov 03, 2025 at 12:11:59PM +0100, Barnabás Pőcze wrote:\n>> 2025. 11. 02. 17:12 keltezéssel, Laurent Pinchart írta:\n>>> On Sat, Nov 01, 2025 at 05:17:53PM +0100, Jacopo Mondi wrote:\n>>>> On Tue, Sep 30, 2025 at 01:58:45PM +0200, Barnabás Pőcze wrote:\n>>>>> 2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n>>>>>> Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n>>>>>>> At the moment all enumerators have a common prefix, in many cases\n>>>>>>> the name of the control, but not always. This is reasonable for\n>>>>>>> C++ because currently non-scoped enumerations are used, so some\n>>>>>>> kind of prefix is needed to differentiate common names like `Auto`,\n>>>>>>> `Manual`, `On`, `Off`, etc.\n>>>>>>\n>>>>>> How does this patch prevent those collisions ?\n>>>>>\n>>>>> Sorry, maybe I wasn't clear. I meant that we cannot do\n>>>>>\n>>>>> enum SomeFeature { On, Off, Auto };\n>>>\n>>> This reminds me of https://thedailywtf.com/articles/What_Is_Truth_0x3f_\n>>>\n>>>>> enum OtherFeature { On, Off, Auto };\n>>>>\n>>>> Is the whole purpose of the patch to remove the \"strip common prefix\"\n>>>> from the Python and gstreamer bindings ? If that was just that I\n>>\n>> Yes. Not only are they differently implemented at the moment, they also need\n>> special hacks to strip the common prefix: e.g. https://patchwork.libcamera.org/patch/24251/\n>>\n>>>> wouldn't say it's worth it the longer names in C++, but I presume\n>>>> we'll have more bindings so this becomes more critical.\n>>\n>> I think the consistency is worth - in my opinion - the minor inconvenience\n>> of longer names in C++.\n>>\n>>>>\n>>>> The thing I'm not sure about is that if it's desirable to have even\n>>>> longer names when referring to the enumerated value from C++ code\n>>>>\n>>>> - case controls::FlickerOff:\n>>>> + case controls::AeFlickerModeOff:\n>>>>\n>>>> as an example result of this change\n>>>>\n>>>> Maybe I'm missing something, but wouldn't scoped enum solve the\n>>>> problem in a better way ? I presume if we didn't use them, there might\n>>>> be a reason I'm now missing.\n>>>>\n>>>> Wouldn't\n>>>> case controls:AeFlickerMode::Off\n>>>>\n>>>> be nicer ?\n>>>\n>>> I quite like that.\n>>>\n>>>> Yes, it requires you to specify the whole namespace, but that's almost\n>>>> the case already with 'controls::AeFlickerModeOff'\n>>>>\n>>>> But I understand that:\n>>>>\n>>>> enum class AeFlickerMode {\n>>>> Off,\n>>>> On,\n>>>> };\n>>>>\n>>>>\n>>>> Would indeed conflic with the control name ('AeFlickerMode' in this\n>>>> case)\n>>>\n>>> The following code compiles:\n>>>\n>>> ----\n>>> enum class Foo {\n>>> \tOn,\n>>> \tOff,\n>>> \tAuto,\n>>> };\n>>>\n>>> static int Foo = 42;\n>>>\n>>> int main()\n>>> {\n>>> \tFoo = static_cast(Foo::On);\n>>>\n>>> \treturn Foo;\n>>> }\n>>> ----\n>>>\n>>> Barnabás, are you aware of issues this could cause ?\n>>\n>> Not an issue per se, but one will have to use elaborated type specifiers\n>> in some cases, e.g. to declare a variable of the type. Same situation as\n>> with `struct stat` vs `stat()`.\n>>\n>> https://en.cppreference.com/w/cpp/language/elaborated_type_specifier.html\n> \n> A bit inconvenient, but maybe not the end of the world.\n> \n>>>> One possibly naive way of avoiding this would be to make the scoped\n>>>> enum names as \"Modes\" by appending an 's' to the enumeration type\n>>>> name:\n>>>>\n>>>> enum class AeFlickerModes {\n>>>> Off,\n>>>> On,\n>>>> };\n>>>>\n>>>> Not sure it's much nicer\n>>\n>> That is also an option, yes. My main concern wrt. scoped enumerations is the\n>> interaction with controls. Enums are saved as `int32_t`, which is not an issue,\n>> but they are also returned as such. This is not an issue for unscoped enumerations\n>> because of implicit casting. But for scoped enumerations, that would make things\n>> quite a bit more inconvenient.\n>>\n>> So I think I would only want to go with scoped enums if the control parts are\n>> extended to directly support them. E.g. something like\n>>\n>> Control AeFlickerMode;\n>> // instead of Control AeFlickerMode\n>>\n>> but I am not yet sure about the implications of such change.\n> \n> This may be more problematic indeed. Would you give it a try to find the\n> implications ? I feel it will be either relative simple to handle, or a\n> complete nightmare.\n\nAs a first approximation I tried to replace int32_t with the unscoped\nenumeration type.\n\nThe fact that bare integers can no longer be used with `ControlList::set()`\nnecessitates a quite a few adjustments.\n\nThere is also the `ControlId::enumerators()` map, which presents an issue since\nit has to use `int32_t` so that enumerators can be accessed in a type-erased fashion,\nbut they are also used to parse configuration files, etc. where the proper type would\nbe important.\n\nThere are also multiple places where the `ControlValue::get()` is used to retrieve\nthe enumerator. The good news is that they can be changed without any dependency on this change.\n\nThen there are the gst parts, which need a couple adjustments. The python parts compile\nwithout any changes (although I have not tested if they work).\n\nUsing scoped enumeration types most likely requires even more casting.\n\nIn any case, I have sent two patches that are prerequisites, but I consider them\nsomewhat useful in any case.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>>>>> because the names would conflict. And this is why the enumerators are prefixed\n>>>>> with the name of the enumeration. But this only really makes sense for something\n>>>>> like C++ and not python or gstreamer, where things would not conflict. However,\n>>>>> the prefixed name is what is in the \"source of truth\" yaml files, which necessitates\n>>>>> workarounds like stripping the common prefix in e.g. the python parts. Having the\n>>>>> \"unprefixed\" name in the source yaml files would address this, and also give a\n>>>>> consistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n>>>>>\n>>>>>>> However, for e.g. language bindings, it might be more desirable to\n>>>>>>> have access to the the unprefixed name. (This is even the case for\n>>>>>>> C++ scoped enumerations.)\n>>>>>>>\n>>>>>>> Currently, both the gstreamer and python bindings have extra code\n>>>>>>> to strip the common prefix. So instead of doing that separately in\n>>>>>>> every binding, etc. store the unprefixed name in the source of truth,\n>>>>>>> the control/property definition yaml files. This affect all C++, python,\n>>>>>>> gstreamer generated code.\n>>>>>>>\n>>>>>>> This is an API break, but it only affects C++ because gst and py already\n>>>>>>> strip the common prefix (TODO: recheck again). And in case of C++ it is\n>>>>\n>>>> How should we re-check ?\n>>\n>> Unfortunately I don't remember what I had in mind. I think what I meant is going\n>> through the generated gst and py code and checking enumerator names to see if\n>> the prefix is indeed removed.\n>>\n>>>>>>> only an API break for enum controls where the common prefix is not the\n>>>>>>> same as the control name:\n>>>>>>\n>>>>>> This sounds reasonable to me.\n>>>>>>\n>>>>>>> * properties::Location\n>>>>>>> * properties::draft::ColorFilterArrangement\n>>>>>>> * controls::AwbMode\n>>>>>>> * controls::AeConstraintMode\n>>>>>>> * controls::AeFlickerMode\n>>>>>>> * controls::AeMeteringMode\n>>>>>>> * controls::AeExposureMode\n>>>>>>> * controls::draft::ColorCorrectionAberrationMode\n>>>>>>> * controls::WdrMode\n>>>> };\n>>>>>>> * TODO: check again\n>>>>\n>>>> I see AwbState too:\n>>>>\n>>>> enum AwbStateEnum {\n>>>> \tAwbStateInactive = 0,\n>>>> \tAwbStateSearching = 1,\n>>>> -\tAwbConverged = 2,\n>>>> -\tAwbLocked = 3,\n>>>> +\tAwbStateConverged = 2,\n>>>> +\tAwbStateLocked = 3,\n>>>> };\n>>\n>> Oops, you're right, there is that as well. In the end I made a script to check:\n>>\n>> ```py\n>> import argparse\n>> import yaml\n>>\n>> def main():\n>> \tparser = argparse.ArgumentParser()\n>> \tparser.add_argument(\"--old\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n>> \tparser.add_argument(\"--new\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n>> \targs = parser.parse_args()\n>>\n>> \told_enums = {}\n>>\n>> \tfor f in args.old:\n>> \t\tc = yaml.safe_load(f)\n>> \t\tv = c.get(\"vendor\")\n>> \t\tfor x in c.get(\"controls\"):\n>> \t\t\tfor (y, z) in x.items():\n>> \t\t\t\tes = z.get(\"enum\")\n>> \t\t\t\tif not es:\n>> \t\t\t\t\tcontinue\n>>\n>> \t\t\t\tfor e in es:\n>> \t\t\t\t\tvals = old_enums.setdefault((v, y), {})\n>> \t\t\t\t\tvals[e[\"value\"]] = e[\"name\"]\n>>\n>> \tchanges = {}\n>>\n>> \tfor f in args.new:\n>> \t\tc = yaml.safe_load(f)\n>> \t\tv = c.get(\"vendor\")\n>> \t\tfor x in c.get(\"controls\"):\n>> \t\t\tfor (y, z) in x.items():\n>> \t\t\t\tes = z.get(\"enum\")\n>> \t\t\t\tif not es:\n>> \t\t\t\t\tcontinue\n>>\n>> \t\t\t\tvals = old_enums[(v, y)]\n>> \t\t\t\tassert len(vals) == len(es)\n>>\n>> \t\t\t\tfor e in es:\n>> \t\t\t\t\told_name = vals[e[\"value\"]]\n>> \t\t\t\t\tif old_name != y + e[\"name\"]:\n>> \t\t\t\t\t\tchanges.setdefault(v, {}).setdefault(y, []).append((old_name, e[\"name\"]))\n>>\n>> \tfor (v, cs) in changes.items():\n>> \t\tprint(v)\n>> \t\tfor (c, vs) in cs.items():\n>> \t\t\tprint(\"\\t\", c, sep=\"\")\n>> \t\t\tfor (o, n) in vs:\n>> \t\t\t\tprint(\"\\t\\t\", o, \" -> \", c + n, sep=\"\")\n>>\n>> if __name__ == \"__main__\":\n>> \tmain()\n>> ```\n>>\n>> which shows:\n>>\n>> ```\n>> $ python asd.py --old src/libcamera/control_ids_core.yaml --old src/libcamera/control_ids_draft.yaml --old src/libcamera/control_ids_rpi.yaml --new ../libcamera/src/libcamera/control_ids_core.yaml --new ../libcamera/src/libcamera/control_ids_draft.yaml --new ../libcamera/src/libcamera/control_ids_rpi.yaml\n>> libcamera\n>> \t AeMeteringMode\n>> \t\t MeteringCentreWeighted -> AeMeteringModeCentreWeighted\n>> \t\t MeteringSpot -> AeMeteringModeSpot\n>> \t\t MeteringMatrix -> AeMeteringModeMatrix\n>> \t\t MeteringCustom -> AeMeteringModeCustom\n>> \t AeConstraintMode\n>> \t\t ConstraintNormal -> AeConstraintModeNormal\n>> \t\t ConstraintHighlight -> AeConstraintModeHighlight\n>> \t\t ConstraintShadows -> AeConstraintModeShadows\n>> \t\t ConstraintCustom -> AeConstraintModeCustom\n>> \t AeExposureMode\n>> \t\t ExposureNormal -> AeExposureModeNormal\n>> \t\t ExposureShort -> AeExposureModeShort\n>> \t\t ExposureLong -> AeExposureModeLong\n>> \t\t ExposureCustom -> AeExposureModeCustom\n>> \t AeFlickerMode\n>> \t\t FlickerOff -> AeFlickerModeOff\n>> \t\t FlickerManual -> AeFlickerModeManual\n>> \t\t FlickerAuto -> AeFlickerModeAuto\n>> \t AwbMode\n>> \t\t AwbAuto -> AwbModeAuto\n>> \t\t AwbIncandescent -> AwbModeIncandescent\n>> \t\t AwbTungsten -> AwbModeTungsten\n>> \t\t AwbFluorescent -> AwbModeFluorescent\n>> \t\t AwbIndoor -> AwbModeIndoor\n>> \t\t AwbDaylight -> AwbModeDaylight\n>> \t\t AwbCloudy -> AwbModeCloudy\n>> \t\t AwbCustom -> AwbModeCustom\n>> \t WdrMode\n>> \t\t WdrOff -> WdrModeOff\n>> \t\t WdrLinear -> WdrModeLinear\n>> \t\t WdrPower -> WdrModePower\n>> \t\t WdrExponential -> WdrModeExponential\n>> \t\t WdrHistogramEqualization -> WdrModeHistogramEqualization\n>> draft\n>> \t ColorCorrectionAberrationMode\n>> \t\t ColorCorrectionAberrationOff -> ColorCorrectionAberrationModeOff\n>> \t\t ColorCorrectionAberrationFast -> ColorCorrectionAberrationModeFast\n>> \t\t ColorCorrectionAberrationHighQuality -> ColorCorrectionAberrationModeHighQuality\n>> \t AwbState\n>> \t\t AwbConverged -> AwbStateConverged\n>> \t\t AwbLocked -> AwbStateLocked\n>> ```\n>>\n>> and\n>>\n>> ```\n>> $ asd.py --old src/libcamera/property_ids_core.yaml --old src/libcamera/property_ids_draft.yaml --new ../libcamera/src/libcamera/property_ids_core.yaml --new ../libcamera/src/libcamera/property_ids_draft.yaml\n>> libcamera\n>> \t Location\n>> \t\t CameraLocationFront -> LocationFront\n>> \t\t CameraLocationBack -> LocationBack\n>> \t\t CameraLocationExternal -> LocationExternal\n>> draft\n>> \t ColorFilterArrangement\n>> \t\t RGGB -> ColorFilterArrangementRGGB\n>> \t\t GRBG -> ColorFilterArrangementGRBG\n>> \t\t GBRG -> ColorFilterArrangementGBRG\n>> \t\t BGGR -> ColorFilterArrangementBGGR\n>> \t\t RGB -> ColorFilterArrangementRGB\n>> \t\t MONO -> ColorFilterArrangementMONO\n>> ```\n>>\n>> so I think it was only `controls::draft::AwbState` that I missed.\n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>>>\n>>>>>>>\n>>>>>>> Additionally, in some cases the corresponding `*NameValueMap` objects\n>>>>>>> are used to parse configuration files, these are also affected:\n>>>>>>>\n>>>>>>> * ipa::AgcMeanLuminance::parseConstraintModes\n>>>>>>> * ipa::AwbAlgorithm::parseModeConfigs\n>>>>>>> * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n>>>>>>> * ConfigParser::parseLocation\n>>>>>>>\n>>>>>>> Signed-off-by: Barnabás Pőcze \n>>>>>>> Reviewed-by: Paul Elder \n>>>>>>> ---\n>>>>>>> More testing and review would be beneficial still.\n>>>>>>>\n>>>>>> ...\n>>>>>>\n>>>>>>> @@ -1170,13 +1170,13 @@ controls:\n>>>>>>> \\sa HdrChannel\n>>>>>>>\n>>>>>>> enum:\n>>>>>>> - - name: HdrModeOff\n>>>>>>> + - name: \"Off\"\n>>>>>>\n>>>>>> What happens here ? Why is this one \"quoted\".\n>>>>>\n>>>>> https://hitchdev.com/strictyaml/why/implicit-typing-removed/\n>>>>>\n>>>>> The plain word `Off` is considered to be the boolean value false:\n>>>>>\n>>>>> $ python\n>>>>> Python 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\n>>>>> Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n>>>>>>>> import yaml\n>>>>>>>> yaml.safe_load(\"name: Off\")\n>>>>> {'name': False}\n>>>>\n>>>> Scaryyyy\n>>>\n>>> YAML is full of \"interesting\" design decisions.\n>>>\n>>>>>> Should all name: entries be quoted ?\n>>>>>>\n>>>>>>> value: 0\n>>>>>>> description: |\n>>>>>>> HDR is disabled.\n>>>>>>>\n>>>>>>> Metadata for this frame will not include the HdrChannel control.\n>>>>>>> - - name: HdrModeMultiExposureUnmerged\n>>>>>>> + - name: MultiExposureUnmerged\n>>>>>>> value: 1\n>>>>>>> description: |\n>>>>>>> Multiple exposures will be generated in an alternating fashion.\n>>>>>>> @@ -1188,7 +1188,7 @@ controls:\n>>>>>>>\n>>>>>>> The expectation is that an application using this mode would merge\n>>>>>>> the frames to create HDR images for itself if it requires them.\n>>>>>>> - - name: HdrModeMultiExposure\n>>>>>>> + - name: MultiExposure\n>>>>>>> value: 2\n>>>>>>> description: |\n>>>>>>> Multiple exposures will be generated and merged to create HDR\n>>>>>>> @@ -1201,7 +1201,7 @@ controls:\n>>>>>>> alternately as the short and long channel. Systems that use three\n>>>>>>> channels for HDR will cycle through the short, medium and long\n>>>>>>> channel before repeating.\n>>>>>>> - - name: HdrModeSingleExposure\n>>>>>>> + - name: SingleExposure\n>>>>>>> value: 3\n>>>>>>> description: |\n>>>>>>> Multiple frames all at a single exposure will be used to create HDR\n>>>>>>> @@ -1209,7 +1209,7 @@ controls:\n>>>>>>>\n>>>>>>> These images should be reported as all corresponding to the HDR\n>>>>>>> short channel.\n>>>>>>> - - name: HdrModeNight\n>>>>>>> + - name: Night\n>>>>>>> value: 4\n>>>>>>> description: |\n>>>>>>> Multiple frames will be combined to produce \"night mode\" images.\n>>>>>>> @@ -1235,20 +1235,20 @@ controls:\n>>>>>>> \\sa HdrMode\n>>>>>>>\n>>>>>>> enum:\n>>>>>>> - - name: HdrChannelNone\n>>>>>>> + - name: None\n>>>>>>> value: 0\n>>>>>>> description: |\n>>>>>>> This image does not correspond to any of the captures used to create\n>>>>>>> an HDR image.\n>>>>>>> - - name: HdrChannelShort\n>>>>>>> + - name: Short\n>>>>>>> value: 1\n>>>>>>> description: |\n>>>>>>> This is a short exposure image.\n>>>>>>> - - name: HdrChannelMedium\n>>>>>>> + - name: Medium\n>>>>>>> value: 2\n>>>>>>> description: |\n>>>>>>> This is a medium exposure image.\n>>>>>>> - - name: HdrChannelLong\n>>>>>>> + - name: Long\n>>>>>>> value: 3\n>>>>>>> description: |\n>>>>>>> This is a long exposure image.\n>>>>>>> @@ -1295,17 +1295,17 @@ controls:\n>>>>>>> The algorithm then compensates for the loss of brightness by applying a\n>>>>>>> global tone mapping curve to the image.\n>>>>>>> enum:\n>>>>>>> - - name: WdrOff\n>>>>>>> + - name: \"Off\"\n>>>>>>\n>>>>>> Do they 'collide' otherwise ?\n>>>\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 867D3C3241\n\tfor ;\n\tMon, 3 Nov 2025 14:52:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1EEF60A80;\n\tMon, 3 Nov 2025 15:52:51 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D941606A0\n\tfor ;\n\tMon, 3 Nov 2025 15:52:49 +0100 (CET)","from [192.168.33.39] (185.221.140.239.nat.pool.zt.hu\n\t[185.221.140.239])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7ECFF7CE;\n\tMon, 3 Nov 2025 15:50:56 +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=\"Bius5KEH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762181456;\n\tbh=04Nt3g/A3aJip2KBOcAKbtWMygEgv5aBR5eRUsMv5Qs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Bius5KEHDEFJxw14xhz/HM/Qvu/WKMTyr0JH/MKjdD16tx17hMZtQWm3WQFMygWLH\n\tWERVG8VuCa7w4i25SOirJ416IDb5JHW63emqjoEt/JlP6KraOfvM7/Id4biV5taysW\n\tiKT2HP58M5NAUqtMAUvi482NFb4wdU/71tIHccwU=","Message-ID":"","Date":"Mon, 3 Nov 2025 15:52:45 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org,\n\tPaul Elder ","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>\n\t\n\t\n\t<20251102161251.GC27255@pendragon.ideasonboard.com>\n\t<01597fab-88fb-4d22-a4de-9c4cf635a65c@ideasonboard.com>\n\t<20251103123130.GU27255@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251103123130.GU27255@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36990,"web_url":"https://patchwork.libcamera.org/comment/36990/","msgid":"","date":"2025-11-21T13:51:36","subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\n resuming the discussion on this patch\n\nOn Mon, Nov 03, 2025 at 03:52:45PM +0100, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 11. 03. 13:31 keltezéssel, Laurent Pinchart írta:\n> > On Mon, Nov 03, 2025 at 12:11:59PM +0100, Barnabás Pőcze wrote:\n> > > 2025. 11. 02. 17:12 keltezéssel, Laurent Pinchart írta:\n> > > > On Sat, Nov 01, 2025 at 05:17:53PM +0100, Jacopo Mondi wrote:\n> > > > > On Tue, Sep 30, 2025 at 01:58:45PM +0200, Barnabás Pőcze wrote:\n> > > > > > 2025. 09. 30. 13:43 keltezéssel, Kieran Bingham írta:\n> > > > > > > Quoting Barnabás Pőcze (2025-09-30 11:32:07)\n> > > > > > > > At the moment all enumerators have a common prefix, in many cases\n> > > > > > > > the name of the control, but not always. This is reasonable for\n> > > > > > > > C++ because currently non-scoped enumerations are used, so some\n> > > > > > > > kind of prefix is needed to differentiate common names like `Auto`,\n> > > > > > > > `Manual`, `On`, `Off`, etc.\n> > > > > > >\n> > > > > > > How does this patch prevent those collisions ?\n> > > > > >\n> > > > > > Sorry, maybe I wasn't clear. I meant that we cannot do\n> > > > > >\n> > > > > > enum SomeFeature { On, Off, Auto };\n> > > >\n> > > > This reminds me of https://thedailywtf.com/articles/What_Is_Truth_0x3f_\n> > > >\n> > > > > > enum OtherFeature { On, Off, Auto };\n> > > > >\n> > > > > Is the whole purpose of the patch to remove the \"strip common prefix\"\n> > > > > from the Python and gstreamer bindings ? If that was just that I\n> > >\n> > > Yes. Not only are they differently implemented at the moment, they also need\n> > > special hacks to strip the common prefix: e.g. https://patchwork.libcamera.org/patch/24251/\n> > >\n> > > > > wouldn't say it's worth it the longer names in C++, but I presume\n> > > > > we'll have more bindings so this becomes more critical.\n> > >\n> > > I think the consistency is worth - in my opinion - the minor inconvenience\n> > > of longer names in C++.\n> > >\n> > > > >\n> > > > > The thing I'm not sure about is that if it's desirable to have even\n> > > > > longer names when referring to the enumerated value from C++ code\n> > > > >\n> > > > > - case controls::FlickerOff:\n> > > > > + case controls::AeFlickerModeOff:\n> > > > >\n> > > > > as an example result of this change\n> > > > >\n> > > > > Maybe I'm missing something, but wouldn't scoped enum solve the\n> > > > > problem in a better way ? I presume if we didn't use them, there might\n> > > > > be a reason I'm now missing.\n> > > > >\n> > > > > Wouldn't\n> > > > > case controls:AeFlickerMode::Off\n> > > > >\n> > > > > be nicer ?\n> > > >\n> > > > I quite like that.\n> > > >\n> > > > > Yes, it requires you to specify the whole namespace, but that's almost\n> > > > > the case already with 'controls::AeFlickerModeOff'\n> > > > >\n> > > > > But I understand that:\n> > > > >\n> > > > > enum class AeFlickerMode {\n> > > > > Off,\n> > > > > On,\n> > > > > };\n> > > > >\n> > > > >\n> > > > > Would indeed conflic with the control name ('AeFlickerMode' in this\n> > > > > case)\n> > > >\n> > > > The following code compiles:\n> > > >\n> > > > ----\n> > > > enum class Foo {\n> > > > \tOn,\n> > > > \tOff,\n> > > > \tAuto,\n> > > > };\n> > > >\n> > > > static int Foo = 42;\n> > > >\n> > > > int main()\n> > > > {\n> > > > \tFoo = static_cast(Foo::On);\n> > > >\n> > > > \treturn Foo;\n> > > > }\n> > > > ----\n> > > >\n> > > > Barnabás, are you aware of issues this could cause ?\n> > >\n> > > Not an issue per se, but one will have to use elaborated type specifiers\n> > > in some cases, e.g. to declare a variable of the type. Same situation as\n> > > with `struct stat` vs `stat()`.\n> > >\n> > > https://en.cppreference.com/w/cpp/language/elaborated_type_specifier.html\n> >\n> > A bit inconvenient, but maybe not the end of the world.\n> >\n> > > > > One possibly naive way of avoiding this would be to make the scoped\n> > > > > enum names as \"Modes\" by appending an 's' to the enumeration type\n> > > > > name:\n> > > > >\n> > > > > enum class AeFlickerModes {\n> > > > > Off,\n> > > > > On,\n> > > > > };\n> > > > >\n> > > > > Not sure it's much nicer\n> > >\n> > > That is also an option, yes. My main concern wrt. scoped enumerations is the\n> > > interaction with controls. Enums are saved as `int32_t`, which is not an issue,\n> > > but they are also returned as such. This is not an issue for unscoped enumerations\n> > > because of implicit casting. But for scoped enumerations, that would make things\n> > > quite a bit more inconvenient.\n> > >\n> > > So I think I would only want to go with scoped enums if the control parts are\n> > > extended to directly support them. E.g. something like\n> > >\n> > > Control AeFlickerMode;\n> > > // instead of Control AeFlickerMode\n> > >\n> > > but I am not yet sure about the implications of such change.\n> >\n> > This may be more problematic indeed. Would you give it a try to find the\n> > implications ? I feel it will be either relative simple to handle, or a\n> > complete nightmare.\n>\n> As a first approximation I tried to replace int32_t with the unscoped\n> enumeration type.\n>\n> The fact that bare integers can no longer be used with `ControlList::set()`\n> necessitates a quite a few adjustments.\n>\n> There is also the `ControlId::enumerators()` map, which presents an issue since\n> it has to use `int32_t` so that enumerators can be accessed in a type-erased fashion,\n> but they are also used to parse configuration files, etc. where the proper type would\n> be important.\n>\n> There are also multiple places where the `ControlValue::get()` is used to retrieve\n> the enumerator. The good news is that they can be changed without any dependency on this change.\n>\n> Then there are the gst parts, which need a couple adjustments. The python parts compile\n> without any changes (although I have not tested if they work).\n>\n> Using scoped enumeration types most likely requires even more casting.\n\nAre you still of the opinion, after your experiment, that scoped enum\nare not the preferred way forward and we should continue with the\napproach proposed in this patch ?\n\nIf that's the case, let's send a new version addressing the controls\nthat have been missed and removing the todos from the commit message.\n\nThanks\n j\n\n>\n> In any case, I have sent two patches that are prerequisites, but I consider them\n> somewhat useful in any case.\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n> >\n> > > > > > because the names would conflict. And this is why the enumerators are prefixed\n> > > > > > with the name of the enumeration. But this only really makes sense for something\n> > > > > > like C++ and not python or gstreamer, where things would not conflict. However,\n> > > > > > the prefixed name is what is in the \"source of truth\" yaml files, which necessitates\n> > > > > > workarounds like stripping the common prefix in e.g. the python parts. Having the\n> > > > > > \"unprefixed\" name in the source yaml files would address this, and also give a\n> > > > > > consistent prefix for all enumerators in C++ (it would always be the name of the enumeration).\n> > > > > >\n> > > > > > > > However, for e.g. language bindings, it might be more desirable to\n> > > > > > > > have access to the the unprefixed name. (This is even the case for\n> > > > > > > > C++ scoped enumerations.)\n> > > > > > > >\n> > > > > > > > Currently, both the gstreamer and python bindings have extra code\n> > > > > > > > to strip the common prefix. So instead of doing that separately in\n> > > > > > > > every binding, etc. store the unprefixed name in the source of truth,\n> > > > > > > > the control/property definition yaml files. This affect all C++, python,\n> > > > > > > > gstreamer generated code.\n> > > > > > > >\n> > > > > > > > This is an API break, but it only affects C++ because gst and py already\n> > > > > > > > strip the common prefix (TODO: recheck again). And in case of C++ it is\n> > > > >\n> > > > > How should we re-check ?\n> > >\n> > > Unfortunately I don't remember what I had in mind. I think what I meant is going\n> > > through the generated gst and py code and checking enumerator names to see if\n> > > the prefix is indeed removed.\n> > >\n> > > > > > > > only an API break for enum controls where the common prefix is not the\n> > > > > > > > same as the control name:\n> > > > > > >\n> > > > > > > This sounds reasonable to me.\n> > > > > > >\n> > > > > > > > * properties::Location\n> > > > > > > > * properties::draft::ColorFilterArrangement\n> > > > > > > > * controls::AwbMode\n> > > > > > > > * controls::AeConstraintMode\n> > > > > > > > * controls::AeFlickerMode\n> > > > > > > > * controls::AeMeteringMode\n> > > > > > > > * controls::AeExposureMode\n> > > > > > > > * controls::draft::ColorCorrectionAberrationMode\n> > > > > > > > * controls::WdrMode\n> > > > > };\n> > > > > > > > * TODO: check again\n> > > > >\n> > > > > I see AwbState too:\n> > > > >\n> > > > > enum AwbStateEnum {\n> > > > > \tAwbStateInactive = 0,\n> > > > > \tAwbStateSearching = 1,\n> > > > > -\tAwbConverged = 2,\n> > > > > -\tAwbLocked = 3,\n> > > > > +\tAwbStateConverged = 2,\n> > > > > +\tAwbStateLocked = 3,\n> > > > > };\n> > >\n> > > Oops, you're right, there is that as well. In the end I made a script to check:\n> > >\n> > > ```py\n> > > import argparse\n> > > import yaml\n> > >\n> > > def main():\n> > > \tparser = argparse.ArgumentParser()\n> > > \tparser.add_argument(\"--old\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n> > > \tparser.add_argument(\"--new\", type=argparse.FileType(\"rb\"), default=[], action=\"append\")\n> > > \targs = parser.parse_args()\n> > >\n> > > \told_enums = {}\n> > >\n> > > \tfor f in args.old:\n> > > \t\tc = yaml.safe_load(f)\n> > > \t\tv = c.get(\"vendor\")\n> > > \t\tfor x in c.get(\"controls\"):\n> > > \t\t\tfor (y, z) in x.items():\n> > > \t\t\t\tes = z.get(\"enum\")\n> > > \t\t\t\tif not es:\n> > > \t\t\t\t\tcontinue\n> > >\n> > > \t\t\t\tfor e in es:\n> > > \t\t\t\t\tvals = old_enums.setdefault((v, y), {})\n> > > \t\t\t\t\tvals[e[\"value\"]] = e[\"name\"]\n> > >\n> > > \tchanges = {}\n> > >\n> > > \tfor f in args.new:\n> > > \t\tc = yaml.safe_load(f)\n> > > \t\tv = c.get(\"vendor\")\n> > > \t\tfor x in c.get(\"controls\"):\n> > > \t\t\tfor (y, z) in x.items():\n> > > \t\t\t\tes = z.get(\"enum\")\n> > > \t\t\t\tif not es:\n> > > \t\t\t\t\tcontinue\n> > >\n> > > \t\t\t\tvals = old_enums[(v, y)]\n> > > \t\t\t\tassert len(vals) == len(es)\n> > >\n> > > \t\t\t\tfor e in es:\n> > > \t\t\t\t\told_name = vals[e[\"value\"]]\n> > > \t\t\t\t\tif old_name != y + e[\"name\"]:\n> > > \t\t\t\t\t\tchanges.setdefault(v, {}).setdefault(y, []).append((old_name, e[\"name\"]))\n> > >\n> > > \tfor (v, cs) in changes.items():\n> > > \t\tprint(v)\n> > > \t\tfor (c, vs) in cs.items():\n> > > \t\t\tprint(\"\\t\", c, sep=\"\")\n> > > \t\t\tfor (o, n) in vs:\n> > > \t\t\t\tprint(\"\\t\\t\", o, \" -> \", c + n, sep=\"\")\n> > >\n> > > if __name__ == \"__main__\":\n> > > \tmain()\n> > > ```\n> > >\n> > > which shows:\n> > >\n> > > ```\n> > > $ python asd.py --old src/libcamera/control_ids_core.yaml --old src/libcamera/control_ids_draft.yaml --old src/libcamera/control_ids_rpi.yaml --new ../libcamera/src/libcamera/control_ids_core.yaml --new ../libcamera/src/libcamera/control_ids_draft.yaml --new ../libcamera/src/libcamera/control_ids_rpi.yaml\n> > > libcamera\n> > > \t AeMeteringMode\n> > > \t\t MeteringCentreWeighted -> AeMeteringModeCentreWeighted\n> > > \t\t MeteringSpot -> AeMeteringModeSpot\n> > > \t\t MeteringMatrix -> AeMeteringModeMatrix\n> > > \t\t MeteringCustom -> AeMeteringModeCustom\n> > > \t AeConstraintMode\n> > > \t\t ConstraintNormal -> AeConstraintModeNormal\n> > > \t\t ConstraintHighlight -> AeConstraintModeHighlight\n> > > \t\t ConstraintShadows -> AeConstraintModeShadows\n> > > \t\t ConstraintCustom -> AeConstraintModeCustom\n> > > \t AeExposureMode\n> > > \t\t ExposureNormal -> AeExposureModeNormal\n> > > \t\t ExposureShort -> AeExposureModeShort\n> > > \t\t ExposureLong -> AeExposureModeLong\n> > > \t\t ExposureCustom -> AeExposureModeCustom\n> > > \t AeFlickerMode\n> > > \t\t FlickerOff -> AeFlickerModeOff\n> > > \t\t FlickerManual -> AeFlickerModeManual\n> > > \t\t FlickerAuto -> AeFlickerModeAuto\n> > > \t AwbMode\n> > > \t\t AwbAuto -> AwbModeAuto\n> > > \t\t AwbIncandescent -> AwbModeIncandescent\n> > > \t\t AwbTungsten -> AwbModeTungsten\n> > > \t\t AwbFluorescent -> AwbModeFluorescent\n> > > \t\t AwbIndoor -> AwbModeIndoor\n> > > \t\t AwbDaylight -> AwbModeDaylight\n> > > \t\t AwbCloudy -> AwbModeCloudy\n> > > \t\t AwbCustom -> AwbModeCustom\n> > > \t WdrMode\n> > > \t\t WdrOff -> WdrModeOff\n> > > \t\t WdrLinear -> WdrModeLinear\n> > > \t\t WdrPower -> WdrModePower\n> > > \t\t WdrExponential -> WdrModeExponential\n> > > \t\t WdrHistogramEqualization -> WdrModeHistogramEqualization\n> > > draft\n> > > \t ColorCorrectionAberrationMode\n> > > \t\t ColorCorrectionAberrationOff -> ColorCorrectionAberrationModeOff\n> > > \t\t ColorCorrectionAberrationFast -> ColorCorrectionAberrationModeFast\n> > > \t\t ColorCorrectionAberrationHighQuality -> ColorCorrectionAberrationModeHighQuality\n> > > \t AwbState\n> > > \t\t AwbConverged -> AwbStateConverged\n> > > \t\t AwbLocked -> AwbStateLocked\n> > > ```\n> > >\n> > > and\n> > >\n> > > ```\n> > > $ asd.py --old src/libcamera/property_ids_core.yaml --old src/libcamera/property_ids_draft.yaml --new ../libcamera/src/libcamera/property_ids_core.yaml --new ../libcamera/src/libcamera/property_ids_draft.yaml\n> > > libcamera\n> > > \t Location\n> > > \t\t CameraLocationFront -> LocationFront\n> > > \t\t CameraLocationBack -> LocationBack\n> > > \t\t CameraLocationExternal -> LocationExternal\n> > > draft\n> > > \t ColorFilterArrangement\n> > > \t\t RGGB -> ColorFilterArrangementRGGB\n> > > \t\t GRBG -> ColorFilterArrangementGRBG\n> > > \t\t GBRG -> ColorFilterArrangementGBRG\n> > > \t\t BGGR -> ColorFilterArrangementBGGR\n> > > \t\t RGB -> ColorFilterArrangementRGB\n> > > \t\t MONO -> ColorFilterArrangementMONO\n> > > ```\n> > >\n> > > so I think it was only `controls::draft::AwbState` that I missed.\n> > >\n> > >\n> > > Regards,\n> > > Barnabás Pőcze\n> > >\n> > > > >\n> > > > > > > >\n> > > > > > > > Additionally, in some cases the corresponding `*NameValueMap` objects\n> > > > > > > > are used to parse configuration files, these are also affected:\n> > > > > > > >\n> > > > > > > > * ipa::AgcMeanLuminance::parseConstraintModes\n> > > > > > > > * ipa::AwbAlgorithm::parseModeConfigs\n> > > > > > > > * ipa::rkisp1::algorithms::Agc::parseMeteringModes\n> > > > > > > > * ConfigParser::parseLocation\n> > > > > > > >\n> > > > > > > > Signed-off-by: Barnabás Pőcze \n> > > > > > > > Reviewed-by: Paul Elder \n> > > > > > > > ---\n> > > > > > > > More testing and review would be beneficial still.\n> > > > > > > >\n> > > > > > > ...\n> > > > > > >\n> > > > > > > > @@ -1170,13 +1170,13 @@ controls:\n> > > > > > > > \\sa HdrChannel\n> > > > > > > >\n> > > > > > > > enum:\n> > > > > > > > - - name: HdrModeOff\n> > > > > > > > + - name: \"Off\"\n> > > > > > >\n> > > > > > > What happens here ? Why is this one \"quoted\".\n> > > > > >\n> > > > > > https://hitchdev.com/strictyaml/why/implicit-typing-removed/\n> > > > > >\n> > > > > > The plain word `Off` is considered to be the boolean value false:\n> > > > > >\n> > > > > > $ python\n> > > > > > Python 3.13.7 (main, Aug 15 2025, 12:34:02) [GCC 15.2.1 20250813] on linux\n> > > > > > Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n> > > > > > > > > import yaml\n> > > > > > > > > yaml.safe_load(\"name: Off\")\n> > > > > > {'name': False}\n> > > > >\n> > > > > Scaryyyy\n> > > >\n> > > > YAML is full of \"interesting\" design decisions.\n> > > >\n> > > > > > > Should all name: entries be quoted ?\n> > > > > > >\n> > > > > > > > value: 0\n> > > > > > > > description: |\n> > > > > > > > HDR is disabled.\n> > > > > > > >\n> > > > > > > > Metadata for this frame will not include the HdrChannel control.\n> > > > > > > > - - name: HdrModeMultiExposureUnmerged\n> > > > > > > > + - name: MultiExposureUnmerged\n> > > > > > > > value: 1\n> > > > > > > > description: |\n> > > > > > > > Multiple exposures will be generated in an alternating fashion.\n> > > > > > > > @@ -1188,7 +1188,7 @@ controls:\n> > > > > > > >\n> > > > > > > > The expectation is that an application using this mode would merge\n> > > > > > > > the frames to create HDR images for itself if it requires them.\n> > > > > > > > - - name: HdrModeMultiExposure\n> > > > > > > > + - name: MultiExposure\n> > > > > > > > value: 2\n> > > > > > > > description: |\n> > > > > > > > Multiple exposures will be generated and merged to create HDR\n> > > > > > > > @@ -1201,7 +1201,7 @@ controls:\n> > > > > > > > alternately as the short and long channel. Systems that use three\n> > > > > > > > channels for HDR will cycle through the short, medium and long\n> > > > > > > > channel before repeating.\n> > > > > > > > - - name: HdrModeSingleExposure\n> > > > > > > > + - name: SingleExposure\n> > > > > > > > value: 3\n> > > > > > > > description: |\n> > > > > > > > Multiple frames all at a single exposure will be used to create HDR\n> > > > > > > > @@ -1209,7 +1209,7 @@ controls:\n> > > > > > > >\n> > > > > > > > These images should be reported as all corresponding to the HDR\n> > > > > > > > short channel.\n> > > > > > > > - - name: HdrModeNight\n> > > > > > > > + - name: Night\n> > > > > > > > value: 4\n> > > > > > > > description: |\n> > > > > > > > Multiple frames will be combined to produce \"night mode\" images.\n> > > > > > > > @@ -1235,20 +1235,20 @@ controls:\n> > > > > > > > \\sa HdrMode\n> > > > > > > >\n> > > > > > > > enum:\n> > > > > > > > - - name: HdrChannelNone\n> > > > > > > > + - name: None\n> > > > > > > > value: 0\n> > > > > > > > description: |\n> > > > > > > > This image does not correspond to any of the captures used to create\n> > > > > > > > an HDR image.\n> > > > > > > > - - name: HdrChannelShort\n> > > > > > > > + - name: Short\n> > > > > > > > value: 1\n> > > > > > > > description: |\n> > > > > > > > This is a short exposure image.\n> > > > > > > > - - name: HdrChannelMedium\n> > > > > > > > + - name: Medium\n> > > > > > > > value: 2\n> > > > > > > > description: |\n> > > > > > > > This is a medium exposure image.\n> > > > > > > > - - name: HdrChannelLong\n> > > > > > > > + - name: Long\n> > > > > > > > value: 3\n> > > > > > > > description: |\n> > > > > > > > This is a long exposure image.\n> > > > > > > > @@ -1295,17 +1295,17 @@ controls:\n> > > > > > > > The algorithm then compensates for the loss of brightness by applying a\n> > > > > > > > global tone mapping curve to the image.\n> > > > > > > > enum:\n> > > > > > > > - - name: WdrOff\n> > > > > > > > + - name: \"Off\"\n> > > > > > >\n> > > > > > > Do they 'collide' otherwise ?\n> > > >\n> > >\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 7BF17C3333\n\tfor ;\n\tFri, 21 Nov 2025 13:51:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9998260A8B;\n\tFri, 21 Nov 2025 14:51:42 +0100 (CET)","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 D79D160805\n\tfor ;\n\tFri, 21 Nov 2025 14:51:40 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 744BA66B;\n\tFri, 21 Nov 2025 14:49:34 +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=\"Xs+mt8gc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763732974;\n\tbh=MtzfWvLaue8bBbhGOd08h4xBHKq93eDR6dPzS+Mgf9g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Xs+mt8gc6IZOs7PXC8O3lrlajqxrxpsACqMNsxcPlA366o37itgOVZf54G460WgmS\n\toMYH5AY3jJWgu55HIQo+hb5MNdMXWk09BWe51x7Mq5uYvXSaVDTj0uBmOac9bIKlJt\n\tO9XKn7VxQmdiYVTNw3pjM7ehD16LsFlTwPed1v9A=","Date":"Fri, 21 Nov 2025 14:51:36 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart , \n\tJacopo Mondi ,\n\tKieran Bingham , \n\tlibcamera-devel@lists.libcamera.org,\n\tPaul Elder ","Subject":"Re: [RFC PATCH v3] libcamera: controls: Remove common enum prefix","Message-ID":"","References":"<20250930103207.626006-1-barnabas.pocze@ideasonboard.com>\n\t<175923261693.4000262.5040062523588319914@ping.linuxembedded.co.uk>\n\t\n\t\n\t<20251102161251.GC27255@pendragon.ideasonboard.com>\n\t<01597fab-88fb-4d22-a4de-9c4cf635a65c@ideasonboard.com>\n\t<20251103123130.GU27255@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","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\" "}}]