apps: cam: Try raw role if default viewfinder role fails

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 <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

3 files changed, 29 insertions, 6 deletions

diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp
index 97c1ae44..f63fcb22 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<StreamRole> roles = StreamKeyValueParser::roles(options_[OptStream]);
+	std::vector<StreamRole> roles =
+		StreamKeyValueParser::roles(options_[OptStream]);
+	std::vector<std::vector<StreamRole>> 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<CameraConfiguration> config;
+	bool valid = false;
+	for (std::vector<StreamRole> &rolesIt : tryRoles) {
+		config = camera_->generateConfiguration(rolesIt);
+		if (config && config->size() == rolesIt.size()) {
+			roles = rolesIt;
+			valid = true;
+			break;
+		}
+	}
 
-	std::unique_ptr<CameraConfiguration> 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 99239e07..288f8653 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<StreamRole> StreamKeyValueParser::roles(const OptionValue &values)
 {
-	/* If no configuration values to examine default to viewfinder. */
 	if (values.empty())
-		return { StreamRole::Viewfinder };
+		return {};
 
 	const std::vector<OptionValue> &streamParameters = values.toArray();
 
diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
index d2ccbd23..224a7e5a 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";