[libcamera-devel,v3,3/3] cam: Add optional argument to --capture to specify how many frames to capture

8990 diff mbox series

Extend the '--capture' option with and optional numerical argument to be
able to specify how many frames to capture before exiting. If the
optional argument is not provided the old behavior of running until the
user interrupts with a SIGINT is retained.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v2
- Move check in Capture::requestComplete() as to not leak the created
  but never used Request.
* Changes since v1
- Extend --capture option instead of adding a new --count option.
---
 src/cam/capture.cpp | 18 ++++++++++++++++--
 src/cam/capture.h   |  2 ++
 src/cam/main.cpp    |  5 +++--
 3 files changed, 21 insertions(+), 4 deletions(-)

Hi Niklas,

Thanks for extending --capture to capture a certain no. of frames.
It super handy to use.

Just want to let you know, it does not work if I use short-hand "-C=10".
However, cam -c 1 --capture=10 seems to work just fine.

On $cam -c 1 -C=10 O get the following:

[uajain@localhost libcamera]$ sudo ./build/src/cam/cam  -c 1 -C=10
Can't parse integer argument for option --capture
Options:
...

On 7/25/20 12:48 AM, Niklas Söderlund wrote:
> Extend the '--capture' option with and optional numerical argument to be
> able to specify how many frames to capture before exiting. If the
> optional argument is not provided the old behavior of running until the
> user interrupts with a SIGINT is retained.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v2
> - Move check in Capture::requestComplete() as to not leak the created
>    but never used Request.
> * Changes since v1
> - Extend --capture option instead of adding a new --count option.
> ---
>   src/cam/capture.cpp | 18 ++++++++++++++++--
>   src/cam/capture.h   |  2 ++
>   src/cam/main.cpp    |  5 +++--
>   3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index f811a18c579508a1..3d8e89d52c7ab68d 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -18,7 +18,8 @@ using namespace libcamera;
>   
>   Capture::Capture(std::shared_ptr<Camera> camera, CameraConfiguration *config,
>   		 EventLoop *loop)
> -	: camera_(camera), config_(config), writer_(nullptr), loop_(loop)
> +	: camera_(camera), config_(config), writer_(nullptr), loop_(loop),
> +	  captureCount_(0), captureLimit_(0)
>   {
>   }
>   
> @@ -26,6 +27,9 @@ int Capture::run(const OptionsParser::Options &options)
>   {
>   	int ret;
>   
> +	captureCount_ = 0;
> +	captureLimit_ = options[OptCapture].toInteger();
> +
>   	if (!camera_) {
>   		std::cout << "Can't capture without a camera" << std::endl;
>   		return -ENODEV;
> @@ -132,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;
> @@ -184,6 +192,12 @@ void Capture::requestComplete(Request *request)
>   
>   	std::cout << info.str() << std::endl;
>   
> +	captureCount_++;
> +	if (captureLimit_ && captureCount_ >= captureLimit_) {
> +		loop_->exit(0);
> +		return;
> +	}
> +
>   	/*
>   	 * Create a new request and populate it with one buffer for each
>   	 * stream.
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index acdefc47b24314d2..32940a2a12138887 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -41,6 +41,8 @@ private:
>   	std::chrono::steady_clock::time_point last_;
>   
>   	EventLoop *loop_;
> +	unsigned int captureCount_;
> +	unsigned int captureLimit_;
>   };
>   
>   #endif /* __CAM_CAPTURE_H__ */
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 3e83feabe64e036c..f5aba041d7c4c181 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -167,8 +167,9 @@ int CamApp::parseOptions(int argc, char *argv[])
>   	parser.addOption(OptCamera, OptionString,
>   			 "Specify which camera to operate on, by name or by index", "camera",
>   			 ArgumentRequired, "camera");
> -	parser.addOption(OptCapture, OptionNone,
> -			 "Capture until interrupted by user", "capture");
> +	parser.addOption(OptCapture, OptionInteger,
> +			 "Capture until interrupted by user or until <count> frames captured",
> +			 "capture", ArgumentOptional, "count");
>   	parser.addOption(OptFile, OptionString,
>   			 "Write captured frames to disk\n"
>   			 "The first '#' character in the file name is expanded to the stream name and frame sequence number.\n"

Hi Umang,

Thanks for your comment.

On 2020-07-28 16:07:56 +0000, Umang Jain wrote:
> Hi Niklas,
> 
> Thanks for extending --capture to capture a certain no. of frames.
> It super handy to use.
> 
> Just want to let you know, it does not work if I use short-hand "-C=10".
> However, cam -c 1 --capture=10 seems to work just fine.
> 
> On $cam -c 1 -C=10 O get the following:
> 
> [uajain@localhost libcamera]$ sudo ./build/src/cam/cam  -c 1 -C=10
> Can't parse integer argument for option --capture
> Options:
> ...

That is because for optional arguments to short options the = character 
is dropped :-)

    $ cam -c 1 -C10
    Using camera Logitech Webcam C930e
    Capture 10 frames
    fps: 0.00 stream0 seq: 000000 bytesused: 49517
    fps: 37.04 stream0 seq: 000001 bytesused: 50679
    fps: 8.13 stream0 seq: 000002 bytesused: 49837
    fps: 8.13 stream0 seq: 000003 bytesused: 50275
    fps: 7.87 stream0 seq: 000004 bytesused: 50851
    fps: 8.13 stream0 seq: 000005 bytesused: 51369
    fps: 8.06 stream0 seq: 000006 bytesused: 50756
    fps: 14.93 stream0 seq: 000007 bytesused: 51042
    fps: 15.62 stream0 seq: 000008 bytesused: 52641
    fps: 14.93 stream0 seq: 000009 bytesused: 51621

> 
> On 7/25/20 12:48 AM, Niklas Söderlund wrote:
> > Extend the '--capture' option with and optional numerical argument to be
> > able to specify how many frames to capture before exiting. If the
> > optional argument is not provided the old behavior of running until the
> > user interrupts with a SIGINT is retained.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > * Changes since v2
> > - Move check in Capture::requestComplete() as to not leak the created
> >    but never used Request.
> > * Changes since v1
> > - Extend --capture option instead of adding a new --count option.
> > ---
> >   src/cam/capture.cpp | 18 ++++++++++++++++--
> >   src/cam/capture.h   |  2 ++
> >   src/cam/main.cpp    |  5 +++--
> >   3 files changed, 21 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > index f811a18c579508a1..3d8e89d52c7ab68d 100644
> > --- a/src/cam/capture.cpp
> > +++ b/src/cam/capture.cpp
> > @@ -18,7 +18,8 @@ using namespace libcamera;
> >   Capture::Capture(std::shared_ptr<Camera> camera, CameraConfiguration *config,
> >   		 EventLoop *loop)
> > -	: camera_(camera), config_(config), writer_(nullptr), loop_(loop)
> > +	: camera_(camera), config_(config), writer_(nullptr), loop_(loop),
> > +	  captureCount_(0), captureLimit_(0)
> >   {
> >   }
> > @@ -26,6 +27,9 @@ int Capture::run(const OptionsParser::Options &options)
> >   {
> >   	int ret;
> > +	captureCount_ = 0;
> > +	captureLimit_ = options[OptCapture].toInteger();
> > +
> >   	if (!camera_) {
> >   		std::cout << "Can't capture without a camera" << std::endl;
> >   		return -ENODEV;
> > @@ -132,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;
> > @@ -184,6 +192,12 @@ void Capture::requestComplete(Request *request)
> >   	std::cout << info.str() << std::endl;
> > +	captureCount_++;
> > +	if (captureLimit_ && captureCount_ >= captureLimit_) {
> > +		loop_->exit(0);
> > +		return;
> > +	}
> > +
> >   	/*
> >   	 * Create a new request and populate it with one buffer for each
> >   	 * stream.
> > diff --git a/src/cam/capture.h b/src/cam/capture.h
> > index acdefc47b24314d2..32940a2a12138887 100644
> > --- a/src/cam/capture.h
> > +++ b/src/cam/capture.h
> > @@ -41,6 +41,8 @@ private:
> >   	std::chrono::steady_clock::time_point last_;
> >   	EventLoop *loop_;
> > +	unsigned int captureCount_;
> > +	unsigned int captureLimit_;
> >   };
> >   #endif /* __CAM_CAPTURE_H__ */
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 3e83feabe64e036c..f5aba041d7c4c181 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -167,8 +167,9 @@ int CamApp::parseOptions(int argc, char *argv[])
> >   	parser.addOption(OptCamera, OptionString,
> >   			 "Specify which camera to operate on, by name or by index", "camera",
> >   			 ArgumentRequired, "camera");
> > -	parser.addOption(OptCapture, OptionNone,
> > -			 "Capture until interrupted by user", "capture");
> > +	parser.addOption(OptCapture, OptionInteger,
> > +			 "Capture until interrupted by user or until <count> frames captured",
> > +			 "capture", ArgumentOptional, "count");
> >   	parser.addOption(OptFile, OptionString,
> >   			 "Write captured frames to disk\n"
> >   			 "The first '#' character in the file name is expanded to the stream name and frame sequence number.\n"