[libcamera-devel] [PATCH v4 07/21] v4l2: v4l2_camera_proxy: Get stride and frameSize from stream config
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 8 16:43:33 CEST 2020
Hi Paul,
Thank you for the patch.
On Wed, Jul 08, 2020 at 10:44:03PM +0900, Paul Elder wrote:
> The stride and frameSize should be obtained through StreamConfiguration
> rather than PixelFormatInfo, as pipeline handlers might have different
> values (eg. for alignment). Get the stride and frameSize values from
> StreamConfiguration instead of from PixelFormatInfo.
>
> This removes the need for V4L2CameraProxy's calculation helper functions
> (bplMultiplier, imageSize, v4l2ToDrm, drmToV4L2, calculateSizeImage) and
> formats, so remove them.
>
> This also removes the need for V4L2CameraProxy::calculateSizeImage, so
> remove it,.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
> Changes in v4:
> - squashed with "v4l2: v4l2_camera_proxy: Use libcamera formats"
>
> New in v3
> ---
> src/v4l2/v4l2_camera_proxy.cpp | 192 ++++++---------------------------
> src/v4l2/v4l2_camera_proxy.h | 8 --
> 2 files changed, 32 insertions(+), 168 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index c246570..305ede0 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"
>
> @@ -70,7 +71,6 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)
>
> vcam_->getStreamConfig(&streamConfig_);
> setFmtFromConfig(streamConfig_);
> - sizeimage_ = calculateSizeImage(streamConfig_);
>
> files_.insert(file);
>
> @@ -164,33 +164,22 @@ 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.stride(size.width, 0);
> + curV4L2Format_.fmt.pix.sizeimage = info.frameSize(size);
The commit message states you get the stride and frame size from the
stream configuration, but they're still retrieved from the pixel format
info here.
> + 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)
> -{
> - /*
> - * \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);
> + sizeimage_ = info.frameSize(size);
> }
>
> void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
> @@ -253,12 +242,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 +269,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. */
> 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 +298,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 +310,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.stride(size.width, 0);
> + arg->fmt.pix.sizeimage = formatInfo.frameSize(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,18 +342,13 @@ 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;
>
> - unsigned int sizeimage = calculateSizeImage(streamConfig_);
> - if (sizeimage == 0)
> - return -EINVAL;
> -
> - sizeimage_ = sizeimage;
> -
> setFmtFromConfig(streamConfig_);
>
> return 0;
> @@ -495,27 +482,13 @@ 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;
>
> - sizeimage_ = calculateSizeImage(streamConfig_);
> - /*
> - * If we return -EINVAL here then the application will think that we
> - * don't support streaming mmap. Since we don't support readwrite and
> - * userptr either, the application will get confused and think that
> - * we don't support anything.
> - * On the other hand, if the set format at the time of reqbufs has a
> - * zero sizeimage we'll get a floating point exception when we try to
> - * stream it.
> - */
> - if (sizeimage_ == 0)
> - LOG(V4L2Compat, Warning)
> - << "sizeimage of at least one format is zero. "
> - << "Streaming this format will cause a floating point exception.";
> -
> setFmtFromConfig(streamConfig_);
>
> arg->count = streamConfig_.bufferCount;
> @@ -835,104 +808,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..e962694 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -40,7 +40,6 @@ private:
> bool validateBufferType(uint32_t type);
> bool validateMemoryType(uint32_t memory);
> void setFmtFromConfig(StreamConfiguration &streamConfig);
> - unsigned int calculateSizeImage(StreamConfiguration &streamConfig);
> void querycap(std::shared_ptr<Camera> camera);
> void tryFormat(struct v4l2_format *arg);
> enum v4l2_priority maxPriority();
> @@ -69,13 +68,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