[libcamera-devel] [RFC PATCH v3 2/2] libcamera: Remove `StreamRoles` alias

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Tue, May 09, 2023 at 11:15:43PM +0000, Barnabás Pőcze via libcamera-devel wrote:
> Now that `Camera::generateConfiguration()` takes a `libcamera::Span`
> of `StreamRole`, remove the `StreamRoles` type, which was an alias
> to `std::vector<libcamera::StreamRole>`.
> 
> The removal has two reasons:
>  - it is no longer strictly necessary,
>  - its presence may suggest that that is the preferred (or correct)
>    way to build/pass a list of `StreamRole`.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/stream.h             | 2 --
>  src/apps/cam/camera_session.cpp        | 2 +-
>  src/apps/common/stream_options.cpp     | 4 ++--
>  src/apps/common/stream_options.h       | 2 +-
>  src/apps/qcam/main_window.cpp          | 2 +-
>  src/gstreamer/gstlibcameraprovider.cpp | 5 +++--
>  src/gstreamer/gstlibcamerasrc.cpp      | 2 +-
>  src/libcamera/stream.cpp               | 5 -----
>  8 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 29235ddf..4e94187d 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -69,8 +69,6 @@ enum class StreamRole {
>  	Viewfinder,
>  };
>  
> -using StreamRoles = std::vector<StreamRole>;
> -
>  std::ostream &operator<<(std::ostream &out, StreamRole role);
>  
>  class Stream
> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp
> index 8fcec630..066e397b 100644
> --- a/src/apps/cam/camera_session.cpp
> +++ b/src/apps/cam/camera_session.cpp
> @@ -55,7 +55,7 @@ CameraSession::CameraSession(CameraManager *cm,
>  		return;
>  	}
>  
> -	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> +	std::vector<StreamRole> roles = StreamKeyValueParser::roles(options_[OptStream]);
>  
>  	std::unique_ptr<CameraConfiguration> config =
>  		camera_->generateConfiguration(roles);
> diff --git a/src/apps/common/stream_options.cpp b/src/apps/common/stream_options.cpp
> index 19dfe051..663b979a 100644
> --- a/src/apps/common/stream_options.cpp
> +++ b/src/apps/common/stream_options.cpp
> @@ -40,7 +40,7 @@ KeyValueParser::Options StreamKeyValueParser::parse(const char *arguments)
>  	return options;
>  }
>  
> -StreamRoles StreamKeyValueParser::roles(const OptionValue &values)
> +std::vector<StreamRole> StreamKeyValueParser::roles(const OptionValue &values)
>  {
>  	/* If no configuration values to examine default to viewfinder. */
>  	if (values.empty())
> @@ -48,7 +48,7 @@ StreamRoles StreamKeyValueParser::roles(const OptionValue &values)
>  
>  	const std::vector<OptionValue> &streamParameters = values.toArray();
>  
> -	StreamRoles roles;
> +	std::vector<StreamRole> roles;
>  	for (auto const &value : streamParameters) {
>  		/* If a role is invalid default it to viewfinder. */
>  		roles.push_back(parseRole(value.toKeyValues()).value_or(StreamRole::Viewfinder));
> diff --git a/src/apps/common/stream_options.h b/src/apps/common/stream_options.h
> index fe298c84..a5f3bde0 100644
> --- a/src/apps/common/stream_options.h
> +++ b/src/apps/common/stream_options.h
> @@ -20,7 +20,7 @@ public:
>  
>  	KeyValueParser::Options parse(const char *arguments) override;
>  
> -	static libcamera::StreamRoles roles(const OptionValue &values);
> +	static std::vector<libcamera::StreamRole> roles(const OptionValue &values);
>  	static int updateConfiguration(libcamera::CameraConfiguration *config,
>  				       const OptionValue &values);
>  
> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> index 680668df..0f16c038 100644
> --- a/src/apps/qcam/main_window.cpp
> +++ b/src/apps/qcam/main_window.cpp
> @@ -362,7 +362,7 @@ void MainWindow::toggleCapture(bool start)
>   */
>  int MainWindow::startCapture()
>  {
> -	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> +	std::vector<StreamRole> roles = StreamKeyValueParser::roles(options_[OptStream]);
>  	int ret;
>  
>  	/* Verify roles are supported. */
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index 6eb0a0eb..494f778b 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -6,6 +6,8 @@
>   * gstlibcameraprovider.c - GStreamer Device Provider
>   */
>  
> +#include <array>
> +
>  #include "gstlibcameraprovider.h"
>  
>  #include <libcamera/camera.h>
> @@ -126,11 +128,10 @@ gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass)
>  static GstDevice *
>  gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>  {
> +	static const std::array roles { StreamRole::VideoRecording };
>  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
>  	const gchar *name = camera->id().c_str();
> -	StreamRoles roles;
>  
> -	roles.push_back(StreamRole::VideoRecording);
>  	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
>  	if (!config || config->size() != roles.size()) {
>  		GST_ERROR("Failed to generate a default configuration for %s", name);
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index a10cbd4f..46a5400e 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -468,7 +468,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
>  	GST_DEBUG_OBJECT(self, "Streaming thread has started");
>  
>  	gint stream_id_num = 0;
> -	StreamRoles roles;
> +	std::vector<StreamRole> roles;
>  	for (GstPad *srcpad : state->srcpads_) {
>  		/* Create stream-id and push stream-start. */
>  		g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i", state->group_id_, stream_id_num++);
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 67f30815..272222b7 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -436,11 +436,6 @@ std::ostream &operator<<(std::ostream &out, StreamRole role)
>  	return out;
>  }
>  
> -/**
> - * \typedef StreamRoles
> - * \brief A vector of StreamRole
> - */
> -
>  /**
>   * \class Stream
>   * \brief Video stream for a camera

-- 
Regards,

Laurent Pinchart