[MIR] libcamera

Output on the current 0.0.1 version

# lintian --pedantic
W: libcamera source: bad-exception-format-in-dep5-copyright gpl-2+ with linux-syscall-note [debian/copyright:106]
W: libcamera0: lacks-unversioned-link-to-shared-library example: usr/lib/x86_64-linux-gnu/v4l2-compat.so [usr/lib/x86_64-linux-gnu/v4l2-compat.so]
W: libcamera-dev: mismatched-override repeated-path-segment libcamera usr/include/libcamera/libcamera/* [usr/share/lintian/overrides/libcamera-dev:1]
W: libcamera source: missing-license-paragraph-in-dep5-copyright gpl-2+ with linux-syscall-note [debian/copyright:106]
W: libcamera-tools: no-manual-page [usr/bin/cam]
W: libcamera-tools: no-manual-page [usr/bin/lc-compliance]
W: libcamera-tools: no-manual-page [usr/bin/libcamerify]
W: libcamera-tools: no-manual-page ... use "--tag-display-limit 0" to see all (or pipe to a file/program)
W: libcamera0: package-name-doesnt-match-sonames libcamera-base0.0.1 libcamera0.0.1 v4l2-compat
W: libcamera0: shared-library-lacks-version usr/lib/x86_64-linux-gnu/v4l2-compat.so v4l2-compat.so
W: libcamera source: superfluous-file-pattern include/linux/drm.h [debian/copyright:85]
W: libcamera source: superfluous-file-pattern include/linux/drm_mode.h [debian/copyright:94]
W: libcamera source: superfluous-file-pattern include/linux/vc_sm_cma_ioctl.h [debian/copyright:108]
W: libcamera source: superfluous-file-pattern ... use "--tag-display-limit 0" to see all (or pipe to a file/program)
W: libcamera source: tab-in-license-text [debian/copyright:722]
P: libcamera source: package-does-not-install-examples [src/py/examples/]
P: libcamera source: redundant-globbing-patterns (*/*/meson.build */meson.build) for include/android/meson.build [debian/copyright:20]
P: libcamera source: redundant-globbing-patterns (*/*/meson.build */meson.build) for include/libcamera/base/meson.build [debian/copyright:20]
P: libcamera source: redundant-globbing-patterns (*/*/meson.build */meson.build) for include/libcamera/internal/meson.build [debian/copyright:20]
P: libcamera source: redundant-globbing-patterns ... use "--tag-display-limit 0" to see all (or pipe to a file/program)
P: libcamera-dev: repeated-path-segment libcamera [usr/include/libcamera/libcamera/]
P: libcamera source: very-long-line-length-in-source-file 547 > 512 [debian/libcamera0.symbols:547]
P: libcamera source: very-long-line-length-in-source-file 886 > 512 [LICENSES/CC-BY-4.0.txt:154]
P: libcamera source: very-long-line-length-in-source-file 887 > 512 [debian/copyright:876]

@MIRteam, I'm opening the MIR knowing it needs work before being ready (current rev failing to build in lunar-proposed, build tests and autopkgtest missing, lintian warning cleanups), we will work on the issues mentioned but if that requires a security team review it would be nice to get it queued without blocking on the packaging fixes. If you believe it doesn't need a security review feel free to set to Incomplete and we will reopen when ready

The lintian warnings have been reduced with the patch sent to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1024803 which has also been uploaded to Lunar now. The remaining ones are about missing manpages for the tools (which we don't plan to install by default) and length of some of lines in symbols/copyright/license which isn't really an issue.

The package has been updated to fix the build and enable build tests (using some hacks for now while some of the issues are investigated so a bit more work is needed)

The upstream tests have also been integrated as an autopkgtest

With those changes we should be in a situation acceptable to get the package reviewed (knowing we still need a bit of cleanup work on some of the build/tests hacks mentioned)

Corresponding upload, https://launchpad.net/ubuntu/+source/libcamera/0.0.1-4ubuntu1

Review for Package: src:libcamera

[Summary]
libcamera is supposed to be a new hardware abstraction layer that sits between
multimedia applications/frameworks and the v4l2 kernel API:
old: Application -> [gstreamer ->] v4l2 -> kernel/hardware
new: Application -> [gstreamer ->] pipewire/libcamera -> v4l2 -> kernel/hardware

I'm all in favor of supporting the new and unified way of handling camera
devices, but this projects still seems to be in its infancy and I feel like
it will be a big maintenance burden to support the early versions, as shown
by the amount of problems I spotted below. Therefore:

MIR team NACK
(for now... sorry) we can still revise this once we get the package into
a better shape.

This does need a security review

List of specific binary packages to be promoted to main: libcamera0
Specific binary packages built, but NOT to be promoted to main: libcamera-tools

Notes:
- package is already subscribed to ~desktop packages
  => good
- deals with (video-)data streams from untrusted (potentially binary blobs)
  sources, passing around data via unix sockets and IPA module signing
  => needs security review
#0 will we be demoting lib4vl2 in favor of this? How to avoid the duplication?
#1 libcamera-tools should be excluded and stay in "universe" (due to libqt5) dep
   libcamera-doc seems to be fine
#2 v0.0.1 is a very early (initial) release, which was just recently tagged,
    is it really mature enough for main inclusion?

Required TODOs:
#3 symbols tracking is in place, but failures being ignored via "-c0"
   => what's the plan to avoid ABI incompatibilities? (especially with such an
      early package version?)
