| Message ID | 20190519150047.12444-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sun, May 19, 2019 at 06:00:43PM +0300, Laurent Pinchart wrote: > In order to prepare for an API overhall of the camera configuration > generation, remove the StreamUsage class and replace its uses by stream > roles. The size hints can't be specified anymore, and will be replaced > with an API on the StreamConfiguration to negotiate configuration > parameters with cameras. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/camera.h | 8 +-- > include/libcamera/stream.h | 44 ++---------- > src/cam/main.cpp | 13 ++-- > src/libcamera/camera.cpp | 16 ++--- > src/libcamera/include/pipeline_handler.h | 6 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 28 ++++---- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- > src/libcamera/pipeline/uvcvideo.cpp | 5 +- > src/libcamera/pipeline/vimc.cpp | 5 +- > src/libcamera/pipeline_handler.cpp | 10 +-- > src/libcamera/stream.cpp | 89 ++++-------------------- > src/qcam/main_window.cpp | 2 +- > test/camera/capture.cpp | 2 +- > test/camera/configuration_default.cpp | 6 +- > test/camera/configuration_set.cpp | 2 +- > test/camera/statemachine.cpp | 2 +- > 16 files changed, 69 insertions(+), 173 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 306739b7014a..42ba5201eabc 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -14,16 +14,13 @@ > > #include <libcamera/request.h> > #include <libcamera/signal.h> > +#include <libcamera/stream.h> > > namespace libcamera { > > class Buffer; > class PipelineHandler; > class Request; > -class Stream; > -class StreamUsage; I guess this would just need a StreamUsage/StreamRoles but I'm fine with dropping forward declarations and include stream.h > - > -struct StreamConfiguration; > > class CameraConfiguration > { > @@ -74,8 +71,7 @@ public: > int release(); > > const std::set<Stream *> &streams() const; > - CameraConfiguration > - generateConfiguration(const std::vector<StreamUsage> &usage); > + CameraConfiguration generateConfiguration(const StreamRoles &roles); > int configure(const CameraConfiguration &config); > > int allocateBuffers(); > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > index 0c986310939b..59bdf217eb31 100644 > --- a/include/libcamera/stream.h > +++ b/include/libcamera/stream.h > @@ -8,6 +8,7 @@ > #define __LIBCAMERA_STREAM_H__ > > #include <string> > +#include <vector> > > #include <libcamera/buffer.h> > #include <libcamera/geometry.h> > @@ -25,48 +26,17 @@ struct StreamConfiguration { > std::string toString() const; > }; > > -class StreamUsage > -{ > -public: > - enum Role { > - StillCapture, > - VideoRecording, > - Viewfinder, > - }; > - > - Role role() const { return role_; } > - const Size &size() const { return size_; } > - > -protected: > - explicit StreamUsage(Role role); > - StreamUsage(Role role, const Size &size); > - > -private: > - Role role_; > - Size size_; > +enum StreamRole { > + StillCapture, > + VideoRecording, > + Viewfinder, > }; > > +using StreamRoles = std::vector<StreamRole>; > + This is indeed nicer to write, it's true that for a user of it, dealing with an std::vector directly would make it clear which API to use without having to look at the type def here. > class Stream > { > public: > - class StillCapture : public StreamUsage > - { > - public: > - StillCapture(); > - }; > - > - class VideoRecording : public StreamUsage > - { > - public: > - VideoRecording(); > - }; > - > - class Viewfinder : public StreamUsage > - { > - public: > - Viewfinder(int width, int height); > - }; > - > Stream(); > BufferPool &bufferPool() { return bufferPool_; } > const StreamConfiguration &configuration() const { return configuration_; } > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 6a2508dd3bd9..d603228c0116 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -87,13 +87,13 @@ static int parseOptions(int argc, char *argv[]) > > static int prepareCameraConfig(CameraConfiguration *config) > { > - std::vector<StreamUsage> roles; > + StreamRoles roles; > > streamInfo.clear(); > > /* If no configuration is provided assume a single video stream. */ > if (!options.isSet(OptStream)) { > - *config = camera->generateConfiguration({ Stream::VideoRecording() }); > + *config = camera->generateConfiguration({ StreamRole::VideoRecording }); > streamInfo[config->front()] = "stream0"; > return 0; > } > @@ -106,14 +106,13 @@ static int prepareCameraConfig(CameraConfiguration *config) > KeyValueParser::Options conf = value.toKeyValues(); > > if (!conf.isSet("role")) { > - roles.push_back(Stream::VideoRecording()); > + roles.push_back(StreamRole::VideoRecording); > } else if (conf["role"].toString() == "viewfinder") { > - roles.push_back(Stream::Viewfinder(conf["width"], > - conf["height"])); > + roles.push_back(StreamRole::Viewfinder); > } else if (conf["role"].toString() == "video") { > - roles.push_back(Stream::VideoRecording()); > + roles.push_back(StreamRole::VideoRecording); > } else if (conf["role"].toString() == "still") { > - roles.push_back(Stream::StillCapture()); > + roles.push_back(StreamRole::StillCapture); > } else { > std::cerr << "Unknown stream role " > << conf["role"].toString() << std::endl; > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 359174a41823..a3921f91f1c9 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -542,23 +542,23 @@ const std::set<Stream *> &Camera::streams() const > } > > /** > - * \brief Generate a default camera configuration according to stream usages > - * \param[in] usages A list of stream usages > + * \brief Generate a default camera configuration according to stream roles > + * \param[in] roles A list of stream roles > * > - * Generate a camera configuration for a set of desired usages. The caller > - * specifies a list of stream usages and the camera returns a configuration > + * Generate a camera configuration for a set of desired stream roles. The caller > + * specifies a list of stream roles and the camera returns a configuration > * containing suitable streams and their suggested default configurations. > * > - * \return A valid CameraConfiguration if the requested usages can be satisfied, > + * \return A valid CameraConfiguration if the requested roles can be satisfied, > * or a invalid one otherwise > */ > CameraConfiguration > -Camera::generateConfiguration(const std::vector<StreamUsage> &usages) > +Camera::generateConfiguration(const StreamRoles &roles) > { > - if (disconnected_ || !usages.size() || usages.size() > streams_.size()) > + if (disconnected_ || !roles.size() || roles.size() > streams_.size()) > return CameraConfiguration(); > > - CameraConfiguration config = pipe_->generateConfiguration(this, usages); > + CameraConfiguration config = pipe_->generateConfiguration(this, roles); > > std::ostringstream msg("streams configuration:", std::ios_base::ate); > unsigned int index = 0; > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index 9cc11a8e192e..3352cb0e5bc9 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -14,6 +14,8 @@ > #include <string> > #include <vector> > > +#include <libcamera/stream.h> > + > namespace libcamera { > > class Buffer; > @@ -26,8 +28,6 @@ class DeviceMatch; > class MediaDevice; > class PipelineHandler; > class Request; > -class Stream; > -class StreamUsage; Same comment as above. Or would you need to forward declare both StreamRoles and StreamRole? > > class CameraData > { > @@ -61,7 +61,7 @@ public: > void unlock(); > > virtual CameraConfiguration > - generateConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0; > + generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; > virtual int configure(Camera *camera, const CameraConfiguration &config) = 0; > > virtual int allocateBuffers(Camera *camera, > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index ba0c708f9e1e..d234a8ac5289 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -151,8 +151,7 @@ public: > PipelineHandlerIPU3(CameraManager *manager); > > CameraConfiguration > - generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) override; > + generateConfiguration(Camera *camera, const StreamRoles &roles) override; > int configure(Camera *camera, > const CameraConfiguration &config) override; > > @@ -211,7 +210,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) > > CameraConfiguration > PipelineHandlerIPU3::generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) > + const StreamRoles &roles) > { > IPU3CameraData *data = cameraData(camera); > CameraConfiguration config = {}; > @@ -220,13 +219,12 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > &data->vfStream_, > }; > > - for (const StreamUsage &usage : usages) { > + for (const StreamRole role : roles) { > StreamConfiguration cfg = {}; > - StreamUsage::Role role = usage.role(); > IPU3Stream *stream = nullptr; > > switch (role) { > - case StreamUsage::Role::StillCapture: > + case StreamRole::StillCapture: > /* > * Pick the output stream by default as the Viewfinder > * and VideoRecording roles are not allowed on > @@ -256,11 +254,11 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > break; > > - case StreamUsage::Role::Viewfinder: > - case StreamUsage::Role::VideoRecording: { > + case StreamRole::Viewfinder: > + case StreamRole::VideoRecording: { > /* > * We can't use the 'output' stream for viewfinder or > - * video capture usages. > + * video capture roles. > * > * \todo This is an artificial limitation until we > * figure out the exact capabilities of the hardware. > @@ -275,15 +273,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > stream = &data->vfStream_; > > /* > - * Align the requested viewfinder size to the > - * maximum available sensor resolution and to the > - * IPU3 alignment constraints. > + * Align the default viewfinder size to the maximum > + * available sensor resolution and to the IPU3 > + * alignment constraints. > */ > const Size &res = data->cio2_.sensor_->resolution(); > - unsigned int width = std::min(usage.size().width, > - res.width); > - unsigned int height = std::min(usage.size().height, > - res.height); > + unsigned int width = std::min(1280U, res.width); > + unsigned int height = std::min(720U, res.height); Why 1280x720 ? > cfg.size = { width & ~7, height & ~3 }; > > break; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4d02f9604ad9..4bd8c5101a96 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -35,7 +35,7 @@ public: > ~PipelineHandlerRkISP1(); > > CameraConfiguration generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) override; > + const StreamRoles &roles) override; > int configure(Camera *camera, > const CameraConfiguration &config) override; > > @@ -107,7 +107,7 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() > */ > > CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) > + const StreamRoles &roles) > { > RkISP1CameraData *data = cameraData(camera); > CameraConfiguration config; > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 118b97457d2a..d2e1f7d4e5b2 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -26,8 +26,7 @@ public: > PipelineHandlerUVC(CameraManager *manager); > > CameraConfiguration > - generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) override; > + generateConfiguration(Camera *camera, const StreamRoles &roles) override; > int configure(Camera *camera, > const CameraConfiguration &config) override; > > @@ -77,7 +76,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) > > CameraConfiguration > PipelineHandlerUVC::generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) > + const StreamRoles &roles) > { > UVCCameraData *data = cameraData(camera); > CameraConfiguration config; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 74959581a7ef..17e2491e5c27 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -26,8 +26,7 @@ public: > PipelineHandlerVimc(CameraManager *manager); > > CameraConfiguration > - generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) override; > + generateConfiguration(Camera *camera, const StreamRoles &roles) override; > int configure(Camera *camera, > const CameraConfiguration &config) override; > > @@ -77,7 +76,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) > > CameraConfiguration > PipelineHandlerVimc::generateConfiguration(Camera *camera, > - const std::vector<StreamUsage> &usages) > + const StreamRoles &roles) > { > VimcCameraData *data = cameraData(camera); > CameraConfiguration config; > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index b9ac64328f1d..81c11149c9fe 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -221,18 +221,18 @@ void PipelineHandler::unlock() > * \fn PipelineHandler::generateConfiguration() > * \brief Generate a camera configuration for a specified camera > * \param[in] camera The camera to generate a default configuration for > - * \param[in] usages A list of stream usages > + * \param[in] roles A list of stream roles > * > - * Generate a default configuration for the \a camera for a specified group of > - * use-cases. The caller shall populate the \a usages array with the use-cases > - * it wishes to fetch the default configuration for. The returned configuration > + * Generate a default configuration for the \a camera for a specified list of > + * stream roles. The caller shall populate the \a roles with the use-cases it > + * wishes to fetch the default configuration for. The returned configuration > * can then be examined by the caller to learn about the selected streams and > * their default parameters. > * > * The intended companion to this is \a configure() which can be used to change > * the group of streams parameters. > * > - * \return A valid CameraConfiguration if the requested usages can be satisfied, > + * \return A valid CameraConfiguration if the requested roles can be satisfied, > * or a invalid configuration otherwise > */ > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index af259510b19c..fe4c4ecf4150 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -75,61 +75,31 @@ std::string StreamConfiguration::toString() const > } > > /** > - * \class StreamUsage > - * \brief Stream usage information > - * > - * The StreamUsage class describes how an application intends to use a stream. > - * Usages are specified by applications and passed to cameras, that then select > - * the most appropriate streams and their default configurations. > - */ > - > -/** > - * \enum StreamUsage::Role > + * \enum StreamRole > * \brief Identify the role a stream is intended to play > - * \var StreamUsage::StillCapture > + * > + * The StreamRole describes how an application intends to use a stream. Roles > + * are specified by applications and passed to cameras, that then select the s/select/selects ? Minors apart, and the resolution selected on IPU3 clarified: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > + * most appropriate streams and their default configurations. > + * > + * \var StillCapture > * The stream is intended to capture high-resolution, high-quality still images > * with low frame rate. The captured frames may be exposed with flash. > - * \var StreamUsage::VideoRecording > + * \var VideoRecording > * The stream is intended to capture video for the purpose of recording or > * streaming. The video stream may produce a high frame rate and may be > * enhanced with video stabilization. > - * \var StreamUsage::Viewfinder > + * \var Viewfinder > * The stream is intended to capture video for the purpose of display on the > - * local screen. The StreamUsage includes the desired resolution. Trade-offs > - * between quality and usage of system resources are acceptable. > + * local screen. Trade-offs between quality and usage of system resources are > + * acceptable. > */ > > /** > - * \fn StreamUsage::role() > - * \brief Retrieve the stream role > - * \return The stream role > + * \typedef StreamRoles > + * \brief A vector of StreamRole > */ > > -/** > - * \fn StreamUsage::size() > - * \brief Retrieve desired size > - * \return The desired size > - */ > - > -/** > - * \brief Create a stream usage > - * \param[in] role Stream role > - */ > -StreamUsage::StreamUsage(Role role) > - : role_(role) > -{ > -} > - > -/** > - * \brief Create a stream usage with a desired size > - * \param[in] role Stream role > - * \param[in] size The desired size > - */ > -StreamUsage::StreamUsage(Role role, const Size &size) > - : role_(role), size_(size) > -{ > -} > - > /** > * \class Stream > * \brief Video stream for a camera > @@ -148,39 +118,6 @@ StreamUsage::StreamUsage(Role role, const Size &size) > * optimal stream for the task. > */ > > -/** > - * \class Stream::StillCapture > - * \brief Describe a still capture usage > - */ > -Stream::StillCapture::StillCapture() > - : StreamUsage(Role::StillCapture) > -{ > -} > - > -/** > - * \class Stream::VideoRecording > - * \brief Describe a video recording usage > - */ > -Stream::VideoRecording::VideoRecording() > - : StreamUsage(Role::VideoRecording) > -{ > -} > - > -/** > - * \class Stream::Viewfinder > - * \brief Describe a viewfinder usage > - */ > - > -/** > - * \brief Create a viewfinder usage with a desired dimension > - * \param[in] width The desired viewfinder width > - * \param[in] height The desired viewfinder height > - */ > -Stream::Viewfinder::Viewfinder(int width, int height) > - : StreamUsage(Role::Viewfinder, Size(width, height)) > -{ > -} > - > /** > * \brief Construct a stream with default parameters > */ > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index c91b82727ec6..a984aaca764f 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -97,7 +97,7 @@ int MainWindow::startCapture() > { > int ret; > > - config_ = camera_->generateConfiguration({ Stream::VideoRecording() }); > + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); > Stream *stream = config_.front(); > ret = camera_->configure(config_); > if (ret < 0) { > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp > index bc3a4d6cb9f2..e7e6438203b9 100644 > --- a/test/camera/capture.cpp > +++ b/test/camera/capture.cpp > @@ -43,7 +43,7 @@ protected: > int run() > { > CameraConfiguration config = > - camera_->generateConfiguration({ Stream::VideoRecording() }); > + camera_->generateConfiguration({ StreamRole::VideoRecording }); > Stream *stream = config.front(); > StreamConfiguration *cfg = &config[stream]; > > diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp > index 340b5f58f04c..8c4a03db498a 100644 > --- a/test/camera/configuration_default.cpp > +++ b/test/camera/configuration_default.cpp > @@ -21,7 +21,7 @@ protected: > CameraConfiguration config; > > /* Test asking for configuration for a video stream. */ > - config = camera_->generateConfiguration({ Stream::VideoRecording() }); > + config = camera_->generateConfiguration({ StreamRole::VideoRecording }); > if (!config.isValid()) { > cout << "Default configuration invalid" << endl; > return TestFail; > @@ -29,11 +29,11 @@ protected: > > /* > * Test that asking for configuration for an empty array of > - * stream usages returns an empty list of configurations. > + * stream roles returns an empty list of configurations. > */ > config = camera_->generateConfiguration({}); > if (config.isValid()) { > - cout << "Failed to retrieve configuration for empty usage list" > + cout << "Failed to retrieve configuration for empty roles list" > << endl; > return TestFail; > } > diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp > index 24d5ca6690b7..76d8bc3e40a4 100644 > --- a/test/camera/configuration_set.cpp > +++ b/test/camera/configuration_set.cpp > @@ -19,7 +19,7 @@ protected: > int run() > { > CameraConfiguration config = > - camera_->generateConfiguration({ Stream::VideoRecording() }); > + camera_->generateConfiguration({ StreamRole::VideoRecording }); > StreamConfiguration *cfg = &config[config.front()]; > > if (!config.isValid()) { > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp > index bd2e61ff2939..7a74cd85a37a 100644 > --- a/test/camera/statemachine.cpp > +++ b/test/camera/statemachine.cpp > @@ -235,7 +235,7 @@ protected: > > int run() > { > - defconf_ = camera_->generateConfiguration({ Stream::VideoRecording() }); > + defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); > > if (testAvailable() != TestPass) { > cout << "State machine in Available state failed" << endl; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Tue, May 21, 2019 at 12:12:06PM +0200, Jacopo Mondi wrote: > On Sun, May 19, 2019 at 06:00:43PM +0300, Laurent Pinchart wrote: > > In order to prepare for an API overhall of the camera configuration > > generation, remove the StreamUsage class and replace its uses by stream > > roles. The size hints can't be specified anymore, and will be replaced > > with an API on the StreamConfiguration to negotiate configuration > > parameters with cameras. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > include/libcamera/camera.h | 8 +-- > > include/libcamera/stream.h | 44 ++---------- > > src/cam/main.cpp | 13 ++-- > > src/libcamera/camera.cpp | 16 ++--- > > src/libcamera/include/pipeline_handler.h | 6 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 28 ++++---- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- > > src/libcamera/pipeline/uvcvideo.cpp | 5 +- > > src/libcamera/pipeline/vimc.cpp | 5 +- > > src/libcamera/pipeline_handler.cpp | 10 +-- > > src/libcamera/stream.cpp | 89 ++++-------------------- > > src/qcam/main_window.cpp | 2 +- > > test/camera/capture.cpp | 2 +- > > test/camera/configuration_default.cpp | 6 +- > > test/camera/configuration_set.cpp | 2 +- > > test/camera/statemachine.cpp | 2 +- > > 16 files changed, 69 insertions(+), 173 deletions(-) > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index 306739b7014a..42ba5201eabc 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -14,16 +14,13 @@ > > > > #include <libcamera/request.h> > > #include <libcamera/signal.h> > > +#include <libcamera/stream.h> > > > > namespace libcamera { > > > > class Buffer; > > class PipelineHandler; > > class Request; > > -class Stream; > > -class StreamUsage; > > I guess this would just need a StreamUsage/StreamRoles but I'm fine > with dropping forward declarations and include stream.h StreamRoles is defined as using StreamRoles = std::vector<StreamRole>; so I would have to duplicate that for the forward-declaration, which wouldn't be nice. > > - > > -struct StreamConfiguration; > > > > class CameraConfiguration > > { > > @@ -74,8 +71,7 @@ public: > > int release(); > > > > const std::set<Stream *> &streams() const; > > - CameraConfiguration > > - generateConfiguration(const std::vector<StreamUsage> &usage); > > + CameraConfiguration generateConfiguration(const StreamRoles &roles); > > int configure(const CameraConfiguration &config); > > > > int allocateBuffers(); > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > > index 0c986310939b..59bdf217eb31 100644 > > --- a/include/libcamera/stream.h > > +++ b/include/libcamera/stream.h > > @@ -8,6 +8,7 @@ > > #define __LIBCAMERA_STREAM_H__ > > > > #include <string> > > +#include <vector> > > > > #include <libcamera/buffer.h> > > #include <libcamera/geometry.h> > > @@ -25,48 +26,17 @@ struct StreamConfiguration { > > std::string toString() const; > > }; > > > > -class StreamUsage > > -{ > > -public: > > - enum Role { > > - StillCapture, > > - VideoRecording, > > - Viewfinder, > > - }; > > - > > - Role role() const { return role_; } > > - const Size &size() const { return size_; } > > - > > -protected: > > - explicit StreamUsage(Role role); > > - StreamUsage(Role role, const Size &size); > > - > > -private: > > - Role role_; > > - Size size_; > > +enum StreamRole { > > + StillCapture, > > + VideoRecording, > > + Viewfinder, > > }; > > > > +using StreamRoles = std::vector<StreamRole>; > > + > > This is indeed nicer to write, it's true that for a user of it, > dealing with an std::vector directly would make it clear which API to > use without having to look at the type def here. I was a bit in two minds, but I decided to go for simplicity. > > class Stream > > { > > public: > > - class StillCapture : public StreamUsage > > - { > > - public: > > - StillCapture(); > > - }; > > - > > - class VideoRecording : public StreamUsage > > - { > > - public: > > - VideoRecording(); > > - }; > > - > > - class Viewfinder : public StreamUsage > > - { > > - public: > > - Viewfinder(int width, int height); > > - }; > > - > > Stream(); > > BufferPool &bufferPool() { return bufferPool_; } > > const StreamConfiguration &configuration() const { return configuration_; } > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index 6a2508dd3bd9..d603228c0116 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -87,13 +87,13 @@ static int parseOptions(int argc, char *argv[]) > > > > static int prepareCameraConfig(CameraConfiguration *config) > > { > > - std::vector<StreamUsage> roles; > > + StreamRoles roles; > > > > streamInfo.clear(); > > > > /* If no configuration is provided assume a single video stream. */ > > if (!options.isSet(OptStream)) { > > - *config = camera->generateConfiguration({ Stream::VideoRecording() }); > > + *config = camera->generateConfiguration({ StreamRole::VideoRecording }); > > streamInfo[config->front()] = "stream0"; > > return 0; > > } > > @@ -106,14 +106,13 @@ static int prepareCameraConfig(CameraConfiguration *config) > > KeyValueParser::Options conf = value.toKeyValues(); > > > > if (!conf.isSet("role")) { > > - roles.push_back(Stream::VideoRecording()); > > + roles.push_back(StreamRole::VideoRecording); > > } else if (conf["role"].toString() == "viewfinder") { > > - roles.push_back(Stream::Viewfinder(conf["width"], > > - conf["height"])); > > + roles.push_back(StreamRole::Viewfinder); > > } else if (conf["role"].toString() == "video") { > > - roles.push_back(Stream::VideoRecording()); > > + roles.push_back(StreamRole::VideoRecording); > > } else if (conf["role"].toString() == "still") { > > - roles.push_back(Stream::StillCapture()); > > + roles.push_back(StreamRole::StillCapture); > > } else { > > std::cerr << "Unknown stream role " > > << conf["role"].toString() << std::endl; > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index 359174a41823..a3921f91f1c9 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -542,23 +542,23 @@ const std::set<Stream *> &Camera::streams() const > > } > > > > /** > > - * \brief Generate a default camera configuration according to stream usages > > - * \param[in] usages A list of stream usages > > + * \brief Generate a default camera configuration according to stream roles > > + * \param[in] roles A list of stream roles > > * > > - * Generate a camera configuration for a set of desired usages. The caller > > - * specifies a list of stream usages and the camera returns a configuration > > + * Generate a camera configuration for a set of desired stream roles. The caller > > + * specifies a list of stream roles and the camera returns a configuration > > * containing suitable streams and their suggested default configurations. > > * > > - * \return A valid CameraConfiguration if the requested usages can be satisfied, > > + * \return A valid CameraConfiguration if the requested roles can be satisfied, > > * or a invalid one otherwise > > */ > > CameraConfiguration > > -Camera::generateConfiguration(const std::vector<StreamUsage> &usages) > > +Camera::generateConfiguration(const StreamRoles &roles) > > { > > - if (disconnected_ || !usages.size() || usages.size() > streams_.size()) > > + if (disconnected_ || !roles.size() || roles.size() > streams_.size()) > > return CameraConfiguration(); > > > > - CameraConfiguration config = pipe_->generateConfiguration(this, usages); > > + CameraConfiguration config = pipe_->generateConfiguration(this, roles); > > > > std::ostringstream msg("streams configuration:", std::ios_base::ate); > > unsigned int index = 0; > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > > index 9cc11a8e192e..3352cb0e5bc9 100644 > > --- a/src/libcamera/include/pipeline_handler.h > > +++ b/src/libcamera/include/pipeline_handler.h > > @@ -14,6 +14,8 @@ > > #include <string> > > #include <vector> > > > > +#include <libcamera/stream.h> > > + > > namespace libcamera { > > > > class Buffer; > > @@ -26,8 +28,6 @@ class DeviceMatch; > > class MediaDevice; > > class PipelineHandler; > > class Request; > > -class Stream; > > -class StreamUsage; > > Same comment as above. Or would you need to forward declare both StreamRoles > and StreamRole? I would have to copy the using line :-( > > > > class CameraData > > { > > @@ -61,7 +61,7 @@ public: > > void unlock(); > > > > virtual CameraConfiguration > > - generateConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0; > > + generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; > > virtual int configure(Camera *camera, const CameraConfiguration &config) = 0; > > > > virtual int allocateBuffers(Camera *camera, > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index ba0c708f9e1e..d234a8ac5289 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -151,8 +151,7 @@ public: > > PipelineHandlerIPU3(CameraManager *manager); > > > > CameraConfiguration > > - generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) override; > > + generateConfiguration(Camera *camera, const StreamRoles &roles) override; > > int configure(Camera *camera, > > const CameraConfiguration &config) override; > > > > @@ -211,7 +210,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) > > > > CameraConfiguration > > PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) > > + const StreamRoles &roles) > > { > > IPU3CameraData *data = cameraData(camera); > > CameraConfiguration config = {}; > > @@ -220,13 +219,12 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > &data->vfStream_, > > }; > > > > - for (const StreamUsage &usage : usages) { > > + for (const StreamRole role : roles) { > > StreamConfiguration cfg = {}; > > - StreamUsage::Role role = usage.role(); > > IPU3Stream *stream = nullptr; > > > > switch (role) { > > - case StreamUsage::Role::StillCapture: > > + case StreamRole::StillCapture: > > /* > > * Pick the output stream by default as the Viewfinder > > * and VideoRecording roles are not allowed on > > @@ -256,11 +254,11 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > > > break; > > > > - case StreamUsage::Role::Viewfinder: > > - case StreamUsage::Role::VideoRecording: { > > + case StreamRole::Viewfinder: > > + case StreamRole::VideoRecording: { > > /* > > * We can't use the 'output' stream for viewfinder or > > - * video capture usages. > > + * video capture roles. > > * > > * \todo This is an artificial limitation until we > > * figure out the exact capabilities of the hardware. > > @@ -275,15 +273,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > > stream = &data->vfStream_; > > > > /* > > - * Align the requested viewfinder size to the > > - * maximum available sensor resolution and to the > > - * IPU3 alignment constraints. > > + * Align the default viewfinder size to the maximum > > + * available sensor resolution and to the IPU3 > > + * alignment constraints. > > */ > > const Size &res = data->cio2_.sensor_->resolution(); > > - unsigned int width = std::min(usage.size().width, > > - res.width); > > - unsigned int height = std::min(usage.size().height, > > - res.height); > > + unsigned int width = std::min(1280U, res.width); > > + unsigned int height = std::min(720U, res.height); > > Why 1280x720 ? It's just a random default, suitable for today's devices in general. Applications are expected to override it. Do you think another default would be best ? > > cfg.size = { width & ~7, height & ~3 }; > > > > break; > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 4d02f9604ad9..4bd8c5101a96 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -35,7 +35,7 @@ public: > > ~PipelineHandlerRkISP1(); > > > > CameraConfiguration generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) override; > > + const StreamRoles &roles) override; > > int configure(Camera *camera, > > const CameraConfiguration &config) override; > > > > @@ -107,7 +107,7 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() > > */ > > > > CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) > > + const StreamRoles &roles) > > { > > RkISP1CameraData *data = cameraData(camera); > > CameraConfiguration config; > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index 118b97457d2a..d2e1f7d4e5b2 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -26,8 +26,7 @@ public: > > PipelineHandlerUVC(CameraManager *manager); > > > > CameraConfiguration > > - generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) override; > > + generateConfiguration(Camera *camera, const StreamRoles &roles) override; > > int configure(Camera *camera, > > const CameraConfiguration &config) override; > > > > @@ -77,7 +76,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) > > > > CameraConfiguration > > PipelineHandlerUVC::generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) > > + const StreamRoles &roles) > > { > > UVCCameraData *data = cameraData(camera); > > CameraConfiguration config; > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > index 74959581a7ef..17e2491e5c27 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -26,8 +26,7 @@ public: > > PipelineHandlerVimc(CameraManager *manager); > > > > CameraConfiguration > > - generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) override; > > + generateConfiguration(Camera *camera, const StreamRoles &roles) override; > > int configure(Camera *camera, > > const CameraConfiguration &config) override; > > > > @@ -77,7 +76,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) > > > > CameraConfiguration > > PipelineHandlerVimc::generateConfiguration(Camera *camera, > > - const std::vector<StreamUsage> &usages) > > + const StreamRoles &roles) > > { > > VimcCameraData *data = cameraData(camera); > > CameraConfiguration config; > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index b9ac64328f1d..81c11149c9fe 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -221,18 +221,18 @@ void PipelineHandler::unlock() > > * \fn PipelineHandler::generateConfiguration() > > * \brief Generate a camera configuration for a specified camera > > * \param[in] camera The camera to generate a default configuration for > > - * \param[in] usages A list of stream usages > > + * \param[in] roles A list of stream roles > > * > > - * Generate a default configuration for the \a camera for a specified group of > > - * use-cases. The caller shall populate the \a usages array with the use-cases > > - * it wishes to fetch the default configuration for. The returned configuration > > + * Generate a default configuration for the \a camera for a specified list of > > + * stream roles. The caller shall populate the \a roles with the use-cases it > > + * wishes to fetch the default configuration for. The returned configuration > > * can then be examined by the caller to learn about the selected streams and > > * their default parameters. > > * > > * The intended companion to this is \a configure() which can be used to change > > * the group of streams parameters. > > * > > - * \return A valid CameraConfiguration if the requested usages can be satisfied, > > + * \return A valid CameraConfiguration if the requested roles can be satisfied, > > * or a invalid configuration otherwise > > */ > > > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > > index af259510b19c..fe4c4ecf4150 100644 > > --- a/src/libcamera/stream.cpp > > +++ b/src/libcamera/stream.cpp > > @@ -75,61 +75,31 @@ std::string StreamConfiguration::toString() const > > } > > > > /** > > - * \class StreamUsage > > - * \brief Stream usage information > > - * > > - * The StreamUsage class describes how an application intends to use a stream. > > - * Usages are specified by applications and passed to cameras, that then select > > - * the most appropriate streams and their default configurations. > > - */ > > - > > -/** > > - * \enum StreamUsage::Role > > + * \enum StreamRole > > * \brief Identify the role a stream is intended to play > > - * \var StreamUsage::StillCapture > > + * > > + * The StreamRole describes how an application intends to use a stream. Roles > > + * are specified by applications and passed to cameras, that then select the > > s/select/selects ? The subject is "cameras", which is a plural, hence "select". > Minors apart, and the resolution selected on IPU3 clarified: > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > + * most appropriate streams and their default configurations. > > + * > > + * \var StillCapture > > * The stream is intended to capture high-resolution, high-quality still images > > * with low frame rate. The captured frames may be exposed with flash. > > - * \var StreamUsage::VideoRecording > > + * \var VideoRecording > > * The stream is intended to capture video for the purpose of recording or > > * streaming. The video stream may produce a high frame rate and may be > > * enhanced with video stabilization. > > - * \var StreamUsage::Viewfinder > > + * \var Viewfinder > > * The stream is intended to capture video for the purpose of display on the > > - * local screen. The StreamUsage includes the desired resolution. Trade-offs > > - * between quality and usage of system resources are acceptable. > > + * local screen. Trade-offs between quality and usage of system resources are > > + * acceptable. > > */ > > > > /** > > - * \fn StreamUsage::role() > > - * \brief Retrieve the stream role > > - * \return The stream role > > + * \typedef StreamRoles > > + * \brief A vector of StreamRole > > */ > > > > -/** > > - * \fn StreamUsage::size() > > - * \brief Retrieve desired size > > - * \return The desired size > > - */ > > - > > -/** > > - * \brief Create a stream usage > > - * \param[in] role Stream role > > - */ > > -StreamUsage::StreamUsage(Role role) > > - : role_(role) > > -{ > > -} > > - > > -/** > > - * \brief Create a stream usage with a desired size > > - * \param[in] role Stream role > > - * \param[in] size The desired size > > - */ > > -StreamUsage::StreamUsage(Role role, const Size &size) > > - : role_(role), size_(size) > > -{ > > -} > > - > > /** > > * \class Stream > > * \brief Video stream for a camera > > @@ -148,39 +118,6 @@ StreamUsage::StreamUsage(Role role, const Size &size) > > * optimal stream for the task. > > */ > > > > -/** > > - * \class Stream::StillCapture > > - * \brief Describe a still capture usage > > - */ > > -Stream::StillCapture::StillCapture() > > - : StreamUsage(Role::StillCapture) > > -{ > > -} > > - > > -/** > > - * \class Stream::VideoRecording > > - * \brief Describe a video recording usage > > - */ > > -Stream::VideoRecording::VideoRecording() > > - : StreamUsage(Role::VideoRecording) > > -{ > > -} > > - > > -/** > > - * \class Stream::Viewfinder > > - * \brief Describe a viewfinder usage > > - */ > > - > > -/** > > - * \brief Create a viewfinder usage with a desired dimension > > - * \param[in] width The desired viewfinder width > > - * \param[in] height The desired viewfinder height > > - */ > > -Stream::Viewfinder::Viewfinder(int width, int height) > > - : StreamUsage(Role::Viewfinder, Size(width, height)) > > -{ > > -} > > - > > /** > > * \brief Construct a stream with default parameters > > */ > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index c91b82727ec6..a984aaca764f 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -97,7 +97,7 @@ int MainWindow::startCapture() > > { > > int ret; > > > > - config_ = camera_->generateConfiguration({ Stream::VideoRecording() }); > > + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); > > Stream *stream = config_.front(); > > ret = camera_->configure(config_); > > if (ret < 0) { > > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp > > index bc3a4d6cb9f2..e7e6438203b9 100644 > > --- a/test/camera/capture.cpp > > +++ b/test/camera/capture.cpp > > @@ -43,7 +43,7 @@ protected: > > int run() > > { > > CameraConfiguration config = > > - camera_->generateConfiguration({ Stream::VideoRecording() }); > > + camera_->generateConfiguration({ StreamRole::VideoRecording }); > > Stream *stream = config.front(); > > StreamConfiguration *cfg = &config[stream]; > > > > diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp > > index 340b5f58f04c..8c4a03db498a 100644 > > --- a/test/camera/configuration_default.cpp > > +++ b/test/camera/configuration_default.cpp > > @@ -21,7 +21,7 @@ protected: > > CameraConfiguration config; > > > > /* Test asking for configuration for a video stream. */ > > - config = camera_->generateConfiguration({ Stream::VideoRecording() }); > > + config = camera_->generateConfiguration({ StreamRole::VideoRecording }); > > if (!config.isValid()) { > > cout << "Default configuration invalid" << endl; > > return TestFail; > > @@ -29,11 +29,11 @@ protected: > > > > /* > > * Test that asking for configuration for an empty array of > > - * stream usages returns an empty list of configurations. > > + * stream roles returns an empty list of configurations. > > */ > > config = camera_->generateConfiguration({}); > > if (config.isValid()) { > > - cout << "Failed to retrieve configuration for empty usage list" > > + cout << "Failed to retrieve configuration for empty roles list" > > << endl; > > return TestFail; > > } > > diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp > > index 24d5ca6690b7..76d8bc3e40a4 100644 > > --- a/test/camera/configuration_set.cpp > > +++ b/test/camera/configuration_set.cpp > > @@ -19,7 +19,7 @@ protected: > > int run() > > { > > CameraConfiguration config = > > - camera_->generateConfiguration({ Stream::VideoRecording() }); > > + camera_->generateConfiguration({ StreamRole::VideoRecording }); > > StreamConfiguration *cfg = &config[config.front()]; > > > > if (!config.isValid()) { > > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp > > index bd2e61ff2939..7a74cd85a37a 100644 > > --- a/test/camera/statemachine.cpp > > +++ b/test/camera/statemachine.cpp > > @@ -235,7 +235,7 @@ protected: > > > > int run() > > { > > - defconf_ = camera_->generateConfiguration({ Stream::VideoRecording() }); > > + defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); > > > > if (testAvailable() != TestPass) { > > cout << "State machine in Available state failed" << endl;
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 306739b7014a..42ba5201eabc 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -14,16 +14,13 @@ #include <libcamera/request.h> #include <libcamera/signal.h> +#include <libcamera/stream.h> namespace libcamera { class Buffer; class PipelineHandler; class Request; -class Stream; -class StreamUsage; - -struct StreamConfiguration; class CameraConfiguration { @@ -74,8 +71,7 @@ public: int release(); const std::set<Stream *> &streams() const; - CameraConfiguration - generateConfiguration(const std::vector<StreamUsage> &usage); + CameraConfiguration generateConfiguration(const StreamRoles &roles); int configure(const CameraConfiguration &config); int allocateBuffers(); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 0c986310939b..59bdf217eb31 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -8,6 +8,7 @@ #define __LIBCAMERA_STREAM_H__ #include <string> +#include <vector> #include <libcamera/buffer.h> #include <libcamera/geometry.h> @@ -25,48 +26,17 @@ struct StreamConfiguration { std::string toString() const; }; -class StreamUsage -{ -public: - enum Role { - StillCapture, - VideoRecording, - Viewfinder, - }; - - Role role() const { return role_; } - const Size &size() const { return size_; } - -protected: - explicit StreamUsage(Role role); - StreamUsage(Role role, const Size &size); - -private: - Role role_; - Size size_; +enum StreamRole { + StillCapture, + VideoRecording, + Viewfinder, }; +using StreamRoles = std::vector<StreamRole>; + class Stream { public: - class StillCapture : public StreamUsage - { - public: - StillCapture(); - }; - - class VideoRecording : public StreamUsage - { - public: - VideoRecording(); - }; - - class Viewfinder : public StreamUsage - { - public: - Viewfinder(int width, int height); - }; - Stream(); BufferPool &bufferPool() { return bufferPool_; } const StreamConfiguration &configuration() const { return configuration_; } diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 6a2508dd3bd9..d603228c0116 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -87,13 +87,13 @@ static int parseOptions(int argc, char *argv[]) static int prepareCameraConfig(CameraConfiguration *config) { - std::vector<StreamUsage> roles; + StreamRoles roles; streamInfo.clear(); /* If no configuration is provided assume a single video stream. */ if (!options.isSet(OptStream)) { - *config = camera->generateConfiguration({ Stream::VideoRecording() }); + *config = camera->generateConfiguration({ StreamRole::VideoRecording }); streamInfo[config->front()] = "stream0"; return 0; } @@ -106,14 +106,13 @@ static int prepareCameraConfig(CameraConfiguration *config) KeyValueParser::Options conf = value.toKeyValues(); if (!conf.isSet("role")) { - roles.push_back(Stream::VideoRecording()); + roles.push_back(StreamRole::VideoRecording); } else if (conf["role"].toString() == "viewfinder") { - roles.push_back(Stream::Viewfinder(conf["width"], - conf["height"])); + roles.push_back(StreamRole::Viewfinder); } else if (conf["role"].toString() == "video") { - roles.push_back(Stream::VideoRecording()); + roles.push_back(StreamRole::VideoRecording); } else if (conf["role"].toString() == "still") { - roles.push_back(Stream::StillCapture()); + roles.push_back(StreamRole::StillCapture); } else { std::cerr << "Unknown stream role " << conf["role"].toString() << std::endl; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 359174a41823..a3921f91f1c9 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -542,23 +542,23 @@ const std::set<Stream *> &Camera::streams() const } /** - * \brief Generate a default camera configuration according to stream usages - * \param[in] usages A list of stream usages + * \brief Generate a default camera configuration according to stream roles + * \param[in] roles A list of stream roles * - * Generate a camera configuration for a set of desired usages. The caller - * specifies a list of stream usages and the camera returns a configuration + * Generate a camera configuration for a set of desired stream roles. The caller + * specifies a list of stream roles and the camera returns a configuration * containing suitable streams and their suggested default configurations. * - * \return A valid CameraConfiguration if the requested usages can be satisfied, + * \return A valid CameraConfiguration if the requested roles can be satisfied, * or a invalid one otherwise */ CameraConfiguration -Camera::generateConfiguration(const std::vector<StreamUsage> &usages) +Camera::generateConfiguration(const StreamRoles &roles) { - if (disconnected_ || !usages.size() || usages.size() > streams_.size()) + if (disconnected_ || !roles.size() || roles.size() > streams_.size()) return CameraConfiguration(); - CameraConfiguration config = pipe_->generateConfiguration(this, usages); + CameraConfiguration config = pipe_->generateConfiguration(this, roles); std::ostringstream msg("streams configuration:", std::ios_base::ate); unsigned int index = 0; diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 9cc11a8e192e..3352cb0e5bc9 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -14,6 +14,8 @@ #include <string> #include <vector> +#include <libcamera/stream.h> + namespace libcamera { class Buffer; @@ -26,8 +28,6 @@ class DeviceMatch; class MediaDevice; class PipelineHandler; class Request; -class Stream; -class StreamUsage; class CameraData { @@ -61,7 +61,7 @@ public: void unlock(); virtual CameraConfiguration - generateConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0; + generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; virtual int configure(Camera *camera, const CameraConfiguration &config) = 0; virtual int allocateBuffers(Camera *camera, diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ba0c708f9e1e..d234a8ac5289 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -151,8 +151,7 @@ public: PipelineHandlerIPU3(CameraManager *manager); CameraConfiguration - generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) override; + generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -211,7 +210,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) CameraConfiguration PipelineHandlerIPU3::generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) + const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); CameraConfiguration config = {}; @@ -220,13 +219,12 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, &data->vfStream_, }; - for (const StreamUsage &usage : usages) { + for (const StreamRole role : roles) { StreamConfiguration cfg = {}; - StreamUsage::Role role = usage.role(); IPU3Stream *stream = nullptr; switch (role) { - case StreamUsage::Role::StillCapture: + case StreamRole::StillCapture: /* * Pick the output stream by default as the Viewfinder * and VideoRecording roles are not allowed on @@ -256,11 +254,11 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, break; - case StreamUsage::Role::Viewfinder: - case StreamUsage::Role::VideoRecording: { + case StreamRole::Viewfinder: + case StreamRole::VideoRecording: { /* * We can't use the 'output' stream for viewfinder or - * video capture usages. + * video capture roles. * * \todo This is an artificial limitation until we * figure out the exact capabilities of the hardware. @@ -275,15 +273,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, stream = &data->vfStream_; /* - * Align the requested viewfinder size to the - * maximum available sensor resolution and to the - * IPU3 alignment constraints. + * Align the default viewfinder size to the maximum + * available sensor resolution and to the IPU3 + * alignment constraints. */ const Size &res = data->cio2_.sensor_->resolution(); - unsigned int width = std::min(usage.size().width, - res.width); - unsigned int height = std::min(usage.size().height, - res.height); + unsigned int width = std::min(1280U, res.width); + unsigned int height = std::min(720U, res.height); cfg.size = { width & ~7, height & ~3 }; break; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4d02f9604ad9..4bd8c5101a96 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -35,7 +35,7 @@ public: ~PipelineHandlerRkISP1(); CameraConfiguration generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) override; + const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -107,7 +107,7 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() */ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) + const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); CameraConfiguration config; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 118b97457d2a..d2e1f7d4e5b2 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -26,8 +26,7 @@ public: PipelineHandlerUVC(CameraManager *manager); CameraConfiguration - generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) override; + generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -77,7 +76,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) CameraConfiguration PipelineHandlerUVC::generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) + const StreamRoles &roles) { UVCCameraData *data = cameraData(camera); CameraConfiguration config; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 74959581a7ef..17e2491e5c27 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -26,8 +26,7 @@ public: PipelineHandlerVimc(CameraManager *manager); CameraConfiguration - generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) override; + generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -77,7 +76,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) CameraConfiguration PipelineHandlerVimc::generateConfiguration(Camera *camera, - const std::vector<StreamUsage> &usages) + const StreamRoles &roles) { VimcCameraData *data = cameraData(camera); CameraConfiguration config; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index b9ac64328f1d..81c11149c9fe 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -221,18 +221,18 @@ void PipelineHandler::unlock() * \fn PipelineHandler::generateConfiguration() * \brief Generate a camera configuration for a specified camera * \param[in] camera The camera to generate a default configuration for - * \param[in] usages A list of stream usages + * \param[in] roles A list of stream roles * - * Generate a default configuration for the \a camera for a specified group of - * use-cases. The caller shall populate the \a usages array with the use-cases - * it wishes to fetch the default configuration for. The returned configuration + * Generate a default configuration for the \a camera for a specified list of + * stream roles. The caller shall populate the \a roles with the use-cases it + * wishes to fetch the default configuration for. The returned configuration * can then be examined by the caller to learn about the selected streams and * their default parameters. * * The intended companion to this is \a configure() which can be used to change * the group of streams parameters. * - * \return A valid CameraConfiguration if the requested usages can be satisfied, + * \return A valid CameraConfiguration if the requested roles can be satisfied, * or a invalid configuration otherwise */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index af259510b19c..fe4c4ecf4150 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -75,61 +75,31 @@ std::string StreamConfiguration::toString() const } /** - * \class StreamUsage - * \brief Stream usage information - * - * The StreamUsage class describes how an application intends to use a stream. - * Usages are specified by applications and passed to cameras, that then select - * the most appropriate streams and their default configurations. - */ - -/** - * \enum StreamUsage::Role + * \enum StreamRole * \brief Identify the role a stream is intended to play - * \var StreamUsage::StillCapture + * + * The StreamRole describes how an application intends to use a stream. Roles + * are specified by applications and passed to cameras, that then select the + * most appropriate streams and their default configurations. + * + * \var StillCapture * The stream is intended to capture high-resolution, high-quality still images * with low frame rate. The captured frames may be exposed with flash. - * \var StreamUsage::VideoRecording + * \var VideoRecording * The stream is intended to capture video for the purpose of recording or * streaming. The video stream may produce a high frame rate and may be * enhanced with video stabilization. - * \var StreamUsage::Viewfinder + * \var Viewfinder * The stream is intended to capture video for the purpose of display on the - * local screen. The StreamUsage includes the desired resolution. Trade-offs - * between quality and usage of system resources are acceptable. + * local screen. Trade-offs between quality and usage of system resources are + * acceptable. */ /** - * \fn StreamUsage::role() - * \brief Retrieve the stream role - * \return The stream role + * \typedef StreamRoles + * \brief A vector of StreamRole */ -/** - * \fn StreamUsage::size() - * \brief Retrieve desired size - * \return The desired size - */ - -/** - * \brief Create a stream usage - * \param[in] role Stream role - */ -StreamUsage::StreamUsage(Role role) - : role_(role) -{ -} - -/** - * \brief Create a stream usage with a desired size - * \param[in] role Stream role - * \param[in] size The desired size - */ -StreamUsage::StreamUsage(Role role, const Size &size) - : role_(role), size_(size) -{ -} - /** * \class Stream * \brief Video stream for a camera @@ -148,39 +118,6 @@ StreamUsage::StreamUsage(Role role, const Size &size) * optimal stream for the task. */ -/** - * \class Stream::StillCapture - * \brief Describe a still capture usage - */ -Stream::StillCapture::StillCapture() - : StreamUsage(Role::StillCapture) -{ -} - -/** - * \class Stream::VideoRecording - * \brief Describe a video recording usage - */ -Stream::VideoRecording::VideoRecording() - : StreamUsage(Role::VideoRecording) -{ -} - -/** - * \class Stream::Viewfinder - * \brief Describe a viewfinder usage - */ - -/** - * \brief Create a viewfinder usage with a desired dimension - * \param[in] width The desired viewfinder width - * \param[in] height The desired viewfinder height - */ -Stream::Viewfinder::Viewfinder(int width, int height) - : StreamUsage(Role::Viewfinder, Size(width, height)) -{ -} - /** * \brief Construct a stream with default parameters */ diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index c91b82727ec6..a984aaca764f 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -97,7 +97,7 @@ int MainWindow::startCapture() { int ret; - config_ = camera_->generateConfiguration({ Stream::VideoRecording() }); + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); Stream *stream = config_.front(); ret = camera_->configure(config_); if (ret < 0) { diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index bc3a4d6cb9f2..e7e6438203b9 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -43,7 +43,7 @@ protected: int run() { CameraConfiguration config = - camera_->generateConfiguration({ Stream::VideoRecording() }); + camera_->generateConfiguration({ StreamRole::VideoRecording }); Stream *stream = config.front(); StreamConfiguration *cfg = &config[stream]; diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 340b5f58f04c..8c4a03db498a 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -21,7 +21,7 @@ protected: CameraConfiguration config; /* Test asking for configuration for a video stream. */ - config = camera_->generateConfiguration({ Stream::VideoRecording() }); + config = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (!config.isValid()) { cout << "Default configuration invalid" << endl; return TestFail; @@ -29,11 +29,11 @@ protected: /* * Test that asking for configuration for an empty array of - * stream usages returns an empty list of configurations. + * stream roles returns an empty list of configurations. */ config = camera_->generateConfiguration({}); if (config.isValid()) { - cout << "Failed to retrieve configuration for empty usage list" + cout << "Failed to retrieve configuration for empty roles list" << endl; return TestFail; } diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 24d5ca6690b7..76d8bc3e40a4 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -19,7 +19,7 @@ protected: int run() { CameraConfiguration config = - camera_->generateConfiguration({ Stream::VideoRecording() }); + camera_->generateConfiguration({ StreamRole::VideoRecording }); StreamConfiguration *cfg = &config[config.front()]; if (!config.isValid()) { diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index bd2e61ff2939..7a74cd85a37a 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -235,7 +235,7 @@ protected: int run() { - defconf_ = camera_->generateConfiguration({ Stream::VideoRecording() }); + defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (testAvailable() != TestPass) { cout << "State machine in Available state failed" << endl;