From patchwork Fri May 22 14:54:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3839 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 49220603D6 for ; Fri, 22 May 2020 16:55:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="FK2cZNKA"; dkim-atps=neutral Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BF67A24D for ; Fri, 22 May 2020 16:55:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1590159318; bh=h6lJfhzFywCDm7bEHl3pw8stvmhiSr9vBtfx/uYer+c=; h=From:To:Subject:Date:From; b=FK2cZNKApySAS40J8qQXl2DkNYoxOF7i5EcU+50J1NegZFJdU4GTBZ4e8xK5VWwR3 9kBSM5O/oZKDuUIY8gLRdepDOh8OlZsuHVJHTY0ZqXm9VWYIuLy0/bvAtP+sasii8s s+arr/XT3bTy77045OTu5C7YgSW7Ki/8twZyc09s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 22 May 2020 17:54:47 +0300 Message-Id: <20200522145459.16836-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 00/11] Introduce formats:: namespace for libcamera pixel formats X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 May 2020 14:55:19 -0000 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%)