#4 some concerning Lintian warnings: https://udd.debian.org/lintian/?packages=libcamera
   + executable-stack-in-shared-library
   + symbols-file-contains-[current-version-with-]debian-revision

Recommended TODOs:
#5 contains some embedded code (mostly android related) and static linking, which
   seems to be unused – I'll leave making a call about how to handle this to the
   security team
#6 You wrote: "We will test on a selection of that hardware"
   => please provide a written test plan for this
#7 weak autopkgtests: package is re-built during tests,
   instead of testing integration of pre-built binaries
#8 FTBFS (dh_auto_test) locally (lunar sbuild), but passes in PPA (see below),
   I'll blame my local environment for now, but that might be investigated
#9 Ubuntu delta carries quirks (skipping tests, fixing copyright),
   which should rather be handled in upstream/Debian
#10 We should upgrade to upstream's current v0.0.2 release
#11 Errors/warnings during the build, to be discussed with upstream/Debian:
    + sh: 1: latex: not found: error: Problems running latex.
    + WARNING: Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated.
    + warning: no [uniquely] matching class member found for
    + dpkg-gensymbols: warning: some symbols or patterns disappeared in the symbols file: see diff output below
    + dpkg-gensymbols: warning: debian/libcamera0/DEBIAN/symbols doesn't match completely debian/libcamera0.symbols
    + dpkg-gencontrol: warning: Depends f...

This is needed for OEM reasons for (at least) the next LTS.

The desktop team want's to re-apply for main inclusion next cycle. Hopefully the project will have matured a bit by then.

Upstream started cutting actual releases and focusing on stability recently, so there is a good chance for the situation to improve. The desktop team could help fix things from the review above in universe (or Debian) in the meantime.

We still want to get this into the security-teams queue, so it will be ready when the MIR is re-requested.

I reviewed libcamera 0.0.2-1ubuntu2 as checked into lunar. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libcamera is an open source camera stack for many platforms with a core
userspace library, and support from the Linux kernel APIs and drivers already
in place. It aims to control the complexity of embedded camera hardware by
providing an intuitive API and method of separating untrusted vendor code
from the open source core.

- CVE History:
  - No CVEs
- Build-Depends?
  - debhelper-compat, meson, pkgconf, libevent-dev, libgtest-dev,
    liblttng-ust-dev, libdw-dev, libudev-dev, libgnutls28-dev, libyaml-dev,
    doxygen, graphviz, python3-sphinx, python3-jinja2, python3-ply,
 python3-yaml, libjs-jquery, libjs-sphinxdoc, libjs-underscore
 libboost-dev, libtiff-dev, libjpeg-dev, libsdl2-dev, openssl, qtbase5-dev,
 libgstreamer-plugins-base1.0-dev
- pre/post inst/rm scripts?
  - None
- init scripts?
  - None
- systemd units?
  - None
- dbus services?
  - None
- setuid binaries?
  - None
- binaries in PATH?
  - /usr/bin/cam
  - /usr/bin/lc-compliance
  - /usr/bin/libcamerify
  - /usr/bin/qcam
- sudo fragments?
  - None
- polkit files?
  - None
- udev rules?
  - None
- unit tests / autopkgtests?
  - 103 tests using meson / ninja / gtest
- cron jobs?
  - None
- Build logs:
  - Lintian complains that lc-compliance, libcamerify, and qcam (all from
    libcamera-tools) don't have manual pages.

- Processes spawned?
  - There are some process spawned, most important in libcamera but looks
    sane.
- Memory management?
  - Looks sane too. Coverity found some miror issues but looks like upstream
    fixed or is working on a fix.
