[libcamera-devel] [RFC PATCH v3 2/2] libcamera: Remove `StreamRoles` alias
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue May 16 01:09:48 CEST 2023
Quoting Barnabás Pőcze via libcamera-devel (2023-05-10 00:15:43)
> 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>
> ---
> 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>;
> -
I was curious how we might handle this moving forwards if we need to
remove things. (I think removing this is the right thing to do, so this
is just 'how' we remove it)
I wonder if we should have an include/libcamera/deprecated.h to place
things like:
using StreamRoles [[deprecated("Use a span, array or vector directly")]]
= std::vector<StreamRole>;
Perhaps even with a reference to something that makes it clearer what to
do to migrate with the issue.
StreamRoles has been in all the examples so far, so I think all apps
that use libcamera are probably already using this.
I know we explicitly don't have ABI/API stability - but if we break
things perhaps it would be helpful to have a system that lets us inform
the user what they need to do next to fix it again.
This probably gets quite relevant to how we handle things with:
https://patchwork.libcamera.org/project/libcamera/list/?series=3877 so
any thoughts on that series are welcome!
> 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
> --
> 2.40.1
>
>
More information about the libcamera-devel
mailing list