[{"id":23693,"web_url":"https://patchwork.libcamera.org/comment/23693/","msgid":"<165662318709.2049236.4764643638940438408@Monstersaurus>","date":"2022-06-30T21:06:27","subject":"Re: [libcamera-devel] [PATCH v3 03/23] libcamera: Introduce\n\tinternal controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:42)\n> Introduce the enumeration of internal controls in\n> internal_control_ids.yaml.\n> \n> Plumb in the build system the command to generate the definition of\n> internal controls by re-using the same mechanism used for public\n> controls to make it easy to extend it to also handle internal properties\n> in future.\n\nI'm afraid these are bugging me ... Perhaps I missed this before when we\ndiscussed internal controls...\n \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>\n> ---\n>  include/libcamera/internal/meson.build  | 15 ++++++++++++++\n>  src/libcamera/internal_control_ids.yaml | 27 +++++++++++++++++++++++++\n>  src/libcamera/meson.build               | 17 ++++++++++++++++\n>  3 files changed, 59 insertions(+)\n>  create mode 100644 src/libcamera/internal_control_ids.yaml\n> \n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 7a780d48ee57..1559c3c368c4 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -9,6 +9,21 @@ libcamera_tracepoint_header = custom_target(\n>      command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'],\n>  )\n>  \n> +# Generate the list of internal controls identifiers\n> +internal_control_source_files = ['control_ids']\n> +\n> +libcamera_internal_control_headers = []\n> +\n> +foreach header : internal_control_source_files\n> +    input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\\\n> +                        '../' + header + '.h.in')\n> +    libcamera_internal_control_headers += custom_target(\n> +            'internal_' + header + '_h',\n> +            input : input_files,\n> +            output : header + '.h',\n> +            command : [gen_controls, '--internal=True','-o', '@OUTPUT@', '@INPUT@'])\n> +endforeach\n> +\n>  libcamera_internal_headers = files([\n>      'bayer_format.h',\n>      'byte_stream_buffer.h',\n> diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml\n> new file mode 100644\n> index 000000000000..6d3775afcf67\n> --- /dev/null\n> +++ b/src/libcamera/internal_control_ids.yaml\n> @@ -0,0 +1,27 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2022, Google Inc.\n> +#\n> +%YAML 1.2\n> +---\n> +# Enumeration of internal libcamera controls\n> +# Not exposed to application, for library use only\n> +\n> +controls:\n> +\n> +  - ExposureTime:\n\nWe have: libcamera::controls::ExposureTime already\nhttps://libcamera.org/api-html/namespacelibcamera_1_1controls.html#a4e1ca45653b62cd969d4d67a741076eb\n\n> +      type: int32_t\n> +      description: |\n> +        The sensor exposure time in milliseconds.\n> +\n> +  - FrameDuration:\n\n\nAnd libcamera::controls::FrameDuration\n\nhttps://libcamera.org/api-html/namespacelibcamera_1_1controls.html#a37d99a76c7249c143beecd70917469be\n\n\n\n> +      type: int64_t\n> +      description: |\n> +        The sensor frame duration time in milliseconds.\n> +\n> +  - AnalogueGain:\n\nAnd ... libcamera::controls::AnalogueGain\n\nhttps://libcamera.org/api-html/namespacelibcamera_1_1controls.html#ab34ebeaa9cbfb3f3fc6996b089ca52b0\n\nThese are 'metadata' controls that are supposed to be returned by the\nPipeline handler in the metadata. But they mean exactly the same thing\nhere.\n\nSo, is there a reason we aren't using them directly?\n\n--\nKieran\n\n> +      type: float\n> +      description: |\n> +        The sensor analogue gain value.\n> +\n> +...\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index b57bee7ef6ca..89fdf347c708 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -52,6 +52,7 @@ libcamera_sources = files([\n>  libcamera_sources += libcamera_public_headers\n>  libcamera_sources += libcamera_generated_ipa_headers\n>  libcamera_sources += libcamera_tracepoint_header\n> +libcamera_sources += libcamera_internal_control_headers\n>  \n>  includes = [\n>      libcamera_includes,\n> @@ -99,6 +100,7 @@ if not libyaml.found()\n>      libyaml = libyaml_wrap.dependency('yaml')\n>  endif\n>  \n> +# Generate control_ids.cpp and property_ids.cpp\n>  control_sources = []\n>  \n>  foreach source : control_source_files\n> @@ -111,6 +113,21 @@ endforeach\n>  \n>  libcamera_sources += control_sources\n>  \n> +# Generate internal_control_ids.cpp\n> +internal_control_source_files = ['control_ids']\n> +internal_control_sources = []\n> +\n> +foreach source : internal_control_source_files\n> +    input_files = files('internal_' + source +'.yaml', source + '.cpp.in')\n> +    internal_control_sources += custom_target('internal_' + source + '_cpp',\n> +                                              input : input_files,\n> +                                              output : 'internal_' + source + '.cpp',\n> +                                              command : [gen_controls, '--internal=True',\\\n> +                                                                       '-o', '@OUTPUT@',\\\n> +                                                                       '@INPUT@'])\n> +endforeach\n> +libcamera_sources += internal_control_sources\n> +\n>  gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh'\n>  \n>  # Use vcs_tag() and not configure_file() or run_command(), to ensure that the\n> -- \n> 2.36.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 25BBEBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 30 Jun 2022 21:06:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 779326564E;\n\tThu, 30 Jun 2022 23:06:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 13CA66559A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Jun 2022 23:06:30 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 90E4545F;\n\tThu, 30 Jun 2022 23:06:29 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656623191;\n\tbh=xrVVqLU/s2nFPuDAIlXwAxdvqFVdKd5apW1gTrXCfK8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=OPx4xpnZ4ry2FNwcwmjscgBIuJqaaMYQre0bZzAe0XU9k4MvbLpgT8qXfEav8AaVC\n\tCIIvGgETF/EdMkxEVg6jJjhVpROueO8pDlIQMhSwYVhopnNBUAfGVowbHJhDK4aieB\n\tU6lXZTlHiwiBj/91PkIn6ZKoB0y9mo0Kp6oGjDh27hKy5npvn+Qok9MoO+lRpmOCht\n\trGdi/m4lgkWM3jVMLLs4DDtk/tVOrUUJBI/3uZnaCA9o0EhatqhiLny8kyUmnvlSUb\n\tOWsJFkn4neOwNyRG5V37EQdsgjrGhzCzv5/AKV3SMirDFuvQ00BR3Ugd0RAU249lem\n\t8rUMgWt57SuqQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656623189;\n\tbh=xrVVqLU/s2nFPuDAIlXwAxdvqFVdKd5apW1gTrXCfK8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=rnRCJbjUyWwaw43vb1hRY/ZcwzDcTtdxEt2AjWVFrZRGVssdTR2l56H6Jx7rb6xiN\n\tfEh5eoeiQbGhiwlLvFDpHiZ+XSZ6bi1Y/xFOTJY+w39VJk9+wGONhkoMdIlyxTSBJ2\n\t3Vx1Qj8gZAzpOLXsqYvFzSwbso+Duq3KxQQjCUhE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rnRCJbjU\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220630133902.321099-4-jacopo@jmondi.org>","References":"<20220630133902.321099-1-jacopo@jmondi.org>\n\t<20220630133902.321099-4-jacopo@jmondi.org>","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Date":"Thu, 30 Jun 2022 22:06:27 +0100","Message-ID":"<165662318709.2049236.4764643638940438408@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 03/23] libcamera: Introduce\n\tinternal controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23703,"web_url":"https://patchwork.libcamera.org/comment/23703/","msgid":"<20220701082513.eqdivsmteejkhomi@uno.localdomain>","date":"2022-07-01T08:25:13","subject":"Re: [libcamera-devel] [PATCH v3 03/23] libcamera: Introduce\n\tinternal controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Thu, Jun 30, 2022 at 10:06:27PM +0100, Kieran Bingham wrote:\n> Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:42)\n> > Introduce the enumeration of internal controls in\n> > internal_control_ids.yaml.\n> >\n> > Plumb in the build system the command to generate the definition of\n> > internal controls by re-using the same mechanism used for public\n> > controls to make it easy to extend it to also handle internal properties\n> > in future.\n>\n> I'm afraid these are bugging me ... Perhaps I missed this before when we\n> discussed internal controls...\n>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>\n> > ---\n> >  include/libcamera/internal/meson.build  | 15 ++++++++++++++\n> >  src/libcamera/internal_control_ids.yaml | 27 +++++++++++++++++++++++++\n> >  src/libcamera/meson.build               | 17 ++++++++++++++++\n> >  3 files changed, 59 insertions(+)\n> >  create mode 100644 src/libcamera/internal_control_ids.yaml\n> >\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 7a780d48ee57..1559c3c368c4 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -9,6 +9,21 @@ libcamera_tracepoint_header = custom_target(\n> >      command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'],\n> >  )\n> >\n> > +# Generate the list of internal controls identifiers\n> > +internal_control_source_files = ['control_ids']\n> > +\n> > +libcamera_internal_control_headers = []\n> > +\n> > +foreach header : internal_control_source_files\n> > +    input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\\\n> > +                        '../' + header + '.h.in')\n> > +    libcamera_internal_control_headers += custom_target(\n> > +            'internal_' + header + '_h',\n> > +            input : input_files,\n> > +            output : header + '.h',\n> > +            command : [gen_controls, '--internal=True','-o', '@OUTPUT@', '@INPUT@'])\n> > +endforeach\n> > +\n> >  libcamera_internal_headers = files([\n> >      'bayer_format.h',\n> >      'byte_stream_buffer.h',\n> > diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml\n> > new file mode 100644\n> > index 000000000000..6d3775afcf67\n> > --- /dev/null\n> > +++ b/src/libcamera/internal_control_ids.yaml\n> > @@ -0,0 +1,27 @@\n> > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > +#\n> > +# Copyright (C) 2022, Google Inc.\n> > +#\n> > +%YAML 1.2\n> > +---\n> > +# Enumeration of internal libcamera controls\n> > +# Not exposed to application, for library use only\n> > +\n> > +controls:\n> > +\n> > +  - ExposureTime:\n>\n> We have: libcamera::controls::ExposureTime already\n> https://libcamera.org/api-html/namespacelibcamera_1_1controls.html#a4e1ca45653b62cd969d4d67a741076eb\n>\n> > +      type: int32_t\n> > +      description: |\n> > +        The sensor exposure time in milliseconds.\n> > +\n> > +  - FrameDuration:\n>\n>\n> And libcamera::controls::FrameDuration\n>\n> https://libcamera.org/api-html/namespacelibcamera_1_1controls.html#a37d99a76c7249c143beecd70917469be\n>\n>\n>\n> > +      type: int64_t\n> > +      description: |\n> > +        The sensor frame duration time in milliseconds.\n> > +\n> > +  - AnalogueGain:\n>\n> And ... libcamera::controls::AnalogueGain\n>\n> https://libcamera.org/api-html/namespacelibcamera_1_1controls.html#ab34ebeaa9cbfb3f3fc6996b089ca52b0\n>\n> These are 'metadata' controls that are supposed to be returned by the\n> Pipeline handler in the metadata. But they mean exactly the same thing\n> here.\n>\n> So, is there a reason we aren't using them directly?\n\nYes, the names are the same.\n\nAlthough the application visibile controls have additional semantic\nthat the internal/camera_sensor controls do not have.\n\ncontrols::ExposureTime in example is not metadata only, but (will,\nonce we finalize the AE controls rework) actually drive how the\nIPA handles auto/manual exposure, something which does not apply to\nthe internal version.\n\nSo we should append to each of the application visible controls a\nparagraph about their usage to drive the camera sensor, and we already\nhave no clear division between controls/metadata in their definition\nwhich is already confusing sometimes.\n\nSo, to me it's seems more appropriate to separate the controls space,\nalso to let the two interface evolve separately, but maybe I'm just\nover-concerned and we could just re-use the public controls without\neven mentioning they're used internally, as applications do not care\nafter all.\n\n>\n> --\n> Kieran\n>\n> > +      type: float\n> > +      description: |\n> > +        The sensor analogue gain value.\n> > +\n> > +...\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index b57bee7ef6ca..89fdf347c708 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -52,6 +52,7 @@ libcamera_sources = files([\n> >  libcamera_sources += libcamera_public_headers\n> >  libcamera_sources += libcamera_generated_ipa_headers\n> >  libcamera_sources += libcamera_tracepoint_header\n> > +libcamera_sources += libcamera_internal_control_headers\n> >\n> >  includes = [\n> >      libcamera_includes,\n> > @@ -99,6 +100,7 @@ if not libyaml.found()\n> >      libyaml = libyaml_wrap.dependency('yaml')\n> >  endif\n> >\n> > +# Generate control_ids.cpp and property_ids.cpp\n> >  control_sources = []\n> >\n> >  foreach source : control_source_files\n> > @@ -111,6 +113,21 @@ endforeach\n> >\n> >  libcamera_sources += control_sources\n> >\n> > +# Generate internal_control_ids.cpp\n> > +internal_control_source_files = ['control_ids']\n> > +internal_control_sources = []\n> > +\n> > +foreach source : internal_control_source_files\n> > +    input_files = files('internal_' + source +'.yaml', source + '.cpp.in')\n> > +    internal_control_sources += custom_target('internal_' + source + '_cpp',\n> > +                                              input : input_files,\n> > +                                              output : 'internal_' + source + '.cpp',\n> > +                                              command : [gen_controls, '--internal=True',\\\n> > +                                                                       '-o', '@OUTPUT@',\\\n> > +                                                                       '@INPUT@'])\n> > +endforeach\n> > +libcamera_sources += internal_control_sources\n> > +\n> >  gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh'\n> >\n> >  # Use vcs_tag() and not configure_file() or run_command(), to ensure that the\n> > --\n> > 2.36.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D3704BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  1 Jul 2022 08:25:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 183F66564E;\n\tFri,  1 Jul 2022 10:25:18 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AEEE06054A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  1 Jul 2022 10:25:16 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id A8364FF813;\n\tFri,  1 Jul 2022 08:25:15 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656663918;\n\tbh=/8gm/FeJv0ahB33HjKPy4Q+q+JWA5fm7037qS4BMv6Q=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=m/O+lOkMTFJlbbxORvSRwSe5xOM81gjj8vEQet6JEbY+CTn+jJPYHvW/m+Isz3EVS\n\tAsOdWdUYzINzFhwYrTDAqUKsFNt/q/twwH3gt2hT55yuSqYXfexLQghpuIH2WdlOon\n\tOCWObNwXCUYGlaRyr+f4FpBRBpGf5DP3oru3rcCiEm3Bj4iDk981RtlC2s3si/azg1\n\tnICQjZB6KJsfRb+FLKhcrJRy47zAavautFXJ3iuRb5iFS0Z21OJMNP4tPYdyhAl4H3\n\tXh+D8Ue3RnkhkO7pGbzrBhzseyFALvNQPuQYgx+8lAbeEFb2hC8ekU8UciooPXvSAW\n\tSUbL95xzTVDYw==","Date":"Fri, 1 Jul 2022 10:25:13 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20220701082513.eqdivsmteejkhomi@uno.localdomain>","References":"<20220630133902.321099-1-jacopo@jmondi.org>\n\t<20220630133902.321099-4-jacopo@jmondi.org>\n\t<165662318709.2049236.4764643638940438408@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<165662318709.2049236.4764643638940438408@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 03/23] libcamera: Introduce\n\tinternal controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23706,"web_url":"https://patchwork.libcamera.org/comment/23706/","msgid":"<165666546449.1516045.3077249302203349199@Monstersaurus>","date":"2022-07-01T08:51:04","subject":"Re: [libcamera-devel] [PATCH v3 03/23] libcamera: Introduce\n\tinternal controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2022-07-01 09:25:13)\n> Hi Kieran\n> \n> On Thu, Jun 30, 2022 at 10:06:27PM +0100, Kieran Bingham wrote:\n> > Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:42)\n> > > Introduce the enumeration of internal controls in\n> > > internal_control_ids.yaml.\n> > >\n> > > Plumb in the build system the command to generate the definition of\n> > > internal controls by re-using the same mechanism used for public\n> > > controls to make it easy to extend it to also handle internal properties\n> > > in future.\n> >\n> > I'm afraid these are bugging me ... Perhaps I missed this before when we\n> > discussed internal controls...\n> >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>\n> > > ---\n> > >  include/libcamera/internal/meson.build  | 15 ++++++++++++++\n> > >  src/libcamera/internal_control_ids.yaml | 27 +++++++++++++++++++++++++\n> > >  src/libcamera/meson.build               | 17 ++++++++++++++++\n> > >  3 files changed, 59 insertions(+)\n> > >  create mode 100644 src/libcamera/internal_control_ids.yaml\n> > >\n> > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > > index 7a780d48ee57..1559c3c368c4 100644\n> > > --- a/include/libcamera/internal/meson.build\n> > > +++ b/include/libcamera/internal/meson.build\n> > > @@ -9,6 +9,21 @@ libcamera_tracepoint_header = custom_target(\n> > >      command: [gen_tracepoints_header, '@OUTPUT@', '@INPUT@'],\n> > >  )\n> > >\n> > > +# Generate the list of internal controls identifiers\n> > > +internal_control_source_files = ['control_ids']\n> > > +\n> > > +libcamera_internal_control_headers = []\n> > > +\n> > > +foreach header : internal_control_source_files\n> > > +    input_files = files('../../../src/libcamera/internal_' + header +'.yaml',\\\n> > > +                        '../' + header + '.h.in')\n> > > +    libcamera_internal_control_headers += custom_target(\n> > > +            'internal_' + header + '_h',\n> > > +            input : input_files,\n> > > +            output : header + '.h',\n> > > +            command : [gen_controls, '--internal=True','-o', '@OUTPUT@', '@INPUT@'])\n> > > +endforeach\n> > > +\n> > >  libcamera_internal_headers = files([\n> > >      'bayer_format.h',\n> > >      'byte_stream_buffer.h',\n> > > diff --git a/src/libcamera/internal_control_ids.yaml b/src/libcamera/internal_control_ids.yaml\n> > > new file mode 100644\n> > > index 000000000000..6d3775afcf67\n> > > --- /dev/null\n> > > +++ b/src/libcamera/internal_control_ids.yaml\n> > > @@ -0,0 +1,27 @@\n> > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > +#\n> > > +# Copyright (C) 2022, Google Inc.\n> > > +#\n> > > +%YAML 1.2\n> > > +---\n> > > +# Enumeration of internal libcamera controls\n> > > +# Not exposed to application, for library use only\n> > > +\n> > > +controls:\n> > > +\n> > > +  - ExposureTime:\n> >\n> > We have: libcamera::controls::ExposureTime already\n> > https://libcamera.org/api-html/namespacelibcamera_1_1controls.html#a4e1ca45653b62cd969d4d67a741076eb\n> >\n> > > +      type: int32_t\n> > > +      description: |\n> > > +        The sensor exposure time in milliseconds.\n> > > +\n> > > +  - FrameDuration:\n> >\n> >\n> > And libcamera::controls::FrameDuration\n> >\n> > https://libcamera.org/api-html/namespacelibcamera_1_1controls.html#a37d99a76c7249c143beecd70917469be\n> >\n> >\n> >\n> > > +      type: int64_t\n> > > +      description: |\n> > > +        The sensor frame duration time in milliseconds.\n> > > +\n> > > +  - AnalogueGain:\n> >\n> > And ... libcamera::controls::AnalogueGain\n> >\n> > https://libcamera.org/api-html/namespacelibcamera_1_1controls.html#ab34ebeaa9cbfb3f3fc6996b089ca52b0\n> >\n> > These are 'metadata' controls that are supposed to be returned by the\n> > Pipeline handler in the metadata. But they mean exactly the same thing\n> > here.\n> >\n> > So, is there a reason we aren't using them directly?\n> \n> Yes, the names are the same.\n> \n> Although the application visibile controls have additional semantic\n> that the internal/camera_sensor controls do not have.\n> \n> controls::ExposureTime in example is not metadata only, but (will,\n> once we finalize the AE controls rework) actually drive how the\n> IPA handles auto/manual exposure, something which does not apply to\n> the internal version.\n> \n> So we should append to each of the application visible controls a\n> paragraph about their usage to drive the camera sensor, and we already\n> have no clear division between controls/metadata in their definition\n> which is already confusing sometimes.\n> \n> So, to me it's seems more appropriate to separate the controls space,\n> also to let the two interface evolve separately, but maybe I'm just\n> over-concerned and we could just re-use the public controls without\n> even mentioning they're used internally, as applications do not care\n> after all.\n\nWell - to me the distinction would be ... could the metadata and the\ninternal control ever have a different value to mean the same thing.\nThat would warrant a different control namespace/scope - but as I\nunderstand it here, these internal controls should be the specific\nvalues that are exposed via metadata for each frame.\n\n--\nKieran\n\n\n> \n> >\n> > --\n> > Kieran\n> >\n> > > +      type: float\n> > > +      description: |\n> > > +        The sensor analogue gain value.\n> > > +\n> > > +...\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index b57bee7ef6ca..89fdf347c708 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -52,6 +52,7 @@ libcamera_sources = files([\n> > >  libcamera_sources += libcamera_public_headers\n> > >  libcamera_sources += libcamera_generated_ipa_headers\n> > >  libcamera_sources += libcamera_tracepoint_header\n> > > +libcamera_sources += libcamera_internal_control_headers\n> > >\n> > >  includes = [\n> > >      libcamera_includes,\n> > > @@ -99,6 +100,7 @@ if not libyaml.found()\n> > >      libyaml = libyaml_wrap.dependency('yaml')\n> > >  endif\n> > >\n> > > +# Generate control_ids.cpp and property_ids.cpp\n> > >  control_sources = []\n> > >\n> > >  foreach source : control_source_files\n> > > @@ -111,6 +113,21 @@ endforeach\n> > >\n> > >  libcamera_sources += control_sources\n> > >\n> > > +# Generate internal_control_ids.cpp\n> > > +internal_control_source_files = ['control_ids']\n> > > +internal_control_sources = []\n> > > +\n> > > +foreach source : internal_control_source_files\n> > > +    input_files = files('internal_' + source +'.yaml', source + '.cpp.in')\n> > > +    internal_control_sources += custom_target('internal_' + source + '_cpp',\n> > > +                                              input : input_files,\n> > > +                                              output : 'internal_' + source + '.cpp',\n> > > +                                              command : [gen_controls, '--internal=True',\\\n> > > +                                                                       '-o', '@OUTPUT@',\\\n> > > +                                                                       '@INPUT@'])\n> > > +endforeach\n> > > +libcamera_sources += internal_control_sources\n> > > +\n> > >  gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh'\n> > >\n> > >  # Use vcs_tag() and not configure_file() or run_command(), to ensure that the\n> > > --\n> > > 2.36.1\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 35EA7BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  1 Jul 2022 08:51:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D12B76054A;\n\tFri,  1 Jul 2022 10:51:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 600D06054A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  1 Jul 2022 10:51:07 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E66E025C;\n\tFri,  1 Jul 2022 10:51:06 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656665468;\n\tbh=EYKJpIuc73Ph3pI7JdbXx+e0DVXfpEkshLXuMYvUd5k=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=s7qfzmk0dOW37XlgqNzx28myCMn23DJuvP/CyNzuLYSH5YK0WQCwYGG+lcw8/eKqX\n\tCcp+OfRZfWqEse3GvQUaTOe8KQFYx+DTaCinDzqw9l3Zr4EkuHt2JrGfxFuQiU0S4v\n\tISBaejskTQ8p4Y9EhcQB2t9Y25z2FZtt7gKA5nqNqeFwJvN0X2OlN6TwnL3Bkngpsf\n\tkeCLZkKat5XU5J0MTWCYJmbb+IW4ShIh5ggbY0ERX10mQHotmOofMh1mjT7yTPCD90\n\txMMG8yZf3bo6XjdHHo12P2IIZTs6UOViRdrfMq5w1THVTpSBlyY57LM0mq9ucAbBik\n\tl0iHx0wzgPtJw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656665467;\n\tbh=EYKJpIuc73Ph3pI7JdbXx+e0DVXfpEkshLXuMYvUd5k=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=LiqTIU2FQkjgz0qYXtp52txa8ujL/6nw3tHhs4hDQ62R05yqyOkLUOqX4hULGLr9i\n\tHAeTvRrH8GwRvUh21bWXtlg1NdzH5B9ArSy+TgrzHM1Qy2YWzI/wa44tGrzUQ0hyAG\n\tlF0tC+RClYK3qqratB+uJYSxh5KVbZyV7C9riNl0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LiqTIU2F\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220701082513.eqdivsmteejkhomi@uno.localdomain>","References":"<20220630133902.321099-1-jacopo@jmondi.org>\n\t<20220630133902.321099-4-jacopo@jmondi.org>\n\t<165662318709.2049236.4764643638940438408@Monstersaurus>\n\t<20220701082513.eqdivsmteejkhomi@uno.localdomain>","To":"Jacopo Mondi <jacopo@jmondi.org>","Date":"Fri, 01 Jul 2022 09:51:04 +0100","Message-ID":"<165666546449.1516045.3077249302203349199@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 03/23] libcamera: Introduce\n\tinternal controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]