- File IO?
  - Configurations files and device IO found. All them looks sane.
- Logging?
  - Logging implementation in src/libcamera/base/log.cpp.
  - Only in C++. Log for python or other languages not found.
  - Moderated use, seems sane.
- Environment variable usage?
  - A few environment variables in use.
  - Documented in Documentation/environment_variables.rst
  - Seems sane.
- Use of privileged functions?
  - None
- Use of cryptography / random number sources etc?
  - libcamera uses gnutls to implemente signature verification.
  - See src/libcamera/pub_key.cpp
  - Looks correct.
- Use of temp files?
  - There are some use of temp file in utils that apparently to have
    predictable names.
- Use of WebKit?
  - None
- Use of PolicyKit?
  - None

- Any significant cppcheck results?
  - No
- Any significant Coverity results?
  - There are two "string not null terminated" detected (log_process and
    log_api) that might cause a memory leak.
  - There are some out-of-bounds detected in alsc.cpp and exif.cpp that could
    be used to cause a memory corruption.
  - Both problems above are specific for raspberry and could not be identified
    as a real issue for Ubuntu. Just in case I filed a bug report [1].
  - There is a possible use-after-free in lc-compliance/main.cpp that can make
    libcamera to crash. This is part of the libcamera-tools project and
    not commonly used by the users.
- Any significant shellcheck results?
  - Lot's of warnings related to undefined variables and ...

Requestion MIR review again since I think the issues raised in the first review are mostly addressed, there are some open questions around symbols and tests, see details bellow

> #0 will we be demoting lib4vl2 in favor of this? How to avoid the duplication?

There is no plan at this point to demote lib4vl2, those are different APIs. Libcamera provides a compatibility layer through LD_PRELOAD (https://libcamera.org/docs.html#v4l2-compatibility-layer) but it is described as 'best-effort basis' and not something we plan to rely on.

> #1 libcamera-tools should be excluded and stay in "universe" (due to libqt5) dep

ack

> #2 v0.0.1 is a very early (initial) release, which was just recently tagged,
> is it really mature enough for main inclusion?

We are at 0.0.5 now, there isn't much we can do to fix the age factor though so it doesn't feel a fair reason to block promotion of something we need to support newer hardware.

> Required TODOs:
> #3 symbols tracking is in place, but failures being ignored via "-c0"
> => what's the plan to avoid ABI incompatibilities? (especially with such an
> early package version?)

Debian removed the .symbols now in https://salsa.debian.org/multimedia-team/libcamera/-/commit/bcc162f8

The rational is that upstream is currently bumping the soname with each release. Would that be also an acceptable situation for the Ubuntu MIR team?

> #4 some concerning Lintian warnings: https://udd.debian.org/lintian/?packages=libcamera
> + executable-stack-in-shared-library
> + symbols-file-contains-[current-version-with-]debian-revision

Those warnings have been resolved in the current version

> Recommended TODOs:
> #5 contains some embedded code (mostly android related) and static linking, which
> seems to be unused – I'll leave making a call about how to handle this to the
> security team

ack

> #6 You wrote: "We will test on a selection of that hardware"
> => please provide a written test plan for this

https://wiki.ubuntu.com/DesktopTeam/TestPlans/Libcamera

> #7 weak autopkgtests: package is re-built during tests,
> instead of testing integration of pre-built binaries

Debian added also a simple autopkgtest calling
$ cam --list
and
$ lc-compliance --list

Would that be enough? The project doesn't have installed test, we could hack something around the existing tests but that's not great

> #8 FTBFS (dh_auto_test) locally (lunar sbuild), but passes in PPA (see below),
> I'll blame my local environment for now, but that might be investigated

Could you provide a build log if you still see the issue? We had some new versions uploaded since the review and they built fine on launchpad, local build is already working for me on a lunar desktop.

> #9 Ubuntu delta carries quirks (skipping tests, fixing copyright),
> which should rather be handled in upstream/Debian

Those changes got included in Debian now

> #10 We should upgrade to upstream's current v0.0.2 release

We have 0.0.5 now which is the current upstream version

> #11 Errors/warnings during the build, to be discussed with upstream/Debian:

Those warnings aren't in ...

I've also added a section to the description about why we aren't enabling the .symbols atm

[Note: I added the link to your manual testplan to the bug description above]

Re-considering the MIR for libcamera 0.0.5-1ubuntu1 by checking the delta:

