Patch Detail
Show a patch.
GET /api/patches/4177/?format=api
{ "id": 4177, "url": "https://patchwork.libcamera.org/api/patches/4177/?format=api", "web_url": "https://patchwork.libcamera.org/patch/4177/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200623190836.53446-4-paul.elder@ideasonboard.com>", "date": "2020-06-23T19:08:17", "name": "[libcamera-devel,v3,03/22] v4l2: v4l2_compat: Support multiple open", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "28f1010a7804d03aa3ae8daa2b897ae638dfb616", "submitter": { "id": 17, "url": "https://patchwork.libcamera.org/api/people/17/?format=api", "name": "Paul Elder", "email": "paul.elder@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/4177/mbox/", "series": [ { "id": 1034, "url": "https://patchwork.libcamera.org/api/series/1034/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1034", "date": "2020-06-23T19:08:14", "name": "Support v4l2-compliance", "version": 3, "mbox": "https://patchwork.libcamera.org/series/1034/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/4177/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/4177/checks/", "tags": {}, "headers": { "Return-Path": "<paul.elder@ideasonboard.com>", "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 A8B5F609A7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jun 2020 21:09:09 +0200 (CEST)", "from jade.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:8147:f2a2:a8c6:9087])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3368E329;\n\tTue, 23 Jun 2020 21:09:07 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"asW8hx07\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592939349;\n\tbh=CNI07xPVFaxx9sRcBYHZW10aL6IHbfGQFJJV6UkxiB4=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=asW8hx078i150GYwFQQbEzp6XBh+oWNtBbKLScScyWizY2M5Ht3PKyCkWcd7QJyPv\n\tdrPItMTkjRdtZX2dBaZ9M8ViHV8hJg+dVMw8TOS4ZJ7chRYI9Phh97yEzIEKWSFW4I\n\tqtaEt32iHSMc/YenWP0h/V/cYRbbeqtPueiC7Lo8=", "From": "Paul Elder <paul.elder@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 24 Jun 2020 04:08:17 +0900", "Message-Id": "<20200623190836.53446-4-paul.elder@ideasonboard.com>", "X-Mailer": "git-send-email 2.27.0", "In-Reply-To": "<20200623190836.53446-1-paul.elder@ideasonboard.com>", "References": "<20200623190836.53446-1-paul.elder@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 03/22] v4l2: v4l2_compat: Support\n\tmultiple open", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Tue, 23 Jun 2020 19:09:10 -0000" }, "content": "Previously, since we acquired the libcamera camera upon open(), it was\nimpossible to support multiple open, as any subsequent opens would\nreturn error because the camera would already be acquired.\n\nTo fix this, we first initialize the camera in the first call to\nV4L2CameraProxy::open(), just to heat up the stream format cache. We\nthen add ownership by a V4L2CameraFile of a V4L2Camera via the\nV4L2CameraProxy. All vidioc ioctls prior to reqbufs > 0 (except for\ns_fmt) are able to access the camera without ownership. A call to\nreqbufs > 0 (and s_fmt) will take ownership, and the ownership will be\nreleased at reqbufs = 0. While ownership is assigned, the eventfd that\nshould be signaled (and cleared) by V4L2Camera and V4L2CameraProxy is\nset to the V4L2CameraFile that has ownership, and is cleared when the\nownership is released. In case close() is called without a\nreqbufs = 0 first, the ownership is also released on close().\n\nWe also use the V4L2CameraFile to contain all the information specific\nto an open instance of the file. This removes the need to keep track of\nsuch information within V4L2CameraProxy via multiple maps from int fd to\ninfo.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\n---\nChanges in v3:\n- split V4L2CompatManager stuff into separate patch\n- split changing the fd argument to camera file argument in all proxy\n methods into the separate patch\n- remove V4L2CameraProxy::efd_ (it's duplicated from\n V4L2CameraFile::efd())\n- rename cf in all the V4L2CameraProxy methods to file\n- rename V4L2CameraProxy::acquiredCf_ to V4L2CameraProxy::owner_\n- remove V4L2CameraProxy::initialized_; just use V4L2CameraProxy::refcount_\n- move bufferCount_ > 0 in reqbufs to different patch\n- move TIMESTAMP_MONOTONIC to different patch\n- don't take the lock for querybuf\n- add V4L2CameraProxy::hasOwnership\n- rename lock() to acquire() and unlock() to release()\n- only lock on s_fmt and reqbufs > 0, in other ioctls only check for\n ownership\n- add explanation about not supporting polling for events from an fd\n different than the one that owns the device for polling for bufs\n\nChanges in v2:\n- use V4L2CameraFile in V4L2CompatManager to deal with dup, and in\n V4L2CameraProxy to deal with information specific to the open file\n (nonblock and priority)\n- unlock exclusive camera lock on close()\n---\n src/v4l2/v4l2_camera.cpp | 9 +++-\n src/v4l2/v4l2_camera.h | 1 +\n src/v4l2/v4l2_camera_proxy.cpp | 99 ++++++++++++++++++++++++++++------\n src/v4l2/v4l2_camera_proxy.h | 16 +++++-\n 4 files changed, 106 insertions(+), 19 deletions(-)", "diff": "diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\nindex 9a1ebc8..2557320 100644\n--- a/src/v4l2/v4l2_camera.cpp\n+++ b/src/v4l2/v4l2_camera.cpp\n@@ -17,7 +17,8 @@ using namespace libcamera;\n LOG_DECLARE_CATEGORY(V4L2Compat);\n \n V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)\n-\t: camera_(camera), isRunning_(false), bufferAllocator_(nullptr)\n+\t: camera_(camera), isRunning_(false), bufferAllocator_(nullptr),\n+\t efd_(-1)\n {\n \tcamera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);\n }\n@@ -29,7 +30,6 @@ V4L2Camera::~V4L2Camera()\n \n int V4L2Camera::open()\n {\n-\t/* \\todo Support multiple open. */\n \tif (camera_->acquire() < 0) {\n \t\tLOG(V4L2Compat, Error) << \"Failed to acquire camera\";\n \t\treturn -EINVAL;\n@@ -59,6 +59,11 @@ void V4L2Camera::bind(int efd)\n \tefd_ = efd;\n }\n \n+void V4L2Camera::unbind()\n+{\n+\tefd_ = -1;\n+}\n+\n void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig)\n {\n \t*streamConfig = config_->at(0);\ndiff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\nindex 33f5eb0..30114ed 100644\n--- a/src/v4l2/v4l2_camera.h\n+++ b/src/v4l2/v4l2_camera.h\n@@ -40,6 +40,7 @@ public:\n \tint open();\n \tvoid close();\n \tvoid bind(int efd);\n+\tvoid unbind();\n \tvoid getStreamConfig(StreamConfiguration *streamConfig);\n \tstd::vector<Buffer> completedBuffers();\n \ndiff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\nindex c1ee1be..eb59d49 100644\n--- a/src/v4l2/v4l2_camera_proxy.cpp\n+++ b/src/v4l2/v4l2_camera_proxy.cpp\n@@ -35,7 +35,7 @@ LOG_DECLARE_CATEGORY(V4L2Compat);\n V4L2CameraProxy::V4L2CameraProxy(unsigned int index,\n \t\t\t\t std::shared_ptr<Camera> camera)\n \t: refcount_(0), index_(index), bufferCount_(0), currentBuf_(0),\n-\t vcam_(std::make_unique<V4L2Camera>(camera))\n+\t vcam_(std::make_unique<V4L2Camera>(camera)), owner_(nullptr)\n {\n \tquerycap(camera);\n }\n@@ -44,18 +44,29 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)\n {\n \tLOG(V4L2Compat, Debug) << \"Servicing open fd = \" << file->efd();\n \n+\tif (refcount_++)\n+\t\treturn 0;\n+\n+\t/*\n+\t * We open the camera here, once, and keep it open until the last\n+\t * V4L2CameraFile is closed. The proxy is initially not owned by any\n+\t * file. The first file that calls reqbufs with count > 0 will become\n+\t * the owner, and no other file will be allowed to call buffer-related\n+\t * ioctls (except querycap, querybuf, try_fmt, g/s_priority,\n+\t * enum/g/s_input), set the format or start or stop the stream until\n+\t * ownership is released with a call to reqbufs with count = 0.\n+\t */\n+\n \tint ret = vcam_->open();\n \tif (ret < 0) {\n-\t\terrno = -ret;\n-\t\treturn -1;\n+\t\trefcount_--;\n+\t\treturn ret;\n \t}\n \n \tvcam_->getStreamConfig(&streamConfig_);\n \tsetFmtFromConfig(streamConfig_);\n \tsizeimage_ = calculateSizeImage(streamConfig_);\n \n-\trefcount_++;\n-\n \treturn 0;\n }\n \n@@ -63,6 +74,7 @@ void V4L2CameraProxy::close(V4L2CameraFile *file)\n {\n \tLOG(V4L2Compat, Debug) << \"Servicing close fd = \" << file->efd();\n \n+\trelease(file);\n \n \tif (--refcount_ > 0)\n \t\treturn;\n@@ -277,12 +289,16 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)\n \tif (!validateBufferType(arg->type))\n \t\treturn -EINVAL;\n \n+\tint ret = acquire(file);\n+\tif (ret < 0)\n+\t\treturn ret;\n+\n \ttryFormat(arg);\n \n \tSize size(arg->fmt.pix.width, arg->fmt.pix.height);\n-\tint ret = vcam_->configure(&streamConfig_, size,\n-\t\t\t\t v4l2ToDrm(arg->fmt.pix.pixelformat),\n-\t\t\t\t bufferCount_);\n+\tret = vcam_->configure(&streamConfig_, size,\n+\t\t\t v4l2ToDrm(arg->fmt.pix.pixelformat),\n+\t\t\t bufferCount_);\n \tif (ret < 0)\n \t\treturn -EINVAL;\n \n@@ -334,15 +350,21 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf\n \n \tLOG(V4L2Compat, Debug) << arg->count << \" buffers requested \";\n \n+\tint ret = acquire(file);\n+\tif (ret < 0)\n+\t\treturn ret;\n+\n \targ->capabilities = V4L2_BUF_CAP_SUPPORTS_MMAP;\n \n-\tif (arg->count == 0)\n+\tif (arg->count == 0) {\n+\t\trelease(file);\n \t\treturn freeBuffers();\n+\t}\n \n \tSize size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);\n-\tint ret = vcam_->configure(&streamConfig_, size,\n-\t\t\t\t v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),\n-\t\t\t\t arg->count);\n+\tret = vcam_->configure(&streamConfig_, size,\n+\t\t\t v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),\n+\t\t\t arg->count);\n \tif (ret < 0)\n \t\treturn -EINVAL;\n \n@@ -409,6 +431,9 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n \t\t\t << arg->index << \" fd = \" << file->efd();\n \n+\tif (!hasOwnership(file))\n+\t\treturn -EBUSY;\n+\n \tif (!validateBufferType(arg->type) ||\n \t !validateMemoryType(arg->memory) ||\n \t arg->index >= bufferCount_)\n@@ -428,6 +453,9 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n {\n \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_dqbuf fd = \" << file->efd();\n \n+\tif (!hasOwnership(file))\n+\t\treturn -EBUSY;\n+\n \tif (!validateBufferType(arg->type) ||\n \t !validateMemoryType(arg->memory))\n \t\treturn -EINVAL;\n@@ -462,6 +490,9 @@ int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n \tif (!validateBufferType(*arg))\n \t\treturn -EINVAL;\n \n+\tif (!hasOwnership(file))\n+\t\treturn -EBUSY;\n+\n \tcurrentBuf_ = 0;\n \n \treturn vcam_->streamOn();\n@@ -474,6 +505,9 @@ int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg)\n \tif (!validateBufferType(*arg))\n \t\treturn -EINVAL;\n \n+\tif (!hasOwnership(file) && owner_)\n+\t\treturn -EBUSY;\n+\n \tint ret = vcam_->streamOff();\n \n \tfor (struct v4l2_buffer &buf : buffers_)\n@@ -532,10 +566,45 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n \treturn ret;\n }\n \n-void V4L2CameraProxy::bind(int fd)\n+bool V4L2CameraProxy::hasOwnership(V4L2CameraFile *file)\n+{\n+\treturn owner_ == file;\n+}\n+\n+/**\n+ * \\brief Acquire exclusive ownership of the V4L2Camera\n+ *\n+ * \\return Zero on success or if already acquired, and negative error on\n+ * failure.\n+ *\n+ * This is sufficient for poll()ing for buffers. Events, however, are signaled\n+ * on the file level, so all fds must be signaled. poll()ing from a different\n+ * fd than the one that locks the device is a corner case, and is currently not\n+ * supported.\n+ */\n+int V4L2CameraProxy::acquire(V4L2CameraFile *file)\n {\n-\tefd_ = fd;\n-\tvcam_->bind(fd);\n+\tif (owner_ == file)\n+\t\treturn 0;\n+\n+\tif (owner_)\n+\t\treturn -EBUSY;\n+\n+\tvcam_->bind(file->efd());\n+\n+\towner_ = file;\n+\n+\treturn 0;\n+}\n+\n+void V4L2CameraProxy::release(V4L2CameraFile *file)\n+{\n+\tif (owner_ != file)\n+\t\treturn;\n+\n+\tvcam_->unbind();\n+\n+\towner_ = nullptr;\n }\n \n struct PixelFormatPlaneInfo {\ndiff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\nindex b2197ef..2be46e6 100644\n--- a/src/v4l2/v4l2_camera_proxy.h\n+++ b/src/v4l2/v4l2_camera_proxy.h\n@@ -33,7 +33,6 @@ public:\n \tvoid *mmap(void *addr, size_t length, int prot, int flags, off64_t offset);\n \tint munmap(void *addr, size_t length);\n \n-\tvoid bind(int fd);\n \tint ioctl(V4L2CameraFile *file, unsigned long request, void *arg);\n \n private:\n@@ -58,6 +57,10 @@ private:\n \tint vidioc_streamon(V4L2CameraFile *file, int *arg);\n \tint vidioc_streamoff(V4L2CameraFile *file, int *arg);\n \n+\tbool hasOwnership(V4L2CameraFile *file);\n+\tint acquire(V4L2CameraFile *file);\n+\tvoid release(V4L2CameraFile *file);\n+\n \tstatic unsigned int bplMultiplier(uint32_t format);\n \tstatic unsigned int imageSize(uint32_t format, unsigned int width,\n \t\t\t\t unsigned int height);\n@@ -80,7 +83,16 @@ private:\n \n \tstd::unique_ptr<V4L2Camera> vcam_;\n \n-\tint efd_;\n+\t/*\n+\t * This is the exclusive owner of this V4L2CameraProxy instance.\n+\t * When there is no owner, anybody can call any ioctl before reqbufs.\n+\t * The first file to call reqbufs with count > 0 will become the owner,\n+\t * and when the owner calls reqbufs with count = 0 it will release\n+\t * ownership. Any ioctl that is call by a non-owner while there exists\n+\t * and owner will return -EBUSY (except querycap, querybuf, try_fmt,\n+\t * g/s_priority, enum/g/s_input\n+\t */\n+\tV4L2CameraFile *owner_;\n };\n \n #endif /* __V4L2_CAMERA_PROXY_H__ */\n", "prefixes": [ "libcamera-devel", "v3", "03/22" ] }