[libcamera-devel] [PATCH/RFC 00/11] Introduce formats:: namespace for libcamera pixel formats

[Laurent Pinchart]

Hello,

This patch series is an attempt to fix a problem caused by a direct
dependency on drm_fourcc.h in the libcamera public API.

libcamera specifies pixel formats using the DRM pixel format FourCC and
modifiers. This requires both internal code and applications to include
drm_fourcc.h. For internal code, we carry a copy of the header to avoid
external dependencies. For third-party applications, however,
drm_fourcc.h needs to be accessible from system includes. It turns out
that the file is shipped by two different packages:

- libdrm, which typically installs it in /usr/include/libdrm/
- kernel headers or libc headers, which typically installs it in
  /usr/include/drm

We don't want to make libdrm a dependency for applications using
libcamera. Unfortunately, depending on drm_fourcc.h being provided by
the distribution kernel headers or libc headers package causes issues,
as not all distributions install the header in /usr/include/drm/. Ubuntu
and Gentoo do, but Debian and Arch don't.

The best option may be to remove the dependency on the macros provided
by drm_fourcc.h, while keeping the pixel format numerical values
identical. This is what this patch series attempts to do.

Patches 01/11 to 04/11 are preparatory cleanups, please see individual
patches for details. I believe they make sense regardless of whether the
rest is accepted or rejected.

Patch 05/11 creates a new libcamera::formats:: namespace and populates
it with constants (in the form of constexpr) for all supported pixel
formats. The values have been written manually, which isn't ideal. We
could partly automate this process by generating the header from a list
of formats (likely in a YAML file), and fetching the numerical values
from drm_fourcc.h to make sure they will always match. This could allow
documenting the pixel formats in the YAML file and generating Doxygen
documentation, like we do for controls.

Patches 06/11 to 11/11 then replace usage of the DRM_FORMAT_* macros
through the code, with one exception in the IPU3 pipeline handler to
demonstrate how DRM_FORMAT_* values can still be used for internal
formats. I however believe this will be dropped too, as applications may
want to capture RAW data from the IPU3, so the format should likely be
defined in the public API.

Laurent Pinchart (11):
  libcamera: Rename pixelformats.{cpp,h} to pixel_format.{cpp,h}
  libcamera: Replace C++ comments with C comments
  libcamera: Rename header guards for internal headers
  libcamera: pixel_format: Make PixelFormat usable as a constexpr
  libcamera: Define constants for pixel formats in the public API
  gst: Replace explicit DRM FourCCs with libcamera formats
  qcam: Replace explicit DRM FourCCs with libcamera formats
  v4l2: Replace explicit DRM FourCCs with libcamera formats
  test: Replace explicit DRM FourCCs with libcamera formats
  libcamera: pipeline: Replace explicit DRM FourCCs with libcamera
    formats
  libcamera: Replace explicit DRM FourCCs with libcamera formats

 include/libcamera/control_ids.h.in            |   4 +-
 include/libcamera/formats.h                   |  94 +++++++++++
 .../libcamera/internal/byte_stream_buffer.h   |   6 +-
 include/libcamera/internal/camera_controls.h  |   6 +-
 include/libcamera/internal/camera_sensor.h    |   6 +-
 .../libcamera/internal/control_serializer.h   |   6 +-
 .../libcamera/internal/control_validator.h    |   6 +-
 .../libcamera/internal/device_enumerator.h    |   6 +-
 .../internal/device_enumerator_sysfs.h        |   6 +-
 .../internal/device_enumerator_udev.h         |   6 +-
 .../internal/event_dispatcher_poll.h          |   6 +-
 include/libcamera/internal/file.h             |   6 +-
 include/libcamera/internal/formats.h          |   8 +-
 .../libcamera/internal/ipa_context_wrapper.h  |   6 +-
 include/libcamera/internal/ipa_manager.h      |   6 +-
 include/libcamera/internal/ipa_module.h       |   6 +-
 include/libcamera/internal/ipa_proxy.h        |   6 +-
 include/libcamera/internal/ipc_unixsocket.h   |   6 +-
 include/libcamera/internal/log.h              |   6 +-
 include/libcamera/internal/media_device.h     |   6 +-
 include/libcamera/internal/media_object.h     |   6 +-
 include/libcamera/internal/message.h          |   6 +-
 include/libcamera/internal/pipeline_handler.h |   6 +-
 include/libcamera/internal/process.h          |   6 +-
 include/libcamera/internal/pub_key.h          |   6 +-
 include/libcamera/internal/semaphore.h        |   6 +-
 include/libcamera/internal/thread.h           |   6 +-
 include/libcamera/internal/utils.h            |   6 +-
 include/libcamera/internal/v4l2_controls.h    |   6 +-
 include/libcamera/internal/v4l2_device.h      |   6 +-
 include/libcamera/internal/v4l2_pixelformat.h |   8 +-
 include/libcamera/internal/v4l2_subdevice.h   |   6 +-
 include/libcamera/internal/v4l2_videodevice.h |   8 +-
 include/libcamera/meson.build                 |   3 +-
 include/libcamera/pixel_format.h              |  48 ++++++
 include/libcamera/pixelformats.h              |  43 -----
 include/libcamera/property_ids.h.in           |   6 +-
 include/libcamera/stream.h                    |   2 +-
 src/gstreamer/gstlibcamera-utils.cpp          |  62 ++++----
 src/libcamera/formats.cpp                     | 148 +++++++++---------
 src/libcamera/meson.build                     |   2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |   8 +-
 .../pipeline/raspberrypi/raspberrypi.cpp      |  10 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  19 +--
 src/libcamera/pipeline/simple/converter.h     |   2 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |  11 +-
 .../{pixelformats.cpp => pixel_format.cpp}    |  19 +--
 src/libcamera/v4l2_pixelformat.cpp            |  85 +++++-----
 src/qcam/dng_writer.cpp                       |  17 +-
 src/qcam/format_converter.cpp                 |  36 +++--
 src/qcam/format_converter.h                   |   2 +-
 src/qcam/viewfinder.cpp                       |  10 +-
 src/qcam/viewfinder.h                         |   2 +-
 src/v4l2/v4l2_camera_proxy.cpp                |  27 ++--
 test/v4l2_videodevice/buffer_cache.cpp        |   3 +-
 55 files changed, 477 insertions(+), 378 deletions(-)
 create mode 100644 include/libcamera/formats.h
 create mode 100644 include/libcamera/pixel_format.h
 delete mode 100644 include/libcamera/pixelformats.h
 rename src/libcamera/{pixelformats.cpp => pixel_format.cpp} (89%)

-- 
Regards,

Laurent Pinchart