[{"id":969,"web_url":"https://patchwork.libcamera.org/comment/969/","msgid":"<20190228215431.GL7811@pendragon.ideasonboard.com>","date":"2019-02-28T21:54:31","subject":"Re: [libcamera-devel] [PATCH v5 4/9] libcamera: v4l2_device: Add\n\tsupport for META_CAPTURE devices","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Feb 28, 2019 at 09:01:46PM +0100, Jacopo Mondi wrote:\n> Add support for devices that provide video meta-data to v4l2_device.cpp\n> and re-arrange bufferType handling in open() method.\n> \n> Reviewed-by: Kieran Bingham \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/v4l2_device.h | 4 +++\n> src/libcamera/v4l2_device.cpp | 38 +++++++++++++++++------------\n> 2 files changed, 27 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 1d31d1b403bc..733bd69959f3 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -53,6 +53,10 @@ struct V4L2Capability final : v4l2_capability {\n> \t\treturn device_caps() & (V4L2_CAP_VIDEO_CAPTURE |\n> \t\t\t\t\tV4L2_CAP_VIDEO_CAPTURE_MPLANE);\n> \t}\n> +\tbool isMetaCapture() const\n> +\t{\n> +\t\treturn device_caps() & V4L2_CAP_META_CAPTURE;\n> +\t}\n> \tbool isOutput() const\n\nAs explained in a previous review, I think you should reorganize these\nfunctions.\n\n> \t{\n> \t\treturn device_caps() & (V4L2_CAP_VIDEO_OUTPUT |\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 24e115554a99..9bfa10e8a151 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -79,6 +79,15 @@ LOG_DEFINE_CATEGORY(V4L2)\n> * \\return True if the device can output video frames\n> */\n> \n> +/**\n> + * \\fn bool V4L2Capability::isMetaCapture()\n> + * \\brief Identify if the device is capable of providing video meta-data\n> + *\n> + * TODO: add support for META_OUTPUT, introduced in Linux v5.0\n\nYou can use \\todo instead of TODO:.\n\nWith these,\n\nReviewed-by: Laurent Pinchart \n\n> + *\n> + * \\return True if the device can capture video meta-data\n> + */\n> +\n> /**\n> * \\fn bool V4L2Capability::hasStreaming()\n> * \\brief Determine if the device can perform Streaming I/O\n> @@ -280,33 +289,32 @@ int V4L2Device::open()\n> \t\t<< \"Opened device \" << caps_.bus_info() << \": \"\n> \t\t<< caps_.driver() << \": \" << caps_.card();\n> \n> -\tif (!caps_.isCapture() && !caps_.isOutput()) {\n> -\t\tLOG(V4L2, Debug) << \"Device is not a supported type\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> \tif (!caps_.hasStreaming()) {\n> \t\tLOG(V4L2, Error) << \"Device does not support streaming I/O\";\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tif (caps_.isCapture())\n> +\t/*\n> +\t * Set buffer type and wait for read notifications on CAPTURE devices\n> +\t * (POLLIN), and write notifications for OUTPUT devices (POLLOUT).\n> +\t */\n> +\tif (caps_.isCapture()) {\n> +\t\tfdEvent_ = new EventNotifier(fd_, EventNotifier::Read);\n> \t\tbufferType_ = caps_.isMultiplanar()\n> \t\t\t ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE\n> \t\t\t : V4L2_BUF_TYPE_VIDEO_CAPTURE;\n> -\telse\n> +\t} else if (caps_.isOutput()) {\n> +\t\tfdEvent_ = new EventNotifier(fd_, EventNotifier::Write);\n> \t\tbufferType_ = caps_.isMultiplanar()\n> \t\t\t ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE\n> \t\t\t : V4L2_BUF_TYPE_VIDEO_OUTPUT;\n> -\n> -\t/*\n> -\t * We wait for Read notifications on CAPTURE devices (POLLIN), and\n> -\t * Write notifications for OUTPUT devices (POLLOUT).\n> -\t */\n> -\tif (caps_.isCapture())\n> +\t} else if (caps_.isMetaCapture()) {\n> \t\tfdEvent_ = new EventNotifier(fd_, EventNotifier::Read);\n> -\telse\n> -\t\tfdEvent_ = new EventNotifier(fd_, EventNotifier::Write);\n> +\t\tbufferType_ = V4L2_BUF_TYPE_META_CAPTURE;\n> +\t} else {\n> +\t\tLOG(V4L2, Debug) << \"Device is not a supported type\";\n> +\t\treturn -EINVAL;\n> +\t}\n> \n> \tfdEvent_->activated.connect(this, &V4L2Device::bufferAvailable);\n> \tfdEvent_->setEnabled(false);","headers":{"Return-Path":"","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 0A7C3610B6\n\tfor ;\n\tThu, 28 Feb 2019 22:54:37 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 84C8E49;\n\tThu, 28 Feb 2019 22:54:36 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551390876;\n\tbh=+vzYu10rFqsDeWU29C8Ic3pVZXegQ/y2s90i47CCJic=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aJcqmglvyxrC9SWhYUOoE69RLrhu0atxiQb3QjaWXPhtYFR5zGcD1Z51uL7yx86oZ\n\tAZC0aJJAtAzYSxTKW2sYuLDqDt7inVLCQZuR/WchsnL6hPsFPW0G68NqX8rZH/9CDS\n\t817x6OZW4NtSA1Ulf2D0nfNJFEz73t5BlaCkTbyw=","Date":"Thu, 28 Feb 2019 23:54:31 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190228215431.GL7811@pendragon.ideasonboard.com>","References":"<20190228200151.2948-1-jacopo@jmondi.org>\n\t<20190228200151.2948-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190228200151.2948-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 4/9] libcamera: v4l2_device: Add\n\tsupport for META_CAPTURE devices","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 28 Feb 2019 21:54:37 -0000"}}]