From patchwork Fri Feb 5 13:10:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helen Koike X-Patchwork-Id: 11176 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 43052BD162 for ; Fri, 5 Feb 2021 13:11:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1CDFE614B1; Fri, 5 Feb 2021 14:11:04 +0100 (CET) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2EE64614B0 for ; Fri, 5 Feb 2021 14:11:03 +0100 (CET) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: koike) with ESMTPSA id 781101F466AB From: Helen Koike To: libcamera-devel@lists.libcamera.org Date: Fri, 5 Feb 2021 10:10:43 -0300 Message-Id: <20210205131044.512128-3-helen.koike@collabora.com> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210205131044.512128-1-helen.koike@collabora.com> References: <20210205131044.512128-1-helen.koike@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 2/3] libcamera: Use VIDIOC_{S, G, TRY}_EXT_PIX_FMT API 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: , Cc: kieran.bingham+renesas@ideasonboard.com, frkoenig@chromium.org, kernel@collabora.com Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use Ext API for format negotiation. Unify multi vs single planar handling, prepare to use modifiers. Signed-off-by: Helen Koike --- include/libcamera/internal/v4l2_videodevice.h | 8 +- src/libcamera/pipeline/ipu3/cio2.cpp | 1 - src/libcamera/pipeline/ipu3/imgu.cpp | 1 - .../pipeline/raspberrypi/raspberrypi.cpp | 4 + src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 4 +- src/libcamera/pipeline/simple/converter.cpp | 6 +- src/libcamera/pipeline/simple/simple.cpp | 8 +- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 + src/libcamera/pipeline/vimc/vimc.cpp | 2 + src/libcamera/v4l2_videodevice.cpp | 183 ++++++------------ 10 files changed, 75 insertions(+), 144 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 626dfbcd..d2de44c0 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -163,7 +163,6 @@ public: Size size; std::array planes; - unsigned int planesCount = 0; const std::string toString() const; }; @@ -222,11 +221,8 @@ private: int getFormatMeta(V4L2DeviceFormat *format); int trySetFormatMeta(V4L2DeviceFormat *format, bool set); - int getFormatMultiplane(V4L2DeviceFormat *format); - int trySetFormatMultiplane(V4L2DeviceFormat *format, bool set); - - int getFormatSingleplane(V4L2DeviceFormat *format); - int trySetFormatSingleplane(V4L2DeviceFormat *format, bool set); + int getFormatExt(V4L2DeviceFormat *format); + int trySetFormatExt(V4L2DeviceFormat *format, bool set); std::vector enumPixelformats(uint32_t code); std::vector enumSizes(V4L2PixelFormat pixelFormat); diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 0ef3bc04..ea0c2b59 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -184,7 +184,6 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) outputFormat->fourcc = info.v4l2Format; outputFormat->size = sensorFormat.size; - outputFormat->planesCount = 1; ret = output_->setFormat(outputFormat); if (ret) diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index d5cf05b0..7684b466 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -527,7 +527,6 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad, *outputFormat = {}; outputFormat->fourcc = dev->toV4L2PixelFormat(formats::NV12); outputFormat->size = cfg.size; - outputFormat->planesCount = 2; ret = dev->setFormat(outputFormat); if (ret) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 0804a439..5f7a1488 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -373,6 +373,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() cfg.stride = sensorFormat.planes[0].bpl; cfg.frameSize = sensorFormat.planes[0].size; + for (unsigned int i = 1; i < sensorFormat.planes.size(); ++i) + cfg.frameSize += sensorFormat.planes[i].size; rawCount++; } else { @@ -445,6 +447,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; + for (unsigned int j = 1; j < format.planes.size(); ++j) + cfg.frameSize += format.planes[j].size; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 25f482eb..adea5590 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -89,6 +89,8 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) cfg->stride = format.planes[0].bpl; cfg->frameSize = format.planes[0].size; + for (unsigned int i = 1; i < format.planes.size(); ++i) + cfg->frameSize += format.planes[i].size; if (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) { LOG(RkISP1, Debug) @@ -144,11 +146,9 @@ int RkISP1Path::configure(const StreamConfiguration &config, << "Configured " << name_ << " resizer output pad with " << ispFormat.toString(); - const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat); V4L2DeviceFormat outputFormat; outputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat); outputFormat.size = config.size; - outputFormat.planesCount = info.numPlanes(); ret = video_->setFormat(&outputFormat); if (ret) diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index a6a8e139..ccdd30cf 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -263,12 +263,16 @@ SimpleConverter::strideAndFrameSize(const Size &size, V4L2DeviceFormat format; format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); format.size = size; + uint32_t frameSize = 0; int ret = m2m_->capture()->tryFormat(&format); if (ret < 0) return std::make_tuple(0, 0); - return std::make_tuple(format.planes[0].bpl, format.planes[0].size); + for (unsigned int i = 0; i < format.planes.size(); ++i) + frameSize += format.planes[i].size; + + return std::make_tuple(format.planes[0].bpl, frameSize); } } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 23320d26..97111d30 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -487,6 +487,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; + for (unsigned int i = 1; i < format.planes.size(); ++i) + cfg.frameSize += format.planes[i].size; return status; } @@ -590,12 +592,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) if (ret) return ret; - if (captureFormat.planesCount != 1) { - LOG(SimplePipeline, Error) - << "Planar formats using non-contiguous memory not supported"; - return -EINVAL; - } - if (captureFormat.fourcc != videoFormat || captureFormat.size != pipeConfig.captureSize) { LOG(SimplePipeline, Error) diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 08a59417..136399b6 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -160,6 +160,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; + for (unsigned int i = 1; i < format.planes.size(); ++i) + cfg.frameSize += format.planes[i].size; return status; } diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 36325ffb..d42c9ec0 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -171,6 +171,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; + for (unsigned int i = 1; i < format.planes.size(); ++i) + cfg.frameSize += format.planes[i].size; return status; } diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index c77e1aff..fab3ba4c 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -337,27 +337,24 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const * stride length and size, but only a global stride length which is applied to * all planes. * - * The V4L2DeviceFormat class describes both packed and planar image formats, - * regardless of the API type (single or multi plane) implemented by the video - * device the format has to be applied to. The total size and bytes per line - * of images represented with packed formats are configured using the first - * entry of the V4L2DeviceFormat::planes array, while the per-plane size and - * per-plane stride length of images represented with planar image formats are - * configured using the opportune number of entries of the - * V4L2DeviceFormat::planes array, as prescribed by the image format - * definition (semi-planar formats use 2 entries, while planar formats use the - * whole 3 entries). The number of valid entries of the - * V4L2DeviceFormat::planes array is defined by the - * V4L2DeviceFormat::planesCount value. + * The V4L2DeviceFormat class describes formats per color component and not + * memory planes. Even if a single memory buffer is used, color planes are still + * described separately. + * The size and bytes per line of each color component is defined separately + * with the V4L2DeviceFormat:::planes array. + * Except for metadata, this class uses the Ext API that unifies the + * multiplanar and single planar API. + * The number of valid entries of the V4L2DeviceFormat::planes array is defined + * by the number of non-zero bytesperline field, up to 3 planes. */ /** * \struct V4L2DeviceFormat::Plane - * \brief Per-plane memory size information + * \brief Per color plane size information * \var V4L2DeviceFormat::Plane::size - * \brief The plane total memory size (in bytes) + * \brief The color plane total memory size (in bytes) * \var V4L2DeviceFormat::Plane::bpl - * \brief The plane line stride (in bytes) + * \brief The color plane line stride (in bytes) */ /** @@ -375,17 +372,11 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const /** * \var V4L2DeviceFormat::planes - * \brief The per-plane memory size information + * \brief The per color plane memory size information * * Images are stored in memory in one or more data planes. Each data plane has a * specific line stride and memory size, which could differ from the image * visible sizes to accommodate padding at the end of lines and end of planes. - * Only the first \ref planesCount entries are considered valid. - */ - -/** - * \var V4L2DeviceFormat::planesCount - * \brief The number of valid data planes */ /** @@ -723,10 +714,8 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat *format) { if (caps_.isMeta()) return getFormatMeta(format); - else if (caps_.isMultiplanar()) - return getFormatMultiplane(format); else - return getFormatSingleplane(format); + return getFormatExt(format); } /** @@ -743,10 +732,8 @@ int V4L2VideoDevice::tryFormat(V4L2DeviceFormat *format) { if (caps_.isMeta()) return trySetFormatMeta(format, false); - else if (caps_.isMultiplanar()) - return trySetFormatMultiplane(format, false); else - return trySetFormatSingleplane(format, false); + return trySetFormatExt(format, false); } /** @@ -762,10 +749,8 @@ int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format) { if (caps_.isMeta()) return trySetFormatMeta(format, true); - else if (caps_.isMultiplanar()) - return trySetFormatMultiplane(format, true); else - return trySetFormatSingleplane(format, true); + return trySetFormatExt(format, true); } int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) @@ -784,9 +769,12 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format) format->size.width = 0; format->size.height = 0; format->fourcc = V4L2PixelFormat(pix->dataformat); - format->planesCount = 1; format->planes[0].bpl = pix->buffersize; format->planes[0].size = pix->buffersize; + for (unsigned int i = 1; i < format->planes.size(); ++i) { + format->planes[i].bpl = 0; + format->planes[i].size = 0; + } return 0; } @@ -800,6 +788,8 @@ int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set) v4l2Format.type = bufferType_; pix->dataformat = format->fourcc; pix->buffersize = format->planes[0].size; + for (unsigned int i = 1; i < format->planes.size(); ++i) + pix->buffersize += format->planes[i].size; ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format); if (ret) { LOG(V4L2, Error) @@ -815,122 +805,60 @@ int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set) format->size.width = 0; format->size.height = 0; format->fourcc = V4L2PixelFormat(pix->dataformat); - format->planesCount = 1; format->planes[0].bpl = pix->buffersize; format->planes[0].size = pix->buffersize; + for (unsigned int i = 1; i < format->planes.size(); ++i) { + format->planes[i].bpl = 0; + format->planes[i].size = 0; + } return 0; } -int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format) +int V4L2VideoDevice::getFormatExt(V4L2DeviceFormat *format) { - struct v4l2_format v4l2Format = {}; - struct v4l2_pix_format_mplane *pix = &v4l2Format.fmt.pix_mp; + struct v4l2_ext_pix_format v4l2FormatExt = {}; int ret; - v4l2Format.type = bufferType_; - ret = ioctl(VIDIOC_G_FMT, &v4l2Format); + v4l2FormatExt.type = bufferType_; + ret = ioctl(VIDIOC_G_EXT_PIX_FMT, &v4l2FormatExt); if (ret) { - LOG(V4L2, Error) << "Unable to get format: " << strerror(-ret); + LOG(V4L2, Error) << "Unable to get format ext: " << strerror(-ret); return ret; } - format->size.width = pix->width; - format->size.height = pix->height; - format->fourcc = V4L2PixelFormat(pix->pixelformat); - format->planesCount = pix->num_planes; + format->size.width = v4l2FormatExt.width; + format->size.height = v4l2FormatExt.height; + format->fourcc = V4L2PixelFormat(v4l2FormatExt.pixelformat); - for (unsigned int i = 0; i < format->planesCount; ++i) { - format->planes[i].bpl = pix->plane_fmt[i].bytesperline; - format->planes[i].size = pix->plane_fmt[i].sizeimage; + for (unsigned int i = 0; i < format->planes.size(); ++i) { + format->planes[i].bpl = v4l2FormatExt.plane_fmt[i].bytesperline; + format->planes[i].size = v4l2FormatExt.plane_fmt[i].sizeimage; } return 0; } -int V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set) +int V4L2VideoDevice::trySetFormatExt(V4L2DeviceFormat *format, bool set) { - struct v4l2_format v4l2Format = {}; - struct v4l2_pix_format_mplane *pix = &v4l2Format.fmt.pix_mp; + struct v4l2_ext_pix_format v4l2FormatExt = {}; int ret; - v4l2Format.type = bufferType_; - pix->width = format->size.width; - pix->height = format->size.height; - pix->pixelformat = format->fourcc; - pix->num_planes = format->planesCount; - pix->field = V4L2_FIELD_NONE; - - ASSERT(pix->num_planes <= std::size(pix->plane_fmt)); - - for (unsigned int i = 0; i < pix->num_planes; ++i) { - pix->plane_fmt[i].bytesperline = format->planes[i].bpl; - pix->plane_fmt[i].sizeimage = format->planes[i].size; - } - - ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format); - if (ret) { - LOG(V4L2, Error) - << "Unable to " << (set ? "set" : "try") - << " format: " << strerror(-ret); - return ret; - } - - /* - * Return to caller the format actually applied on the video device, - * which might differ from the requested one. - */ - format->size.width = pix->width; - format->size.height = pix->height; - format->fourcc = V4L2PixelFormat(pix->pixelformat); - format->planesCount = pix->num_planes; - for (unsigned int i = 0; i < format->planesCount; ++i) { - format->planes[i].bpl = pix->plane_fmt[i].bytesperline; - format->planes[i].size = pix->plane_fmt[i].sizeimage; - } - - return 0; -} + v4l2FormatExt.type = bufferType_; + v4l2FormatExt.width = format->size.width; + v4l2FormatExt.height = format->size.height; + v4l2FormatExt.pixelformat = format->fourcc; + v4l2FormatExt.field = V4L2_FIELD_NONE; -int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format) -{ - struct v4l2_format v4l2Format = {}; - struct v4l2_pix_format *pix = &v4l2Format.fmt.pix; - int ret; - - v4l2Format.type = bufferType_; - ret = ioctl(VIDIOC_G_FMT, &v4l2Format); - if (ret) { - LOG(V4L2, Error) << "Unable to get format: " << strerror(-ret); - return ret; + for (unsigned int i = 0; i < format->planes.size(); ++i) { + v4l2FormatExt.plane_fmt[i].bytesperline = format->planes[i].bpl; + v4l2FormatExt.plane_fmt[i].sizeimage = format->planes[i].size; } - format->size.width = pix->width; - format->size.height = pix->height; - format->fourcc = V4L2PixelFormat(pix->pixelformat); - format->planesCount = 1; - format->planes[0].bpl = pix->bytesperline; - format->planes[0].size = pix->sizeimage; - - return 0; -} - -int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set) -{ - struct v4l2_format v4l2Format = {}; - struct v4l2_pix_format *pix = &v4l2Format.fmt.pix; - int ret; - - v4l2Format.type = bufferType_; - pix->width = format->size.width; - pix->height = format->size.height; - pix->pixelformat = format->fourcc; - pix->bytesperline = format->planes[0].bpl; - pix->field = V4L2_FIELD_NONE; - ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format); + ret = ioctl(set ? VIDIOC_S_EXT_PIX_FMT : VIDIOC_TRY_EXT_PIX_FMT, &v4l2FormatExt); if (ret) { LOG(V4L2, Error) - << "Unable to " << (set ? "set" : "try") + << "Unable to " << (set ? "set ext pix" : "try ext pi") << " format: " << strerror(-ret); return ret; } @@ -939,12 +867,13 @@ int V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set) * Return to caller the format actually applied on the device, * which might differ from the requested one. */ - format->size.width = pix->width; - format->size.height = pix->height; - format->fourcc = V4L2PixelFormat(pix->pixelformat); - format->planesCount = 1; - format->planes[0].bpl = pix->bytesperline; - format->planes[0].size = pix->sizeimage; + format->size.width = v4l2FormatExt.width; + format->size.height = v4l2FormatExt.height; + format->fourcc = V4L2PixelFormat(v4l2FormatExt.pixelformat); + for (unsigned int i = 0; i < format->planes.size(); ++i) { + format->planes[i].bpl = v4l2FormatExt.plane_fmt[i].bytesperline; + format->planes[i].size = v4l2FormatExt.plane_fmt[i].sizeimage; + } return 0; }