[libcamera-devel] [PATCH v2 6/6] cam: Add command line option to test IspCrop control

[Niklas Söderlund]

Hi David,

Thanks for your patch.

On 2020-09-25 09:51:27 +0100, David Plowman wrote:
> This allows the user to request digital zoom by adding, for example,
> "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in
> the middle of the image.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/cam/capture.cpp | 25 +++++++++++++++++++++++--
>  src/cam/capture.h   |  2 +-
>  src/cam/main.cpp    |  3 +++
>  src/cam/main.h      |  1 +
>  4 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 5510c00..3b50c59 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -10,6 +10,9 @@
>  #include <limits.h>
>  #include <sstream>
>  
> +#include <libcamera/control_ids.h>
> +#include <libcamera/property_ids.h>
> +
>  #include "capture.h"
>  #include "main.h"
>  
> @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options)
>  
>  	FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);
>  
> -	ret = capture(allocator);
> +	ret = capture(allocator, options);
>  
>  	if (options.isSet(OptFile)) {
>  		delete writer_;
> @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options)
>  	return ret;
>  }
>  
> -int Capture::capture(FrameBufferAllocator *allocator)
> +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options)
>  {
>  	int ret;
>  
> @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator)
>  		requests.push_back(request);
>  	}
>  
> +	/* Set up digital zoom if it was requested on the command line. */
> +	if (options.isSet(OptZoom)) {
> +		std::string zoom = options[OptZoom].toString();
> +		float x, y, width, height;
> +
> +		if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) {

This is the wrong place to parse the input from the user. If the zoom 
options are global something similar like StreamKeyValueParser 
(ZoomKeyValueParser?) should be created.

> +			Size sensorArea = camera_->properties().get(properties::SensorOutputSize);
> +			Rectangle crop(x * sensorArea.width,
> +				       y * sensorArea.height,
> +				       width * sensorArea.width,
> +				       height * sensorArea.height);


> +
> +			requests.front()->controls().set(controls::IspCrop, crop);

Do we wish to expose the ISP in the public API? If we do is the ISP 
(from an application point of view) the correct level to operate on?  
Would not setting this on a stream level cover more hardware?

I'm thinking of an ISP that can apply different crop parameters to two 
(or more) source pads from a single sink pad. Having a control that sets 
a global crop on the ISP would that not limit such hardware or am I 
missing something?

> +		} else {
> +			std::cout << "Invalid zoom values - ignoring" << std::endl;
> +		}
> +	}
> +
>  	ret = camera_->start();
>  	if (ret) {
>  		std::cout << "Failed to start capture" << std::endl;
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index 0aebdac..5280616 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -29,7 +29,7 @@ public:
>  
>  	int run(const OptionsParser::Options &options);
>  private:
> -	int capture(libcamera::FrameBufferAllocator *allocator);
> +	int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options);
>  
>  	void requestComplete(libcamera::Request *request);
>  
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 244720b..8f326d9 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -201,6 +201,9 @@ 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(OptZoom, OptionString,
> +			 "Specify digital zoom by supplying IspCrop control as x,y,width,height e.g. 0.25,0.25,0.5,0.5",
> +			 "zoom", ArgumentRequired, "zoom");
>  
>  	options_ = parser.parse(argc, argv);
>  	if (!options_.valid())
> diff --git a/src/cam/main.h b/src/cam/main.h
> index ea8dfd3..a1fd899 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -17,6 +17,7 @@ enum {
>  	OptListProperties = 'p',
>  	OptMonitor = 'm',
>  	OptStream = 's',
> +	OptZoom = 'z',
>  	OptListControls = 256,
>  	OptStrictFormats = 257,
>  };
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund