Message ID | CAEmqJPohD+Pah=YcP0qJ7=_+o2WAvA+Dnw6qfo8g_DKgpwP2ag@mail.gmail.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Naush, On Thu, Nov 30, 2023 at 08:24:02AM +0000, Naushir Patuck via libcamera-devel wrote: > The following changes since commit 61f6b372421ad96e283220c2c7beb1cb298b1eaf: > > libcamera: pipeline: Fix c++20 compile warning (2023-11-29 02:50:40 +0200) > > are available in the Git repository at: > > https://github.com/naushir/libcamera.git HEAD > > for you to fetch changes up to ada0e9f440d19f41fa222a87b844a1701b26465f: > > documentation: Document vendor specific controls and properties > handling (2023-11-29 09:05:41 +0000) I've left a few review comments on v3. They're all minor. As this pull request hasn't been merged yet, would you like to address them in a new version ? The series can then be merged. > ---------------------------------------------------------------- > Naushir Patuck (7): > controls: Add vendor control/property support to generation scripts > controls: Update argument handling for controls generation scripts > build: controls: Rework how controls and properties are generated > libcamera: control: Add vendor control id range reservation > libcamera: controls: Use vendor tags for draft controls and properties > build: controls: Add Raspberry Pi vendor specific controls > documentation: Document vendor specific controls and properties handling > > Documentation/guides/pipeline-handler.rst | 60 ++++++++++++++++++-- > include/libcamera/control_ids.h.in | 6 +- > include/libcamera/meson.build | 57 ++++++++++++++++--- > include/libcamera/property_ids.h.in | 8 +-- > meson.build | 2 + > src/ipa/rpi/common/ipa_base.cpp | 2 +- > src/ipa/rpi/vc4/vc4.cpp | 2 +- > src/libcamera/control_ids.cpp.in | 14 +---- > src/libcamera/{control_ids.yaml => control_ids_core.yaml} | 233 +----------------------------------------------------------------------------- > src/libcamera/control_ids_draft.yaml | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/libcamera/control_ids_rpi.yaml | 17 ++++++ > src/libcamera/control_ranges.yaml | 18 ++++++ > src/libcamera/meson.build | 25 +++++++-- > src/libcamera/property_ids.cpp.in | 14 +---- > src/libcamera/{property_ids.yaml => property_ids_core.yaml} | 34 +----------- > src/libcamera/property_ids_draft.yaml | 39 +++++++++++++ > src/py/libcamera/gen-py-controls.py | 87 +++++++++++++++++------------ > src/py/libcamera/meson.build | 26 +++++---- > src/py/libcamera/py_controls_generated.cpp.in | 6 +- > src/py/libcamera/py_properties_generated.cpp.in | 6 +- > utils/gen-controls.py | 147 ++++++++++++++++++++++++++++++++++--------------- > 21 files changed, 614 insertions(+), 419 deletions(-) > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (79%) > create mode 100644 src/libcamera/control_ids_draft.yaml > create mode 100644 src/libcamera/control_ids_rpi.yaml > create mode 100644 src/libcamera/control_ranges.yaml > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (96%) > create mode 100644 src/libcamera/property_ids_draft.yaml
Quoting Laurent Pinchart (2023-11-30 13:04:18) > Hi Naush, > > On Thu, Nov 30, 2023 at 08:24:02AM +0000, Naushir Patuck via libcamera-devel wrote: > > The following changes since commit 61f6b372421ad96e283220c2c7beb1cb298b1eaf: > > > > libcamera: pipeline: Fix c++20 compile warning (2023-11-29 02:50:40 +0200) > > > > are available in the Git repository at: > > > > https://github.com/naushir/libcamera.git HEAD > > > > for you to fetch changes up to ada0e9f440d19f41fa222a87b844a1701b26465f: > > > > documentation: Document vendor specific controls and properties > > handling (2023-11-29 09:05:41 +0000) > > I've left a few review comments on v3. They're all minor. As this pull > request hasn't been merged yet, would you like to address them in a new > version ? The series can then be merged. Ok this was ... 'seconds' away from being merged. Integration tests all pass including no regressions on CTS. -- Kieran > > > ---------------------------------------------------------------- > > Naushir Patuck (7): > > controls: Add vendor control/property support to generation scripts > > controls: Update argument handling for controls generation scripts > > build: controls: Rework how controls and properties are generated > > libcamera: control: Add vendor control id range reservation > > libcamera: controls: Use vendor tags for draft controls and properties > > build: controls: Add Raspberry Pi vendor specific controls > > documentation: Document vendor specific controls and properties handling > > > > Documentation/guides/pipeline-handler.rst | 60 ++++++++++++++++++-- > > include/libcamera/control_ids.h.in | 6 +- > > include/libcamera/meson.build | 57 ++++++++++++++++--- > > include/libcamera/property_ids.h.in | 8 +-- > > meson.build | 2 + > > src/ipa/rpi/common/ipa_base.cpp | 2 +- > > src/ipa/rpi/vc4/vc4.cpp | 2 +- > > src/libcamera/control_ids.cpp.in | 14 +---- > > src/libcamera/{control_ids.yaml => control_ids_core.yaml} | 233 +----------------------------------------------------------------------------- > > src/libcamera/control_ids_draft.yaml | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > src/libcamera/control_ids_rpi.yaml | 17 ++++++ > > src/libcamera/control_ranges.yaml | 18 ++++++ > > src/libcamera/meson.build | 25 +++++++-- > > src/libcamera/property_ids.cpp.in | 14 +---- > > src/libcamera/{property_ids.yaml => property_ids_core.yaml} | 34 +----------- > > src/libcamera/property_ids_draft.yaml | 39 +++++++++++++ > > src/py/libcamera/gen-py-controls.py | 87 +++++++++++++++++------------ > > src/py/libcamera/meson.build | 26 +++++---- > > src/py/libcamera/py_controls_generated.cpp.in | 6 +- > > src/py/libcamera/py_properties_generated.cpp.in | 6 +- > > utils/gen-controls.py | 147 ++++++++++++++++++++++++++++++++++--------------- > > 21 files changed, 614 insertions(+), 419 deletions(-) > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (79%) > > create mode 100644 src/libcamera/control_ids_draft.yaml > > create mode 100644 src/libcamera/control_ids_rpi.yaml > > create mode 100644 src/libcamera/control_ranges.yaml > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (96%) > > create mode 100644 src/libcamera/property_ids_draft.yaml > > -- > Regards, > > Laurent Pinchart
On Thu, Nov 30, 2023 at 01:06:59PM +0000, Kieran Bingham wrote: > Quoting Laurent Pinchart (2023-11-30 13:04:18) > > On Thu, Nov 30, 2023 at 08:24:02AM +0000, Naushir Patuck via libcamera-devel wrote: > > > The following changes since commit 61f6b372421ad96e283220c2c7beb1cb298b1eaf: > > > > > > libcamera: pipeline: Fix c++20 compile warning (2023-11-29 02:50:40 +0200) > > > > > > are available in the Git repository at: > > > > > > https://github.com/naushir/libcamera.git HEAD > > > > > > for you to fetch changes up to ada0e9f440d19f41fa222a87b844a1701b26465f: > > > > > > documentation: Document vendor specific controls and properties > > > handling (2023-11-29 09:05:41 +0000) > > > > I've left a few review comments on v3. They're all minor. As this pull > > request hasn't been merged yet, would you like to address them in a new > > version ? The series can then be merged. > > Ok this was ... 'seconds' away from being merged. Integration tests all > pass including no regressions on CTS. We can possibly fix things on top if preferred, as long as it doesn't take long. > > > ---------------------------------------------------------------- > > > Naushir Patuck (7): > > > controls: Add vendor control/property support to generation scripts > > > controls: Update argument handling for controls generation scripts > > > build: controls: Rework how controls and properties are generated > > > libcamera: control: Add vendor control id range reservation > > > libcamera: controls: Use vendor tags for draft controls and properties > > > build: controls: Add Raspberry Pi vendor specific controls > > > documentation: Document vendor specific controls and properties handling > > > > > > Documentation/guides/pipeline-handler.rst | 60 ++++++++++++++++++-- > > > include/libcamera/control_ids.h.in | 6 +- > > > include/libcamera/meson.build | 57 ++++++++++++++++--- > > > include/libcamera/property_ids.h.in | 8 +-- > > > meson.build | 2 + > > > src/ipa/rpi/common/ipa_base.cpp | 2 +- > > > src/ipa/rpi/vc4/vc4.cpp | 2 +- > > > src/libcamera/control_ids.cpp.in | 14 +---- > > > src/libcamera/{control_ids.yaml => control_ids_core.yaml} | 233 +----------------------------------------------------------------------------- > > > src/libcamera/control_ids_draft.yaml | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > src/libcamera/control_ids_rpi.yaml | 17 ++++++ > > > src/libcamera/control_ranges.yaml | 18 ++++++ > > > src/libcamera/meson.build | 25 +++++++-- > > > src/libcamera/property_ids.cpp.in | 14 +---- > > > src/libcamera/{property_ids.yaml => property_ids_core.yaml} | 34 +----------- > > > src/libcamera/property_ids_draft.yaml | 39 +++++++++++++ > > > src/py/libcamera/gen-py-controls.py | 87 +++++++++++++++++------------ > > > src/py/libcamera/meson.build | 26 +++++---- > > > src/py/libcamera/py_controls_generated.cpp.in | 6 +- > > > src/py/libcamera/py_properties_generated.cpp.in | 6 +- > > > utils/gen-controls.py | 147 ++++++++++++++++++++++++++++++++++--------------- > > > 21 files changed, 614 insertions(+), 419 deletions(-) > > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (79%) > > > create mode 100644 src/libcamera/control_ids_draft.yaml > > > create mode 100644 src/libcamera/control_ids_rpi.yaml > > > create mode 100644 src/libcamera/control_ranges.yaml > > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (96%) > > > create mode 100644 src/libcamera/property_ids_draft.yaml
Hi Laurent, On Thu, 30 Nov 2023 at 13:23, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Thu, Nov 30, 2023 at 01:06:59PM +0000, Kieran Bingham wrote: > > Quoting Laurent Pinchart (2023-11-30 13:04:18) > > > On Thu, Nov 30, 2023 at 08:24:02AM +0000, Naushir Patuck via libcamera-devel wrote: > > > > The following changes since commit 61f6b372421ad96e283220c2c7beb1cb298b1eaf: > > > > > > > > libcamera: pipeline: Fix c++20 compile warning (2023-11-29 02:50:40 +0200) > > > > > > > > are available in the Git repository at: > > > > > > > > https://github.com/naushir/libcamera.git HEAD > > > > > > > > for you to fetch changes up to ada0e9f440d19f41fa222a87b844a1701b26465f: > > > > > > > > documentation: Document vendor specific controls and properties > > > > handling (2023-11-29 09:05:41 +0000) > > > > > > I've left a few review comments on v3. They're all minor. As this pull > > > request hasn't been merged yet, would you like to address them in a new > > > version ? The series can then be merged. > > > > Ok this was ... 'seconds' away from being merged. Integration tests all > > pass including no regressions on CTS. > > We can possibly fix things on top if preferred, as long as it doesn't > take long. I can make the changes now before merging. However, to clarify, here's what I think needs to change: - Capatilise the vendor string in the gen-py-controls.py script. - Remove the rpi vendor controls commit from this series. Other than that, there is the comment about adding core controls to libcamera::controls:core:: namespace. If possible I think we defer this one as it's a bit more effort and I would probably prefer to do that on-top. If I've captured all the changes correctly, should I go ahead and make the changes and do a git-request-pull, or would you prefer to see an update posted to the ML first? Regards, Naush > > > > > ---------------------------------------------------------------- > > > > Naushir Patuck (7): > > > > controls: Add vendor control/property support to generation scripts > > > > controls: Update argument handling for controls generation scripts > > > > build: controls: Rework how controls and properties are generated > > > > libcamera: control: Add vendor control id range reservation > > > > libcamera: controls: Use vendor tags for draft controls and properties > > > > build: controls: Add Raspberry Pi vendor specific controls > > > > documentation: Document vendor specific controls and properties handling > > > > > > > > Documentation/guides/pipeline-handler.rst | 60 ++++++++++++++++++-- > > > > include/libcamera/control_ids.h.in | 6 +- > > > > include/libcamera/meson.build | 57 ++++++++++++++++--- > > > > include/libcamera/property_ids.h.in | 8 +-- > > > > meson.build | 2 + > > > > src/ipa/rpi/common/ipa_base.cpp | 2 +- > > > > src/ipa/rpi/vc4/vc4.cpp | 2 +- > > > > src/libcamera/control_ids.cpp.in | 14 +---- > > > > src/libcamera/{control_ids.yaml => control_ids_core.yaml} | 233 +----------------------------------------------------------------------------- > > > > src/libcamera/control_ids_draft.yaml | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > src/libcamera/control_ids_rpi.yaml | 17 ++++++ > > > > src/libcamera/control_ranges.yaml | 18 ++++++ > > > > src/libcamera/meson.build | 25 +++++++-- > > > > src/libcamera/property_ids.cpp.in | 14 +---- > > > > src/libcamera/{property_ids.yaml => property_ids_core.yaml} | 34 +----------- > > > > src/libcamera/property_ids_draft.yaml | 39 +++++++++++++ > > > > src/py/libcamera/gen-py-controls.py | 87 +++++++++++++++++------------ > > > > src/py/libcamera/meson.build | 26 +++++---- > > > > src/py/libcamera/py_controls_generated.cpp.in | 6 +- > > > > src/py/libcamera/py_properties_generated.cpp.in | 6 +- > > > > utils/gen-controls.py | 147 ++++++++++++++++++++++++++++++++++--------------- > > > > 21 files changed, 614 insertions(+), 419 deletions(-) > > > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (79%) > > > > create mode 100644 src/libcamera/control_ids_draft.yaml > > > > create mode 100644 src/libcamera/control_ids_rpi.yaml > > > > create mode 100644 src/libcamera/control_ranges.yaml > > > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (96%) > > > > create mode 100644 src/libcamera/property_ids_draft.yaml > > -- > Regards, > > Laurent Pinchart
Hi Naush, On Thu, Nov 30, 2023 at 01:48:25PM +0000, Naushir Patuck wrote: > On Thu, 30 Nov 2023 at 13:23, Laurent Pinchart wrote: > > On Thu, Nov 30, 2023 at 01:06:59PM +0000, Kieran Bingham wrote: > > > Quoting Laurent Pinchart (2023-11-30 13:04:18) > > > > On Thu, Nov 30, 2023 at 08:24:02AM +0000, Naushir Patuck via libcamera-devel wrote: > > > > > The following changes since commit 61f6b372421ad96e283220c2c7beb1cb298b1eaf: > > > > > > > > > > libcamera: pipeline: Fix c++20 compile warning (2023-11-29 02:50:40 +0200) > > > > > > > > > > are available in the Git repository at: > > > > > > > > > > https://github.com/naushir/libcamera.git HEAD > > > > > > > > > > for you to fetch changes up to ada0e9f440d19f41fa222a87b844a1701b26465f: > > > > > > > > > > documentation: Document vendor specific controls and properties > > > > > handling (2023-11-29 09:05:41 +0000) > > > > > > > > I've left a few review comments on v3. They're all minor. As this pull > > > > request hasn't been merged yet, would you like to address them in a new > > > > version ? The series can then be merged. > > > > > > Ok this was ... 'seconds' away from being merged. Integration tests all > > > pass including no regressions on CTS. > > > > We can possibly fix things on top if preferred, as long as it doesn't > > take long. > > I can make the changes now before merging. However, to clarify, > here's what I think needs to change: > > - Capatilise the vendor string in the gen-py-controls.py script. > - Remove the rpi vendor controls commit from this series. > > Other than that, there is the comment about adding core controls to > libcamera::controls:core:: namespace. If possible I think we defer > this one as it's a bit more effort and I would probably prefer to do > that on-top. We're on the same page, that's what I had in mind. Thanks for clarifying it, it's always best to try and save a round trip :-) > If I've captured all the changes correctly, should I go ahead and make > the changes and do a git-request-pull, or would you prefer to see an > update posted to the ML first? Could you do both ? I trust you'll get the changes right, so you can post a pull request right away, but I'd like the patch to be on the list for reference. That holds true for future pull requests, if you make non-trivial changes on top of the last version that you include in the pull request, please also send them to the list. For small typos you don't need to bother. > > > > > ---------------------------------------------------------------- > > > > > Naushir Patuck (7): > > > > > controls: Add vendor control/property support to generation scripts > > > > > controls: Update argument handling for controls generation scripts > > > > > build: controls: Rework how controls and properties are generated > > > > > libcamera: control: Add vendor control id range reservation > > > > > libcamera: controls: Use vendor tags for draft controls and properties > > > > > build: controls: Add Raspberry Pi vendor specific controls > > > > > documentation: Document vendor specific controls and properties handling > > > > > > > > > > Documentation/guides/pipeline-handler.rst | 60 ++++++++++++++++++-- > > > > > include/libcamera/control_ids.h.in | 6 +- > > > > > include/libcamera/meson.build | 57 ++++++++++++++++--- > > > > > include/libcamera/property_ids.h.in | 8 +-- > > > > > meson.build | 2 + > > > > > src/ipa/rpi/common/ipa_base.cpp | 2 +- > > > > > src/ipa/rpi/vc4/vc4.cpp | 2 +- > > > > > src/libcamera/control_ids.cpp.in | 14 +---- > > > > > src/libcamera/{control_ids.yaml => control_ids_core.yaml} | 233 +----------------------------------------------------------------------------- > > > > > src/libcamera/control_ids_draft.yaml | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > src/libcamera/control_ids_rpi.yaml | 17 ++++++ > > > > > src/libcamera/control_ranges.yaml | 18 ++++++ > > > > > src/libcamera/meson.build | 25 +++++++-- > > > > > src/libcamera/property_ids.cpp.in | 14 +---- > > > > > src/libcamera/{property_ids.yaml => property_ids_core.yaml} | 34 +----------- > > > > > src/libcamera/property_ids_draft.yaml | 39 +++++++++++++ > > > > > src/py/libcamera/gen-py-controls.py | 87 +++++++++++++++++------------ > > > > > src/py/libcamera/meson.build | 26 +++++---- > > > > > src/py/libcamera/py_controls_generated.cpp.in | 6 +- > > > > > src/py/libcamera/py_properties_generated.cpp.in | 6 +- > > > > > utils/gen-controls.py | 147 ++++++++++++++++++++++++++++++++++--------------- > > > > > 21 files changed, 614 insertions(+), 419 deletions(-) > > > > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (79%) > > > > > create mode 100644 src/libcamera/control_ids_draft.yaml > > > > > create mode 100644 src/libcamera/control_ids_rpi.yaml > > > > > create mode 100644 src/libcamera/control_ranges.yaml > > > > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (96%) > > > > > create mode 100644 src/libcamera/property_ids_draft.yaml