[libcamera-devel] [PATCH v5 6/6] libcamera: camera: extend camera object to support configuration of streams
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 1 00:58:06 CET 2019
Hi Niklas,
On Thu, Jan 31, 2019 at 07:01:13PM +0100, Niklas Söderlund wrote:
> On 2019-01-31 02:30:32 +0200, Laurent Pinchart wrote:
> > On Wed, Jan 30, 2019 at 12:56:15PM +0100, Niklas Söderlund wrote:
> >> Extend the camera to support reading and configuring formats for
> >> groups of streams. The implementation in the Camera are minimalistic as
> >> the heavy lifting are done by the pipeline handler implementations.
> >>
> >> The most important functionality the camera provides in this context is
> >> validation of data structures passed to it from the application and
> >> access control to the pipeline handler.
> >>
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >> ---
> >> include/libcamera/camera.h | 4 +++
> >> src/libcamera/camera.cpp | 61 ++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 65 insertions(+)
> >>
> >> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> >> index 01498935ae4aab58..b21d116edc0f3a57 100644
> >> --- a/include/libcamera/camera.h
> >> +++ b/include/libcamera/camera.h
> >> @@ -7,6 +7,7 @@
> >> #ifndef __LIBCAMERA_CAMERA_H__
> >> #define __LIBCAMERA_CAMERA_H__
> >>
> >> +#include <map>
> >> #include <memory>
> >> #include <string>
> >>
> >> @@ -16,6 +17,7 @@ namespace libcamera {
> >>
> >> class PipelineHandler;
> >> class Stream;
> >> +class StreamConfiguration;
> >>
> >> class Camera final
> >> {
> >> @@ -35,6 +37,8 @@ public:
> >> void release();
> >>
> >> const std::vector<Stream *> streams() const;
> >> + std::map<Stream*, StreamConfiguration> streamConfiguration(std::vector<Stream*> &streams);
> >
> > s/;/ const;/
> >
> >> + int configureStreams(std::map<Stream*, StreamConfiguration> &config);
> >
> > s/(/(const /
> >
> >> private:
> >> Camera(PipelineHandler *pipe, const std::string &name);
> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> >> index 986b74407aed6bd2..a293350f731e18df 100644
> >> --- a/src/libcamera/camera.cpp
> >> +++ b/src/libcamera/camera.cpp
> >> @@ -190,4 +190,65 @@ const std::vector<Stream *> Camera::streams() const
> >> return streams_;
> >> }
> >>
> >> +/**
> >> + * \brief Retrieve a group of stream configurations
> >> + * \param[in] streams A map of stream IDs and configurations to setup
> >> + *
> >> + * Retrieve the camera's configuration for a specified group of streams. The
> >> + * caller can specifies which of the camera's streams to retrieve configuration
> >> + * from by populating \a streams.
> >> + *
> >> + * The easiest way to populate the array of streams to fetch configuration from
> >> + * is to first retrieve the camera's full array of stream by with streams() and
> >
> > "by with" ?
> >
> >> + * then potentially trim it down to only contain the streams the caller
> >> + * are interested in.
> >> + *
> >> + * \return A map of successfully retrieved stream IDs and configurations or an
> >> + * empty list on error.
> >
> > That doesn't allow returning an error code to discriminate between
> > errors :-/ I wonder if we should pass the map as an argument by pointer
> > and return an integer. Or maybe there's no need to discriminate between
> > errors ?
>
> Good question, I'm not sure if we need to discriminate between errors
> here? If we do how would an application act on the different errors?
I'm not sure either, although distinguishing invalid arguments from
disconnection may be useful. Maybe we can leave it as-is for now.
> >> + */
> >> +std::map<Stream*, StreamConfiguration>
> >> +Camera::streamConfiguration(std::vector<Stream*> &streams)
> >> +{
> >> + if (disconnected_ || !streams.size())
> >> + std::map<unsigned int, StreamConfiguration>{};
> >> +
> >> + return pipe_->streamConfiguration(this, streams);
> >> +}
> >> +
> >> +/**
> >> + * \brief Configure the camera's streams prior to capture
> >> + * \param[in] config A map of stream IDs and configurations to setup
> >> + *
> >> + * Prior to starting capture, the camera must be configured to select a
> >> + * group of streams to be involved in the capture and their configuration.
> >> + * The caller specifies which streams are to be involved and their configuration
> >> + * by populating \a config.
> >> + *
> >> + * The easiest way to populate the array of config is to fetch an initial
> >> + * configuration from the camera with streamConfiguration() and then change the
> >> + * parameters to fit the caller's need and once all the streams parameters are
> >> + * configured hand that over to configureStreams() to actually setup the camera.
> >> + *
> >> + * Exclusive access to the camera shall be ensured by a call to acquire() prior
> >> + * to calling this function, otherwise an -EACCES error is be returned.
> >
> > s/is be/will be/
> >
> >> + *
> >> + * \return 0 on success or a negative error code on error.
> >> + * \retval -ENODEV The camera is not connected to any hardware
> >> + * \retval -EACCES The user have not acquired exclusive access to the camera
> >
> > s/have not/has not/
> >
> >> + * \retval -EINVAL The configuration is not valid
> >> + */
> >> +int Camera::configureStreams(std::map<Stream*, StreamConfiguration> &config)
> >> +{
> >> + if (disconnected_)
> >> + return -ENODEV;
> >> +
> >> + if (!acquired_)
> >> + return -EACCES;
> >> +
> >> + if (!config.size())
> >> + return -EINVAL;
> >> +
> >> + return pipe_->configureStreams(this, config);
> >> +}
> >> +
> >> } /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list