[{"id":5156,"web_url":"https://patchwork.libcamera.org/comment/5156/","msgid":"<35491e66-dc2b-6ca1-3758-de07e895ea3f@collabora.com>","date":"2020-06-10T12:49:31","subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","submitter":{"id":54,"url":"https://patchwork.libcamera.org/api/people/54/","name":"Stéphane Cerveau","email":"scerveau@collabora.com"},"content":"Hello Laurent,\n\nYesterday I tried a different approach from previous attempt, where I \nproposed to install the libdrm headers in libcamera include folders.\n\nIndeed I succeeded to make libcamera compile without including \ndrm_fourcc.h in pixel_format.h, see attached patches.\n\nThis is building correctly on my machine (ubuntu 18.04) with a simple \n`meson build`. But I might have missed use cases.\n\nConcerning the libdrm dependency or includes, I haven't noticed with \nDebian/Buster or ArchLinux/latest (docker images) any issue mentioned \nwith libdrm include folder. Both can be found with pkg-config on Ubuntu \nas well.\n\nOn the other hand your approach might be better, to remove the \ndependency to libdrm and create your own define for the format if it's \ngoing to be the only dependency, the project will have to libdrm.\n\nBut in this case the patch is missing to remove those files then:\n\n- drm.h\n- drm_fourcc.h\n- drm_mode.h\n\nStéphane.\n\n\n\n\nOn 10/6/20 1:23, Laurent Pinchart wrote:\n> Hello,\n> \n> This patch series is an attempt to fix a problem caused by a direct\n> dependency on drm_fourcc.h in the libcamera public API.\n> \n> libcamera specifies pixel formats using the DRM pixel format FourCC and\n> modifiers. This requires both internal code and applications to include\n> drm_fourcc.h. For internal code, we carry a copy of the header to avoid\n> external dependencies. For third-party applications, however,\n> drm_fourcc.h needs to be accessible from system includes. It turns out\n> that the file is shipped by two different packages:\n> \n> - libdrm, which typically installs it in /usr/include/libdrm/\n> - kernel headers or libc headers, which typically installs it in\n>    /usr/include/drm\n> \n> We don't want to make libdrm a dependency for applications using\n> libcamera. Unfortunately, depending on drm_fourcc.h being provided by\n> the distribution kernel headers or libc headers package causes issues,\n> as not all distributions install the header in /usr/include/drm/. Ubuntu\n> and Gentoo do, but Debian and Arch don't.\n> \n> The best option may be to remove the dependency on the macros provided\n> by drm_fourcc.h, while keeping the pixel format numerical values\n> identical. This is what this patch series attempts to do.\n> \n> Patch 1/7 creates a new libcamera::formats:: namespace and populates it\n> with constants (in the form of constexpr) for all supported pixel\n> formats. The values are automatically generated from a list of formats,\n> stored in formats.yaml, and the numerical values for the DRM FourCCs and\n> modifiers are extracted from drm_fourcc.h.\n> \n> Patches 2/7 to 7/8 then replace usage of the DRM_FORMAT_* macros through\n> the code.\n> \n> Laurent Pinchart (7):\n>    libcamera: Define constants for pixel formats in the public API\n>    gst: Replace explicit DRM FourCCs with libcamera formats\n>    qcam: Replace explicit DRM FourCCs with libcamera formats\n>    v4l2: Replace explicit DRM FourCCs with libcamera formats\n>    test: Replace explicit DRM FourCCs with libcamera formats\n>    libcamera: pipeline: Replace explicit DRM FourCCs with libcamera\n>      formats\n>    libcamera: Replace explicit DRM FourCCs with libcamera formats\n> \n>   include/libcamera/formats.h.in                |  44 ++++++\n>   include/libcamera/gen-formats.py              | 118 ++++++++++++++\n>   include/libcamera/meson.build                 |  22 +++\n>   include/libcamera/pixel_format.h              |   2 -\n>   src/android/camera_device.cpp                 |   9 +-\n>   src/gstreamer/gstlibcamera-utils.cpp          |  62 ++++----\n>   src/libcamera/formats.cpp                     | 148 +++++++++---------\n>   src/libcamera/formats.yaml                    | 124 +++++++++++++++\n>   src/libcamera/pipeline/ipu3/ipu3.cpp          |  16 +-\n>   .../pipeline/raspberrypi/raspberrypi.cpp      |  10 +-\n>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  19 +--\n>   src/libcamera/pipeline/vimc/vimc.cpp          |  11 +-\n>   src/libcamera/pixel_format.cpp                |   4 +-\n>   src/libcamera/v4l2_pixelformat.cpp            |  83 +++++-----\n>   src/qcam/dng_writer.cpp                       |  17 +-\n>   src/qcam/format_converter.cpp                 |  36 +++--\n>   src/qcam/viewfinder.cpp                       |  10 +-\n>   src/v4l2/v4l2_camera_proxy.cpp                |  29 ++--\n>   test/v4l2_videodevice/buffer_cache.cpp        |   3 +-\n>   19 files changed, 540 insertions(+), 227 deletions(-)\n>   create mode 100644 include/libcamera/formats.h.in\n>   create mode 100755 include/libcamera/gen-formats.py\n>   create mode 100644 src/libcamera/formats.yaml\n>","headers":{"Return-Path":"<scerveau@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 19CDC600F6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Jun 2020 14:49:36 +0200 (CEST)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: scerveau) with ESMTPSA id F0B032A2CEC"],"To":"libcamera-devel@lists.libcamera.org","References":"<20200609232323.29628-1-laurent.pinchart@ideasonboard.com>","From":"scerveau <scerveau@collabora.com>","Message-ID":"<35491e66-dc2b-6ca1-3758-de07e895ea3f@collabora.com>","Date":"Wed, 10 Jun 2020 14:49:31 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200609232323.29628-1-laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/mixed;\n\tboundary=\"------------625E0E7FD47B6C70FA67D6E3\"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 10 Jun 2020 12:49:36 -0000"}},{"id":5157,"web_url":"https://patchwork.libcamera.org/comment/5157/","msgid":"<20200610125734.GA5961@pendragon.ideasonboard.com>","date":"2020-06-10T12:57:34","subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stéphane,\n\nOn Wed, Jun 10, 2020 at 02:49:31PM +0200, scerveau wrote:\n> Hello Laurent,\n> \n> Yesterday I tried a different approach from previous attempt, where I \n> proposed to install the libdrm headers in libcamera include folders.\n> \n> Indeed I succeeded to make libcamera compile without including \n> drm_fourcc.h in pixel_format.h, see attached patches.\n> \n> This is building correctly on my machine (ubuntu 18.04) with a simple \n> `meson build`. But I might have missed use cases.\n> \n> Concerning the libdrm dependency or includes, I haven't noticed with \n> Debian/Buster or ArchLinux/latest (docker images) any issue mentioned \n> with libdrm include folder. Both can be found with pkg-config on Ubuntu \n> as well.\n\nThe issue is that we wanted to depend in the Linux kernel DRM headers,\nnot the libdrm package. libcamera doesn't interact with DRM directly, so\ndepending on libdrm wasn't something we wanted to consider. The Linux\nkernel DRM headers contain drm_fourcc.h, which we initially thought\nwould be enough, but it turned out that distributions don't all ship the\nDRM headers as part of the Linux kernel headers package.\n\n> On the other hand your approach might be better, to remove the \n> dependency to libdrm and create your own define for the format if it's \n> going to be the only dependency, the project will have to libdrm.\n> \n> But in this case the patch is missing to remove those files then:\n> \n> - drm.h\n> - drm_fourcc.h\n> - drm_mode.h\n\ndrm.h and drm_mode.h should indeed be dropped. drm_fourcc.h is still\nused by gen-formats.py to generate the formats.h header, so we need to\nkeep it.4\n\n> On 10/6/20 1:23, Laurent Pinchart wrote:\n> > Hello,\n> > \n> > This patch series is an attempt to fix a problem caused by a direct\n> > dependency on drm_fourcc.h in the libcamera public API.\n> > \n> > libcamera specifies pixel formats using the DRM pixel format FourCC and\n> > modifiers. This requires both internal code and applications to include\n> > drm_fourcc.h. For internal code, we carry a copy of the header to avoid\n> > external dependencies. For third-party applications, however,\n> > drm_fourcc.h needs to be accessible from system includes. It turns out\n> > that the file is shipped by two different packages:\n> > \n> > - libdrm, which typically installs it in /usr/include/libdrm/\n> > - kernel headers or libc headers, which typically installs it in\n> >    /usr/include/drm\n> > \n> > We don't want to make libdrm a dependency for applications using\n> > libcamera. Unfortunately, depending on drm_fourcc.h being provided by\n> > the distribution kernel headers or libc headers package causes issues,\n> > as not all distributions install the header in /usr/include/drm/. Ubuntu\n> > and Gentoo do, but Debian and Arch don't.\n> > \n> > The best option may be to remove the dependency on the macros provided\n> > by drm_fourcc.h, while keeping the pixel format numerical values\n> > identical. This is what this patch series attempts to do.\n> > \n> > Patch 1/7 creates a new libcamera::formats:: namespace and populates it\n> > with constants (in the form of constexpr) for all supported pixel\n> > formats. The values are automatically generated from a list of formats,\n> > stored in formats.yaml, and the numerical values for the DRM FourCCs and\n> > modifiers are extracted from drm_fourcc.h.\n> > \n> > Patches 2/7 to 7/8 then replace usage of the DRM_FORMAT_* macros through\n> > the code.\n> > \n> > Laurent Pinchart (7):\n> >    libcamera: Define constants for pixel formats in the public API\n> >    gst: Replace explicit DRM FourCCs with libcamera formats\n> >    qcam: Replace explicit DRM FourCCs with libcamera formats\n> >    v4l2: Replace explicit DRM FourCCs with libcamera formats\n> >    test: Replace explicit DRM FourCCs with libcamera formats\n> >    libcamera: pipeline: Replace explicit DRM FourCCs with libcamera\n> >      formats\n> >    libcamera: Replace explicit DRM FourCCs with libcamera formats\n> > \n> >   include/libcamera/formats.h.in                |  44 ++++++\n> >   include/libcamera/gen-formats.py              | 118 ++++++++++++++\n> >   include/libcamera/meson.build                 |  22 +++\n> >   include/libcamera/pixel_format.h              |   2 -\n> >   src/android/camera_device.cpp                 |   9 +-\n> >   src/gstreamer/gstlibcamera-utils.cpp          |  62 ++++----\n> >   src/libcamera/formats.cpp                     | 148 +++++++++---------\n> >   src/libcamera/formats.yaml                    | 124 +++++++++++++++\n> >   src/libcamera/pipeline/ipu3/ipu3.cpp          |  16 +-\n> >   .../pipeline/raspberrypi/raspberrypi.cpp      |  10 +-\n> >   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  19 +--\n> >   src/libcamera/pipeline/vimc/vimc.cpp          |  11 +-\n> >   src/libcamera/pixel_format.cpp                |   4 +-\n> >   src/libcamera/v4l2_pixelformat.cpp            |  83 +++++-----\n> >   src/qcam/dng_writer.cpp                       |  17 +-\n> >   src/qcam/format_converter.cpp                 |  36 +++--\n> >   src/qcam/viewfinder.cpp                       |  10 +-\n> >   src/v4l2/v4l2_camera_proxy.cpp                |  29 ++--\n> >   test/v4l2_videodevice/buffer_cache.cpp        |   3 +-\n> >   19 files changed, 540 insertions(+), 227 deletions(-)\n> >   create mode 100644 include/libcamera/formats.h.in\n> >   create mode 100755 include/libcamera/gen-formats.py\n> >   create mode 100644 src/libcamera/formats.yaml","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F96F600F6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Jun 2020 14:57:55 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B42F629E;\n\tWed, 10 Jun 2020 14:57:54 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"u543+5sT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591793874;\n\tbh=FZBReH8/k9Mkish/rkYKREwZRigHTwsOCOGXT8rLEm8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u543+5sTrk9qtkocPcuNh/7wzI7cwFon8ioJaY95BdD8pbpvB1v9ea1EQkATnBcUR\n\tDMldF+afzPNDLWsfGKLGfyDHnTKtmqoXVbTlKMREXg+7uo4VvswtIzpuaAjWsUNbC0\n\twisgKtO8BSQ3KJnOZ6QNtIb3A0A6CVq62OFbEwq0=","Date":"Wed, 10 Jun 2020 15:57:34 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"scerveau <scerveau@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200610125734.GA5961@pendragon.ideasonboard.com>","References":"<20200609232323.29628-1-laurent.pinchart@ideasonboard.com>\n\t<35491e66-dc2b-6ca1-3758-de07e895ea3f@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<35491e66-dc2b-6ca1-3758-de07e895ea3f@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 10 Jun 2020 12:57:55 -0000"}},{"id":5158,"web_url":"https://patchwork.libcamera.org/comment/5158/","msgid":"<5c695258-10f0-d6fb-c41c-710db7d6971a@collabora.com>","date":"2020-06-10T13:08:00","subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","submitter":{"id":54,"url":"https://patchwork.libcamera.org/api/people/54/","name":"Stéphane Cerveau","email":"scerveau@collabora.com"},"content":"Hi Laurent,\n\nI was suspecting something like that regarding the drm includes but\ncould you explain why you dont want to depend on the libdrm package as a \nfew runtime dependencies are necessary already ?\n\nRegards.\n\nStéphane\n\nOn 10/6/20 14:57, Laurent Pinchart wrote:\n> Hi Stéphane,\n> \n> On Wed, Jun 10, 2020 at 02:49:31PM +0200, scerveau wrote:\n>> Hello Laurent,\n>>\n>> Yesterday I tried a different approach from previous attempt, where I\n>> proposed to install the libdrm headers in libcamera include folders.\n>>\n>> Indeed I succeeded to make libcamera compile without including\n>> drm_fourcc.h in pixel_format.h, see attached patches.\n>>\n>> This is building correctly on my machine (ubuntu 18.04) with a simple\n>> `meson build`. But I might have missed use cases.\n>>\n>> Concerning the libdrm dependency or includes, I haven't noticed with\n>> Debian/Buster or ArchLinux/latest (docker images) any issue mentioned\n>> with libdrm include folder. Both can be found with pkg-config on Ubuntu\n>> as well.\n> \n> The issue is that we wanted to depend in the Linux kernel DRM headers,\n> not the libdrm package. libcamera doesn't interact with DRM directly, so\n> depending on libdrm wasn't something we wanted to consider. The Linux\n> kernel DRM headers contain drm_fourcc.h, which we initially thought\n> would be enough, but it turned out that distributions don't all ship the\n> DRM headers as part of the Linux kernel headers package.\n> \n>> On the other hand your approach might be better, to remove the\n>> dependency to libdrm and create your own define for the format if it's\n>> going to be the only dependency, the project will have to libdrm.\n>>\n>> But in this case the patch is missing to remove those files then:\n>>\n>> - drm.h\n>> - drm_fourcc.h\n>> - drm_mode.h\n> \n> drm.h and drm_mode.h should indeed be dropped. drm_fourcc.h is still\n> used by gen-formats.py to generate the formats.h header, so we need to\n> keep it.4\n> \n>> On 10/6/20 1:23, Laurent Pinchart wrote:\n>>> Hello,\n>>>\n>>> This patch series is an attempt to fix a problem caused by a direct\n>>> dependency on drm_fourcc.h in the libcamera public API.\n>>>\n>>> libcamera specifies pixel formats using the DRM pixel format FourCC and\n>>> modifiers. This requires both internal code and applications to include\n>>> drm_fourcc.h. For internal code, we carry a copy of the header to avoid\n>>> external dependencies. For third-party applications, however,\n>>> drm_fourcc.h needs to be accessible from system includes. It turns out\n>>> that the file is shipped by two different packages:\n>>>\n>>> - libdrm, which typically installs it in /usr/include/libdrm/\n>>> - kernel headers or libc headers, which typically installs it in\n>>>     /usr/include/drm\n>>>\n>>> We don't want to make libdrm a dependency for applications using\n>>> libcamera. Unfortunately, depending on drm_fourcc.h being provided by\n>>> the distribution kernel headers or libc headers package causes issues,\n>>> as not all distributions install the header in /usr/include/drm/. Ubuntu\n>>> and Gentoo do, but Debian and Arch don't.\n>>>\n>>> The best option may be to remove the dependency on the macros provided\n>>> by drm_fourcc.h, while keeping the pixel format numerical values\n>>> identical. This is what this patch series attempts to do.\n>>>\n>>> Patch 1/7 creates a new libcamera::formats:: namespace and populates it\n>>> with constants (in the form of constexpr) for all supported pixel\n>>> formats. The values are automatically generated from a list of formats,\n>>> stored in formats.yaml, and the numerical values for the DRM FourCCs and\n>>> modifiers are extracted from drm_fourcc.h.\n>>>\n>>> Patches 2/7 to 7/8 then replace usage of the DRM_FORMAT_* macros through\n>>> the code.\n>>>\n>>> Laurent Pinchart (7):\n>>>     libcamera: Define constants for pixel formats in the public API\n>>>     gst: Replace explicit DRM FourCCs with libcamera formats\n>>>     qcam: Replace explicit DRM FourCCs with libcamera formats\n>>>     v4l2: Replace explicit DRM FourCCs with libcamera formats\n>>>     test: Replace explicit DRM FourCCs with libcamera formats\n>>>     libcamera: pipeline: Replace explicit DRM FourCCs with libcamera\n>>>       formats\n>>>     libcamera: Replace explicit DRM FourCCs with libcamera formats\n>>>\n>>>    include/libcamera/formats.h.in                |  44 ++++++\n>>>    include/libcamera/gen-formats.py              | 118 ++++++++++++++\n>>>    include/libcamera/meson.build                 |  22 +++\n>>>    include/libcamera/pixel_format.h              |   2 -\n>>>    src/android/camera_device.cpp                 |   9 +-\n>>>    src/gstreamer/gstlibcamera-utils.cpp          |  62 ++++----\n>>>    src/libcamera/formats.cpp                     | 148 +++++++++---------\n>>>    src/libcamera/formats.yaml                    | 124 +++++++++++++++\n>>>    src/libcamera/pipeline/ipu3/ipu3.cpp          |  16 +-\n>>>    .../pipeline/raspberrypi/raspberrypi.cpp      |  10 +-\n>>>    src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  19 +--\n>>>    src/libcamera/pipeline/vimc/vimc.cpp          |  11 +-\n>>>    src/libcamera/pixel_format.cpp                |   4 +-\n>>>    src/libcamera/v4l2_pixelformat.cpp            |  83 +++++-----\n>>>    src/qcam/dng_writer.cpp                       |  17 +-\n>>>    src/qcam/format_converter.cpp                 |  36 +++--\n>>>    src/qcam/viewfinder.cpp                       |  10 +-\n>>>    src/v4l2/v4l2_camera_proxy.cpp                |  29 ++--\n>>>    test/v4l2_videodevice/buffer_cache.cpp        |   3 +-\n>>>    19 files changed, 540 insertions(+), 227 deletions(-)\n>>>    create mode 100644 include/libcamera/formats.h.in\n>>>    create mode 100755 include/libcamera/gen-formats.py\n>>>    create mode 100644 src/libcamera/formats.yaml\n>","headers":{"Return-Path":"<scerveau@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E000E6595C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Jun 2020 15:08:04 +0200 (CEST)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: scerveau) with ESMTPSA id 281222A44E8"],"To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20200609232323.29628-1-laurent.pinchart@ideasonboard.com>\n\t<35491e66-dc2b-6ca1-3758-de07e895ea3f@collabora.com>\n\t<20200610125734.GA5961@pendragon.ideasonboard.com>","From":"scerveau <scerveau@collabora.com>","Message-ID":"<5c695258-10f0-d6fb-c41c-710db7d6971a@collabora.com>","Date":"Wed, 10 Jun 2020 15:08:00 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200610125734.GA5961@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 10 Jun 2020 13:08:05 -0000"}},{"id":5159,"web_url":"https://patchwork.libcamera.org/comment/5159/","msgid":"<20200610131042.GB5961@pendragon.ideasonboard.com>","date":"2020-06-10T13:10:42","subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stéphane,\n\nOn Wed, Jun 10, 2020 at 03:08:00PM +0200, scerveau wrote:\n> Hi Laurent,\n> \n> I was suspecting something like that regarding the drm includes but\n> could you explain why you dont want to depend on the libdrm package as a \n> few runtime dependencies are necessary already ?\n\nBecause libcamera doesn't interact with DRM devices, to there's no need\nfor libdrm. The only thing we need is the definitions of the the pixel\nformats.\n\n> On 10/6/20 14:57, Laurent Pinchart wrote:\n> > On Wed, Jun 10, 2020 at 02:49:31PM +0200, scerveau wrote:\n> >> Hello Laurent,\n> >>\n> >> Yesterday I tried a different approach from previous attempt, where I\n> >> proposed to install the libdrm headers in libcamera include folders.\n> >>\n> >> Indeed I succeeded to make libcamera compile without including\n> >> drm_fourcc.h in pixel_format.h, see attached patches.\n> >>\n> >> This is building correctly on my machine (ubuntu 18.04) with a simple\n> >> `meson build`. But I might have missed use cases.\n> >>\n> >> Concerning the libdrm dependency or includes, I haven't noticed with\n> >> Debian/Buster or ArchLinux/latest (docker images) any issue mentioned\n> >> with libdrm include folder. Both can be found with pkg-config on Ubuntu\n> >> as well.\n> > \n> > The issue is that we wanted to depend in the Linux kernel DRM headers,\n> > not the libdrm package. libcamera doesn't interact with DRM directly, so\n> > depending on libdrm wasn't something we wanted to consider. The Linux\n> > kernel DRM headers contain drm_fourcc.h, which we initially thought\n> > would be enough, but it turned out that distributions don't all ship the\n> > DRM headers as part of the Linux kernel headers package.\n> > \n> >> On the other hand your approach might be better, to remove the\n> >> dependency to libdrm and create your own define for the format if it's\n> >> going to be the only dependency, the project will have to libdrm.\n> >>\n> >> But in this case the patch is missing to remove those files then:\n> >>\n> >> - drm.h\n> >> - drm_fourcc.h\n> >> - drm_mode.h\n> > \n> > drm.h and drm_mode.h should indeed be dropped. drm_fourcc.h is still\n> > used by gen-formats.py to generate the formats.h header, so we need to\n> > keep it.4\n> > \n> >> On 10/6/20 1:23, Laurent Pinchart wrote:\n> >>> Hello,\n> >>>\n> >>> This patch series is an attempt to fix a problem caused by a direct\n> >>> dependency on drm_fourcc.h in the libcamera public API.\n> >>>\n> >>> libcamera specifies pixel formats using the DRM pixel format FourCC and\n> >>> modifiers. This requires both internal code and applications to include\n> >>> drm_fourcc.h. For internal code, we carry a copy of the header to avoid\n> >>> external dependencies. For third-party applications, however,\n> >>> drm_fourcc.h needs to be accessible from system includes. It turns out\n> >>> that the file is shipped by two different packages:\n> >>>\n> >>> - libdrm, which typically installs it in /usr/include/libdrm/\n> >>> - kernel headers or libc headers, which typically installs it in\n> >>>     /usr/include/drm\n> >>>\n> >>> We don't want to make libdrm a dependency for applications using\n> >>> libcamera. Unfortunately, depending on drm_fourcc.h being provided by\n> >>> the distribution kernel headers or libc headers package causes issues,\n> >>> as not all distributions install the header in /usr/include/drm/. Ubuntu\n> >>> and Gentoo do, but Debian and Arch don't.\n> >>>\n> >>> The best option may be to remove the dependency on the macros provided\n> >>> by drm_fourcc.h, while keeping the pixel format numerical values\n> >>> identical. This is what this patch series attempts to do.\n> >>>\n> >>> Patch 1/7 creates a new libcamera::formats:: namespace and populates it\n> >>> with constants (in the form of constexpr) for all supported pixel\n> >>> formats. The values are automatically generated from a list of formats,\n> >>> stored in formats.yaml, and the numerical values for the DRM FourCCs and\n> >>> modifiers are extracted from drm_fourcc.h.\n> >>>\n> >>> Patches 2/7 to 7/8 then replace usage of the DRM_FORMAT_* macros through\n> >>> the code.\n> >>>\n> >>> Laurent Pinchart (7):\n> >>>     libcamera: Define constants for pixel formats in the public API\n> >>>     gst: Replace explicit DRM FourCCs with libcamera formats\n> >>>     qcam: Replace explicit DRM FourCCs with libcamera formats\n> >>>     v4l2: Replace explicit DRM FourCCs with libcamera formats\n> >>>     test: Replace explicit DRM FourCCs with libcamera formats\n> >>>     libcamera: pipeline: Replace explicit DRM FourCCs with libcamera\n> >>>       formats\n> >>>     libcamera: Replace explicit DRM FourCCs with libcamera formats\n> >>>\n> >>>    include/libcamera/formats.h.in                |  44 ++++++\n> >>>    include/libcamera/gen-formats.py              | 118 ++++++++++++++\n> >>>    include/libcamera/meson.build                 |  22 +++\n> >>>    include/libcamera/pixel_format.h              |   2 -\n> >>>    src/android/camera_device.cpp                 |   9 +-\n> >>>    src/gstreamer/gstlibcamera-utils.cpp          |  62 ++++----\n> >>>    src/libcamera/formats.cpp                     | 148 +++++++++---------\n> >>>    src/libcamera/formats.yaml                    | 124 +++++++++++++++\n> >>>    src/libcamera/pipeline/ipu3/ipu3.cpp          |  16 +-\n> >>>    .../pipeline/raspberrypi/raspberrypi.cpp      |  10 +-\n> >>>    src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  19 +--\n> >>>    src/libcamera/pipeline/vimc/vimc.cpp          |  11 +-\n> >>>    src/libcamera/pixel_format.cpp                |   4 +-\n> >>>    src/libcamera/v4l2_pixelformat.cpp            |  83 +++++-----\n> >>>    src/qcam/dng_writer.cpp                       |  17 +-\n> >>>    src/qcam/format_converter.cpp                 |  36 +++--\n> >>>    src/qcam/viewfinder.cpp                       |  10 +-\n> >>>    src/v4l2/v4l2_camera_proxy.cpp                |  29 ++--\n> >>>    test/v4l2_videodevice/buffer_cache.cpp        |   3 +-\n> >>>    19 files changed, 540 insertions(+), 227 deletions(-)\n> >>>    create mode 100644 include/libcamera/formats.h.in\n> >>>    create mode 100755 include/libcamera/gen-formats.py\n> >>>    create mode 100644 src/libcamera/formats.yaml","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0878A60C4E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Jun 2020 15:11:03 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4742D29E;\n\tWed, 10 Jun 2020 15:11:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DhfHt11h\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591794662;\n\tbh=wf/t8N2ad2AuSNlALCn+z8GMDdWY3hV3wNUpuKGTVJU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DhfHt11hWc/Qw5siafhGA2aUvU0XdkjgIzV3jHBhRwbcNJT7a50GYLS0pqx9BIt4B\n\taVjLhVNxiDqFs7b1Y9F5gv0II7x5wr7kKjkeTpsMJW6ugkhoBaSEsplo2PDOEA0cES\n\tKOa2x41G8kqgEu7P/jNDlKKjcE+0tRvXHERxn+8o=","Date":"Wed, 10 Jun 2020 16:10:42 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"scerveau <scerveau@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200610131042.GB5961@pendragon.ideasonboard.com>","References":"<20200609232323.29628-1-laurent.pinchart@ideasonboard.com>\n\t<35491e66-dc2b-6ca1-3758-de07e895ea3f@collabora.com>\n\t<20200610125734.GA5961@pendragon.ideasonboard.com>\n\t<5c695258-10f0-d6fb-c41c-710db7d6971a@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5c695258-10f0-d6fb-c41c-710db7d6971a@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v2 0/7] Introduce formats:: namespace\n\tfor libcamera pixel formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 10 Jun 2020 13:11:03 -0000"}}]