[libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace for libcamera pixel formats

[scerveau]

Hi Laurent,

I was suspecting something like that regarding the drm includes but
could you explain why you dont want to depend on the libdrm package as a 
few runtime dependencies are necessary already ?

Regards.

Stéphane

On 10/6/20 14:57, Laurent Pinchart wrote:
> Hi Stéphane,
> 
> On Wed, Jun 10, 2020 at 02:49:31PM +0200, scerveau wrote:
>> Hello Laurent,
>>
>> Yesterday I tried a different approach from previous attempt, where I
>> proposed to install the libdrm headers in libcamera include folders.
>>
>> Indeed I succeeded to make libcamera compile without including
>> drm_fourcc.h in pixel_format.h, see attached patches.
>>
>> This is building correctly on my machine (ubuntu 18.04) with a simple
>> `meson build`. But I might have missed use cases.
>>
>> Concerning the libdrm dependency or includes, I haven't noticed with
>> Debian/Buster or ArchLinux/latest (docker images) any issue mentioned
>> with libdrm include folder. Both can be found with pkg-config on Ubuntu
>> as well.
> 
> The issue is that we wanted to depend in the Linux kernel DRM headers,
> not the libdrm package. libcamera doesn't interact with DRM directly, so
> depending on libdrm wasn't something we wanted to consider. The Linux
> kernel DRM headers contain drm_fourcc.h, which we initially thought
> would be enough, but it turned out that distributions don't all ship the
> DRM headers as part of the Linux kernel headers package.
> 
>> On the other hand your approach might be better, to remove the
>> dependency to libdrm and create your own define for the format if it's
>> going to be the only dependency, the project will have to libdrm.
>>
>> But in this case the patch is missing to remove those files then:
>>
>> - drm.h
>> - drm_fourcc.h
>> - drm_mode.h
> 
> drm.h and drm_mode.h should indeed be dropped. drm_fourcc.h is still
> used by gen-formats.py to generate the formats.h header, so we need to
> keep it.4
> 
>> On 10/6/20 1:23, Laurent Pinchart wrote:
>>> 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.
>>>
>>> Patch 1/7 creates a new libcamera::formats:: namespace and populates it
>>> with constants (in the form of constexpr) for all supported pixel
>>> formats. The values are automatically generated from a list of formats,
>>> stored in formats.yaml, and the numerical values for the DRM FourCCs and
>>> modifiers are extracted from drm_fourcc.h.
>>>
>>> Patches 2/7 to 7/8 then replace usage of the DRM_FORMAT_* macros through
>>> the code.
>>>
>>> Laurent Pinchart (7):
>>>     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/formats.h.in                |  44 ++++++
>>>    include/libcamera/gen-formats.py              | 118 ++++++++++++++
>>>    include/libcamera/meson.build                 |  22 +++
>>>    include/libcamera/pixel_format.h              |   2 -
>>>    src/android/camera_device.cpp                 |   9 +-
>>>    src/gstreamer/gstlibcamera-utils.cpp          |  62 ++++----
>>>    src/libcamera/formats.cpp                     | 148 +++++++++---------
>>>    src/libcamera/formats.yaml                    | 124 +++++++++++++++
>>>    src/libcamera/pipeline/ipu3/ipu3.cpp          |  16 +-
>>>    .../pipeline/raspberrypi/raspberrypi.cpp      |  10 +-
>>>    src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  19 +--
>>>    src/libcamera/pipeline/vimc/vimc.cpp          |  11 +-
>>>    src/libcamera/pixel_format.cpp                |   4 +-
>>>    src/libcamera/v4l2_pixelformat.cpp            |  83 +++++-----
>>>    src/qcam/dng_writer.cpp                       |  17 +-
>>>    src/qcam/format_converter.cpp                 |  36 +++--
>>>    src/qcam/viewfinder.cpp                       |  10 +-
>>>    src/v4l2/v4l2_camera_proxy.cpp                |  29 ++--
>>>    test/v4l2_videodevice/buffer_cache.cpp        |   3 +-
>>>    19 files changed, 540 insertions(+), 227 deletions(-)
>>>    create mode 100644 include/libcamera/formats.h.in
>>>    create mode 100755 include/libcamera/gen-formats.py
>>>    create mode 100644 src/libcamera/formats.yaml
>