Patch Detail
Show a patch.
GET /api/patches/18757/?format=api
{ "id": 18757, "url": "https://patchwork.libcamera.org/api/patches/18757/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18757/", "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": "<20230622111717.32236-1-umang.jain@ideasonboard.com>", "date": "2023-06-22T11:17:17", "name": "[libcamera-devel] gstreamer: Maintain a control list to track libcamerasrc control properties", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "774a033e65a300389698dbd878fa42668064dae3", "submitter": { "id": 86, "url": "https://patchwork.libcamera.org/api/people/86/?format=api", "name": "Umang Jain", "email": "umang.jain@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18757/mbox/", "series": [ { "id": 3934, "url": "https://patchwork.libcamera.org/api/series/3934/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3934", "date": "2023-06-22T11:17:17", "name": "[libcamera-devel] gstreamer: Maintain a control list to track libcamerasrc control properties", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3934/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18757/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18757/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 92BACC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 22 Jun 2023 11:17:28 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA12C61E40;\n\tThu, 22 Jun 2023 13:17:27 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8ECF26002B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 22 Jun 2023 13:17:26 +0200 (CEST)", "from umang.jainideasonboard.com (unknown [103.95.123.182])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCC384DE;\n\tThu, 22 Jun 2023 13:16:48 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687432647;\n\tbh=hx3CRN8TIsb5RH2JzUgcjhX4mDe8eFsmw6PV2no4Tbo=;\n\th=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:Cc:From;\n\tb=q9lOwyMLt6OajpCpK4VL1TZ0K1vu33ITGLXARMcQdmtrHD8xYlYCJRSmZqMmsb+Vf\n\tRfuTlu2GwsDPBGf5pw5KO0nhVsK9Q54f/yRiWzb6pDUmmjM9ngVFIDajipNycUkfeh\n\tLtCcFs4dG5gQXFDEfPa0aItSkcMcIYl9qyH57AcAYLhbA3Mwjv5gu2NlWzBWDJpHIk\n\tK8Et8q5aRRC7aREFwe+VAnYIzsGykmWbsaYqDjAJqFk2QUgyDNDlbmvfO7gu3xo15i\n\t6AP8yFl7rKq4OCVBwe04YxUo0iOsfOpeKGF8CgsUZq+P1qMO4gi+KSnfdJ8cM3oUYW\n\tyS9HfMPiQgCgw==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687432610;\n\tbh=hx3CRN8TIsb5RH2JzUgcjhX4mDe8eFsmw6PV2no4Tbo=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=lUU2ljTCJrUnRVI6+LKpDcoTlE0S1MLinjFHX2MeF2tkMvSYvDAs/BFG4eoqaEXFr\n\tATMuS+RMx/0MOCysAyKEOLvhKu64C3gT+b87Dle1IAJUE4t7S8wz59AC7DSMV0qhHB\n\t5nZHQWKwRZU+GVODgn7t9UoKHfQl89iHiFk2ewIw=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lUU2ljTC\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 22 Jun 2023 16:47:17 +0530", "Message-Id": "<20230622111717.32236-1-umang.jain@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.1", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH] gstreamer: Maintain a control list to\n\ttrack libcamerasrc control properties", "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>", "From": "Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>", "Reply-To": "Umang Jain <umang.jain@ideasonboard.com>", "Cc": "Cedric Nugteren <cedric@plumerai.com>,\n\tCedric Nugteren <web@cedricnugteren.nl>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "As more and more libcamera controls get plumbed, more member variables\nget introduced in struct GstLibcameraSrc. Instead of doing that, now\nmaintain a single ControlList which is more appropriate to keep track\nof controls that get sets on libcamerasrc.\n\nThis also brings easy validation of controls set on libcamerasrc. If\nthe controls are not supported by camera, abort early and report what is\nnot supported.\n\nThe current patch migrates previously introduced control,\nauto-focus-mode, to be set directly in control list.\n\nSigned-off-by: Umang Jain <umang.jain@ideasonboard.com>\n---\nCedric, Can you please help test this?\n\n(I'm travelling so have tested this on my RPi and inferring from\nlibcamera/gstreamer logs, it seems to have not broken anything)\n\nAn additional test from your side would be appreciated.\n---\n src/gstreamer/gstlibcamerasrc.cpp | 45 ++++++++++++++++++++++---------\n src/gstreamer/gstlibcamerasrc.h | 6 ++---\n 2 files changed, 36 insertions(+), 15 deletions(-)", "diff": "diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 47d8ff43..a2266310 100644\n--- a/src/gstreamer/gstlibcamerasrc.cpp\n+++ b/src/gstreamer/gstlibcamerasrc.cpp\n@@ -146,7 +146,7 @@ struct _GstLibcameraSrc {\n \tGstTask *task;\n \n \tgchar *camera_name;\n-\tcontrols::AfModeEnum auto_focus_mode = controls::AfModeManual;\n+\tControlList *controls;\n \n \tGstLibcameraSrcState *state;\n \tGstLibcameraAllocator *allocator;\n@@ -462,7 +462,9 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);\n \tGLibRecLocker lock(&self->stream_lock);\n \tGstLibcameraSrcState *state = self->state;\n+\tconst ControlInfoMap &info_map = state->cam_->controls();\n \tGstFlowReturn flow_ret = GST_FLOW_OK;\n+\tgboolean supported_ctrl = true;\n \tgint ret;\n \n \tg_autoptr(GstStructure) element_caps = gst_structure_new_empty(\"caps\");\n@@ -579,18 +581,31 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n \t}\n \n-\tif (self->auto_focus_mode != controls::AfModeManual) {\n-\t\tconst ControlInfoMap &infoMap = state->cam_->controls();\n-\t\tif (infoMap.find(&controls::AfMode) != infoMap.end()) {\n-\t\t\tstate->initControls_.set(controls::AfMode, self->auto_focus_mode);\n-\t\t} else {\n+\t/*\n+\t * Check the controls set on libcamerasrc is supported by the camera.\n+\t * This can be easily done by CameraControlValidator but it is an internal\n+\t * libcamera class. Hence avoid adding a internal header even though\n+\t * gstreamer links with libcamera-private.\n+\t */\n+\tfor (auto iter = self->controls->begin(); iter != self->controls->end(); iter++) {\n+\t\tif (info_map.find(iter->first) == info_map.end()) {\n+\t\t\tconst ControlIdMap *id_map = self->controls->idMap();\n \t\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n-\t\t\t\t\t (\"Failed to enable auto focus\"),\n-\t\t\t\t\t (\"AfMode not supported by this camera, \"\n-\t\t\t\t\t \"please retry with 'auto-focus-mode=AfModeManual'\"));\n+\t\t\t\t\t (\"Failed to set %s\", id_map->at(iter->first)->name().c_str()),\n+\t\t\t\t\t (\"%s not supported by this camera, \", id_map->at(iter->first)->name().c_str()));\n+\t\t\tsupported_ctrl = false;\n+\t\t\tbreak;\n \t\t}\n \t}\n \n+\tif (!supported_ctrl) {\n+\t\tgst_task_stop(task);\n+\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n+\t\tgoto done;\n+\t}\n+\n+\tstate->initControls_.merge(*self->controls);\n+\n \tret = state->cam_->start(&state->initControls_);\n \tif (ret) {\n \t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n@@ -674,7 +689,8 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n \t\tself->camera_name = g_value_dup_string(value);\n \t\tbreak;\n \tcase PROP_AUTO_FOCUS_MODE:\n-\t\tself->auto_focus_mode = static_cast<controls::AfModeEnum>(g_value_get_enum(value));\n+\t\tself->controls->set(controls::AfMode,\n+\t\t\t\t static_cast<controls::AfModeEnum>(g_value_get_enum(value)));\n \t\tbreak;\n \tdefault:\n \t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n@@ -693,9 +709,11 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n \tcase PROP_CAMERA_NAME:\n \t\tg_value_set_string(value, self->camera_name);\n \t\tbreak;\n-\tcase PROP_AUTO_FOCUS_MODE:\n-\t\tg_value_set_enum(value, static_cast<gint>(self->auto_focus_mode));\n+\tcase PROP_AUTO_FOCUS_MODE: {\n+\t\tauto auto_focus_mode = self->controls->get(controls::AfMode).value_or(controls::AfModeManual);\n+\t\tg_value_set_enum(value, auto_focus_mode);\n \t\tbreak;\n+\t}\n \tdefault:\n \t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n \t\tbreak;\n@@ -760,6 +778,7 @@ gst_libcamera_src_finalize(GObject *object)\n \tg_rec_mutex_clear(&self->stream_lock);\n \tg_clear_object(&self->task);\n \tg_free(self->camera_name);\n+\tdelete self->controls;\n \tdelete self->state;\n \n \treturn klass->finalize(object);\n@@ -783,6 +802,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)\n \t/* C-style friend. */\n \tstate->src_ = self;\n \tself->state = state;\n+\n+\tself->controls = new ControlList(controls::controls, nullptr);\n }\n \n static GstPad *\ndiff --git a/src/gstreamer/gstlibcamerasrc.h b/src/gstreamer/gstlibcamerasrc.h\nindex 0a88ba02..2e6d74cb 100644\n--- a/src/gstreamer/gstlibcamerasrc.h\n+++ b/src/gstreamer/gstlibcamerasrc.h\n@@ -26,17 +26,17 @@ gst_libcamera_auto_focus_get_type()\n \tstatic GType type = 0;\n \tstatic const GEnumValue values[] = {\n \t\t{\n-\t\t\tstatic_cast<gint>(libcamera::controls::AfModeManual),\n+\t\t\tlibcamera::controls::AfModeManual,\n \t\t\t\"AfModeManual\",\n \t\t\t\"manual-focus\",\n \t\t},\n \t\t{\n-\t\t\tstatic_cast<gint>(libcamera::controls::AfModeAuto),\n+\t\t\tlibcamera::controls::AfModeAuto,\n \t\t\t\"AfModeAuto\",\n \t\t\t\"automatic-auto-focus\",\n \t\t},\n \t\t{\n-\t\t\tstatic_cast<gint>(libcamera::controls::AfModeContinuous),\n+\t\t\tlibcamera::controls::AfModeContinuous,\n \t\t\t\"AfModeContinuous\",\n \t\t\t\"continuous-auto-focus\",\n \t\t},\n", "prefixes": [ "libcamera-devel" ] }