| Message ID | 20200923164412.319079-1-kieran.bingham@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patches. On Wed, Sep 23, 2020 at 05:44:08PM +0100, Kieran Bingham wrote: > The simple-cam app needs to be updated with a few API updates to work on > the latest libcamera builds. > > This is based on the code at https://github.com/kbingham/simple-cam > though I hope to move this to a more official location soon. For the whole series, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> It would be interesting to test-compile simple-cam when libcamera gains new commits. It can probably wait until libcamera itself gets CI though :-) > Kieran Bingham (4): > simple-cam: Use the new BufferMap interface > simple-cam: Use a const stream > meson: Remove incorrect default > meson: Update to C++17 > > meson.build | 3 +-- > simple-cam.cpp | 4 ++-- > 2 files changed, 3 insertions(+), 4 deletions(-)
Hi Laurent, On 24/09/2020 04:14, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patches. > > On Wed, Sep 23, 2020 at 05:44:08PM +0100, Kieran Bingham wrote: >> The simple-cam app needs to be updated with a few API updates to work on >> the latest libcamera builds. >> >> This is based on the code at https://github.com/kbingham/simple-cam >> though I hope to move this to a more official location soon. > > For the whole series, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > It would be interesting to test-compile simple-cam when libcamera gains > new commits. It can probably wait until libcamera itself gets CI though > :-) This is something I've been working on. There are two options, an easy way and a hard way (to do this without installing the library on each build). The hard way (which is of course what was recommended in #meson) is to provide a meson wrap for libcamera, so that it can be a subproject to simple-cam. This means if libcamera is not found as a dependency it will clone libcamera, build it - and automatically link against it statically. It doesn't work - due to faults in our set up of the build system and incorrect usage of things like meson.build_root() and meson.source_root(). My conversation led to one of the meson devs deciding to mark those as deprecated : (See https://github.com/mesonbuild/meson/pull/7772) Anyway, the easy and quick way - is that meson creates a pkg-config '-uninstalled' location at $BUILD/meson-uninstalled ... and it's really easy to build an app against an uninstalled library by pointing PKG_CONFIG_DIR=$BUILD/meson-uninstalled So that's a really quick and effective way of also compiling the 'external' simple-cam against a 'just built' libcamera. I'm adding that to my daily builder, (the one that also does a coverity-scan build, and a full matrix build of all compilers on my laptop) - so I'll know as soon as there is breakage. And indeed, sometime we'll move this to a more fully handled CI system too. Anyway, thanks for the review, I'll push these patches to my repo. -- Kieran > >> Kieran Bingham (4): >> simple-cam: Use the new BufferMap interface >> simple-cam: Use a const stream >> meson: Remove incorrect default >> meson: Update to C++17 >> >> meson.build | 3 +-- >> simple-cam.cpp | 4 ++-- >> 2 files changed, 3 insertions(+), 4 deletions(-) >
Hi Kieran, On Thu, Sep 24, 2020 at 09:52:11AM +0100, Kieran Bingham wrote: > On 24/09/2020 04:14, Laurent Pinchart wrote: > > On Wed, Sep 23, 2020 at 05:44:08PM +0100, Kieran Bingham wrote: > >> The simple-cam app needs to be updated with a few API updates to work on > >> the latest libcamera builds. > >> > >> This is based on the code at https://github.com/kbingham/simple-cam > >> though I hope to move this to a more official location soon. > > > > For the whole series, > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > It would be interesting to test-compile simple-cam when libcamera gains > > new commits. It can probably wait until libcamera itself gets CI though > > :-) > > This is something I've been working on. > > There are two options, an easy way and a hard way (to do this without > installing the library on each build). > > The hard way (which is of course what was recommended in #meson) is to > provide a meson wrap for libcamera, so that it can be a subproject to > simple-cam. This means if libcamera is not found as a dependency it will > clone libcamera, build it - and automatically link against it statically. > > It doesn't work - due to faults in our set up of the build system and > incorrect usage of things like meson.build_root() and meson.source_root(). Patches are welcome :-) > My conversation led to one of the meson devs deciding to mark those as > deprecated : > (See https://github.com/mesonbuild/meson/pull/7772) > > Anyway, the easy and quick way - is that meson creates a pkg-config > '-uninstalled' location at $BUILD/meson-uninstalled ... and it's really > easy to build an app against an uninstalled library by pointing > PKG_CONFIG_DIR=$BUILD/meson-uninstalled > > So that's a really quick and effective way of also compiling the > 'external' simple-cam against a 'just built' libcamera. From a build server point of view, I thought the builder could get the latest libcamera master branch, build it, and install it. Why would we need an "uninstalled" location ? > I'm adding that to my daily builder, (the one that also does a > coverity-scan build, and a full matrix build of all compilers on my > laptop) - so I'll know as soon as there is breakage. > > And indeed, sometime we'll move this to a more fully handled CI system too. > > Anyway, thanks for the review, I'll push these patches to my repo. > > >> Kieran Bingham (4): > >> simple-cam: Use the new BufferMap interface > >> simple-cam: Use a const stream > >> meson: Remove incorrect default > >> meson: Update to C++17 > >> > >> meson.build | 3 +-- > >> simple-cam.cpp | 4 ++-- > >> 2 files changed, 3 insertions(+), 4 deletions(-)
Hi Laurent, On 24/09/2020 12:44, Laurent Pinchart wrote: > Hi Kieran, > > On Thu, Sep 24, 2020 at 09:52:11AM +0100, Kieran Bingham wrote: >> On 24/09/2020 04:14, Laurent Pinchart wrote: >>> On Wed, Sep 23, 2020 at 05:44:08PM +0100, Kieran Bingham wrote: >>>> The simple-cam app needs to be updated with a few API updates to work on >>>> the latest libcamera builds. >>>> >>>> This is based on the code at https://github.com/kbingham/simple-cam >>>> though I hope to move this to a more official location soon. >>> >>> For the whole series, >>> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> >>> It would be interesting to test-compile simple-cam when libcamera gains >>> new commits. It can probably wait until libcamera itself gets CI though >>> :-) >> >> This is something I've been working on. >> >> There are two options, an easy way and a hard way (to do this without >> installing the library on each build). >> >> The hard way (which is of course what was recommended in #meson) is to >> provide a meson wrap for libcamera, so that it can be a subproject to >> simple-cam. This means if libcamera is not found as a dependency it will >> clone libcamera, build it - and automatically link against it statically. >> >> It doesn't work - due to faults in our set up of the build system and >> incorrect usage of things like meson.build_root() and meson.source_root(). > > Patches are welcome :-) Sure - they're already on my todo list from yesterday ;-) >> My conversation led to one of the meson devs deciding to mark those as >> deprecated : >> (See https://github.com/mesonbuild/meson/pull/7772) >> >> Anyway, the easy and quick way - is that meson creates a pkg-config >> '-uninstalled' location at $BUILD/meson-uninstalled ... and it's really >> easy to build an app against an uninstalled library by pointing >> PKG_CONFIG_DIR=$BUILD/meson-uninstalled Ahem, s/PKG_CONFIG_DIR/PKG_CONFIG_PATH/ >> >> So that's a really quick and effective way of also compiling the >> 'external' simple-cam against a 'just built' libcamera. > > From a build server point of view, I thought the builder could get the > latest libcamera master branch, build it, and install it. Why would we > need an "uninstalled" location ? Hrm, my original answer here was 'because you need to be root to install' ... but that's not correct. You can install to a non-root owned location (and then make sure the library is on the PKG_CONFIG_PATH...) So that's also an option ... and also validates the install procedures. > >> I'm adding that to my daily builder, (the one that also does a >> coverity-scan build, and a full matrix build of all compilers on my >> laptop) - so I'll know as soon as there is breakage. >> >> And indeed, sometime we'll move this to a more fully handled CI system too. >> >> Anyway, thanks for the review, I'll push these patches to my repo. >> >>>> Kieran Bingham (4): >>>> simple-cam: Use the new BufferMap interface >>>> simple-cam: Use a const stream >>>> meson: Remove incorrect default >>>> meson: Update to C++17 >>>> >>>> meson.build | 3 +-- >>>> simple-cam.cpp | 4 ++-- >>>> 2 files changed, 3 insertions(+), 4 deletions(-) >
Hi Kieran, On Thu, Sep 24, 2020 at 03:47:25PM +0100, Kieran Bingham wrote: > On 24/09/2020 12:44, Laurent Pinchart wrote: > > On Thu, Sep 24, 2020 at 09:52:11AM +0100, Kieran Bingham wrote: > >> On 24/09/2020 04:14, Laurent Pinchart wrote: > >>> On Wed, Sep 23, 2020 at 05:44:08PM +0100, Kieran Bingham wrote: > >>>> The simple-cam app needs to be updated with a few API updates to work on > >>>> the latest libcamera builds. > >>>> > >>>> This is based on the code at https://github.com/kbingham/simple-cam > >>>> though I hope to move this to a more official location soon. > >>> > >>> For the whole series, > >>> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> > >>> It would be interesting to test-compile simple-cam when libcamera gains > >>> new commits. It can probably wait until libcamera itself gets CI though > >>> :-) > >> > >> This is something I've been working on. > >> > >> There are two options, an easy way and a hard way (to do this without > >> installing the library on each build). > >> > >> The hard way (which is of course what was recommended in #meson) is to > >> provide a meson wrap for libcamera, so that it can be a subproject to > >> simple-cam. This means if libcamera is not found as a dependency it will > >> clone libcamera, build it - and automatically link against it statically. > >> > >> It doesn't work - due to faults in our set up of the build system and > >> incorrect usage of things like meson.build_root() and meson.source_root(). > > > > Patches are welcome :-) > > Sure - they're already on my todo list from yesterday ;-) > > >> My conversation led to one of the meson devs deciding to mark those as > >> deprecated : > >> (See https://github.com/mesonbuild/meson/pull/7772) > >> > >> Anyway, the easy and quick way - is that meson creates a pkg-config > >> '-uninstalled' location at $BUILD/meson-uninstalled ... and it's really > >> easy to build an app against an uninstalled library by pointing > >> PKG_CONFIG_DIR=$BUILD/meson-uninstalled > > Ahem, s/PKG_CONFIG_DIR/PKG_CONFIG_PATH/ > > >> So that's a really quick and effective way of also compiling the > >> 'external' simple-cam against a 'just built' libcamera. > > > > From a build server point of view, I thought the builder could get the > > latest libcamera master branch, build it, and install it. Why would we > > need an "uninstalled" location ? > > Hrm, my original answer here was 'because you need to be root to > install' ... but that's not correct. You can install to a non-root owned > location (and then make sure the library is on the PKG_CONFIG_PATH...) > > So that's also an option ... and also validates the install procedures. If the build of simple-cam was automated on a build server, I would assume it would happen in a container, so we can install anything in any way we want :-) > >> I'm adding that to my daily builder, (the one that also does a > >> coverity-scan build, and a full matrix build of all compilers on my > >> laptop) - so I'll know as soon as there is breakage. > >> > >> And indeed, sometime we'll move this to a more fully handled CI system too. > >> > >> Anyway, thanks for the review, I'll push these patches to my repo. > >> > >>>> Kieran Bingham (4): > >>>> simple-cam: Use the new BufferMap interface > >>>> simple-cam: Use a const stream > >>>> meson: Remove incorrect default > >>>> meson: Update to C++17 > >>>> > >>>> meson.build | 3 +-- > >>>> simple-cam.cpp | 4 ++-- > >>>> 2 files changed, 3 insertions(+), 4 deletions(-)