[libcamera-devel] [PATCH v2 6/6] v4l2: v4l2_camera_proxy: Use libcamera formats
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 1 14:29:49 CEST 2020
And another thing.
On Wed, Jul 01, 2020 at 02:15:32PM +0300, Laurent Pinchart wrote:
> On Tue, Jun 30, 2020 at 11:58:08PM +0900, Paul Elder wrote:
> > Now that calculation of bytes per line, image size, and conversions to
>
> s/bytes per line/line stride/ :-) (or just stride)
>
> > and from v4l2 and drm formats have been moved into libcamera core,
> > removed them from V4L2CameraProxy. This has a positive side-effect of
> > cleaning up the code.
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> >
> > ---
> > Changes in v2:
> > - clean up the code a bit more
> > ---
> > src/v4l2/v4l2_camera_proxy.cpp | 165 +++++++--------------------------
> > src/v4l2/v4l2_camera_proxy.h | 7 --
> > 2 files changed, 34 insertions(+), 138 deletions(-)
> >
> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > index c246570..485e4b6 100644
> > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > @@ -20,6 +20,7 @@
> > #include <libcamera/formats.h>
> > #include <libcamera/object.h>
> >
> > +#include "libcamera/internal/formats.h"
> > #include "libcamera/internal/log.h"
> > #include "libcamera/internal/utils.h"
> >
> > @@ -164,22 +165,20 @@ bool V4L2CameraProxy::validateMemoryType(uint32_t memory)
> >
> > void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)
> > {
> > - curV4L2Format_.fmt.pix.width = streamConfig.size.width;
> > - curV4L2Format_.fmt.pix.height = streamConfig.size.height;
> > - curV4L2Format_.fmt.pix.pixelformat = drmToV4L2(streamConfig.pixelFormat);
> > - curV4L2Format_.fmt.pix.field = V4L2_FIELD_NONE;
> > - curV4L2Format_.fmt.pix.bytesperline =
> > - bplMultiplier(curV4L2Format_.fmt.pix.pixelformat) *
> > - curV4L2Format_.fmt.pix.width;
> > - curV4L2Format_.fmt.pix.sizeimage =
> > - imageSize(curV4L2Format_.fmt.pix.pixelformat,
> > - curV4L2Format_.fmt.pix.width,
> > - curV4L2Format_.fmt.pix.height);
> > - curV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> > - curV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> > - curV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
> > + Size size = streamConfig.size;
> > +
> > + curV4L2Format_.fmt.pix.width = size.width;
> > + curV4L2Format_.fmt.pix.height = size.height;
> > + curV4L2Format_.fmt.pix.pixelformat = info.v4l2Format;
> > + curV4L2Format_.fmt.pix.field = V4L2_FIELD_NONE;
> > + curV4L2Format_.fmt.pix.bytesperline = info.bytesPerLine(size.width);
StreamConfiguration has a stride field, that's what you should use here
instead of calculating it. It allows pipeline handlers to convey
additional alignment constraints. The PixelFormatInfo::bytesPerLine()
function is still a good idea, but it should be used by the pipeline
handlers (if needed), not the V4L2 compatibility layer.
> > + curV4L2Format_.fmt.pix.sizeimage = info.imageSize(size);
> > + curV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> > + curV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> > + curV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > curV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT;
> > - curV4L2Format_.fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > + curV4L2Format_.fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > }
> >
> > unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)
> > @@ -188,9 +187,9 @@ unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConf
> > * \todo Merge this method with setFmtFromConfig (need imageSize to
> > * support all libcamera formats first, or filter out MJPEG for now).
> > */
> > - return imageSize(drmToV4L2(streamConfig.pixelFormat),
> > - streamConfig.size.width,
> > - streamConfig.size.height);
> > + const PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);
> > +
> > + return info.imageSize(streamConfig.size);
> > }
> >
> > void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
> > @@ -253,12 +252,13 @@ int V4L2CameraProxy::vidioc_enum_framesizes(V4L2CameraFile *file, struct v4l2_fr
> > {
> > LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_framesizes fd = " << file->efd();
> >
> > - PixelFormat argFormat = v4l2ToDrm(arg->pixel_format);
> > + V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->pixel_format);
> > + PixelFormat format = PixelFormatInfo::info(v4l2Format).format;
> > /*
> > * \todo This might need to be expanded as few pipeline handlers
> > * report StreamFormats.
> > */
> > - const std::vector<Size> &frameSizes = streamConfig_.formats().sizes(argFormat);
> > + const std::vector<Size> &frameSizes = streamConfig_.formats().sizes(format);
> >
> > if (arg->index >= frameSizes.size())
> > return -EINVAL;
> > @@ -279,12 +279,14 @@ int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *
> > arg->index >= streamConfig_.formats().pixelformats().size())
> > return -EINVAL;
> >
> > + PixelFormat format = streamConfig_.formats().pixelformats()[arg->index];
> > +
> > /* \todo Set V4L2_FMT_FLAG_COMPRESSED for compressed formats. */
>
> How about addressing this one ? :-) I think we can simply check if
> bytesPerGroup is zero for now.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > arg->flags = 0;
> > /* \todo Add map from format to description. */
> > utils::strlcpy(reinterpret_cast<char *>(arg->description),
> > "Video Format Description", sizeof(arg->description));
> > - arg->pixelformat = drmToV4L2(streamConfig_.formats().pixelformats()[arg->index]);
> > + arg->pixelformat = PixelFormatInfo::info(format).v4l2Format;
> >
> > memset(arg->reserved, 0, sizeof(arg->reserved));
> >
> > @@ -306,7 +308,8 @@ int V4L2CameraProxy::vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
> >
> > void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
> > {
> > - PixelFormat format = v4l2ToDrm(arg->fmt.pix.pixelformat);
> > + V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
> > + PixelFormat format = PixelFormatInfo::info(v4l2Format).format;
> > const std::vector<PixelFormat> &formats =
> > streamConfig_.formats().pixelformats();
> > if (std::find(formats.begin(), formats.end(), format) == formats.end())
> > @@ -317,15 +320,14 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
> > if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
> > size = streamConfig_.formats().sizes(format)[0];
> >
> > + const PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);
> > +
> > arg->fmt.pix.width = size.width;
> > arg->fmt.pix.height = size.height;
> > - arg->fmt.pix.pixelformat = drmToV4L2(format);
> > + arg->fmt.pix.pixelformat = formatInfo.v4l2Format;
> > arg->fmt.pix.field = V4L2_FIELD_NONE;
> > - arg->fmt.pix.bytesperline = bplMultiplier(drmToV4L2(format)) *
> > - arg->fmt.pix.width;
> > - arg->fmt.pix.sizeimage = imageSize(drmToV4L2(format),
> > - arg->fmt.pix.width,
> > - arg->fmt.pix.height);
> > + arg->fmt.pix.bytesperline = formatInfo.bytesPerLine(size.width);
> > + arg->fmt.pix.sizeimage = formatInfo.imageSize(size);
> > arg->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> > arg->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> > arg->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > @@ -350,8 +352,9 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
> > tryFormat(arg);
> >
> > Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> > + V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
> > ret = vcam_->configure(&streamConfig_, size,
> > - v4l2ToDrm(arg->fmt.pix.pixelformat),
> > + PixelFormatInfo::info(v4l2Format).format,
> > bufferCount_);
> > if (ret < 0)
> > return -EINVAL;
> > @@ -495,8 +498,9 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
> > freeBuffers();
> >
> > Size size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);
> > + V4L2PixelFormat v4l2Format = V4L2PixelFormat(curV4L2Format_.fmt.pix.pixelformat);
> > int ret = vcam_->configure(&streamConfig_, size,
> > - v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),
> > + PixelFormatInfo::info(v4l2Format).format,
> > arg->count);
> > if (ret < 0)
> > return -EINVAL;
> > @@ -835,104 +839,3 @@ void V4L2CameraProxy::release(V4L2CameraFile *file)
> >
> > owner_ = nullptr;
> > }
> > -
> > -struct PixelFormatPlaneInfo {
> > - unsigned int bitsPerPixel;
> > - unsigned int hSubSampling;
> > - unsigned int vSubSampling;
> > -};
> > -
> > -struct PixelFormatInfo {
> > - PixelFormat format;
> > - uint32_t v4l2Format;
> > - unsigned int numPlanes;
> > - std::array<PixelFormatPlaneInfo, 3> planes;
> > -};
> > -
> > -namespace {
> > -
> > -static const std::array<PixelFormatInfo, 16> pixelFormatInfo = {{
> > - /* RGB formats. */
> > - { formats::RGB888, V4L2_PIX_FMT_BGR24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - { formats::BGR888, V4L2_PIX_FMT_RGB24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - { formats::BGRA8888, V4L2_PIX_FMT_ARGB32, 1, {{ { 32, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - /* YUV packed formats. */
> > - { formats::UYVY, V4L2_PIX_FMT_UYVY, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - { formats::VYUY, V4L2_PIX_FMT_VYUY, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - { formats::YUYV, V4L2_PIX_FMT_YUYV, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - { formats::YVYU, V4L2_PIX_FMT_YVYU, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > - /* YUY planar formats. */
> > - { formats::NV12, V4L2_PIX_FMT_NV12, 2, {{ { 8, 1, 1 }, { 16, 2, 2 }, { 0, 0, 0 } }} },
> > - { formats::NV21, V4L2_PIX_FMT_NV21, 2, {{ { 8, 1, 1 }, { 16, 2, 2 }, { 0, 0, 0 } }} },
> > - { formats::NV16, V4L2_PIX_FMT_NV16, 2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },
> > - { formats::NV61, V4L2_PIX_FMT_NV61, 2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },
> > - { formats::NV24, V4L2_PIX_FMT_NV24, 2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },
> > - { formats::NV42, V4L2_PIX_FMT_NV42, 2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },
> > - { formats::YUV420, V4L2_PIX_FMT_YUV420, 3, {{ { 8, 1, 1 }, { 8, 2, 2 }, { 8, 2, 2 } }} },
> > - { formats::YUV422, V4L2_PIX_FMT_YUV422P, 3, {{ { 8, 1, 1 }, { 8, 2, 1 }, { 8, 2, 1 } }} },
> > - /* Compressed formats. */
> > - /*
> > - * \todo Get a better image size estimate for MJPEG, via
> > - * StreamConfiguration, instead of using the worst-case
> > - * width * height * bpp of uncompressed data.
> > - */
> > - { formats::MJPEG, V4L2_PIX_FMT_MJPEG, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
> > -}};
> > -
> > -} /* namespace */
> > -
> > -/* \todo make libcamera export these */
> > -unsigned int V4L2CameraProxy::bplMultiplier(uint32_t format)
> > -{
> > - auto info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> > - [format](const PixelFormatInfo &info) {
> > - return info.v4l2Format == format;
> > - });
> > - if (info == pixelFormatInfo.end())
> > - return 0;
> > -
> > - return info->planes[0].bitsPerPixel / 8;
> > -}
> > -
> > -unsigned int V4L2CameraProxy::imageSize(uint32_t format, unsigned int width,
> > - unsigned int height)
> > -{
> > - auto info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> > - [format](const PixelFormatInfo &info) {
> > - return info.v4l2Format == format;
> > - });
> > - if (info == pixelFormatInfo.end())
> > - return 0;
> > -
> > - unsigned int multiplier = 0;
> > - for (unsigned int i = 0; i < info->numPlanes; ++i)
> > - multiplier += info->planes[i].bitsPerPixel
> > - / info->planes[i].hSubSampling
> > - / info->planes[i].vSubSampling;
> > -
> > - return width * height * multiplier / 8;
> > -}
> > -
> > -PixelFormat V4L2CameraProxy::v4l2ToDrm(uint32_t format)
> > -{
> > - auto info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> > - [format](const PixelFormatInfo &info) {
> > - return info.v4l2Format == format;
> > - });
> > - if (info == pixelFormatInfo.end())
> > - return PixelFormat();
> > -
> > - return info->format;
> > -}
> > -
> > -uint32_t V4L2CameraProxy::drmToV4L2(const PixelFormat &format)
> > -{
> > - auto info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> > - [format](const PixelFormatInfo &info) {
> > - return info.format == format;
> > - });
> > - if (info == pixelFormatInfo.end())
> > - return format;
> > -
> > - return info->v4l2Format;
> > -}
> > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> > index d78a472..49184a1 100644
> > --- a/src/v4l2/v4l2_camera_proxy.h
> > +++ b/src/v4l2/v4l2_camera_proxy.h
> > @@ -69,13 +69,6 @@ private:
> > int acquire(V4L2CameraFile *file);
> > void release(V4L2CameraFile *file);
> >
> > - static unsigned int bplMultiplier(uint32_t format);
> > - static unsigned int imageSize(uint32_t format, unsigned int width,
> > - unsigned int height);
> > -
> > - static PixelFormat v4l2ToDrm(uint32_t format);
> > - static uint32_t drmToV4L2(const PixelFormat &format);
> > -
> > static const std::set<unsigned long> supportedIoctls_;
> >
> > unsigned int refcount_;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list