[{"id":5362,"web_url":"https://patchwork.libcamera.org/comment/5362/","msgid":"<20200623224044.GL5870@pendragon.ideasonboard.com>","date":"2020-06-23T22:40:44","subject":"Re: [libcamera-devel] [PATCH v3 09/22] v4l2: v4l2_camera_proxy:\n\tImplement VIDIOC_G/S_PRIORITY","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jun 24, 2020 at 04:08:23AM +0900, Paul Elder wrote:\n> Implement VIDIOC_G_PRIORITY and VIDIOC_S_PRIORITY. The behaviour\n> documented in the V4L2 specification doesn't match the implementation in\n> the Linux kernel, implement the latter.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v3:\n> - save the priorities of every file\n> - they're saved in V4L2CameraFile, so save a set of these in V4L2CameraProxy\n> - they're unique from the perspective of the proxy so set is fine\n> - actually check the priority for s_fmt, reqbufs, streamon, streamoff,\n> s_input, and s_priority\n> \n> Changes in v2:\n> - use V4L2CameraFile instead of fd and priorities map\n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 59 ++++++++++++++++++++++++++++++++++\n> src/v4l2/v4l2_camera_proxy.h | 5 +++\n> 2 files changed, 64 insertions(+)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index bf14ba0..361abe4 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -45,6 +45,8 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)\n> {\n> \tLOG(V4L2Compat, Debug) << \"Servicing open fd = \" << file->efd();\n> \n> +\tfiles_.insert(file);\n> +\n> \tif (refcount_++)\n> \t\treturn 0;\n\nIn the error paths below, shouldn't you remove file from files_ ? It may\nbe easier to perform the insertion in the refcount_++ case and at the\nvery end of the function instead.\n\n> \n> @@ -75,6 +77,8 @@ void V4L2CameraProxy::close(V4L2CameraFile *file)\n> {\n> \tLOG(V4L2Compat, Debug) << \"Servicing close fd = \" << file->efd();\n> \n> +\tfiles_.erase(file);\n> +\n> \trelease(file);\n> \n> \tif (--refcount_ > 0)\n> @@ -304,6 +308,9 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)\n> \tif (!validateBufferType(arg->type))\n> \t\treturn -EINVAL;\n> \n> +\tif (file->priority() < maxPriority())\n> +\t\treturn -EBUSY;\n> +\n> \tint ret = acquire(file);\n> \tif (ret < 0)\n> \t\treturn ret;\n> @@ -340,6 +347,41 @@ int V4L2CameraProxy::vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *ar\n> \treturn 0;\n> }\n> \n> +enum v4l2_priority V4L2CameraProxy::maxPriority()\n> +{\n> +\tenum v4l2_priority maxPrio = V4L2_PRIORITY_UNSET;\n> +\tfor (V4L2CameraFile *f : files_)\n\n\tfor (V4L2CameraFile *f : files_) {\n\n> +\t\tif (f->priority() > maxPrio)\n> +\t\t\tmaxPrio = f->priority();\n\n\t}\n\nOr\n\n\tauto max = std::max_element(files_.begin(), files_.end(),\n\t\t\t\t [](const V4L2CameraFile *a, const V4L2CameraFile *b) {\n\t\t\t\t\t return a->priority() < b->priority();\n\t\t\t\t });\n\treturn max != files_.end() ? (*max)->priority() : V4L2_PRIORITY_UNSET;\n\nUp to you.\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\treturn maxPrio;\n> +}\n> +\n> +int V4L2CameraProxy::vidioc_g_priority(V4L2CameraFile *file, enum v4l2_priority *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_priority fd = \" << file->efd();\n> +\n> +\t*arg = maxPriority();\n> +\n> +\treturn 0;\n> +}\n> +\n> +int V4L2CameraProxy::vidioc_s_priority(V4L2CameraFile *file, enum v4l2_priority *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug)\n> +\t\t<< \"Servicing vidioc_s_priority fd = \" << file->efd();\n> +\n> +\tif (*arg > V4L2_PRIORITY_RECORD)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (file->priority() < maxPriority())\n> +\t\treturn -EBUSY;\n> +\n> +\tfile->setPriority(*arg);\n> +\n> +\treturn 0;\n> +}\n> +\n> int V4L2CameraProxy::freeBuffers()\n> {\n> \tLOG(V4L2Compat, Debug) << \"Freeing libcamera bufs\";\n> @@ -365,6 +407,9 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf\n> \n> \tLOG(V4L2Compat, Debug) << arg->count << \" buffers requested \";\n> \n> +\tif (file->priority() < maxPriority())\n> +\t\treturn -EBUSY;\n> +\n> \tint ret = acquire(file);\n> \tif (ret < 0)\n> \t\treturn ret;\n> @@ -516,6 +561,9 @@ int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n> \tif (!validateBufferType(*arg))\n> \t\treturn -EINVAL;\n> \n> +\tif (file->priority() < maxPriority())\n> +\t\treturn -EBUSY;\n> +\n> \tif (!hasOwnership(file))\n> \t\treturn -EBUSY;\n> \n> @@ -531,6 +579,9 @@ int V4L2CameraProxy::vidioc_streamoff(V4L2CameraFile *file, int *arg)\n> \tif (!validateBufferType(*arg))\n> \t\treturn -EINVAL;\n> \n> +\tif (file->priority() < maxPriority())\n> +\t\treturn -EBUSY;\n> +\n> \tif (!hasOwnership(file) && owner_)\n> \t\treturn -EBUSY;\n> \n> @@ -548,6 +599,8 @@ const std::set V4L2CameraProxy::supportedIoctls_ = {\n> \tVIDIOC_G_FMT,\n> \tVIDIOC_S_FMT,\n> \tVIDIOC_TRY_FMT,\n> +\tVIDIOC_G_PRIORITY,\n> +\tVIDIOC_S_PRIORITY,\n> \tVIDIOC_REQBUFS,\n> \tVIDIOC_QUERYBUF,\n> \tVIDIOC_QBUF,\n> @@ -590,6 +643,12 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n> \tcase VIDIOC_TRY_FMT:\n> \t\tret = vidioc_try_fmt(file, static_cast(arg));\n> \t\tbreak;\n> +\tcase VIDIOC_G_PRIORITY:\n> +\t\tret = vidioc_g_priority(file, static_cast(arg));\n> +\t\tbreak;\n> +\tcase VIDIOC_S_PRIORITY:\n> +\t\tret = vidioc_s_priority(file, static_cast(arg));\n> +\t\tbreak;\n> \tcase VIDIOC_REQBUFS:\n> \t\tret = vidioc_reqbufs(file, static_cast(arg));\n> \t\tbreak;\n> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> index 2582eb5..a7293bf 100644\n> --- a/src/v4l2/v4l2_camera_proxy.h\n> +++ b/src/v4l2/v4l2_camera_proxy.h\n> @@ -43,6 +43,7 @@ private:\n> \tunsigned int calculateSizeImage(StreamConfiguration &streamConfig);\n> \tvoid querycap(std::shared_ptr camera);\n> \tvoid tryFormat(struct v4l2_format *arg);\n> +\tenum v4l2_priority maxPriority();\n> \tvoid updateBuffers();\n> \tint freeBuffers();\n> \n> @@ -51,6 +52,8 @@ private:\n> \tint vidioc_g_fmt(V4L2CameraFile *file, struct v4l2_format *arg);\n> \tint vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg);\n> \tint vidioc_try_fmt(V4L2CameraFile *file, struct v4l2_format *arg);\n> +\tint vidioc_g_priority(V4L2CameraFile *file, enum v4l2_priority *arg);\n> +\tint vidioc_s_priority(V4L2CameraFile *file, enum v4l2_priority *arg);\n> \tint vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg);\n> \tint vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> \tint vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> @@ -84,6 +87,8 @@ private:\n> \tstd::vector buffers_;\n> \tstd::map mmaps_;\n> \n> +\tstd::set files_;\n> +\n> \tstd::unique_ptr vcam_;\n> \n> \t/*","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 8CFC9603B9\n\tfor ;\n\tWed, 24 Jun 2020 00:41:12 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9688D2A9;\n\tWed, 24 Jun 2020 00:41:11 +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=\"GjO2Bxm+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592952072;\n\tbh=GqxYS1vJ+fctWZ97mokhi9loGrH62vswpJcEN9f+QR8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GjO2Bxm+cOXDyW6Fvp+Hc3Q0JSnE06992qJuwHyjjk00Er4d+QPKmZZ2eGBrSZphX\n\taDgbwgwBcJSCLcY+jwNXlM8WWRGmtqYotLyHyfmpbV2ZjYToe+PNeySpjqS87++sTz\n\txqUZUNRElWvfcyH1GQqlE7P+ViIdHSKGjuiYvTmE=","Date":"Wed, 24 Jun 2020 01:40:44 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200623224044.GL5870@pendragon.ideasonboard.com>","References":"<20200623190836.53446-1-paul.elder@ideasonboard.com>\n\t<20200623190836.53446-10-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200623190836.53446-10-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 09/22] v4l2: v4l2_camera_proxy:\n\tImplement VIDIOC_G/S_PRIORITY","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 23 Jun 2020 22:41:12 -0000"}}]