[libcamera-devel] [PATCH v2 2/5] libcamera: Add user Transform to CameraConfiguration

[Laurent Pinchart]

Hi Kieran,

On Wed, Aug 19, 2020 at 09:34:13AM +0100, Kieran Bingham wrote:
> On 19/08/2020 02:58, Laurent Pinchart wrote:
> > On Thu, Aug 06, 2020 at 05:36:36PM +0100, David Plowman wrote:
> >> Add a field to the CameraConfiguration to represent a 2d transform
> >> requested by the application. All pipeline handlers are amended to
> >> coerce this to the Identity, marking the configuration as "adjusted"
> >> if something different had been requested.
> >>
> >> Pipeline handlers that support Transforms can be amended subsequently.
> > 
> > Missing SoB here too. Maybe that was done on purpose, as the series
> > isn't complete yet. It's also customary to express that with RFC/PATCH
> > in the subject line (git-format-patch has a --subject-prefix options for
> > that).
> 
> it also has --rfc directly as a shorthand ;-)
> 
> git format-patch -3 --cover-letter --rfc -v2

Oh, I didn't know that. Neat :-)

> cat v2-0000-cover-letter.patch  | grep Subject
> Subject: [RFC PATCH v2 0/3] *** SUBJECT HERE ***
> 
> > Apart from the missing documentation, this looks fine to me.
> > 
> >> ---
> >>  include/libcamera/camera.h                         | 3 +++
> >>  src/libcamera/camera.cpp                           | 2 +-
> >>  src/libcamera/pipeline/ipu3/ipu3.cpp               | 5 +++++
> >>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++++
> >>  src/libcamera/pipeline/rkisp1/rkisp1.cpp           | 5 +++++
> >>  src/libcamera/pipeline/simple/simple.cpp           | 5 +++++
> >>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       | 5 +++++
> >>  src/libcamera/pipeline/vimc/vimc.cpp               | 5 +++++
> >>  8 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> >> index 48d88d6..dedc1c6 100644
> >> --- a/include/libcamera/camera.h
> >> +++ b/include/libcamera/camera.h
> >> @@ -17,6 +17,7 @@
> >>  #include <libcamera/request.h>
> >>  #include <libcamera/signal.h>
> >>  #include <libcamera/stream.h>
> >> +#include <libcamera/transform.h>
> >>  
> >>  namespace libcamera {
> >>  
> >> @@ -61,6 +62,8 @@ public:
> >>  	bool empty() const;
> >>  	std::size_t size() const;
> >>  
> >> +	Transform transform;
> >> +
> >>  protected:
> >>  	CameraConfiguration();
> >>  
> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> >> index 820fa1e..4282a02 100644
> >> --- a/src/libcamera/camera.cpp
> >> +++ b/src/libcamera/camera.cpp
> >> @@ -93,7 +93,7 @@ LOG_DECLARE_CATEGORY(Camera)
> >>   * \brief Create an empty camera configuration
> >>   */
> >>  CameraConfiguration::CameraConfiguration()
> >> -	: config_({})
> >> +	: transform(Transform::Identity), config_({})
> >>  {
> >>  }
> >>  
> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> index d931ed3..a9a82a4 100644
> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> @@ -138,6 +138,11 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> >>  	if (config_.empty())
> >>  		return Invalid;
> >>  
> >> +	if (transform != Transform::Identity) {
> >> +		transform = Transform::Identity;
> >> +		status = Adjusted;
> >> +	}
> >> +
> >>  	/* Cap the number of entries to the available streams. */
> >>  	if (config_.size() > IPU3_MAX_STREAMS) {
> >>  		config_.resize(IPU3_MAX_STREAMS);
> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> index eeaf335..236aa5c 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> @@ -400,6 +400,11 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >>  	if (config_.empty())
> >>  		return Invalid;
> >>  
> >> +	if (transform != Transform::Identity) {
> >> +		transform = Transform::Identity;
> >> +		status = Adjusted;
> >> +	}
> >> +
> >>  	unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;
> >>  	std::pair<int, Size> outSize[2];
> >>  	Size maxSize;
> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> >> index b7609cb..002f8e5 100644
> >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> >> @@ -478,6 +478,11 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >>  	if (config_.empty())
> >>  		return Invalid;
> >>  
> >> +	if (transform != Transform::Identity) {
> >> +		transform = Transform::Identity;
> >> +		status = Adjusted;
> >> +	}
> >> +
> >>  	/* Cap the number of entries to the available streams. */
> >>  	if (config_.size() > 1) {
> >>  		config_.resize(1);
> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> >> index eb72e3b..10223a9 100644
> >> --- a/src/libcamera/pipeline/simple/simple.cpp
> >> +++ b/src/libcamera/pipeline/simple/simple.cpp
> >> @@ -438,6 +438,11 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
> >>  	if (config_.empty())
> >>  		return Invalid;
> >>  
> >> +	if (transform != Transform::Identity) {
> >> +		transform = Transform::Identity;
> >> +		status = Adjusted;
> >> +	}
> >> +
> >>  	/* Cap the number of entries to the available streams. */
> >>  	if (config_.size() > 1) {
> >>  		config_.resize(1);
> >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> >> index bc892ec..fd14248 100644
> >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> >> @@ -108,6 +108,11 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
> >>  	if (config_.empty())
> >>  		return Invalid;
> >>  
> >> +	if (transform != Transform::Identity) {
> >> +		transform = Transform::Identity;
> >> +		status = Adjusted;
> >> +	}
> >> +
> >>  	/* Cap the number of entries to the available streams. */
> >>  	if (config_.size() > 1) {
> >>  		config_.resize(1);
> >> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> >> index cf244f1..bb791d6 100644
> >> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> >> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> >> @@ -130,6 +130,11 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
> >>  	if (config_.empty())
> >>  		return Invalid;
> >>  
> >> +	if (transform != Transform::Identity) {
> >> +		transform = Transform::Identity;
> >> +		status = Adjusted;
> >> +	}
> >> +
> >>  	/* Cap the number of entries to the available streams. */
> >>  	if (config_.size() > 1) {
> >>  		config_.resize(1);

-- 
Regards,

Laurent Pinchart