| Message ID | 20210126130415.26849-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Tue, Jan 26, 2021 at 02:04:13PM +0100, Jean-Michel Hautbois wrote: > Some applications may not be compliant with C++17 (Chromium, as an > example). Keep the C++17 features for libcamera internals, and C++14 > compliance for public API. This should be captured in the commit message of the two patches, as the cover letter won't end up in the git history. The series make sense, the more lightweight our requirements will be in terms of modern features in the public API, the more applications will be able to use libcamera. libcamera itself stays C++17, but at this point of time, sticking to C++14 in the public API isn't a huge hassle. This is of course a matter of finding a proper balance, I'm sure we will eventually want to move the public API to C++17 too. Chromium is the only application I'm aware of that is affected by the lack of C++17 support, and it will eventually get migrated, solving this particular issue. We will then be able to revert the reverts, if desired. I've also been wondering if the libcamera integration in Chromium could be compiled with -std=c++17, with the rest of the browser still compiled in C++14 mode. It could be useful to get a confirmation from Google on whether this is possible or not. I've asked and am waiting for feedback, but this isn't blocking and we can move ahead with this series. With the commit message updated (which can be done when applying), Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'll give others a few days to shime in if desired before pushing this. > --- > Changes in v2: > - keep each revert separate > > Jean-Michel Hautbois (2): > Revert "libcamera: span: Provide and use helper variable templates for > type traits" > Revert "libcamera: Use helper variable template for type traits" > > include/libcamera/bound_method.h | 2 +- > include/libcamera/controls.h | 24 +++++------ > include/libcamera/object.h | 2 +- > include/libcamera/signal.h | 4 +- > include/libcamera/span.h | 70 +++++++++++++++----------------- > 5 files changed, 48 insertions(+), 54 deletions(-)
On Tue, Jan 26, 2021 at 11:19:06PM +0200, Laurent Pinchart wrote: > Hi Jean-Michel, > > Thank you for the patch. > > On Tue, Jan 26, 2021 at 02:04:13PM +0100, Jean-Michel Hautbois wrote: > > Some applications may not be compliant with C++17 (Chromium, as an > > example). Keep the C++17 features for libcamera internals, and C++14 > > compliance for public API. > > This should be captured in the commit message of the two patches, as the > cover letter won't end up in the git history. > > The series make sense, the more lightweight our requirements will be in > terms of modern features in the public API, the more applications will > be able to use libcamera. libcamera itself stays C++17, but at this > point of time, sticking to C++14 in the public API isn't a huge hassle. > > This is of course a matter of finding a proper balance, I'm sure we will > eventually want to move the public API to C++17 too. Chromium is the > only application I'm aware of that is affected by the lack of C++17 > support, and it will eventually get migrated, solving this particular > issue. We will then be able to revert the reverts, if desired. > > I've also been wondering if the libcamera integration in Chromium could > be compiled with -std=c++17, with the rest of the browser still compiled > in C++14 mode. It could be useful to get a confirmation from Google on > whether this is possible or not. I've asked and am waiting for feedback, > but this isn't blocking and we can move ahead with this series. https://github.com/jhautbois/chromium/blob/master/skia/BUILD.gn#L205 Could something similar be done for libcamera ? > With the commit message updated (which can be done when applying), > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I'll give others a few days to shime in if desired before pushing this. > > > --- > > Changes in v2: > > - keep each revert separate > > > > Jean-Michel Hautbois (2): > > Revert "libcamera: span: Provide and use helper variable templates for > > type traits" > > Revert "libcamera: Use helper variable template for type traits" > > > > include/libcamera/bound_method.h | 2 +- > > include/libcamera/controls.h | 24 +++++------ > > include/libcamera/object.h | 2 +- > > include/libcamera/signal.h | 4 +- > > include/libcamera/span.h | 70 +++++++++++++++----------------- > > 5 files changed, 48 insertions(+), 54 deletions(-)
Hi Laurent, On 26/01/2021 22:25, Laurent Pinchart wrote: > On Tue, Jan 26, 2021 at 11:19:06PM +0200, Laurent Pinchart wrote: >> Hi Jean-Michel, >> >> Thank you for the patch. >> >> On Tue, Jan 26, 2021 at 02:04:13PM +0100, Jean-Michel Hautbois wrote: >>> Some applications may not be compliant with C++17 (Chromium, as an >>> example). Keep the C++17 features for libcamera internals, and C++14 >>> compliance for public API. >> >> This should be captured in the commit message of the two patches, as the >> cover letter won't end up in the git history. >> >> The series make sense, the more lightweight our requirements will be in >> terms of modern features in the public API, the more applications will >> be able to use libcamera. libcamera itself stays C++17, but at this >> point of time, sticking to C++14 in the public API isn't a huge hassle. >> >> This is of course a matter of finding a proper balance, I'm sure we will >> eventually want to move the public API to C++17 too. Chromium is the >> only application I'm aware of that is affected by the lack of C++17 >> support, and it will eventually get migrated, solving this particular >> issue. We will then be able to revert the reverts, if desired. >> >> I've also been wondering if the libcamera integration in Chromium could >> be compiled with -std=c++17, with the rest of the browser still compiled >> in C++14 mode. It could be useful to get a confirmation from Google on >> whether this is possible or not. I've asked and am waiting for feedback, >> but this isn't blocking and we can move ahead with this series. > > https://github.com/jhautbois/chromium/blob/master/skia/BUILD.gn#L205 > > Could something similar be done for libcamera ? Oh, nice catch, this is why I hate BUILD.gn it is not clear where the options should lie to be correctly applied :-p. I will try and get back to you ! >> With the commit message updated (which can be done when applying), >> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> I'll give others a few days to shime in if desired before pushing this. >> >>> --- >>> Changes in v2: >>> - keep each revert separate >>> >>> Jean-Michel Hautbois (2): >>> Revert "libcamera: span: Provide and use helper variable templates for >>> type traits" >>> Revert "libcamera: Use helper variable template for type traits" >>> >>> include/libcamera/bound_method.h | 2 +- >>> include/libcamera/controls.h | 24 +++++------ >>> include/libcamera/object.h | 2 +- >>> include/libcamera/signal.h | 4 +- >>> include/libcamera/span.h | 70 +++++++++++++++----------------- >>> 5 files changed, 48 insertions(+), 54 deletions(-) >
On 27/01/2021 07:46, Jean-Michel Hautbois wrote: > Hi Laurent, > > On 26/01/2021 22:25, Laurent Pinchart wrote: >> On Tue, Jan 26, 2021 at 11:19:06PM +0200, Laurent Pinchart wrote: >>> Hi Jean-Michel, >>> >>> Thank you for the patch. >>> >>> On Tue, Jan 26, 2021 at 02:04:13PM +0100, Jean-Michel Hautbois wrote: >>>> Some applications may not be compliant with C++17 (Chromium, as an >>>> example). Keep the C++17 features for libcamera internals, and C++14 >>>> compliance for public API. >>> >>> This should be captured in the commit message of the two patches, as the >>> cover letter won't end up in the git history. >>> >>> The series make sense, the more lightweight our requirements will be in >>> terms of modern features in the public API, the more applications will >>> be able to use libcamera. libcamera itself stays C++17, but at this >>> point of time, sticking to C++14 in the public API isn't a huge hassle. >>> >>> This is of course a matter of finding a proper balance, I'm sure we will >>> eventually want to move the public API to C++17 too. Chromium is the >>> only application I'm aware of that is affected by the lack of C++17 >>> support, and it will eventually get migrated, solving this particular >>> issue. We will then be able to revert the reverts, if desired. >>> >>> I've also been wondering if the libcamera integration in Chromium could >>> be compiled with -std=c++17, with the rest of the browser still compiled >>> in C++14 mode. It could be useful to get a confirmation from Google on >>> whether this is possible or not. I've asked and am waiting for feedback, >>> but this isn't blocking and we can move ahead with this series. >> >> https://github.com/jhautbois/chromium/blob/master/skia/BUILD.gn#L205 >> >> Could something similar be done for libcamera ? > > Oh, nice catch, this is why I hate BUILD.gn it is not clear where the > options should lie to be correctly applied :-p. > I will try and get back to you ! I can't make it work, so, if Google replies it can help, as I tried it in several places in the BUILD.gn file and it has no effect... :-(. >>> With the commit message updated (which can be done when applying), >>> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> >>> I'll give others a few days to shime in if desired before pushing this. >>> >>>> --- >>>> Changes in v2: >>>> - keep each revert separate >>>> >>>> Jean-Michel Hautbois (2): >>>> Revert "libcamera: span: Provide and use helper variable templates for >>>> type traits" >>>> Revert "libcamera: Use helper variable template for type traits" >>>> >>>> include/libcamera/bound_method.h | 2 +- >>>> include/libcamera/controls.h | 24 +++++------ >>>> include/libcamera/object.h | 2 +- >>>> include/libcamera/signal.h | 4 +- >>>> include/libcamera/span.h | 70 +++++++++++++++----------------- >>>> 5 files changed, 48 insertions(+), 54 deletions(-) >> >
Some applications may not be compliant with C++17 (Chromium, as an example). Keep the C++17 features for libcamera internals, and C++14 compliance for public API. --- Changes in v2: - keep each revert separate Jean-Michel Hautbois (2): Revert "libcamera: span: Provide and use helper variable templates for type traits" Revert "libcamera: Use helper variable template for type traits" include/libcamera/bound_method.h | 2 +- include/libcamera/controls.h | 24 +++++------ include/libcamera/object.h | 2 +- include/libcamera/signal.h | 4 +- include/libcamera/span.h | 70 +++++++++++++++----------------- 5 files changed, 48 insertions(+), 54 deletions(-)