From patchwork Wed Apr 23 09:12:08 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 23241 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 5433BC327D for ; Wed, 23 Apr 2025 09:12:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5155968AC5; Wed, 23 Apr 2025 11:12:33 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="N54xd/u0"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 76A6F617E5 for ; Wed, 23 Apr 2025 11:12:31 +0200 (CEST) Received: from neptunite.hamster-moth.ts.net (unknown [IPv6:2404:7a81:160:2100:7551:2625:7c9e:4259]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FA6822A; Wed, 23 Apr 2025 11:12:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1745399550; bh=jSlpadac187sUbrkBHG6AuY4BzbIEFj2lcpTzEPDfBc=; h=From:To:Cc:Subject:Date:From; b=N54xd/u00yZ4KPhjzZ+IfT8ehWmrTzLnu8mEgWj7FG6Fr9kIfJxB1HnaKu8yUvoP0 2KnXq38X+cUFHSgDZ2oKDg9+sEz+rfCGmdG42SCE5MatohSJLb5kraNpSoQvHF736Z O+1EFEg0BZfggBQmQwxqJkh8olrm7iZmVwdfhQgs= From: Paul Elder To: libcamera-devel@lists.libcamera.org Cc: Paul Elder Subject: [PATCH] apps: cam: Try raw role if default viewfinder role fails Date: Wed, 23 Apr 2025 18:12:08 +0900 Message-ID: <20250423091208.2935632-1-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.47.2 MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" cam currently defaults to the viewfinder role when no role is specified. This means that on platforms that only support the raw role (such as a raw sensor with no softISP on a simple pipeline platform), generateConfiguration() would return nullptr and cam would bail out. At least this is what is supposed to happen based on the little documentation that we have written regarding generateConfiguration(), specifically in the application writer's guide, which is probably the most influential piece of documentation: The ``Camera::generateConfiguration()`` function accepts a list of desired roles and generates a ``CameraConfiguration`` with the best stream parameters configuration for each of the requested roles. If the camera can handle the requested roles, it returns an initialized ``CameraConfiguration`` and a null pointer if it can't. Currently the simple pipeline handler will return a raw configuration anyway (if it only supports raw) even if a non-raw role was requested. Thus cam receives a raw configuration instead of a nullptr when no role is specified and viewfinder is requested. However, in the near future, support for raw streams with softISP on the simple pipeline handler will be merged. This will notably change the behavior of the simple pipeline handler to return nullptr if a non-raw role was requested on a platform that only supports raw. This is proper behavior according to documentation, but changes cam's behavior as it used to capture fine with no parameters but will no longer be able to. Technically this is an issue with the roles API, as we are mixing roles in the sense of "configuration hints" (eg. viewfinder vs recording vs still capture) with roles in the sense of "platform capabilities" (raw vs everything else). In the long term the proper solution is to rework the roles API. In the meantime, fix cam so that it will try the raw role if the default viewfinder role returns no configuration. cam is an app that is capable of using the raw stream, so this is appropriate behavior. If roles are specified, then do not retry, as in this situation the user knows what streams they can use and what they want. Signed-off-by: Paul Elder Reviewed-by: Kieran Bingham --- src/apps/cam/camera_session.cpp | 29 +++++++++++++++++++++++++---- src/apps/common/stream_options.cpp | 3 +-- src/apps/qcam/main_window.cpp | 3 +++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp index 97c1ae44995e..f63fcb228519 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -62,11 +62,32 @@ CameraSession::CameraSession(CameraManager *cm, return; } - std::vector roles = StreamKeyValueParser::roles(options_[OptStream]); + std::vector roles = + StreamKeyValueParser::roles(options_[OptStream]); + std::vector> tryRoles; + if (!roles.empty()) { + /* + * If the roles are explicitly specified then there's no need + * to try other roles + */ + tryRoles.push_back(roles); + } else { + tryRoles.push_back({ StreamRole::Viewfinder }); + tryRoles.push_back({ StreamRole::Raw }); + } + + std::unique_ptr config; + bool valid = false; + for (std::vector &rolesIt : tryRoles) { + config = camera_->generateConfiguration(rolesIt); + if (config && config->size() == rolesIt.size()) { + roles = rolesIt; + valid = true; + break; + } + } - std::unique_ptr config = - camera_->generateConfiguration(roles); - if (!config || config->size() != roles.size()) { + if (!valid) { std::cerr << "Failed to get default stream configuration" << std::endl; return; diff --git a/src/apps/common/stream_options.cpp b/src/apps/common/stream_options.cpp index 99239e07e302..288f86530351 100644 --- a/src/apps/common/stream_options.cpp +++ b/src/apps/common/stream_options.cpp @@ -42,9 +42,8 @@ KeyValueParser::Options StreamKeyValueParser::parse(const char *arguments) std::vector StreamKeyValueParser::roles(const OptionValue &values) { - /* If no configuration values to examine default to viewfinder. */ if (values.empty()) - return { StreamRole::Viewfinder }; + return {}; const std::vector &streamParameters = values.toArray(); diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp index d2ccbd2318fa..224a7e5a693a 100644 --- a/src/apps/qcam/main_window.cpp +++ b/src/apps/qcam/main_window.cpp @@ -356,6 +356,9 @@ int MainWindow::startCapture() /* Verify roles are supported. */ switch (roles.size()) { + case 0: + roles[0] = StreamRole::Viewfinder; + break; case 1: if (roles[0] != StreamRole::Viewfinder) { qWarning() << "Only viewfinder supported for single stream";