| Message ID | 20220427223004.115381-1-Rauch.Christian@gmx.de |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi all, Can you let me know what is missing to go forward with this? I think I resolved all comments. Best, Christian Am 27.04.22 um 23:30 schrieb Christian Rauch: > Hello, > > In version 4 of this patch set I applied minor modification from feedback that I received so far. I reorderd the commits such that the "std::optional" commit appears first and can be used without the changes related to fixed-sized Spans. > > Best, > > Christian Rauch (4): > use std::optional to handle invalid control values > libcamera: controls: Define size of array controls as a shape vector > generate fixed- and variable-sized Span Controls > apply explicit fixed-sized Span type casts > > include/libcamera/controls.h | 7 ++-- > src/cam/main.cpp | 4 +-- > src/ipa/raspberrypi/raspberrypi.cpp | 21 ++++++------ > src/libcamera/control_ids.yaml | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++--- > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++--- > src/libcamera/property_ids.yaml | 4 +-- > src/qcam/dng_writer.cpp | 15 +++++---- > utils/gen-controls.py | 32 +++++++++++++------ > 9 files changed, 61 insertions(+), 43 deletions(-) > > -- > 2.34.1
Quoting Christian Rauch via libcamera-devel (2022-05-17 23:44:31) > Hi all, > > Can you let me know what is missing to go forward with this? I think I > resolved all comments. > The outstanding investigation was not knowing if opencv could support features from C++17. It looks like they use C++11, and we're already using C++14 - so this may mean that it is already not possible for open-cv to implement a libcamera plugin directly until they update their C++ standard. The difficulty is, as a library that could be widely used - to support more projects would require conservative updating of the C++ standard. So somehow we have to decide when we can use C++ 17 features on our public API. We could also simply choose to do so - and therefore enforce any projects that want to use libcamera to update... For applciations that's probably trivial - but for other libraries, like opencv, well then they face the same issue - that they might not be able to update easily. -- Kieran > Best, > Christian > > > Am 27.04.22 um 23:30 schrieb Christian Rauch: > > Hello, > > > > In version 4 of this patch set I applied minor modification from feedback that I received so far. I reorderd the commits such that the "std::optional" commit appears first and can be used without the changes related to fixed-sized Spans. > > > > Best, > > > > Christian Rauch (4): > > use std::optional to handle invalid control values > > libcamera: controls: Define size of array controls as a shape vector > > generate fixed- and variable-sized Span Controls > > apply explicit fixed-sized Span type casts > > > > include/libcamera/controls.h | 7 ++-- > > src/cam/main.cpp | 4 +-- > > src/ipa/raspberrypi/raspberrypi.cpp | 21 ++++++------ > > src/libcamera/control_ids.yaml | 2 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++--- > > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++--- > > src/libcamera/property_ids.yaml | 4 +-- > > src/qcam/dng_writer.cpp | 15 +++++---- > > utils/gen-controls.py | 32 +++++++++++++------ > > 9 files changed, 61 insertions(+), 43 deletions(-) > > > > -- > > 2.34.1
Dear Kieran, Is this a question of ABI or API compatibility? I understand that if you raise the required C++ standard for libcamera, you also have to compile OpenCV with that higher C++ standard. But why is this a problem? C++17 compilers are now widely available. Since newer standards are backwards compatible, other projects do not have to make changes to their source. Am I missing something here? What is the concrete problem? Are you worried that other projects will not compile with newer C++ versions? Best, Christian Am 18.05.22 um 00:56 schrieb Kieran Bingham: > Quoting Christian Rauch via libcamera-devel (2022-05-17 23:44:31) >> Hi all, >> >> Can you let me know what is missing to go forward with this? I think I >> resolved all comments. >> > > The outstanding investigation was not knowing if opencv could support > features from C++17. > > It looks like they use C++11, and we're already using C++14 - so this > may mean that it is already not possible for open-cv to implement a > libcamera plugin directly until they update their C++ standard. > > The difficulty is, as a library that could be widely used - to support > more projects would require conservative updating of the C++ standard. > So somehow we have to decide when we can use C++ 17 features on our > public API. > > We could also simply choose to do so - and therefore enforce any > projects that want to use libcamera to update... For applciations that's > probably trivial - but for other libraries, like opencv, well then they > face the same issue - that they might not be able to update easily. > > -- > Kieran > >> Best, >> Christian >> >> >> Am 27.04.22 um 23:30 schrieb Christian Rauch: >>> Hello, >>> >>> In version 4 of this patch set I applied minor modification from feedback that I received so far. I reorderd the commits such that the "std::optional" commit appears first and can be used without the changes related to fixed-sized Spans. >>> >>> Best, >>> >>> Christian Rauch (4): >>> use std::optional to handle invalid control values >>> libcamera: controls: Define size of array controls as a shape vector >>> generate fixed- and variable-sized Span Controls >>> apply explicit fixed-sized Span type casts >>> >>> include/libcamera/controls.h | 7 ++-- >>> src/cam/main.cpp | 4 +-- >>> src/ipa/raspberrypi/raspberrypi.cpp | 21 ++++++------ >>> src/libcamera/control_ids.yaml | 2 +- >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++--- >>> .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++--- >>> src/libcamera/property_ids.yaml | 4 +-- >>> src/qcam/dng_writer.cpp | 15 +++++---- >>> utils/gen-controls.py | 32 +++++++++++++------ >>> 9 files changed, 61 insertions(+), 43 deletions(-) >>> >>> -- >>> 2.34.1
Hi Kieran, Looking at this again ... libcamera already uses "std::optional" in the public API and requires C++17. Using "std::optional" additionally for the control values does not increase the requirements. Best, Christian Am 18.05.22 um 00:56 schrieb Kieran Bingham: > Quoting Christian Rauch via libcamera-devel (2022-05-17 23:44:31) >> Hi all, >> >> Can you let me know what is missing to go forward with this? I think I >> resolved all comments. >> > > The outstanding investigation was not knowing if opencv could support > features from C++17. > > It looks like they use C++11, and we're already using C++14 - so this > may mean that it is already not possible for open-cv to implement a > libcamera plugin directly until they update their C++ standard. > > The difficulty is, as a library that could be widely used - to support > more projects would require conservative updating of the C++ standard. > So somehow we have to decide when we can use C++ 17 features on our > public API. > > We could also simply choose to do so - and therefore enforce any > projects that want to use libcamera to update... For applciations that's > probably trivial - but for other libraries, like opencv, well then they > face the same issue - that they might not be able to update easily. > > -- > Kieran > >> Best, >> Christian >> >> >> Am 27.04.22 um 23:30 schrieb Christian Rauch: >>> Hello, >>> >>> In version 4 of this patch set I applied minor modification from feedback that I received so far. I reorderd the commits such that the "std::optional" commit appears first and can be used without the changes related to fixed-sized Spans. >>> >>> Best, >>> >>> Christian Rauch (4): >>> use std::optional to handle invalid control values >>> libcamera: controls: Define size of array controls as a shape vector >>> generate fixed- and variable-sized Span Controls >>> apply explicit fixed-sized Span type casts >>> >>> include/libcamera/controls.h | 7 ++-- >>> src/cam/main.cpp | 4 +-- >>> src/ipa/raspberrypi/raspberrypi.cpp | 21 ++++++------ >>> src/libcamera/control_ids.yaml | 2 +- >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++--- >>> .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++--- >>> src/libcamera/property_ids.yaml | 4 +-- >>> src/qcam/dng_writer.cpp | 15 +++++---- >>> utils/gen-controls.py | 32 +++++++++++++------ >>> 9 files changed, 61 insertions(+), 43 deletions(-) >>> >>> -- >>> 2.34.1