[Buildroot] [RFC PATCH] package/libcamera: Add libcamera package

Kieran Bingham kieran.bingham at ideasonboard.com
Tue Mar 19 14:12:03 UTC 2019


Hi Yann,

Thank you for the review,

On 19/03/2019 12:50, Yann E. MORIN wrote:
> Kieran, All,
> 
> On 2019-03-19 11:03 +0000, Kieran Bingham spake thusly:
>>   http://libcamera.org/
>>
>> Cameras are complex devices that need heavy hardware image processing
>> operations. Control of the processing is based on advanced algorithms
>> that must run on a programmable processor. This has traditionally been
>> implemented in a dedicated MCU in the camera, but in embedded devices
>> algorithms have been moved to the main CPU to save cost. Blurring the
>                                                            ^^^^^^^^
> I see what you did there! ;-)

Cameras on these new platforms are certainly blurry without some new
software support :)

But yes, this was the blurb from libcamera.org to provide context of the
library addition.

>> boundary between camera devices and Linux often left the user with no
>> other option than a vendor-specific closed-source solution.
>>
>> To address this problem the Linux media community has very recently
>> started collaboration with the industry to develop a camera stack that
>> will be open-source-friendly while still protecting vendor core IP.
>> libcamera was born out of that collaboration and will offer modern
>> camera support to Linux-based systems, including traditional Linux
>> distributions, ChromeOS and Android.
> 
> While I appreciate the blurb about the context around libcamera, what I
> find even more interesting in a commit log is an explanations about the
> complexity of the packaging in Buildrot, and why such or such hack was

Buildrot? I hope that's not some sort of freudian slip :-)


> done.
> 
> Here, you don't seem to have much to say, though, except...
> 
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> We do not yet have an official 'release', so I'm sending this as an
>> early RFC, with
>>   LIBCAMERA_VERSION = origin/master
>> and no .hash file.
> 
> ... this blurb would have had its place in the commit log:

I think I essentially knew origin/master was not going to be acceptable,
hence it was RFC to find out the right thing to do and I had only put
this in the comments,

> 
>     The project has not made an official release as of yet, so we're
>     using the latest sha1 from master.

I agree, This is a good addition to the commit message.


> 
> which is about the only thing that I find important for a commit in
> Buildroot.
> 
>> Is this still suitable to go in and be updated when we have a tagged
>> release later?
> 
> As I explained on IRC, using a branch name is not acceptable, because it
> does not work as you think it would, as explaiend in the manual (quting
> here for your convenience):
> 
>     Note: Using a branch name as FOO_VERSION is not supported, because it
>     does not and can not work as people would expect it should:
> 
>      1. due to local caching, Buildroot will not re-fetch the repository, so
>         people who expect to be able to follow the remote repository would be
>         quite surprised and disappointed;
>      2. because two builds can never be perfectly simultaneous, and because
>         the remote repository may get new commits on the branch anytime, two
>         users, using the same Buildroot tree and building the same
>         configuration, may get different source, thus rendering the build non
>         reproducible, and people would be quite surprised and disappointed. 
> 
> So, use a sha1. ;-)

Agreed. I'll hold off until the fix required to get compilation to work
in buildroot is in... which shouldn't be long :)



> 
>>  DEVELOPERS                     |  3 +++
>>  package/Config.in              |  1 +
>>  package/libcamera/Config.in    | 11 +++++++++++
>>  package/libcamera/libcamera.mk | 13 +++++++++++++
>>  4 files changed, 28 insertions(+)
>>  create mode 100644 package/libcamera/Config.in
>>  create mode 100644 package/libcamera/libcamera.mk
>>
>> diff --git a/DEVELOPERS b/DEVELOPERS
>> index c91325e28486..5bcdf208a62b 100644
>> --- a/DEVELOPERS
>> +++ b/DEVELOPERS
>> @@ -1260,6 +1260,9 @@ F:	package/ramsmp/
>>  N:	Kevin Joly <kevin.joly at sensefly.com>
>>  F:	package/libgphoto2/
>>  
>> +N:	Kieran Bingham <kieran.bingham at ideasonboard.com>
>> +F:	package/libcamera/
> 
> Nice, thanks! :-)


