[{"id":28145,"web_url":"https://patchwork.libcamera.org/comment/28145/","msgid":"<20231123093004.GE15697@pendragon.ideasonboard.com>","date":"2023-11-23T09:30:04","subject":"Re: [libcamera-devel] [PATCH v2 5/6] libcamera: controls: Use\n\tvendor tags for draft controls and properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Nov 21, 2023 at 02:53:49PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Label draft controls and properties through the \"draft\" vendor tag\n> and deprecate the existing \"draft: true\" mechanism. This uses the new\n> vendor tags mechanism to place draft controls in the same\n> libcamera::controls::draft namespace and provide a defined control id\n> range for these controls. This requires moving all draft controls from\n> control_ids.yaml to control_ids_draft.yaml.\n> \n> One breaking change in this commit is that draft control ids also move\n> to the libcamera::controls::draft namespace from the existing\n> libcamera::controls namespace. This is desirable to avoid API breakages\n> when adding new libcamera controls. So, for example, the use of\n> controls::NOISE_REDUCTION_MODE will need to be replaced with\n> controls::draft::NOISE_REDUCTION_MODE.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  include/libcamera/control_ids.h.in            |   6 -\n>  include/libcamera/meson.build                 |   3 +-\n>  include/libcamera/property_ids.h.in           |   6 -\n>  src/ipa/rpi/common/ipa_base.cpp               |   2 +-\n>  src/ipa/rpi/vc4/vc4.cpp                       |   2 +-\n>  src/libcamera/control_ids.cpp.in              |  16 +-\n>  src/libcamera/control_ids.yaml                | 232 -----------------\n>  src/libcamera/control_ids_draft.yaml          | 240 ++++++++++++++++++\n>  src/libcamera/meson.build                     |   2 +-\n>  src/libcamera/property_ids.cpp.in             |  16 +-\n>  src/libcamera/property_ids.yaml               |  33 ---\n>  src/libcamera/property_ids_draft.yaml         |  39 +++\n>  src/py/libcamera/gen-py-controls.py           |   5 +-\n>  src/py/libcamera/meson.build                  |   6 +-\n>  src/py/libcamera/py_controls_generated.cpp.in |   5 -\n>  .../libcamera/py_properties_generated.cpp.in  |   3 +-\n>  utils/gen-controls.py                         |  39 +--\n>  17 files changed, 303 insertions(+), 352 deletions(-)\n>  create mode 100644 src/libcamera/control_ids_draft.yaml\n>  create mode 100644 src/libcamera/property_ids_draft.yaml\n> \n> diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\n> index c97b09a82450..d53b1cf7beb2 100644\n> --- a/include/libcamera/control_ids.h.in\n> +++ b/include/libcamera/control_ids.h.in\n> @@ -26,12 +26,6 @@ ${controls}\n>  \n>  extern const ControlIdMap controls;\n>  \n> -namespace draft {\n> -\n> -${draft_controls}\n> -\n> -} /* namespace draft */\n> -\n>  ${vendor_controls}\n>  \n>  } /* namespace controls */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 6cac385fdee4..63fc120a0818 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -76,7 +76,8 @@ foreach header, mode : control_source_files\n>          vendor_prop_files = vendor_files\n>      endif\n>  \n> -    input_files = files('../../src/libcamera/' + header + '.yaml')\n> +    input_files = files(['../../src/libcamera/' + header +'.yaml',\n> +                         '../../src/libcamera/' + header +'_draft.yaml'])\n>  \n>      foreach file : vendor_files\n>          input_files += files('../../src/libcamera/' + file)\n> diff --git a/include/libcamera/property_ids.h.in b/include/libcamera/property_ids.h.in\n> index 47c5d6bf2e28..43372c718fc9 100644\n> --- a/include/libcamera/property_ids.h.in\n> +++ b/include/libcamera/property_ids.h.in\n> @@ -23,12 +23,6 @@ ${ids}\n>  \n>  ${controls}\n>  \n> -namespace draft {\n> -\n> -${draft_controls}\n> -\n> -} /* namespace draft */\n> -\n>  extern const ControlIdMap properties;\n>  \n>  ${vendor_controls}\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index a1fec3aa3dd1..6ac9d5de2f88 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -1024,7 +1024,7 @@ void IpaBase::applyControls(const ControlList &controls)\n>  \t\t\tbreak;\n>  \t\t}\n>  \n> -\t\tcase controls::NOISE_REDUCTION_MODE:\n> +\t\tcase controls::draft::NOISE_REDUCTION_MODE:\n>  \t\t\t/* Handled below in handleControls() */\n>  \t\t\tlibcameraMetadata_.set(controls::draft::NoiseReductionMode,\n>  \t\t\t\t\t       ctrl.second.get<int32_t>());\n> diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp\n> index c4baf04fb1e7..c165a5b8b0b6 100644\n> --- a/src/ipa/rpi/vc4/vc4.cpp\n> +++ b/src/ipa/rpi/vc4/vc4.cpp\n> @@ -260,7 +260,7 @@ void IpaVc4::handleControls(const ControlList &controls)\n>  \n>  \tfor (auto const &ctrl : controls) {\n>  \t\tswitch (ctrl.first) {\n> -\t\tcase controls::NOISE_REDUCTION_MODE: {\n> +\t\tcase controls::draft::NOISE_REDUCTION_MODE: {\n>  \t\t\tRPiController::DenoiseAlgorithm *sdn = dynamic_cast<RPiController::DenoiseAlgorithm *>(\n>  \t\t\t\tcontroller_.getAlgorithm(\"SDN\"));\n>  \t\t\t/* Some platforms may have a combined \"denoise\" algorithm instead. */\n> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\n> index d26eb930b30c..284589567cfb 100644\n> --- a/src/libcamera/control_ids.cpp.in\n> +++ b/src/libcamera/control_ids.cpp.in\n> @@ -24,15 +24,6 @@ namespace controls {\n>  \n>  ${controls_doc}\n>  \n> -/**\n> - * \\brief Namespace for libcamera draft controls\n> - */\n> -namespace draft {\n> -\n> -${draft_controls_doc}\n> -\n> -} /* namespace draft */\n> -\n>  ${vendor_controls_doc}\n>  \n>  #ifndef __DOXYGEN__\n> @@ -42,13 +33,8 @@ ${vendor_controls_doc}\n>   */\n>  ${controls_def}\n>  \n> -namespace draft {\n> -\n> -${draft_controls_def}\n> -\n> -} /* namespace draft */\n> -\n>  ${vendor_controls_def}\n> +\n>  #endif\n>  \n>  /**\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index ff74ce1deedb..76fb9347d8e3 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -865,236 +865,4 @@ controls:\n>            description: |\n>              This is a long exposure image.\n>  \n> -  # ----------------------------------------------------------------------------\n> -  # Draft controls section\n> -\n> -  - AePrecaptureTrigger:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -        Control for AE metering trigger. Currently identical to\n> -        ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER.\n> -\n> -        Whether the camera device will trigger a precapture metering sequence\n> -        when it processes this request.\n> -      enum:\n> -        - name: AePrecaptureTriggerIdle\n> -          value: 0\n> -          description: The trigger is idle.\n> -        - name: AePrecaptureTriggerStart\n> -          value: 1\n> -          description: The pre-capture AE metering is started by the camera.\n> -        - name: AePrecaptureTriggerCancel\n> -          value: 2\n> -          description: |\n> -            The camera will cancel any active or completed metering sequence.\n> -            The AE algorithm is reset to its initial state.\n> -\n> -  - NoiseReductionMode:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -       Control to select the noise reduction algorithm mode. Currently\n> -       identical to ANDROID_NOISE_REDUCTION_MODE.\n> -\n> -        Mode of operation for the noise reduction algorithm.\n> -      enum:\n> -        - name: NoiseReductionModeOff\n> -          value: 0\n> -          description: No noise reduction is applied\n> -        - name: NoiseReductionModeFast\n> -          value: 1\n> -          description: |\n> -            Noise reduction is applied without reducing the frame rate.\n> -        - name: NoiseReductionModeHighQuality\n> -          value: 2\n> -          description: |\n> -            High quality noise reduction at the expense of frame rate.\n> -        - name: NoiseReductionModeMinimal\n> -          value: 3\n> -          description: |\n> -            Minimal noise reduction is applied without reducing the frame rate.\n> -        - name: NoiseReductionModeZSL\n> -          value: 4\n> -          description: |\n> -            Noise reduction is applied at different levels to different streams.\n> -\n> -  - ColorCorrectionAberrationMode:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -       Control to select the color correction aberration mode. Currently\n> -       identical to ANDROID_COLOR_CORRECTION_ABERRATION_MODE.\n> -\n> -        Mode of operation for the chromatic aberration correction algorithm.\n> -      enum:\n> -        - name: ColorCorrectionAberrationOff\n> -          value: 0\n> -          description: No aberration correction is applied.\n> -        - name: ColorCorrectionAberrationFast\n> -          value: 1\n> -          description: Aberration correction will not slow down the frame rate.\n> -        - name: ColorCorrectionAberrationHighQuality\n> -          value: 2\n> -          description: |\n> -            High quality aberration correction which might reduce the frame\n> -            rate.\n> -\n> -  - AeState:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -       Control to report the current AE algorithm state. Currently identical to\n> -       ANDROID_CONTROL_AE_STATE.\n> -\n> -        Current state of the AE algorithm.\n> -      enum:\n> -        - name: AeStateInactive\n> -          value: 0\n> -          description: The AE algorithm is inactive.\n> -        - name: AeStateSearching\n> -          value: 1\n> -          description: The AE algorithm has not converged yet.\n> -        - name: AeStateConverged\n> -          value: 2\n> -          description: The AE algorithm has converged.\n> -        - name: AeStateLocked\n> -          value: 3\n> -          description: The AE algorithm is locked.\n> -        - name: AeStateFlashRequired\n> -          value: 4\n> -          description: The AE algorithm would need a flash for good results\n> -        - name: AeStatePrecapture\n> -          value: 5\n> -          description: |\n> -            The AE algorithm has started a pre-capture metering session.\n> -            \\sa AePrecaptureTrigger\n> -\n> -  - AwbState:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -       Control to report the current AWB algorithm state. Currently identical\n> -       to ANDROID_CONTROL_AWB_STATE.\n> -\n> -        Current state of the AWB algorithm.\n> -      enum:\n> -        - name: AwbStateInactive\n> -          value: 0\n> -          description: The AWB algorithm is inactive.\n> -        - name: AwbStateSearching\n> -          value: 1\n> -          description: The AWB algorithm has not converged yet.\n> -        - name: AwbConverged\n> -          value: 2\n> -          description: The AWB algorithm has converged.\n> -        - name: AwbLocked\n> -          value: 3\n> -          description: The AWB algorithm is locked.\n> -\n> -  - SensorRollingShutterSkew:\n> -      type: int64_t\n> -      draft: true\n> -      description: |\n> -       Control to report the time between the start of exposure of the first\n> -       row and the start of exposure of the last row. Currently identical to\n> -       ANDROID_SENSOR_ROLLING_SHUTTER_SKEW\n> -\n> -  - LensShadingMapMode:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -       Control to report if the lens shading map is available. Currently\n> -       identical to ANDROID_STATISTICS_LENS_SHADING_MAP_MODE.\n> -      enum:\n> -        - name: LensShadingMapModeOff\n> -          value: 0\n> -          description: No lens shading map mode is available.\n> -        - name: LensShadingMapModeOn\n> -          value: 1\n> -          description: The lens shading map mode is available.\n> -\n> -  - PipelineDepth:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -        Specifies the number of pipeline stages the frame went through from when\n> -        it was exposed to when the final completed result was available to the\n> -        framework. Always less than or equal to PipelineMaxDepth. Currently\n> -        identical to ANDROID_REQUEST_PIPELINE_DEPTH.\n> -\n> -        The typical value for this control is 3 as a frame is first exposed,\n> -        captured and then processed in a single pass through the ISP. Any\n> -        additional processing step performed after the ISP pass (in example face\n> -        detection, additional format conversions etc) count as an additional\n> -        pipeline stage.\n> -\n> -  - MaxLatency:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -        The maximum number of frames that can occur after a request (different\n> -        than the previous) has been submitted, and before the result's state\n> -        becomes synchronized. A value of -1 indicates unknown latency, and 0\n> -        indicates per-frame control. Currently identical to\n> -        ANDROID_SYNC_MAX_LATENCY.\n> -\n> -  - TestPatternMode:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -        Control to select the test pattern mode. Currently identical to\n> -        ANDROID_SENSOR_TEST_PATTERN_MODE.\n> -      enum:\n> -        - name: TestPatternModeOff\n> -          value: 0\n> -          description: |\n> -            No test pattern mode is used. The camera device returns frames from\n> -            the image sensor.\n> -        - name: TestPatternModeSolidColor\n> -          value: 1\n> -          description: |\n> -            Each pixel in [R, G_even, G_odd, B] is replaced by its respective\n> -            color channel provided in test pattern data.\n> -            \\todo Add control for test pattern data.\n> -        - name: TestPatternModeColorBars\n> -          value: 2\n> -          description: |\n> -            All pixel data is replaced with an 8-bar color pattern. The vertical\n> -            bars (left-to-right) are as follows; white, yellow, cyan, green,\n> -            magenta, red, blue and black. Each bar should take up 1/8 of the\n> -            sensor pixel array width. When this is not possible, the bar size\n> -            should be rounded down to the nearest integer and the pattern can\n> -            repeat on the right side. Each bar's height must always take up the\n> -            full sensor pixel array height.\n> -        - name: TestPatternModeColorBarsFadeToGray\n> -          value: 3\n> -          description: |\n> -            The test pattern is similar to TestPatternModeColorBars,\n> -            except that each bar should start at its specified color at the top\n> -            and fade to gray at the bottom. Furthermore each bar is further\n> -            subdevided into a left and right half. The left half should have a\n> -            smooth gradient, and the right half should have a quantized\n> -            gradient. In particular, the right half's should consist of blocks\n> -            of the same color for 1/16th active sensor pixel array width. The\n> -            least significant bits in the quantized gradient should be copied\n> -            from the most significant bits of the smooth gradient. The height of\n> -            each bar should always be a multiple of 128. When this is not the\n> -            case, the pattern should repeat at the bottom of the image.\n> -        - name: TestPatternModePn9\n> -          value: 4\n> -          description: |\n> -            All pixel data is replaced by a pseudo-random sequence generated\n> -            from a PN9 512-bit sequence (typically implemented in hardware with\n> -            a linear feedback shift register). The generator should be reset at\n> -            the beginning of each frame, and thus each subsequent raw frame with\n> -            this test pattern should be exactly the same as the last.\n> -        - name: TestPatternModeCustom1\n> -          value: 256\n> -          description: |\n> -            The first custom test pattern. All custom patterns that are\n> -            available only on this camera device are at least this numeric\n> -            value. All of the custom test patterns will be static (that is the\n> -            raw image must not vary from frame to frame).\n> -\n>  ...\n> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml\n> new file mode 100644\n> index 000000000000..e4f53ea51d7a\n> --- /dev/null\n> +++ b/src/libcamera/control_ids_draft.yaml\n> @@ -0,0 +1,240 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2019, Google Inc.\n> +#\n> +%YAML 1.1\n> +---\n> +# Unless otherwise stated, all controls are bi-directional, i.e. they can be\n> +# set through Request::controls() and returned out through Request::metadata().\n> +vendor: draft\n> +controls:\n> +  - AePrecaptureTrigger:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +        Control for AE metering trigger. Currently identical to\n> +        ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER.\n> +\n> +        Whether the camera device will trigger a precapture metering sequence\n> +        when it processes this request.\n> +      enum:\n> +        - name: AePrecaptureTriggerIdle\n> +          value: 0\n> +          description: The trigger is idle.\n> +        - name: AePrecaptureTriggerStart\n> +          value: 1\n> +          description: The pre-capture AE metering is started by the camera.\n> +        - name: AePrecaptureTriggerCancel\n> +          value: 2\n> +          description: |\n> +            The camera will cancel any active or completed metering sequence.\n> +            The AE algorithm is reset to its initial state.\n> +\n> +  - NoiseReductionMode:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +       Control to select the noise reduction algorithm mode. Currently\n> +       identical to ANDROID_NOISE_REDUCTION_MODE.\n> +\n> +        Mode of operation for the noise reduction algorithm.\n> +      enum:\n> +        - name: NoiseReductionModeOff\n> +          value: 0\n> +          description: No noise reduction is applied\n> +        - name: NoiseReductionModeFast\n> +          value: 1\n> +          description: |\n> +            Noise reduction is applied without reducing the frame rate.\n> +        - name: NoiseReductionModeHighQuality\n> +          value: 2\n> +          description: |\n> +            High quality noise reduction at the expense of frame rate.\n> +        - name: NoiseReductionModeMinimal\n> +          value: 3\n> +          description: |\n> +            Minimal noise reduction is applied without reducing the frame rate.\n> +        - name: NoiseReductionModeZSL\n> +          value: 4\n> +          description: |\n> +            Noise reduction is applied at different levels to different streams.\n> +\n> +  - ColorCorrectionAberrationMode:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +       Control to select the color correction aberration mode. Currently\n> +       identical to ANDROID_COLOR_CORRECTION_ABERRATION_MODE.\n> +\n> +        Mode of operation for the chromatic aberration correction algorithm.\n> +      enum:\n> +        - name: ColorCorrectionAberrationOff\n> +          value: 0\n> +          description: No aberration correction is applied.\n> +        - name: ColorCorrectionAberrationFast\n> +          value: 1\n> +          description: Aberration correction will not slow down the frame rate.\n> +        - name: ColorCorrectionAberrationHighQuality\n> +          value: 2\n> +          description: |\n> +            High quality aberration correction which might reduce the frame\n> +            rate.\n> +\n> +  - AeState:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +       Control to report the current AE algorithm state. Currently identical to\n> +       ANDROID_CONTROL_AE_STATE.\n> +\n> +        Current state of the AE algorithm.\n> +      enum:\n> +        - name: AeStateInactive\n> +          value: 0\n> +          description: The AE algorithm is inactive.\n> +        - name: AeStateSearching\n> +          value: 1\n> +          description: The AE algorithm has not converged yet.\n> +        - name: AeStateConverged\n> +          value: 2\n> +          description: The AE algorithm has converged.\n> +        - name: AeStateLocked\n> +          value: 3\n> +          description: The AE algorithm is locked.\n> +        - name: AeStateFlashRequired\n> +          value: 4\n> +          description: The AE algorithm would need a flash for good results\n> +        - name: AeStatePrecapture\n> +          value: 5\n> +          description: |\n> +            The AE algorithm has started a pre-capture metering session.\n> +            \\sa AePrecaptureTrigger\n> +\n> +  - AwbState:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +       Control to report the current AWB algorithm state. Currently identical\n> +       to ANDROID_CONTROL_AWB_STATE.\n> +\n> +        Current state of the AWB algorithm.\n> +      enum:\n> +        - name: AwbStateInactive\n> +          value: 0\n> +          description: The AWB algorithm is inactive.\n> +        - name: AwbStateSearching\n> +          value: 1\n> +          description: The AWB algorithm has not converged yet.\n> +        - name: AwbConverged\n> +          value: 2\n> +          description: The AWB algorithm has converged.\n> +        - name: AwbLocked\n> +          value: 3\n> +          description: The AWB algorithm is locked.\n> +\n> +  - SensorRollingShutterSkew:\n> +      type: int64_t\n> +      draft: true\n> +      description: |\n> +       Control to report the time between the start of exposure of the first\n> +       row and the start of exposure of the last row. Currently identical to\n> +       ANDROID_SENSOR_ROLLING_SHUTTER_SKEW\n> +\n> +  - LensShadingMapMode:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +       Control to report if the lens shading map is available. Currently\n> +       identical to ANDROID_STATISTICS_LENS_SHADING_MAP_MODE.\n> +      enum:\n> +        - name: LensShadingMapModeOff\n> +          value: 0\n> +          description: No lens shading map mode is available.\n> +        - name: LensShadingMapModeOn\n> +          value: 1\n> +          description: The lens shading map mode is available.\n> +\n> +  - PipelineDepth:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +        Specifies the number of pipeline stages the frame went through from when\n> +        it was exposed to when the final completed result was available to the\n> +        framework. Always less than or equal to PipelineMaxDepth. Currently\n> +        identical to ANDROID_REQUEST_PIPELINE_DEPTH.\n> +\n> +        The typical value for this control is 3 as a frame is first exposed,\n> +        captured and then processed in a single pass through the ISP. Any\n> +        additional processing step performed after the ISP pass (in example face\n> +        detection, additional format conversions etc) count as an additional\n> +        pipeline stage.\n> +\n> +  - MaxLatency:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +        The maximum number of frames that can occur after a request (different\n> +        than the previous) has been submitted, and before the result's state\n> +        becomes synchronized. A value of -1 indicates unknown latency, and 0\n> +        indicates per-frame control. Currently identical to\n> +        ANDROID_SYNC_MAX_LATENCY.\n> +\n> +  - TestPatternMode:\n> +      type: int32_t\n> +      draft: true\n> +      description: |\n> +        Control to select the test pattern mode. Currently identical to\n> +        ANDROID_SENSOR_TEST_PATTERN_MODE.\n> +      enum:\n> +        - name: TestPatternModeOff\n> +          value: 0\n> +          description: |\n> +            No test pattern mode is used. The camera device returns frames from\n> +            the image sensor.\n> +        - name: TestPatternModeSolidColor\n> +          value: 1\n> +          description: |\n> +            Each pixel in [R, G_even, G_odd, B] is replaced by its respective\n> +            color channel provided in test pattern data.\n> +            \\todo Add control for test pattern data.\n> +        - name: TestPatternModeColorBars\n> +          value: 2\n> +          description: |\n> +            All pixel data is replaced with an 8-bar color pattern. The vertical\n> +            bars (left-to-right) are as follows; white, yellow, cyan, green,\n> +            magenta, red, blue and black. Each bar should take up 1/8 of the\n> +            sensor pixel array width. When this is not possible, the bar size\n> +            should be rounded down to the nearest integer and the pattern can\n> +            repeat on the right side. Each bar's height must always take up the\n> +            full sensor pixel array height.\n> +        - name: TestPatternModeColorBarsFadeToGray\n> +          value: 3\n> +          description: |\n> +            The test pattern is similar to TestPatternModeColorBars,\n> +            except that each bar should start at its specified color at the top\n> +            and fade to gray at the bottom. Furthermore each bar is further\n> +            subdevided into a left and right half. The left half should have a\n> +            smooth gradient, and the right half should have a quantized\n> +            gradient. In particular, the right half's should consist of blocks\n> +            of the same color for 1/16th active sensor pixel array width. The\n> +            least significant bits in the quantized gradient should be copied\n> +            from the most significant bits of the smooth gradient. The height of\n> +            each bar should always be a multiple of 128. When this is not the\n> +            case, the pattern should repeat at the bottom of the image.\n> +        - name: TestPatternModePn9\n> +          value: 4\n> +          description: |\n> +            All pixel data is replaced by a pseudo-random sequence generated\n> +            from a PN9 512-bit sequence (typically implemented in hardware with\n> +            a linear feedback shift register). The generator should be reset at\n> +            the beginning of each frame, and thus each subsequent raw frame with\n> +            this test pattern should be exactly the same as the last.\n> +        - name: TestPatternModeCustom1\n> +          value: 256\n> +          description: |\n> +            The first custom test pattern. All custom patterns that are\n> +            available only on this camera device are at least this numeric\n> +            value. All of the custom test patterns will be static (that is the\n> +            raw image must not vary from frame to frame).\n> +\n> +...\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 303eee84a77e..e965b72e015a 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -128,7 +128,7 @@ endif\n>  control_sources = []\n>  \n>  foreach source, mode : control_source_files\n> -    input_files = files(source +'.yaml')\n> +    input_files = files([source +'.yaml', source + '_draft.yaml'])\n>  \n>      # Add the vendor specific files to the input.\n>      if mode == 'controls'\n> diff --git a/src/libcamera/property_ids.cpp.in b/src/libcamera/property_ids.cpp.in\n> index ddbe714c3f00..b72e12e4cc70 100644\n> --- a/src/libcamera/property_ids.cpp.in\n> +++ b/src/libcamera/property_ids.cpp.in\n> @@ -23,14 +23,7 @@ namespace properties {\n>  \n>  ${controls_doc}\n>  \n> -/**\n> - * \\brief Namespace for libcamera draft properties\n> - */\n> -namespace draft {\n> -\n> -${draft_controls_doc}\n> -\n> -} /* namespace draft */\n> +${vendor_controls_doc}\n>  \n>  ${vendor_controls_doc}\n\nThis doesn't look right, vendor_controls_doc is used twice.\n\n>  \n> @@ -41,13 +34,8 @@ ${vendor_controls_doc}\n>   */\n>  ${controls_def}\n>  \n> -namespace draft {\n> -\n> -${draft_controls_def}\n> -\n> -} /* namespace draft */\n> -\n>  ${vendor_controls_def}\n> +\n>  #endif\n>  \n>  /**\n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> index 45f3609b4236..834454a4e087 100644\n> --- a/src/libcamera/property_ids.yaml\n> +++ b/src/libcamera/property_ids.yaml\n> @@ -701,37 +701,4 @@ controls:\n>  \n>          Different cameras may report identical devices.\n>  \n> -  # ----------------------------------------------------------------------------\n> -  # Draft properties section\n> -\n> -  - ColorFilterArrangement:\n> -      type: int32_t\n> -      draft: true\n> -      description: |\n> -        The arrangement of color filters on sensor; represents the colors in the\n> -        top-left 2x2 section of the sensor, in reading order. Currently\n> -        identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.\n> -      enum:\n> -        - name: RGGB\n> -          value: 0\n> -          description: RGGB Bayer pattern\n> -        - name: GRBG\n> -          value: 1\n> -          description: GRBG Bayer pattern\n> -        - name: GBRG\n> -          value: 2\n> -          description: GBRG Bayer pattern\n> -        - name: BGGR\n> -          value: 3\n> -          description: BGGR Bayer pattern\n> -        - name: RGB\n> -          value: 4\n> -          description: |\n> -            Sensor is not Bayer; output has 3 16-bit values for each pixel,\n> -            instead of just 1 16-bit value per pixel.\n> -        - name: MONO\n> -          value: 5\n> -          description: |\n> -            Sensor is not Bayer; output consists of a single colour channel.\n> -\n>  ...\n> diff --git a/src/libcamera/property_ids_draft.yaml b/src/libcamera/property_ids_draft.yaml\n> new file mode 100644\n> index 000000000000..62f0e242d0c6\n> --- /dev/null\n> +++ b/src/libcamera/property_ids_draft.yaml\n> @@ -0,0 +1,39 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2019, Google Inc.\n> +#\n> +%YAML 1.1\n> +---\n> +vendor: draft\n> +controls:\n> +  - ColorFilterArrangement:\n> +      type: int32_t\n> +      vendor: draft\n> +      description: |\n> +        The arrangement of color filters on sensor; represents the colors in the\n> +        top-left 2x2 section of the sensor, in reading order. Currently\n> +        identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT.\n> +      enum:\n> +        - name: RGGB\n> +          value: 0\n> +          description: RGGB Bayer pattern\n> +        - name: GRBG\n> +          value: 1\n> +          description: GRBG Bayer pattern\n> +        - name: GBRG\n> +          value: 2\n> +          description: GBRG Bayer pattern\n> +        - name: BGGR\n> +          value: 3\n> +          description: BGGR Bayer pattern\n> +        - name: RGB\n> +          value: 4\n> +          description: |\n> +            Sensor is not Bayer; output has 3 16-bit values for each pixel,\n> +            instead of just 1 16-bit value per pixel.\n> +        - name: MONO\n> +          value: 5\n> +          description: |\n> +            Sensor is not Bayer; output consists of a single colour channel.\n> +\n> +...\n> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index e2ddad8581fd..e0695635dbcc 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -36,10 +36,7 @@ def generate_py(controls, mode):\n>                  vendor_defs.append('\\tauto {} = py::class_<Py{}Controls>(controls, \\\"{}\\\");'.format(vendor, vendor, vendor))\n>                  vendors.append(vendor)\n>  \n> -            if ctrl.get('draft'):\n> -                ns = 'libcamera::{}::draft::'.format(mode)\n> -                container = 'draft'\n> -            elif vendor != 'libcamera':\n> +            if vendor != 'libcamera':\n>                  ns = 'libcamera::{}::{}::'.format(mode, vendor)\n>                  container = vendor\n>              else:\n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> index ea38ad6ca829..9641862a4e9b 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -28,7 +28,8 @@ pycamera_sources = files([\n>  \n>  # Generate controls\n>  \n> -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> +gen_py_controls_input_files = files(['../../libcamera/control_ids.yaml',\n> +                                     '../../libcamera/control_ids_draft.yaml'])\n>  gen_py_controls_template = files('py_controls_generated.cpp.in')\n>  \n>  gen_py_controls = files('gen-py-controls.py')\n> @@ -45,7 +46,8 @@ pycamera_sources += custom_target('py_gen_controls',\n>  \n>  # Generate properties\n>  \n> -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> +gen_py_property_enums_input_files = files(['../../libcamera/property_ids.yaml',\n> +                                           '../../libcamera/property_ids_draft.yaml'])\n>  gen_py_properties_template = files('py_properties_generated.cpp.in')\n>  \n>  foreach file : vendor_prop_files\n> diff --git a/src/py/libcamera/py_controls_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in\n> index ec4b55ef2011..8d282ce51856 100644\n> --- a/src/py/libcamera/py_controls_generated.cpp.in\n> +++ b/src/py/libcamera/py_controls_generated.cpp.in\n> @@ -17,16 +17,11 @@ class PyControls\n>  {\n>  };\n>  \n> -class PyDraftControls\n> -{\n> -};\n> -\n>  ${vendors_class_def}\n>  \n>  void init_py_controls_generated(py::module& m)\n>  {\n>  \tauto controls = py::class_<PyControls>(m, \"controls\");\n> -\tauto draft = py::class_<PyDraftControls>(controls, \"draft\");\n>  ${vendors_defs}\n>  \n>  ${controls}\n> diff --git a/src/py/libcamera/py_properties_generated.cpp.in b/src/py/libcamera/py_properties_generated.cpp.in\n> index f7b5ec8c635d..1fa1967a9672 100644\n> --- a/src/py/libcamera/py_properties_generated.cpp.in\n> +++ b/src/py/libcamera/py_properties_generated.cpp.in\n> @@ -26,8 +26,7 @@ ${vendors_class_def}\n>  void init_py_properties_generated(py::module& m)\n>  {\n>  \tauto controls = py::class_<PyProperties>(m, \"properties\");\n> -\tauto draft = py::class_<PyDraftProperties>(controls, \"draft\");\n> -${vendors_defs}\n> +${vendors_defs}\t\n\nTrailing whitespace here.\n\nWith those small issues addressed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  \n>  ${controls}\n>  }\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index 9eede8940f24..f20e4eafcc2f 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -85,11 +85,6 @@ class Control(object):\n>          \"\"\"Is the control an enumeration\"\"\"\n>          return self.__enum_values is not None\n>  \n> -    @property\n> -    def is_draft(self):\n> -        \"\"\"Is the control a draft control\"\"\"\n> -        return self.__data.get('draft') is not None\n> -\n>      @property\n>      def vendor(self):\n>          \"\"\"The vendor string, or None\"\"\"\n> @@ -100,12 +95,6 @@ class Control(object):\n>          \"\"\"The control name (CamelCase)\"\"\"\n>          return self.__name\n>  \n> -    @property\n> -    def q_name(self):\n> -        \"\"\"The control name, qualified with a namespace\"\"\"\n> -        ns = 'draft::' if self.is_draft else ''\n> -        return ns + self.__name\n> -\n>      @property\n>      def type(self):\n>          typ = self.__data.get('type')\n> @@ -158,7 +147,7 @@ ${description}\n>      for ctrl in controls:\n>          id_name = snake_case(ctrl.name).upper()\n>  \n> -        vendor = 'draft' if ctrl.is_draft else ctrl.vendor\n> +        vendor = ctrl.vendor\n>          if vendor not in ctrls_doc:\n>              ctrls_doc[vendor] = []\n>              ctrls_def[vendor] = []\n> @@ -209,7 +198,7 @@ ${description}\n>          target_doc.append(doc_template.substitute(info))\n>          target_def.append(def_template.substitute(info))\n>  \n> -        target_ctrls_map.append('\\t{ ' + id_name + ', &' + ctrl.q_name + ' },')\n> +        target_ctrls_map.append('\\t{ ' + id_name + ', &' + ctrl.name + ' },')\n>  \n>      vendor_ctrl_doc_sub = []\n>      vendor_ctrl_template = string.Template('''\n> @@ -219,11 +208,11 @@ ${vendor_controls_str}\n>  \n>  } /* namespace ${vendor} */''')\n>  \n> -    for vendor in [v for v in ctrls_map.keys() if v not in ['libcamera', 'draft']]:\n> +    for vendor in [v for v in ctrls_map.keys() if v != 'libcamera']:\n>          vendor_ctrl_doc_sub.append(vendor_ctrl_template.substitute({'vendor': vendor, 'vendor_controls_str': '\\n\\n'.join(ctrls_doc[vendor])}))\n>  \n>      vendor_ctrl_def_sub = []\n> -    for vendor in [v for v in ctrls_map.keys() if v not in ['libcamera', 'draft']]:\n> +    for vendor in [v for v in ctrls_map.keys() if v != 'libcamera']:\n>          vendor_ctrl_def_sub.append(vendor_ctrl_template.substitute({'vendor': vendor, 'vendor_controls_str': '\\n'.join(ctrls_def[vendor])}))\n>  \n>      vendor_ctrl_map_sub = []\n> @@ -241,16 +230,14 @@ ${vendor_controls_map}\n>  } /* namespace ${vendor} */\n>  ''')\n>  \n> -    for vendor in [v for v in ctrls_map.keys() if v not in ['libcamera', 'draft']]:\n> +    for vendor in [v for v in ctrls_map.keys() if v != 'libcamera']:\n>          vendor_ctrl_map_sub.append(vendor_ctrl_template.substitute({'vendor': vendor,\n>                                                                      'vendor_controls_map': '\\n'.join(ctrls_map[vendor])}))\n>  \n>      return {\n>          'controls_doc': '\\n\\n'.join(ctrls_doc['libcamera']),\n>          'controls_def': '\\n'.join(ctrls_def['libcamera']),\n> -        'draft_controls_doc': '\\n\\n'.join(ctrls_doc['draft']),\n> -        'draft_controls_def': '\\n\\n'.join(ctrls_def['draft']),\n> -        'controls_map': '\\n'.join(ctrls_map['libcamera'] + ctrls_map['draft']),\n> +        'controls_map': '\\n'.join(ctrls_map['libcamera']),\n>          'vendor_controls_doc': '\\n'.join(vendor_ctrl_doc_sub),\n>          'vendor_controls_def': '\\n'.join(vendor_ctrl_def_sub),\n>          'vendor_controls_map': '\\n'.join(vendor_ctrl_map_sub),\n> @@ -270,7 +257,7 @@ def generate_h(controls, mode, ranges):\n>      for ctrl in controls:\n>          id_name = snake_case(ctrl.name).upper()\n>  \n> -        vendor = 'draft' if ctrl.is_draft else ctrl.vendor\n> +        vendor = ctrl.vendor\n>          if vendor not in ctrls:\n>              if vendor not in ranges.keys():\n>                  raise RuntimeError(f'Control id range is not defined for vendor {vendor}')\n> @@ -278,8 +265,7 @@ def generate_h(controls, mode, ranges):\n>              ids[vendor] = []\n>              ctrls[vendor] = []\n>  \n> -        # Core and draft controls use the same ID value\n> -        target_ids = ids['libcamera'] if vendor in ['libcamera', 'draft'] else ids[vendor]\n> +        target_ids = ids[vendor]\n>          target_ids.append('\\t' + id_name + ' = ' + str(id_value[vendor]) + ',')\n>  \n>          info = {\n> @@ -287,11 +273,7 @@ def generate_h(controls, mode, ranges):\n>              'type': ctrl.type,\n>          }\n>  \n> -        target_ctrls = ctrls['libcamera']\n> -        if ctrl.is_draft:\n> -            target_ctrls = ctrls['draft']\n> -        elif vendor != 'libcamera':\n> -            target_ctrls = ctrls[vendor]\n> +        target_ctrls = ctrls[vendor]\n>  \n>          if ctrl.is_enum:\n>              target_ctrls.append(enum_template_start.substitute(info))\n> @@ -332,7 +314,7 @@ ${vendor_controls}\n>  ''')\n>  \n>      vendor_sub = []\n> -    for vendor in [v for v in ctrls.keys() if v not in ['libcamera', 'draft']]:\n> +    for vendor in [v for v in ctrls.keys() if v != 'libcamera']:\n>          vendor_sub.append(vendor_template.substitute({'mode': mode.upper(),\n>                                                        'vendor': vendor,\n>                                                        'vendor_def': vendor.upper(),\n> @@ -342,7 +324,6 @@ ${vendor_controls}\n>      return {\n>          'ids': '\\n'.join(ids['libcamera']),\n>          'controls': '\\n'.join(ctrls['libcamera']),\n> -        'draft_controls': '\\n'.join(ctrls['draft']),\n>          'vendor_controls': '\\n'.join(vendor_sub)\n>      }\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 D6566BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 23 Nov 2023 09:30:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E489629BC;\n\tThu, 23 Nov 2023 10:30:00 +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 2498961DAA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 Nov 2023 10:29:58 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E8BE25A;\n\tThu, 23 Nov 2023 10:29:26 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700731800;\n\tbh=YUqCqkOJoZmbk4hUTuMMIpgs+goImh1NNYycuL7aBHY=;\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=iRRTgqfb3GYCnwFjCQlSTjkRLZPF8XYW7QiJQcpHCPBaXvW/oRF7FtUjphpc7TlkH\n\thPUR+nOMsxS0EKc6d8XskXPlkOKP213IopjkMGbxotfzf41W8OMUb8N6gUzcJj0DcZ\n\tTQEl/WXzQ2qKKezNWxyB+j+zhiUtFQYv85K0Io53Y2evRwsoD2E5nR7V52dx8NzC4S\n\tOKmlAip//BT5/P6GAgXPxKBR90ALstb2LzeaA5OOutSLyzouTIteY/NjINxGyc+/aE\n\tz7KnvYCKMm3kKaGh1acUF9KRlhyS4t94lmXC/2xFGCEFMd8AwF5RNkxkXlRDAqWup7\n\t6PwfnRNo+m3Cw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700731766;\n\tbh=YUqCqkOJoZmbk4hUTuMMIpgs+goImh1NNYycuL7aBHY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gp+VlD/fNFRJS+fKjnMmPvqfeYSHOFkGeZsVF4mlUZJ8vq8TbXjVHDqHUB/Vd/LVW\n\tF/A8b4U7MjsYsgMMLs5XAaQtNAFaloRWmZSLMyEBbQl1oPIYLD+FME/1UBzhZTfT0G\n\tXxhK6DF0N7gJ6ew/nyhHLr7QmK1wyKrvdI53Aw7A="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gp+VlD/f\"; dkim-atps=neutral","Date":"Thu, 23 Nov 2023 11:30:04 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20231123093004.GE15697@pendragon.ideasonboard.com>","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-6-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231121145350.5956-6-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 5/6] libcamera: controls: Use\n\tvendor tags for draft controls and properties","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":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@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>"}}]