[libcamera-devel] [PATCH 3/3] cam: Add option to exit capture loop after n frames

[Kieran Bingham]

Hi Niklas,

Andrey beat me to the option value point, but still a Q below:


On 24/07/2020 14:43, Niklas Söderlund wrote:
> Add an option '--count' to specify how many frames to capture before
> terminating the capture loop.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

<edit, actually there isn't anything but minors here, so addressed how
you feel best:>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> ---
>  src/cam/capture.cpp | 17 +++++++++++++++--
>  src/cam/capture.h   |  2 ++
>  src/cam/main.cpp    |  2 ++
>  src/cam/main.h      |  1 +
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index aa53c407d7b71b44..3ca70d9c6a8c7b7e 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -17,7 +17,8 @@
>  using namespace libcamera;
>  
>  Capture::Capture(std::shared_ptr<Camera> camera, CameraConfiguration *config)
> -	: camera_(camera), config_(config), writer_(nullptr), loop_(nullptr)
> +	: camera_(camera), config_(config), writer_(nullptr), loop_(nullptr),
> +	  captureCount_(0), captureLimit_(0)
>  {
>  }
>  
> @@ -26,6 +27,8 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)
>  	int ret;
>  
>  	loop_ = loop;
> +	captureCount_ = 0;
> +	captureLimit_ = options.isSet(OptCount) ? options[OptCount].toInteger() : 0;

This prevents setting a limit of 0. I don't know if that's useful,
except for testing stream_on, followed by immediate stream_off ...

Actually that wouldn't work anyway, as the check would only happen in
request complete, so we know it's always going to do at least one capture...



>  	if (!camera_) {
>  		std::cout << "Can't capture without a camera" << std::endl;
> @@ -133,7 +136,11 @@ int Capture::capture(FrameBufferAllocator *allocator)
>  		}
>  	}
>  
> -	std::cout << "Capture until user interrupts by SIGINT" << std::endl;
> +	if (captureLimit_)
> +		std::cout << "Capture " << captureLimit_ << " frames" << std::endl;
> +	else
> +		std::cout << "Capture until user interrupts by SIGINT" << std::endl;
> +
>  	ret = loop_->exec();
>  	if (ret)
>  		std::cout << "Failed to run capture loop" << std::endl;
> @@ -202,5 +209,11 @@ void Capture::requestComplete(Request *request)
>  		request->addBuffer(stream, buffer);
>  	}
>  
> +	captureCount_++;
> +	if (captureLimit_ && captureCount_ >= captureLimit_) {
> +		loop_->exit(0);
> +		return;
> +	}
> +
>  	camera_->queueRequest(request);
>  }
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index 67c6260bfe24aefc..0a1cbc519f2a90c5 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -40,6 +40,8 @@ private:
>  	std::chrono::steady_clock::time_point last_;
>  
>  	EventLoop *loop_;
> +	unsigned int captureCount_;
> +	unsigned int captureLimit_;

As these are already in the capture class, I guess they could just be
count, and limit ... but that's just some bikeshed colour ...


>  };
>  
>  #endif /* __CAM_CAPTURE_H__ */
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index a223563ad37e9fe5..f3bacb4a895600ee 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -188,6 +188,8 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	parser.addOption(OptStrictFormats, OptionNone,
>  			 "Do not allow requested stream format(s) to be adjusted",
>  			 "strict-formats");
> +	parser.addOption(OptCount, OptionInteger, "Number of frames to capture",
> +			 "count", ArgumentRequired, "count");


Would reusing the capture option simplify the args?

>  
>  	options_ = parser.parse(argc, argv);
>  	if (!options_.valid())
> diff --git a/src/cam/main.h b/src/cam/main.h
> index 6f95add31a6341cf..d4c93fee35681e86 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -18,6 +18,7 @@ enum {
>  	OptStream = 's',
>  	OptListControls = 256,
>  	OptStrictFormats = 257,
> +	OptCount = 258,
>  };
>  
>  #endif /* __CAM_MAIN_H__ */
> 

-- 
Regards
--
Kieran