| Message ID | 20220209071917.559993-1-hanlinchen@chromium.org |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Han-Lin, I realise this replay might be very (very) late now :-) However, I have been working on something very similar to this work for the Raspberry Pi pipeline handler: https://patchwork.libcamera.org/project/libcamera/list/?series=3659 . I just wanted to reach out to see if there is any common ground between both our patch series that would help us both. Having briefly looked through your series, I think the key difference is how we provide the config file to the respective pipeline handlers. I've opted for a simpler approach of using an environment variable to provide a filename to the pipeline handler. There is a helper function PipelineHandler::configurationFile() that will attempt to locate the file from either the system install path, or the source directory. It's up to the respective pipeline handler to parse the file and interpret the parameters as required. Is this approach something you can make use of for your series, replacing the "options" mechanism that you have implemented? As always, feel free to ask any questions if you have them. Regards, Naush On Wed, 9 Feb 2022 at 07:19, Han-Lin Chen <hanlinchen@chromium.org> wrote: > Hello, > > This patch series is to introduce the pipeline configuration preference > for IPU3. Before the series, IPU3 calculates the ImgU configuration > based on the algorithm from Intel: > https://github.com/intel/intel-ipu3-pipecfg > > However, IPU3 seems not well defined with the limitation on ImgU and > leading to configuration failure in some cases. See: > https://bugs.libcamera.org/show_bug.cgi?id=67 > > Due to this, on ChromeOS, Intel made manual adjusts on the validated > setting for each camera module. The patch series is to introduce the > feature for IPU3, and possibly other platforms with similar issues. > > The patch 1-2 is to introduce Option control for user to enable features > which need to be selected before querying capabilities of a camera. > > The patch 3 Adds helper class to wrap libyaml for easier reading yaml > files. > > The patch 4-9 Supports the Pipeline configuration preference of IPU3 and > default to be enabled on ChromeOS is it's specified on hal_config.yaml. > > Han-Lin Chen (9): > libcamera: Introduce option to customize behavior for a camera module > libcamera: Add options() and setOptions() operations to Camera > libcamera: Introduce YamlParser as a helper to parse yaml files > android: camera_hal_config: Use YamlParser to parse android hal config > android: Add pipeline_config_file parameter for camera_hal.yaml > android: Set PipelineConfigFile option if it's supported by the camera > libcamera: ipu3: Add helper class PipeConfigPreference > libcamera: ipu3: Support PipelineConfigFile option > android: Elevate min duration to 30 FPS if it's lower within 2% > > README.rst | 2 +- > include/libcamera/camera.h | 3 + > include/libcamera/geometry.h | 4 + > include/libcamera/internal/camera.h | 2 + > include/libcamera/internal/meson.build | 1 + > include/libcamera/internal/pipeline_handler.h | 2 + > include/libcamera/internal/yaml_parser.h | 82 ++ > include/libcamera/ipa/ipa_controls.h | 1 + > include/libcamera/meson.build | 3 +- > include/libcamera/option_ids.h.in | 36 + > src/android/camera_capabilities.cpp | 31 +- > src/android/camera_device.cpp | 19 + > src/android/camera_hal_config.cpp | 344 ++------ > src/android/camera_hal_config.h | 1 + > src/android/data/nautilus/camera_hal.yaml | 2 + > src/android/data/nautilus/imx258.yaml | 248 ++++++ > src/android/data/soraka/camera_hal.yaml | 2 + > src/android/data/soraka/ov13858.yaml | 236 ++++++ > src/android/data/soraka/ov5670.yaml | 242 ++++++ > src/libcamera/camera.cpp | 59 ++ > src/libcamera/control_serializer.cpp | 12 + > src/libcamera/geometry.cpp | 20 + > src/libcamera/meson.build | 3 + > src/libcamera/option_ids.cpp.in | 58 ++ > src/libcamera/option_ids.yaml | 16 + > src/libcamera/pipeline/ipu3/ipu3.cpp | 215 ++++- > src/libcamera/pipeline/ipu3/meson.build | 1 + > .../pipeline/ipu3/pipe_config_pref.cpp | 285 +++++++ > .../pipeline/ipu3/pipe_config_pref.h | 80 ++ > src/libcamera/pipeline_handler.cpp | 26 + > src/libcamera/yaml_parser.cpp | 796 ++++++++++++++++++ > 31 files changed, 2563 insertions(+), 269 deletions(-) > create mode 100644 include/libcamera/internal/yaml_parser.h > create mode 100644 include/libcamera/option_ids.h.in > create mode 100644 src/android/data/nautilus/imx258.yaml > create mode 100644 src/android/data/soraka/ov13858.yaml > create mode 100644 src/android/data/soraka/ov5670.yaml > create mode 100644 src/libcamera/option_ids.cpp.in > create mode 100644 src/libcamera/option_ids.yaml > create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.cpp > create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.h > create mode 100644 src/libcamera/yaml_parser.cpp > > -- > 2.35.0.263.gb82422642f-goog > >
Hi Naushir, Thanks for reaching out. In fact, I have no preference either way, as long as we can get the config file for the pipeline handler :) Just that ChromeOS usually does not use environment variables to define config files. If the helper function PipelineHandler::configurationFile() can have a default path, maybe only for the ChromeOS builds, that would be good enough for me. Please don't hesitate to continue your changes. I will follow up when I get cycles to update my series :P On Thu, Dec 8, 2022 at 6:28 PM Naushir Patuck <naush@raspberrypi.com> wrote: > > Hi Han-Lin, > > I realise this replay might be very (very) late now :-) However, I have been > working on something very similar to this work for the Raspberry Pi pipeline > handler: https://patchwork.libcamera.org/project/libcamera/list/?series=3659. > I just wanted to reach out to see if there is any common ground between both our > patch series that would help us both. > > Having briefly looked through your series, I think the key difference is how we > provide the config file to the respective pipeline handlers. I've opted for a > simpler approach of using an environment variable to provide a filename to the > pipeline handler. There is a helper function PipelineHandler::configurationFile() > that will attempt to locate the file from either the system install path, or > the source directory. It's up to the respective pipeline handler to parse the > file and interpret the parameters as required. > > Is this approach something you can make use of for your series, replacing the > "options" mechanism that you have implemented? As always, feel free to ask any > questions if you have them. > > Regards, > Naush > > On Wed, 9 Feb 2022 at 07:19, Han-Lin Chen <hanlinchen@chromium.org> wrote: >> >> Hello, >> >> This patch series is to introduce the pipeline configuration preference >> for IPU3. Before the series, IPU3 calculates the ImgU configuration >> based on the algorithm from Intel: >> https://github.com/intel/intel-ipu3-pipecfg >> >> However, IPU3 seems not well defined with the limitation on ImgU and >> leading to configuration failure in some cases. See: >> https://bugs.libcamera.org/show_bug.cgi?id=67 >> >> Due to this, on ChromeOS, Intel made manual adjusts on the validated >> setting for each camera module. The patch series is to introduce the >> feature for IPU3, and possibly other platforms with similar issues. >> >> The patch 1-2 is to introduce Option control for user to enable features >> which need to be selected before querying capabilities of a camera. >> >> The patch 3 Adds helper class to wrap libyaml for easier reading yaml >> files. >> >> The patch 4-9 Supports the Pipeline configuration preference of IPU3 and >> default to be enabled on ChromeOS is it's specified on hal_config.yaml. >> >> Han-Lin Chen (9): >> libcamera: Introduce option to customize behavior for a camera module >> libcamera: Add options() and setOptions() operations to Camera >> libcamera: Introduce YamlParser as a helper to parse yaml files >> android: camera_hal_config: Use YamlParser to parse android hal config >> android: Add pipeline_config_file parameter for camera_hal.yaml >> android: Set PipelineConfigFile option if it's supported by the camera >> libcamera: ipu3: Add helper class PipeConfigPreference >> libcamera: ipu3: Support PipelineConfigFile option >> android: Elevate min duration to 30 FPS if it's lower within 2% >> >> README.rst | 2 +- >> include/libcamera/camera.h | 3 + >> include/libcamera/geometry.h | 4 + >> include/libcamera/internal/camera.h | 2 + >> include/libcamera/internal/meson.build | 1 + >> include/libcamera/internal/pipeline_handler.h | 2 + >> include/libcamera/internal/yaml_parser.h | 82 ++ >> include/libcamera/ipa/ipa_controls.h | 1 + >> include/libcamera/meson.build | 3 +- >> include/libcamera/option_ids.h.in | 36 + >> src/android/camera_capabilities.cpp | 31 +- >> src/android/camera_device.cpp | 19 + >> src/android/camera_hal_config.cpp | 344 ++------ >> src/android/camera_hal_config.h | 1 + >> src/android/data/nautilus/camera_hal.yaml | 2 + >> src/android/data/nautilus/imx258.yaml | 248 ++++++ >> src/android/data/soraka/camera_hal.yaml | 2 + >> src/android/data/soraka/ov13858.yaml | 236 ++++++ >> src/android/data/soraka/ov5670.yaml | 242 ++++++ >> src/libcamera/camera.cpp | 59 ++ >> src/libcamera/control_serializer.cpp | 12 + >> src/libcamera/geometry.cpp | 20 + >> src/libcamera/meson.build | 3 + >> src/libcamera/option_ids.cpp.in | 58 ++ >> src/libcamera/option_ids.yaml | 16 + >> src/libcamera/pipeline/ipu3/ipu3.cpp | 215 ++++- >> src/libcamera/pipeline/ipu3/meson.build | 1 + >> .../pipeline/ipu3/pipe_config_pref.cpp | 285 +++++++ >> .../pipeline/ipu3/pipe_config_pref.h | 80 ++ >> src/libcamera/pipeline_handler.cpp | 26 + >> src/libcamera/yaml_parser.cpp | 796 ++++++++++++++++++ >> 31 files changed, 2563 insertions(+), 269 deletions(-) >> create mode 100644 include/libcamera/internal/yaml_parser.h >> create mode 100644 include/libcamera/option_ids.h.in >> create mode 100644 src/android/data/nautilus/imx258.yaml >> create mode 100644 src/android/data/soraka/ov13858.yaml >> create mode 100644 src/android/data/soraka/ov5670.yaml >> create mode 100644 src/libcamera/option_ids.cpp.in >> create mode 100644 src/libcamera/option_ids.yaml >> create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.cpp >> create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.h >> create mode 100644 src/libcamera/yaml_parser.cpp >> >> -- >> 2.35.0.263.gb82422642f-goog >>
Quoting Hanlin Chen via libcamera-devel (2023-01-07 12:30:43) > Hi Naushir, > > Thanks for reaching out. > In fact, I have no preference either way, as long as we can get the > config file for the pipeline handler :) It looks like I'm going to merge at least the core support for pipeline configuration files from Naush's series quite soon. > Just that ChromeOS usually does not use environment variables to > define config files. > If the helper function PipelineHandler::configurationFile() can have a > default path, maybe only for the ChromeOS builds, that would be good > enough for me. I think I would like to see a 'core' libcamera configuration file, with a well defined path that can set anything that we currently set by environment variable. -- Kieran > Please don't hesitate to continue your changes. I will follow up when > I get cycles to update my series :P > > On Thu, Dec 8, 2022 at 6:28 PM Naushir Patuck <naush@raspberrypi.com> wrote: > > > > Hi Han-Lin, > > > > I realise this replay might be very (very) late now :-) However, I have been > > working on something very similar to this work for the Raspberry Pi pipeline > > handler: https://patchwork.libcamera.org/project/libcamera/list/?series=3659. > > I just wanted to reach out to see if there is any common ground between both our > > patch series that would help us both. > > > > Having briefly looked through your series, I think the key difference is how we > > provide the config file to the respective pipeline handlers. I've opted for a > > simpler approach of using an environment variable to provide a filename to the > > pipeline handler. There is a helper function PipelineHandler::configurationFile() > > that will attempt to locate the file from either the system install path, or > > the source directory. It's up to the respective pipeline handler to parse the > > file and interpret the parameters as required. > > > > Is this approach something you can make use of for your series, replacing the > > "options" mechanism that you have implemented? As always, feel free to ask any > > questions if you have them. > > > > Regards, > > Naush > > > > On Wed, 9 Feb 2022 at 07:19, Han-Lin Chen <hanlinchen@chromium.org> wrote: > >> > >> Hello, > >> > >> This patch series is to introduce the pipeline configuration preference > >> for IPU3. Before the series, IPU3 calculates the ImgU configuration > >> based on the algorithm from Intel: > >> https://github.com/intel/intel-ipu3-pipecfg > >> > >> However, IPU3 seems not well defined with the limitation on ImgU and > >> leading to configuration failure in some cases. See: > >> https://bugs.libcamera.org/show_bug.cgi?id=67 > >> > >> Due to this, on ChromeOS, Intel made manual adjusts on the validated > >> setting for each camera module. The patch series is to introduce the > >> feature for IPU3, and possibly other platforms with similar issues. > >> > >> The patch 1-2 is to introduce Option control for user to enable features > >> which need to be selected before querying capabilities of a camera. > >> > >> The patch 3 Adds helper class to wrap libyaml for easier reading yaml > >> files. > >> > >> The patch 4-9 Supports the Pipeline configuration preference of IPU3 and > >> default to be enabled on ChromeOS is it's specified on hal_config.yaml. > >> > >> Han-Lin Chen (9): > >> libcamera: Introduce option to customize behavior for a camera module > >> libcamera: Add options() and setOptions() operations to Camera > >> libcamera: Introduce YamlParser as a helper to parse yaml files > >> android: camera_hal_config: Use YamlParser to parse android hal config > >> android: Add pipeline_config_file parameter for camera_hal.yaml > >> android: Set PipelineConfigFile option if it's supported by the camera > >> libcamera: ipu3: Add helper class PipeConfigPreference > >> libcamera: ipu3: Support PipelineConfigFile option > >> android: Elevate min duration to 30 FPS if it's lower within 2% > >> > >> README.rst | 2 +- > >> include/libcamera/camera.h | 3 + > >> include/libcamera/geometry.h | 4 + > >> include/libcamera/internal/camera.h | 2 + > >> include/libcamera/internal/meson.build | 1 + > >> include/libcamera/internal/pipeline_handler.h | 2 + > >> include/libcamera/internal/yaml_parser.h | 82 ++ > >> include/libcamera/ipa/ipa_controls.h | 1 + > >> include/libcamera/meson.build | 3 +- > >> include/libcamera/option_ids.h.in | 36 + > >> src/android/camera_capabilities.cpp | 31 +- > >> src/android/camera_device.cpp | 19 + > >> src/android/camera_hal_config.cpp | 344 ++------ > >> src/android/camera_hal_config.h | 1 + > >> src/android/data/nautilus/camera_hal.yaml | 2 + > >> src/android/data/nautilus/imx258.yaml | 248 ++++++ > >> src/android/data/soraka/camera_hal.yaml | 2 + > >> src/android/data/soraka/ov13858.yaml | 236 ++++++ > >> src/android/data/soraka/ov5670.yaml | 242 ++++++ > >> src/libcamera/camera.cpp | 59 ++ > >> src/libcamera/control_serializer.cpp | 12 + > >> src/libcamera/geometry.cpp | 20 + > >> src/libcamera/meson.build | 3 + > >> src/libcamera/option_ids.cpp.in | 58 ++ > >> src/libcamera/option_ids.yaml | 16 + > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 215 ++++- > >> src/libcamera/pipeline/ipu3/meson.build | 1 + > >> .../pipeline/ipu3/pipe_config_pref.cpp | 285 +++++++ > >> .../pipeline/ipu3/pipe_config_pref.h | 80 ++ > >> src/libcamera/pipeline_handler.cpp | 26 + > >> src/libcamera/yaml_parser.cpp | 796 ++++++++++++++++++ > >> 31 files changed, 2563 insertions(+), 269 deletions(-) > >> create mode 100644 include/libcamera/internal/yaml_parser.h > >> create mode 100644 include/libcamera/option_ids.h.in > >> create mode 100644 src/android/data/nautilus/imx258.yaml > >> create mode 100644 src/android/data/soraka/ov13858.yaml > >> create mode 100644 src/android/data/soraka/ov5670.yaml > >> create mode 100644 src/libcamera/option_ids.cpp.in > >> create mode 100644 src/libcamera/option_ids.yaml > >> create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.cpp > >> create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.h > >> create mode 100644 src/libcamera/yaml_parser.cpp > >> > >> -- > >> 2.35.0.263.gb82422642f-goog > >>