| Message ID | 20200806163639.12971-4-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for the patch. On Thu, Aug 06, 2020 at 05:36:37PM +0100, David Plowman wrote: > The Raspberry Pi pipeline handler allows all transforms except those > involving a transpose. The user transform is combined with any > inherent rotation of the camera, and the camera's H and V flip bits > are set accordingly. > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 20 +++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 236aa5c..9d183e3 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -324,6 +324,8 @@ public: > uint32_t expectedSequence_; > bool sensorMetadata_; > > + Transform transform_; > + > /* > * All the functions in this class are called from a single calling > * thread. So, we do not need to have any mutex to protect access to any > @@ -400,8 +402,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > - if (transform != Transform::Identity) { > - transform = Transform::Identity; > + /* We cannot do Transforms with a transpose in them. */ > + if (!!(transform & Transform::Transpose)) { > + transform = transform ^ Transform::Transpose; I wonder if we should define a ~ operator to be able to write transform = transform & ~Transform::Transpose; And maybe also making the following possible ? transform &= ~Transform::Transpose; > status = Adjusted; > } > > @@ -610,6 +613,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > for (auto const stream : data->streams_) > stream->reset(); > > + /* We will want to know the transform requested by the application. */ > + data->transform_ = config->transform; > + > Size maxSize, sensorSize; > unsigned int maxIndex = 0; > bool rawStream = false; > @@ -1174,8 +1180,14 @@ int RPiCameraData::configureIPA() > /* Configure the H/V flip controls based on the sensor rotation. */ > ControlList ctrls(unicam_[Unicam::Image].dev()->controls()); > int32_t rotation = sensor_->properties().get(properties::Rotation); > - ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation)); > - ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation)); > + bool success; > + Transform combined = transform_ * transformFromRotation(rotation, &success); > + if (!success) { > + LOG(RPI, Error) << "Invalid rotation: " << rotation; > + return -EINVAL; > + } > + ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!(combined & Transform::HFlip))); > + ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!(combined & Transform::VFlip))); Shouldn't this code guard against a 90° or 270° rotation ? Actually, thinking about it, what if the sensor is mounted with a 90° rotation, and the user is fine with that ? How should we combine that with the user transform to make it possible to capture images ? > unicam_[Unicam::Image].dev()->setControls(&ctrls); > } >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 236aa5c..9d183e3 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -324,6 +324,8 @@ public: uint32_t expectedSequence_; bool sensorMetadata_; + Transform transform_; + /* * All the functions in this class are called from a single calling * thread. So, we do not need to have any mutex to protect access to any @@ -400,8 +402,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() if (config_.empty()) return Invalid; - if (transform != Transform::Identity) { - transform = Transform::Identity; + /* We cannot do Transforms with a transpose in them. */ + if (!!(transform & Transform::Transpose)) { + transform = transform ^ Transform::Transpose; status = Adjusted; } @@ -610,6 +613,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) for (auto const stream : data->streams_) stream->reset(); + /* We will want to know the transform requested by the application. */ + data->transform_ = config->transform; + Size maxSize, sensorSize; unsigned int maxIndex = 0; bool rawStream = false; @@ -1174,8 +1180,14 @@ int RPiCameraData::configureIPA() /* Configure the H/V flip controls based on the sensor rotation. */ ControlList ctrls(unicam_[Unicam::Image].dev()->controls()); int32_t rotation = sensor_->properties().get(properties::Rotation); - ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation)); - ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation)); + bool success; + Transform combined = transform_ * transformFromRotation(rotation, &success); + if (!success) { + LOG(RPI, Error) << "Invalid rotation: " << rotation; + return -EINVAL; + } + ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!(combined & Transform::HFlip))); + ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!(combined & Transform::VFlip))); unicam_[Unicam::Image].dev()->setControls(&ctrls); }