[libcamera-devel] [PATCH 1/8] cam: Pass stream roles to Capture class
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue May 19 16:27:50 CEST 2020
Hi Laurent,
Thanks for your work.
On 2020-05-19 06:24:58 +0300, Laurent Pinchart wrote:
> The stream roles will be needed in the Capture class to verify the
> configuration. Store they in the CamApp class and pass them to the
> Capture class constructor.
The patch itself is fine but I wonder if this is the right way. We have
previously endeavored to limit the spread of the roles past the
generateConfiguration() as they somewhat lose there meaning after that,
right?
Even if the user requested a viewfinder and video stream they are only
hints to the pipeline hander is "free" to return any type of camera
configuration, even one with only 1 stream even if more where requested
in the role hints.
For this reason I wonder if instead the check for the combination of the
new option -D and that the stream role hint should be set to viewfinder
should be moved and be validated right after we have parsed the command
line options?
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/cam/capture.cpp | 5 +++--
> src/cam/capture.h | 4 +++-
> src/cam/main.cpp | 9 +++++----
> 3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 55fa2dabcee9..b7e06bcc9463 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -16,8 +16,9 @@
>
> using namespace libcamera;
>
> -Capture::Capture(std::shared_ptr<Camera> camera, CameraConfiguration *config)
> - : camera_(camera), config_(config), writer_(nullptr)
> +Capture::Capture(std::shared_ptr<Camera> camera, CameraConfiguration *config,
> + const StreamRoles &roles)
> + : camera_(camera), config_(config), roles_(roles), writer_(nullptr)
> {
> }
>
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index 9bca5661070e..c0e697b831fb 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -24,7 +24,8 @@ class Capture
> {
> public:
> Capture(std::shared_ptr<libcamera::Camera> camera,
> - libcamera::CameraConfiguration *config);
> + libcamera::CameraConfiguration *config,
> + const libcamera::StreamRoles &roles);
>
> int run(EventLoop *loop, const OptionsParser::Options &options);
> private:
> @@ -35,6 +36,7 @@ private:
>
> std::shared_ptr<libcamera::Camera> camera_;
> libcamera::CameraConfiguration *config_;
> + libcamera::StreamRoles roles_;
>
> std::map<libcamera::Stream *, std::string> streamName_;
> BufferWriter *writer_;
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 2512fe9da782..cdd29d500202 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -47,6 +47,7 @@ private:
> OptionsParser::Options options_;
> CameraManager *cm_;
> std::shared_ptr<Camera> camera_;
> + StreamRoles roles_;
> std::unique_ptr<libcamera::CameraConfiguration> config_;
> EventLoop *loop_;
> };
> @@ -194,10 +195,10 @@ int CamApp::parseOptions(int argc, char *argv[])
>
> int CamApp::prepareConfig()
> {
> - StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> + roles_ = StreamKeyValueParser::roles(options_[OptStream]);
>
> - config_ = camera_->generateConfiguration(roles);
> - if (!config_ || config_->size() != roles.size()) {
> + config_ = camera_->generateConfiguration(roles_);
> + if (!config_ || config_->size() != roles_.size()) {
> std::cerr << "Failed to get default stream configuration"
> << std::endl;
> return -EINVAL;
> @@ -326,7 +327,7 @@ int CamApp::run()
> }
>
> if (options_.isSet(OptCapture)) {
> - Capture capture(camera_, config_.get());
> + Capture capture(camera_, config_.get(), roles_);
> return capture.run(loop_, options_);
> }
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list