From patchwork Wed Nov 23 13:42:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 17845 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 B9473BE08B for ; Wed, 23 Nov 2022 13:42:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 776A36331A; Wed, 23 Nov 2022 14:42:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669210957; bh=SPasf+UFSiUL0pEODjtq2GNuvLMqcOphL5md3BKO8B4=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=s77WSdiA/9L9u+pEzrgX4nM3PJ06HGMbAaHtT5/fHeFO8vAkhSH+szZaIzQOBpuuW KtJcLtQNV2/mWxBJ2N6zMovCKlgB5T8lkteQI5NvtpWcKvFHTJMqDRvv4D16nuMeyl A0ea4lD9XXB1rWuwCKur48lKWBrFMuxYSleARzUraUqgvzJqcOuSMfosc2MNA+CNHW FE2NsSo+9os76oslZhrQY5kX4Sqhs0OBw4rcx0iJLhZ7BetrEOCNGpgfWhGifp8oa9 iTzs6v1qkmpLCM7tpg9+7XmmLq+i/CRcwOj9Chyakf8t2qJsRXjQ1D1MxyH9Ww6NaG 8zNh4MeEc0/oQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC06A63311 for ; Wed, 23 Nov 2022 14:42:36 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AlMGORfY"; dkim-atps=neutral Received: from Monstersaurus.local (cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 227D8890; Wed, 23 Nov 2022 14:42:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1669210956; bh=SPasf+UFSiUL0pEODjtq2GNuvLMqcOphL5md3BKO8B4=; h=From:To:Cc:Subject:Date:From; b=AlMGORfYY6agx8ikF0vT2NYlLo2lVkan/KLNjwqB8p9i9Soml7Ga5hfwwqAKDET22 HIctnY29iiH978qk3IOSxWbn7JFwErXTKDryvLbN/GH+zfO56CUtBQ0AHJDkNHcLit 6K6e+n6gR4PszOAdy8VfwB8wsB2KN2zTcf/KmxRU= To: libcamera devel Date: Wed, 23 Nov 2022 13:42:32 +0000 Message-Id: <20221123134232.1937002-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3] libcamera: v4l2_device: Workaround faulty control menus 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: , X-Patchwork-Original-From: Kieran Bingham via libcamera-devel From: Kieran Bingham Reply-To: Kieran Bingham Cc: Marian Buschsieweke Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Some UVC cameras have been identified that can provide V4L2 menu controls without any menu items. This leads to a segfault where we try to construct a ControlInfo(Span<>,default) with an empty span. Convert the v4l2ControlInfo and v4l2MenuControlInfo helper functions to return std::optional to be able to account in the caller if the control is valid, and only add acceptable controls to the supported control list. Menu controls without a list of menu items are no longer added as a valid control and a warning is logged. This also fixes a potential crash that would have occured in the unlikely event that a ctrl.minimum was set to less than 0. Bug: https://bugs.libcamera.org/show_bug.cgi?id=167 Reported-by: Marian Buschsieweke Signed-off-by: Kieran Bingham Reviewed-by: Jacopo Mondi --- include/libcamera/internal/v4l2_device.h | 4 ++-- src/libcamera/v4l2_device.cpp | 23 ++++++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index 75304be11f77..50d4adbc5f2b 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -70,8 +70,8 @@ protected: private: static ControlType v4l2CtrlType(uint32_t ctrlType); static std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl); - ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl); - ControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl); + std::optional v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl); + std::optional v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl); void listControls(); void updateControls(ControlList *ctrls, diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index c17b323f8af0..c2e21bdf33b7 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -529,7 +529,7 @@ std::unique_ptr V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl & * \param[in] ctrl The v4l2_query_ext_ctrl that represents a V4L2 control * \return A ControlInfo that represents \a ctrl */ -ControlInfo V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl) +std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl) { switch (ctrl.type) { case V4L2_CTRL_TYPE_U8: @@ -566,14 +566,14 @@ ControlInfo V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl) * * \return A ControlInfo that represents \a ctrl */ -ControlInfo V4L2Device::v4l2MenuControlInfo(const struct v4l2_query_ext_ctrl &ctrl) +std::optional V4L2Device::v4l2MenuControlInfo(const struct v4l2_query_ext_ctrl &ctrl) { std::vector indices; struct v4l2_querymenu menu = {}; menu.id = ctrl.id; if (ctrl.minimum < 0) - return ControlInfo(); + return std::nullopt; for (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) { menu.index = index; @@ -583,6 +583,17 @@ ControlInfo V4L2Device::v4l2MenuControlInfo(const struct v4l2_query_ext_ctrl &ct indices.push_back(index); } + /* + * Some faulty UVC drivers are known to return an empty menu control. + * Controls without a menu option can not be set, or read, so they are + * not exposed. + */ + if (indices.size() == 0) { + LOG(V4L2, Warning) + << "Menu control '" << ctrl.name << "' has no entries"; + return std::nullopt; + } + return ControlInfo(indices, ControlValue(static_cast(ctrl.default_value))); } @@ -631,7 +642,9 @@ void V4L2Device::listControls() controlIdMap_[ctrl.id] = controlIds_.back().get(); controlInfo_.emplace(ctrl.id, ctrl); - ctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl)); + std::optional info = v4l2ControlInfo(ctrl); + if (info) + ctrls.emplace(controlIds_.back().get(), *info); } controls_ = ControlInfoMap(std::move(ctrls), controlIdMap_); @@ -670,7 +683,7 @@ void V4L2Device::updateControlInfo() continue; } - info = v4l2ControlInfo(ctrl); + info = *v4l2ControlInfo(ctrl); } }