I see that this package's quality has improved and the security team seems to be happy, too (especially considering the upstream activity). I still wouldn't call it "great" and it's still in its early development stages, but OTOH we need it for hardware enablement. The maintenance burden is on the maintaining team (Desktop) and security team, so considering both teams are fine with the current state and accept to e.g. take special considerations when doing SRUs, to avoid ABI breaking changes, this works for me.

=> MIR team ACK.
For promoting the libcamera0.0.5, libcamera-{ipa,v4l2} and gstreamer1.0-libcamera binary packages. While excluding libcamera-tools.

One Recommended (non-blocking) TODO:

#12 Lintian warning:
    libcamera-v4l2: hardening-no-fortify-functions [usr/lib/x86_64-linux-gnu/v4l2-compat.so]
    => I assume this is related to libcamera-v4l2's LD_PRELOAD nature, but maybe this should be double-checked and any hardeding enabled if possible.

> > #0 will we be demoting lib4vl2 in favor of this? How to avoid the duplication?
>
> There is no plan at this point to demote lib4vl2, those are different APIs. Libcamera provides a compatibility layer through LD_PRELOAD (https://libcamera.org/docs.html#v4l2-compatibility-layer) but it is described as 'best-effort basis' and not something we plan to rely on.

OK, understood. Thanks for the clarification.

> > #1 libcamera-tools should be excluded and stay in "universe" (due to libqt5) dep
> ack

OK, I think we should be fine to promote the (new/split) libcamera0.0.5, libcamera-{ipa,v4l2} and gstreamer1.0-libcamera binary packages.
While avoiding libcamera-tools.

> > #2 v0.0.1 is a very early (initial) release, which was just recently tagged,
> > is it really mature enough for main inclusion?
>
> We are at 0.0.5 now, there isn't much we can do to fix the age factor though so it doesn't feel a fair reason to block promotion of something we need to support newer hardware.

ACK. See my summary above.

> > Required TODOs:
> > #3 symbols tracking is in place, but failures being ignored via "-c0"
> > => what's the plan to avoid ABI incompatibilities? (especially with such an
> > early package version?)
>
> Debian removed the .symbols now in https://salsa.debian.org/multimedia-team/libcamera/-/commit/bcc162f8
>
> The rational is that upstream is currently bumping the soname with each release. Would that be also an > acceptable situation for the Ubuntu MIR team?

This is not the best solution from a MIR perspective, as it introduces potentially unnecessary transitions and we need to be extra cautious when doing SRUs to avoid potentially breaking ABI. But I guess it's acceptable as it avoids breaking ABI during regular package updates.

> > #4 some concerning Lintian warnings: https://udd.debian.org/lintian/?packages=libcamera
> > + executable-stack-in-shared-library
> > + symbols-file-contains-[current-version-with-]debian-revision
>
> Those warnings have been resolved in the current version

ACK

> > Recommended TODOs:
> > #5 contains som...

> One Recommended (non-blocking) TODO:
>
> #12 Lintian warning:
> libcamera-v4l2: hardening-no-fortify-functions [usr/lib/x86_64-linux-gnu/v4l2-compat.so]
> => I assume this is related to libcamera-v4l2's LD_PRELOAD nature, but maybe this should be double- checked and any hardening enabled if possible.

This is set in debian/rules:
export DEB_BUILD_MAINT_OPTIONS = hardening=+all

I have made the seed change (added pipewire's libcamera binary package to Ubuntu supported).

Override component to main
libcamera 0.0.5-1ubuntu1 in mantic: universe/misc -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic amd64: universe/libdevel/optional/100% -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic arm64: universe/libdevel/optional/100% -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic armhf: universe/libdevel/optional/100% -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic i386: universe/libdevel/optional/100% -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic ppc64el: universe/libdevel/optional/100% -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic riscv64: universe/libdevel/optional/100% -> main
libcamera-dev 0.0.5-1ubuntu1 in mantic s390x: universe/libdevel/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic amd64: universe/doc/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic arm64: universe/doc/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic armhf: universe/doc/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic i386: universe/doc/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic ppc64el: universe/doc/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic riscv64: universe/doc/optional/100% -> main
libcamera-doc 0.0.5-1ubuntu1 in mantic s390x: universe/doc/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic amd64: universe/libs/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic arm64: universe/libs/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic armhf: universe/libs/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic i386: universe/libs/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic ppc64el: universe/libs/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic riscv64: universe/libs/optional/100% -> main
libcamera0.0.5 0.0.5-1ubuntu1 in mantic s390x: universe/libs/optional/100% -> main
Override [y|N]? y