[libcamera-devel] [PATCH/RFC 10/11] libcamera: pipeline: Replace explicit DRM FourCCs with libcamera formats
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu May 28 13:26:15 CEST 2020
Hi Kieran,
On Thu, May 28, 2020 at 11:16:39AM +0100, Kieran Bingham wrote:
> On 22/05/2020 15:54, Laurent Pinchart wrote:
> > Use the new pixel format constants to replace usage of macros from
> > drm_fourcc.h.
> >
> > The IPU3 pipeline handler still uses DRM FourCCs for IPU3-specific
> > formats that are not defined in the libcamera public API.
>
> If the header is (mostly)automatically generated from the drm_fourcc,
> would it then be available? (I.e. ... should we define it? or leave it)
>
> Otherwise, would it make sense to add a comment to that usage to state
> /why/ we use the DRM format directly for any casual reader who suddenly
> sees a DRM_FORMAT_ out of the blue (and which differs from the rest of
> the code base)
I've actually decided to reconsider this, as applications may be
interested in capturing raw images with the IPU3, even if the format is
non-standard. We thus need to define IPU3-specific formats, like we do
for the other Bayer formats. I'll change that in the next version.
> Either-way,
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +++++---
> > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++++-----
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++++++++---------
> > src/libcamera/pipeline/vimc/vimc.cpp | 11 ++++++-----
> > 4 files changed, 26 insertions(+), 22 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index b805fea71c2d..f23f338d4eb7 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -11,9 +11,11 @@
> > #include <queue>
> > #include <vector>
> >
> > +#include <linux/drm_fourcc.h>
> > #include <linux/media-bus-format.h>
> >
> > #include <libcamera/camera.h>
> > +#include <libcamera/formats.h>
> > #include <libcamera/request.h>
> > #include <libcamera/stream.h>
> >
> > @@ -261,7 +263,7 @@ IPU3CameraConfiguration::IPU3CameraConfiguration(Camera *camera,
> > void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
> > {
> > /* The only pixel format the driver supports is NV12. */
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> > + cfg.pixelFormat = formats::NV12;
> >
> > if (scale) {
> > /*
> > @@ -430,7 +432,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> > StreamConfiguration cfg = {};
> > IPU3Stream *stream = nullptr;
> >
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> > + cfg.pixelFormat = formats::NV12;
> >
> > switch (role) {
> > case StreamRole::StillCapture:
> > @@ -1193,7 +1195,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output,
> > return 0;
> >
> > *outputFormat = {};
> > - outputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));
> > + outputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12);
> > outputFormat->size = cfg.size;
> > outputFormat->planesCount = 2;
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index e16a9c7f10d3..d3d11f87ecb5 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -13,12 +13,12 @@
> >
> > #include <libcamera/camera.h>
> > #include <libcamera/control_ids.h>
> > +#include <libcamera/formats.h>
> > #include <libcamera/ipa/raspberrypi.h>
> > #include <libcamera/logging.h>
> > #include <libcamera/request.h>
> > #include <libcamera/stream.h>
> >
> > -#include <linux/drm_fourcc.h>
> > #include <linux/videodev2.h>
> >
> > #include "libcamera/internal/camera_sensor.h"
> > @@ -490,7 +490,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >
> > if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt, false)) == fmts.end()) {
> > /* If we cannot find a native format, use a default one. */
> > - cfgPixFmt = PixelFormat(DRM_FORMAT_NV12);
> > + cfgPixFmt = formats::NV12;
> > status = Adjusted;
> > }
> > }
> > @@ -537,20 +537,20 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > break;
> >
> > case StreamRole::StillCapture:
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> > + cfg.pixelFormat = formats::NV12;
> > /* Return the largest sensor resolution. */
> > cfg.size = data->sensor_->resolution();
> > cfg.bufferCount = 1;
> > break;
> >
> > case StreamRole::VideoRecording:
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> > + cfg.pixelFormat = formats::NV12;
> > cfg.size = { 1920, 1080 };
> > cfg.bufferCount = 4;
> > break;
> >
> > case StreamRole::Viewfinder:
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_ARGB8888);
> > + cfg.pixelFormat = formats::ARGB8888;
> > cfg.size = { 800, 600 };
> > cfg.bufferCount = 4;
> > break;
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index d807fc2cf535..401e777fbbf7 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -16,6 +16,7 @@
> > #include <libcamera/buffer.h>
> > #include <libcamera/camera.h>
> > #include <libcamera/control_ids.h>
> > +#include <libcamera/formats.h>
> > #include <libcamera/ipa/rkisp1.h>
> > #include <libcamera/request.h>
> > #include <libcamera/stream.h>
> > @@ -459,13 +460,13 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> > CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > {
> > static const std::array<PixelFormat, 8> formats{
> > - PixelFormat(DRM_FORMAT_YUYV),
> > - PixelFormat(DRM_FORMAT_YVYU),
> > - PixelFormat(DRM_FORMAT_VYUY),
> > - PixelFormat(DRM_FORMAT_NV16),
> > - PixelFormat(DRM_FORMAT_NV61),
> > - PixelFormat(DRM_FORMAT_NV21),
> > - PixelFormat(DRM_FORMAT_NV12),
> > + formats::YUYV,
> > + formats::YVYU,
> > + formats::VYUY,
> > + formats::NV16,
> > + formats::NV61,
> > + formats::NV21,
> > + formats::NV12,
> > /* \todo Add support for 8-bit greyscale to DRM formats */
> > };
> >
> > @@ -487,7 +488,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) ==
> > formats.end()) {
> > LOG(RkISP1, Debug) << "Adjusting format to NV12";
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12),
> > + cfg.pixelFormat = formats::NV12,
> > status = Adjusted;
> > }
> >
> > @@ -566,7 +567,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
> > return config;
> >
> > StreamConfiguration cfg{};
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);
> > + cfg.pixelFormat = formats::NV12;
> > cfg.size = data->sensor_->resolution();
> >
> > config->addConfiguration(cfg);
> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > index ca36348a5eb6..ce6db27ce576 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -17,6 +17,7 @@
> > #include <libcamera/camera.h>
> > #include <libcamera/control_ids.h>
> > #include <libcamera/controls.h>
> > +#include <libcamera/formats.h>
> > #include <libcamera/ipa/ipa_interface.h>
> > #include <libcamera/ipa/ipa_module_info.h>
> > #include <libcamera/request.h>
> > @@ -108,9 +109,9 @@ private:
> > namespace {
> >
> > static const std::array<PixelFormat, 3> pixelformats{
> > - PixelFormat(DRM_FORMAT_RGB888),
> > - PixelFormat(DRM_FORMAT_BGR888),
> > - PixelFormat(DRM_FORMAT_BGRA8888),
> > + formats::RGB888,
> > + formats::BGR888,
> > + formats::BGRA8888,
> > };
> >
> > } /* namespace */
> > @@ -139,7 +140,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
> > if (std::find(pixelformats.begin(), pixelformats.end(), cfg.pixelFormat) ==
> > pixelformats.end()) {
> > LOG(VIMC, Debug) << "Adjusting format to RGB24";
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_BGR888);
> > + cfg.pixelFormat = formats::BGR888;
> > status = Adjusted;
> > }
> >
> > @@ -188,7 +189,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
> >
> > StreamConfiguration cfg(formats);
> >
> > - cfg.pixelFormat = PixelFormat(DRM_FORMAT_BGR888);
> > + cfg.pixelFormat = formats::BGR888;
> > cfg.size = { 1920, 1080 };
> > cfg.bufferCount = 4;
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list