Is it possible to put the libcamera mailinglist as a 'cc' target here as
well? or is it only individuals.

If someone (other than me) updates the package, It would be nice for
that notification to go to the list.


>> diff --git a/package/libcamera/Config.in b/package/libcamera/Config.in
>> new file mode 100644
>> index 000000000000..c80f58c00f17
>> --- /dev/null
>> +++ b/package/libcamera/Config.in
>> @@ -0,0 +1,11 @@
>> +config BR2_PACKAGE_LIBCAMERA
>> +	bool "libcamera"
>> +	depends on BR2_INSTALL_LIBSTDCPP
>> +	depends on BR2_TOOLCHAIN_HAS_THREADS
>> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_7 # C++11
>> +	depends on BR2_PACKAGE_HAS_UDEV
>> +	help
>> +	  libcamera provides a software stack to support complex devices that
>> +	  need heavy hardware image processing operations.
> 
> Did you pass this package through utils/check-package?
> 
>     package/libcamera/Config.in:8: help text: <tab><2 spaces><62 chars>
>     (http://nightly.buildroot.org/#writing-rules-config-in)
>     24 lines processed
>     1 warnings generated

I thought I had - but had no output, so clearly I failed :(
I was incorrectly wrapping at 72. I've now re-wrapped at 62.


> Also, as you already discovered, it's nice to pass a new package through
> utils/test-pkg as well (and although not mandatory, it's nice to provide
> that report after the --- line.)

I was going to ask how to do this - as it was skipping all 6, and then
all 43 toolchains with -a. But I've worked it out, so for the benefit of
others:

I had to create a libcamera.config and specify the EUDEV dependency:

  BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
  BR2_PACKAGE_LIBCAMERA=y

Then:
 ./utils/test-pkg -c libcamera.config -p libcamera


It's now running, and I assume it will take some time...


>> +	  http://www.libcamera.org/
>> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
>> new file mode 100644
>> index 000000000000..4d908c7a3645
>> --- /dev/null
>> +++ b/package/libcamera/libcamera.mk
>> @@ -0,0 +1,13 @@
>> +################################################################################
>> +#
>> +# libcamera
>> +#
>> +################################################################################
>> +
>> +LIBCAMERA_VERSION = origin/master
>> +LIBCAMERA_SITE = git://linuxtv.org/libcamera.git
> 
> Please use the https (or http) URI, as people usually can't use the git
> protocol from behind nasty corporate-class firewalls.
> 
>> +LIBCAMERA_SITE_METHOD = git
>> +LIBCAMERA_DEPENDENCIES = udev
>> +LIBCAMERA_LICENSE = LGPL-2.0+
> 
> This is not LGPL-2.0+, but LGPL-2.1+

Ugh, I should have remembered that, especially as I added the licenses...

> 
> There are actually 2 other licenses applicable to this package: GPLv2.0+
> and CC-BY-SA-4.0 (or later?).
> 
> We usually specify what part of the package they apply to (correct me
> if/where I am wrong):
> 
>     LIBCAMERA_LICENSE = LGPL-2.1+ (library), GPL-2.0+ (utils, test), CC-By_SA-4.0 (doc)


That looks accurate to me.

I've updated the patch.

> 
> Please also specify the files that contain the license texts:
> 
>     LIBCAMERA_LICENSE_FILES = \
>         licenses/cc-by-sa-v4.0.txt \
>         licenses/developer-certificate-of-origin.txt \
>         licenses/gnu-gpl-2.0.txt \
>         licenses/gnu-lgpl-2.1.txt
> 

along with this.


> Thanks! :-)
> 
> Regards,
> Yann E. MORIN.
> 
>> +
>> +$(eval $(meson-package))
>> -- 
>> 2.19.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
Regards
--
Kieran



More information about the buildroot mailing list