| Message ID | 20250930122726.1837524-24-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 09. 30. 14:26 keltezéssel, Stefan Klug írta: > The dw100 allows more features implemented in the dw100 vertex map. > Implement these features for the rkisp1 pipeline. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > 2 files changed, 85 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > index 03309eeac34f..10ee68db6d8c 100644 > --- a/src/libcamera/control_ids_draft.yaml > +++ b/src/libcamera/control_ids_draft.yaml > @@ -206,7 +206,44 @@ controls: > available only on this camera device are at least this numeric > value. All of the custom test patterns will be static (that is the > raw image must not vary from frame to frame). > - > + - Dw100ScaleMode: > + type: int32_t > + direction: inout > + description: | > + Scale mode of the dewarper. > + enum: Wrt. https://patchwork.libcamera.org/patch/24496/ could you prefix the names of the enumerators with exactly the name of the control? Regards, Barnabás Pőcze > + - name: Fill > + value: 0 > + description: | > + Fills the given output size with the largest rectangle possible. > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > + ignored. > + - name: Crop > + value: 1 > + description: | > + Crops to the given output size. The scale factor can be specified > + using Dw100Scale. Aspect ratio is preserved. > + - Dw100Scale: > + type: float > + direction: inout > + description: | > + Scale factor applied to the image when Dw100ScaleMode is set to Crop. > + This value is clamped, so that all pixels have valid input data. > + Therefore a value of 0 always provides the largest possible field of > + view. > + - Dw100Rotation: > + type: float > + direction: inout > + description: | > + Rotates the image by the given angle in degrees. > + - Dw100Offset: > + type: Point > + direction: inout > + description: | > + Moves the image by the given values in x and y direction in output > + coordinate space. This is clamped, so that all output pixels contain > + valid data. The offset is therefore ignored when Dw100ScaleMode is set > + to 'Fit' or the Dw100Scale value is too small. > - FaceDetectMode: > type: int32_t > direction: inout > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 8b78c7f213f6..740791ac9c02 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1364,6 +1364,17 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data) > scalerMaxCrop_); > data->properties_.set(properties::ScalerCropMaximum, scalerMaxCrop_); > activeCrop_ = scalerMaxCrop_; > + > + if (dewarper_->supportsRequests()) { > + controls[&controls::draft::Dw100Scale] = ControlInfo(0.2f, 8.0f, 1.0f); > + controls[&controls::draft::Dw100Rotation] = ControlInfo(-180.0f, 180.0f, 0.0f); > + controls[&controls::draft::Dw100Offset] = ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0)); > + controls[&controls::draft::Dw100ScaleMode] = ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill); > + } else { > + LOG(RkISP1, Warning) > + << "dw100 kernel driver has no requests support." > + " No dynamic configuration possible."; > + } > } > > /* Add the IPA registered controls to list of camera controls. */ > @@ -1637,6 +1648,37 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > availableDewarpRequests_.pop(); > } > > + bool update = false; > + auto &vertexMap = dewarper_->vertexMap(&data->mainPathStream_); > + > + const auto &scale = request->controls().get(controls::draft::Dw100Scale); > + if (scale) { > + vertexMap.setScale(*scale); > + update = true; > + } > + > + const auto &rotation = request->controls().get(controls::draft::Dw100Rotation); > + if (rotation) { > + vertexMap.setRotation(*rotation); > + update = true; > + } > + > + const auto &offset = request->controls().get(controls::draft::Dw100Offset); > + if (offset) { > + vertexMap.setOffset(*offset); > + update = true; > + } > + > + const auto &scaleMode = request->controls().get(controls::draft::Dw100ScaleMode); > + if (scaleMode) { > + vertexMap.setMode(static_cast<Dw100VertexMap::ScaleMode>(*scaleMode)); > + update = true; > + } > + > + if (update || info->frame == 0) { > + dewarper_->applyVertexMap(&data->mainPathStream_, dewarpRequest); > + } > + > /* Handle scaler crop control. */ > const auto &crop = request->controls().get(controls::ScalerCrop); > if (crop) { > @@ -1700,7 +1742,11 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > } > } > > - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); > + auto &meta = request->metadata(); > + meta.set(controls::draft::Dw100Scale, vertexMap.effectiveScale()); > + meta.set(controls::draft::Dw100Rotation, vertexMap.rotation()); > + meta.set(controls::draft::Dw100Offset, vertexMap.effectiveOffset()); > + meta.set(controls::ScalerCrop, activeCrop_.value()); > } > > void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request) > -- > 2.48.1 >
Quoting Barnabás Pőcze (2025-09-30 14:52:34) > Hi > > 2025. 09. 30. 14:26 keltezéssel, Stefan Klug írta: > > The dw100 allows more features implemented in the dw100 vertex map. > > Implement these features for the rkisp1 pipeline. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > > 2 files changed, 85 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > > index 03309eeac34f..10ee68db6d8c 100644 > > --- a/src/libcamera/control_ids_draft.yaml > > +++ b/src/libcamera/control_ids_draft.yaml > > @@ -206,7 +206,44 @@ controls: > > available only on this camera device are at least this numeric > > value. All of the custom test patterns will be static (that is the > > raw image must not vary from frame to frame). > > - > > + - Dw100ScaleMode: > > + type: int32_t > > + direction: inout > > + description: | > > + Scale mode of the dewarper. > > + enum: > > Wrt. https://patchwork.libcamera.org/patch/24496/ could you prefix the > names of the enumerators with exactly the name of the control? Also - I'm really trying to push against ::draft:: and I'd like to see that namespace removed ... it was a conceptual idea that just leads to a dumping ground. It was also before we had vendor/namespaces for controls. If you really think these should be distinguished from core controls - can they go into a DW100 namespace ? > Regards, > Barnabás Pőcze > > > > + - name: Fill > > + value: 0 > > + description: | > > + Fills the given output size with the largest rectangle possible. > > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > > + ignored. > > + - name: Crop > > + value: 1 > > + description: | > > + Crops to the given output size. The scale factor can be specified > > + using Dw100Scale. Aspect ratio is preserved. > > + - Dw100Scale: > > + type: float > > + direction: inout > > + description: | > > + Scale factor applied to the image when Dw100ScaleMode is set to Crop. > > + This value is clamped, so that all pixels have valid input data. > > + Therefore a value of 0 always provides the largest possible field of > > + view. > > + - Dw100Rotation: > > + type: float > > + direction: inout > > + description: | > > + Rotates the image by the given angle in degrees. > > + - Dw100Offset: > > + type: Point > > + direction: inout > > + description: | > > + Moves the image by the given values in x and y direction in output > > + coordinate space. This is clamped, so that all output pixels contain > > + valid data. The offset is therefore ignored when Dw100ScaleMode is set > > + to 'Fit' or the Dw100Scale value is too small. > > - FaceDetectMode: > > type: int32_t > > direction: inout > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 8b78c7f213f6..740791ac9c02 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -1364,6 +1364,17 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data) > > scalerMaxCrop_); > > data->properties_.set(properties::ScalerCropMaximum, scalerMaxCrop_); > > activeCrop_ = scalerMaxCrop_; > > + > > + if (dewarper_->supportsRequests()) { > > + controls[&controls::draft::Dw100Scale] = ControlInfo(0.2f, 8.0f, 1.0f); > > + controls[&controls::draft::Dw100Rotation] = ControlInfo(-180.0f, 180.0f, 0.0f); > > + controls[&controls::draft::Dw100Offset] = ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0)); > > + controls[&controls::draft::Dw100ScaleMode] = ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill); > > + } else { > > + LOG(RkISP1, Warning) > > + << "dw100 kernel driver has no requests support." > > + " No dynamic configuration possible."; > > + } > > } > > > > /* Add the IPA registered controls to list of camera controls. */ > > @@ -1637,6 +1648,37 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > > availableDewarpRequests_.pop(); > > } > > > > + bool update = false; > > + auto &vertexMap = dewarper_->vertexMap(&data->mainPathStream_); > > + > > + const auto &scale = request->controls().get(controls::draft::Dw100Scale); > > + if (scale) { > > + vertexMap.setScale(*scale); > > + update = true; > > + } > > + > > + const auto &rotation = request->controls().get(controls::draft::Dw100Rotation); > > + if (rotation) { > > + vertexMap.setRotation(*rotation); > > + update = true; > > + } > > + > > + const auto &offset = request->controls().get(controls::draft::Dw100Offset); > > + if (offset) { > > + vertexMap.setOffset(*offset); > > + update = true; > > + } > > + > > + const auto &scaleMode = request->controls().get(controls::draft::Dw100ScaleMode); > > + if (scaleMode) { > > + vertexMap.setMode(static_cast<Dw100VertexMap::ScaleMode>(*scaleMode)); > > + update = true; > > + } > > + > > + if (update || info->frame == 0) { > > + dewarper_->applyVertexMap(&data->mainPathStream_, dewarpRequest); > > + } > > + > > /* Handle scaler crop control. */ > > const auto &crop = request->controls().get(controls::ScalerCrop); > > if (crop) { > > @@ -1700,7 +1742,11 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > > } > > } > > > > - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); > > + auto &meta = request->metadata(); > > + meta.set(controls::draft::Dw100Scale, vertexMap.effectiveScale()); > > + meta.set(controls::draft::Dw100Rotation, vertexMap.rotation()); > > + meta.set(controls::draft::Dw100Offset, vertexMap.effectiveOffset()); > > + meta.set(controls::ScalerCrop, activeCrop_.value()); > > } > > > > void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request) > > -- > > 2.48.1 > > >
Hi Stefan, Sorry for jumping in here... :) On Tue, 30 Sept 2025 at 14:22, Stefan Klug <stefan.klug@ideasonboard.com> wrote: > The dw100 allows more features implemented in the dw100 vertex map. > Implement these features for the rkisp1 pipeline. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > 2 files changed, 85 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/control_ids_draft.yaml > b/src/libcamera/control_ids_draft.yaml > index 03309eeac34f..10ee68db6d8c 100644 > --- a/src/libcamera/control_ids_draft.yaml > +++ b/src/libcamera/control_ids_draft.yaml > @@ -206,7 +206,44 @@ controls: > available only on this camera device are at least this numeric > value. All of the custom test patterns will be static (that > is the > raw image must not vary from frame to frame). > - > + - Dw100ScaleMode: > + type: int32_t > + direction: inout > + description: | > + Scale mode of the dewarper. > + enum: > + - name: Fill > + value: 0 > + description: | > + Fills the given output size with the largest rectangle > possible. > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > + ignored. > + - name: Crop > + value: 1 > + description: | > + Crops to the given output size. The scale factor can be > specified > + using Dw100Scale. Aspect ratio is preserved. > + - Dw100Scale: > + type: float > + direction: inout > + description: | > + Scale factor applied to the image when Dw100ScaleMode is set to > Crop. > + This value is clamped, so that all pixels have valid input data. > + Therefore a value of 0 always provides the largest possible field > of > + view. > Couldn't the above 3 be represented by the ScalerCrop control in conjunction with the stream output size? Also, I would suggest that perhaps these controls belong in a new rksip vendor namespace. Regards, Naush > + - Dw100Rotation: > + type: float > + direction: inout > + description: | > + Rotates the image by the given angle in degrees. > + - Dw100Offset: > + type: Point > + direction: inout > + description: | > + Moves the image by the given values in x and y direction in > output > + coordinate space. This is clamped, so that all output pixels > contain > + valid data. The offset is therefore ignored when Dw100ScaleMode > is set > + to 'Fit' or the Dw100Scale value is too small. > - FaceDetectMode: > type: int32_t > direction: inout > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 8b78c7f213f6..740791ac9c02 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1364,6 +1364,17 @@ int > PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data) > > scalerMaxCrop_); > data->properties_.set(properties::ScalerCropMaximum, > scalerMaxCrop_); > activeCrop_ = scalerMaxCrop_; > + > + if (dewarper_->supportsRequests()) { > + controls[&controls::draft::Dw100Scale] = > ControlInfo(0.2f, 8.0f, 1.0f); > + controls[&controls::draft::Dw100Rotation] = > ControlInfo(-180.0f, 180.0f, 0.0f); > + controls[&controls::draft::Dw100Offset] = > ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0)); > + controls[&controls::draft::Dw100ScaleMode] = > ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill); > + } else { > + LOG(RkISP1, Warning) > + << "dw100 kernel driver has no requests > support." > + " No dynamic configuration possible."; > + } > } > > /* Add the IPA registered controls to list of camera controls. */ > @@ -1637,6 +1648,37 @@ void > PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > availableDewarpRequests_.pop(); > } > > + bool update = false; > + auto &vertexMap = dewarper_->vertexMap(&data->mainPathStream_); > + > + const auto &scale = > request->controls().get(controls::draft::Dw100Scale); > + if (scale) { > + vertexMap.setScale(*scale); > + update = true; > + } > + > + const auto &rotation = > request->controls().get(controls::draft::Dw100Rotation); > + if (rotation) { > + vertexMap.setRotation(*rotation); > + update = true; > + } > + > + const auto &offset = > request->controls().get(controls::draft::Dw100Offset); > + if (offset) { > + vertexMap.setOffset(*offset); > + update = true; > + } > + > + const auto &scaleMode = > request->controls().get(controls::draft::Dw100ScaleMode); > + if (scaleMode) { > + > vertexMap.setMode(static_cast<Dw100VertexMap::ScaleMode>(*scaleMode)); > + update = true; > + } > + > + if (update || info->frame == 0) { > + dewarper_->applyVertexMap(&data->mainPathStream_, > dewarpRequest); > + } > + > /* Handle scaler crop control. */ > const auto &crop = request->controls().get(controls::ScalerCrop); > if (crop) { > @@ -1700,7 +1742,11 @@ void > PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > } > } > > - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); > + auto &meta = request->metadata(); > + meta.set(controls::draft::Dw100Scale, vertexMap.effectiveScale()); > + meta.set(controls::draft::Dw100Rotation, vertexMap.rotation()); > + meta.set(controls::draft::Dw100Offset, > vertexMap.effectiveOffset()); > + meta.set(controls::ScalerCrop, activeCrop_.value()); > } > > void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request) > -- > 2.48.1 > >
Quoting Naushir Patuck (2025-10-03 15:31:20) > Hi Stefan, > > Sorry for jumping in here... :) I think jumping in is always desired ;-) Thanks for reviewing! > On Tue, 30 Sept 2025 at 14:22, Stefan Klug <stefan.klug@ideasonboard.com> > wrote: > > > The dw100 allows more features implemented in the dw100 vertex map. > > Implement these features for the rkisp1 pipeline. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > > 2 files changed, 85 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/control_ids_draft.yaml > > b/src/libcamera/control_ids_draft.yaml > > index 03309eeac34f..10ee68db6d8c 100644 > > --- a/src/libcamera/control_ids_draft.yaml > > +++ b/src/libcamera/control_ids_draft.yaml > > @@ -206,7 +206,44 @@ controls: > > available only on this camera device are at least this numeric > > value. All of the custom test patterns will be static (that > > is the > > raw image must not vary from frame to frame). > > - > > + - Dw100ScaleMode: > > + type: int32_t > > + direction: inout > > + description: | > > + Scale mode of the dewarper. > > + enum: > > + - name: Fill > > + value: 0 > > + description: | > > + Fills the given output size with the largest rectangle > > possible. > > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > > + ignored. > > + - name: Crop > > + value: 1 > > + description: | > > + Crops to the given output size. The scale factor can be > > specified > > + using Dw100Scale. Aspect ratio is preserved. > > + - Dw100Scale: > > + type: float > > + direction: inout > > + description: | > > + Scale factor applied to the image when Dw100ScaleMode is set to > > Crop. > > + This value is clamped, so that all pixels have valid input data. > > + Therefore a value of 0 always provides the largest possible field > > of > > + view. > > > > Couldn't the above 3 be represented by the ScalerCrop control in > conjunction with the stream output size? > > Also, I would suggest that perhaps these controls belong in a new > rksip vendor namespace. With a dw100 prefix a namespace indeed definitely makes sense. But if we can model this already on a ScalerCrop (or akin to that if we need something extra) ... is there a way we can model these controls generically so that it could be used by multiple platforms (or even a GPU based dewarper that could run on any platform which I bet would be fairly easy to construct once we have the GPU objects in from the GPUISP?). I don't think anything the DW100 is 'specific only to that exact instance of hardware' ? -- Kieran
Hi Kieran, Quoting Kieran Bingham (2025-10-03 16:14:43) > Quoting Barnabás Pőcze (2025-09-30 14:52:34) > > Hi > > > > 2025. 09. 30. 14:26 keltezéssel, Stefan Klug írta: > > > The dw100 allows more features implemented in the dw100 vertex map. > > > Implement these features for the rkisp1 pipeline. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > > > 2 files changed, 85 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml > > > index 03309eeac34f..10ee68db6d8c 100644 > > > --- a/src/libcamera/control_ids_draft.yaml > > > +++ b/src/libcamera/control_ids_draft.yaml > > > @@ -206,7 +206,44 @@ controls: > > > available only on this camera device are at least this numeric > > > value. All of the custom test patterns will be static (that is the > > > raw image must not vary from frame to frame). > > > - > > > + - Dw100ScaleMode: > > > + type: int32_t > > > + direction: inout > > > + description: | > > > + Scale mode of the dewarper. > > > + enum: > > > > Wrt. https://patchwork.libcamera.org/patch/24496/ could you prefix the > > names of the enumerators with exactly the name of the control? > > Also - I'm really trying to push against ::draft:: and I'd like to see > that namespace removed ... it was a conceptual idea that just leads to a > dumping ground. It was also before we had vendor/namespaces for > controls. Yes, I know. I was thinking about moving them to core before posting v1 but then decided against it because I expected some discussions around the controls. I totally agree with you that draft doesn't help much. > > If you really think these should be distinguished from core controls - > can they go into a DW100 namespace ? That feels a bit small for an own namespace. I'd be fine with core as soon as we decided on the exact names. Best regards, Stefan > > > Regards, > > Barnabás Pőcze > > > > > > > + - name: Fill > > > + value: 0 > > > + description: | > > > + Fills the given output size with the largest rectangle possible. > > > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > > > + ignored. > > > + - name: Crop > > > + value: 1 > > > + description: | > > > + Crops to the given output size. The scale factor can be specified > > > + using Dw100Scale. Aspect ratio is preserved. > > > + - Dw100Scale: > > > + type: float > > > + direction: inout > > > + description: | > > > + Scale factor applied to the image when Dw100ScaleMode is set to Crop. > > > + This value is clamped, so that all pixels have valid input data. > > > + Therefore a value of 0 always provides the largest possible field of > > > + view. > > > + - Dw100Rotation: > > > + type: float > > > + direction: inout > > > + description: | > > > + Rotates the image by the given angle in degrees. > > > + - Dw100Offset: > > > + type: Point > > > + direction: inout > > > + description: | > > > + Moves the image by the given values in x and y direction in output > > > + coordinate space. This is clamped, so that all output pixels contain > > > + valid data. The offset is therefore ignored when Dw100ScaleMode is set > > > + to 'Fit' or the Dw100Scale value is too small. > > > - FaceDetectMode: > > > type: int32_t > > > direction: inout > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > index 8b78c7f213f6..740791ac9c02 100644 > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > @@ -1364,6 +1364,17 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data) > > > scalerMaxCrop_); > > > data->properties_.set(properties::ScalerCropMaximum, scalerMaxCrop_); > > > activeCrop_ = scalerMaxCrop_; > > > + > > > + if (dewarper_->supportsRequests()) { > > > + controls[&controls::draft::Dw100Scale] = ControlInfo(0.2f, 8.0f, 1.0f); > > > + controls[&controls::draft::Dw100Rotation] = ControlInfo(-180.0f, 180.0f, 0.0f); > > > + controls[&controls::draft::Dw100Offset] = ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0)); > > > + controls[&controls::draft::Dw100ScaleMode] = ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill); > > > + } else { > > > + LOG(RkISP1, Warning) > > > + << "dw100 kernel driver has no requests support." > > > + " No dynamic configuration possible."; > > > + } > > > } > > > > > > /* Add the IPA registered controls to list of camera controls. */ > > > @@ -1637,6 +1648,37 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > > > availableDewarpRequests_.pop(); > > > } > > > > > > + bool update = false; > > > + auto &vertexMap = dewarper_->vertexMap(&data->mainPathStream_); > > > + > > > + const auto &scale = request->controls().get(controls::draft::Dw100Scale); > > > + if (scale) { > > > + vertexMap.setScale(*scale); > > > + update = true; > > > + } > > > + > > > + const auto &rotation = request->controls().get(controls::draft::Dw100Rotation); > > > + if (rotation) { > > > + vertexMap.setRotation(*rotation); > > > + update = true; > > > + } > > > + > > > + const auto &offset = request->controls().get(controls::draft::Dw100Offset); > > > + if (offset) { > > > + vertexMap.setOffset(*offset); > > > + update = true; > > > + } > > > + > > > + const auto &scaleMode = request->controls().get(controls::draft::Dw100ScaleMode); > > > + if (scaleMode) { > > > + vertexMap.setMode(static_cast<Dw100VertexMap::ScaleMode>(*scaleMode)); > > > + update = true; > > > + } > > > + > > > + if (update || info->frame == 0) { > > > + dewarper_->applyVertexMap(&data->mainPathStream_, dewarpRequest); > > > + } > > > + > > > /* Handle scaler crop control. */ > > > const auto &crop = request->controls().get(controls::ScalerCrop); > > > if (crop) { > > > @@ -1700,7 +1742,11 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > > > } > > > } > > > > > > - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); > > > + auto &meta = request->metadata(); > > > + meta.set(controls::draft::Dw100Scale, vertexMap.effectiveScale()); > > > + meta.set(controls::draft::Dw100Rotation, vertexMap.rotation()); > > > + meta.set(controls::draft::Dw100Offset, vertexMap.effectiveOffset()); > > > + meta.set(controls::ScalerCrop, activeCrop_.value()); > > > } > > > > > > void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request) > > > -- > > > 2.48.1 > > > > >
Hi Naush, hi Kieran, Quoting Kieran Bingham (2025-10-04 12:15:44) > Quoting Naushir Patuck (2025-10-03 15:31:20) > > Hi Stefan, > > > > Sorry for jumping in here... :) > > I think jumping in is always desired ;-) Thanks for reviewing! I can only consent here. Feedback on this is very valuable. > > > On Tue, 30 Sept 2025 at 14:22, Stefan Klug <stefan.klug@ideasonboard.com> > > wrote: > > > > > The dw100 allows more features implemented in the dw100 vertex map. > > > Implement these features for the rkisp1 pipeline. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > > > 2 files changed, 85 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/libcamera/control_ids_draft.yaml > > > b/src/libcamera/control_ids_draft.yaml > > > index 03309eeac34f..10ee68db6d8c 100644 > > > --- a/src/libcamera/control_ids_draft.yaml > > > +++ b/src/libcamera/control_ids_draft.yaml > > > @@ -206,7 +206,44 @@ controls: > > > available only on this camera device are at least this numeric > > > value. All of the custom test patterns will be static (that > > > is the > > > raw image must not vary from frame to frame). > > > - > > > + - Dw100ScaleMode: > > > + type: int32_t > > > + direction: inout > > > + description: | > > > + Scale mode of the dewarper. > > > + enum: > > > + - name: Fill > > > + value: 0 > > > + description: | > > > + Fills the given output size with the largest rectangle > > > possible. > > > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > > > + ignored. > > > + - name: Crop > > > + value: 1 > > > + description: | > > > + Crops to the given output size. The scale factor can be > > > specified > > > + using Dw100Scale. Aspect ratio is preserved. > > > + - Dw100Scale: > > > + type: float > > > + direction: inout > > > + description: | > > > + Scale factor applied to the image when Dw100ScaleMode is set to > > > Crop. > > > + This value is clamped, so that all pixels have valid input data. > > > + Therefore a value of 0 always provides the largest possible field > > > of > > > + view. > > > > > > > Couldn't the above 3 be represented by the ScalerCrop control in > > conjunction with the stream output size? The problem here is that ScalerCrop is in integer values. We gave it a try to use it for digital zoom and you see the rounding artifacts while zooming. The center of the image jumps around visibly. That could be improved by making ScalerCrop use float rectangles which we don't have and which would have a larger impact on libcamera. The other issue is that ScalerCrop doesn't carry any rotation information. So from a theoretical point I'd like to model a mostly complete 2D transformation matrix. That is (translate(x,y), rotate, scale(x,y) and shear(x,y)) For the sake of usability I dropped shear and made scale a single value. By applying that transformation matrix after the ScalerCrop, ScalerCrop is still fully functional but we can have more control "on top". > > > > Also, I would suggest that perhaps these controls belong in a new > > rksip vendor namespace. > > With a dw100 prefix a namespace indeed definitely makes sense. > > But if we can model this already on a ScalerCrop (or akin to that if we > need something extra) ... is there a way we can model these controls > generically so that it could be used by multiple platforms (or even a > GPU based dewarper that could run on any platform which I bet would be > fairly easy to construct once we have the GPU objects in from the > GPUISP?). As noted above, the most generic would be a 2x3 matrix (the upper rows of the 2D transform https://graphicmaths.com/pure/matrices/matrix-2d-transformations/ ). But using a matrix from a user point of view is a nightmare. So I thought rotation + scale + translate all around the center of the image makes it pretty easy to use. So I think the controls are already generic. For upcoming usecases we can easily add shear (x,y) which defaults to 0 so that it is backwards compatible. To add the second scale in a backwards compatible way, I'd define a Dw100ScalingRatio. So that: scale_x = Dw100Scale scale_y = Dw100ScalingRatio * Dw100Scale This way we can keep the simple "one scale value" interface while allowing more complex usecases with the additional ratio. All this is quite generic and could already be implemented using a GPU. What about naming the controls in a generic way? My proposal: TransformFillMode (fits the function better than ScaleMode) TransformScale TransformRotation TranformOffset (maybe this should be named TransformTranslation) later: TransformShear TransformScaleRatio Best regards, Stefan > > I don't think anything the DW100 is 'specific only to that exact > instance of hardware' ? > > -- > Kieran
Hi Stefan, On Mon, 6 Oct 2025 at 10:03, Stefan Klug <stefan.klug@ideasonboard.com> wrote: > > Hi Naush, hi Kieran, > > Quoting Kieran Bingham (2025-10-04 12:15:44) > > Quoting Naushir Patuck (2025-10-03 15:31:20) > > > Hi Stefan, > > > > > > Sorry for jumping in here... :) > > > > I think jumping in is always desired ;-) Thanks for reviewing! > > I can only consent here. Feedback on this is very valuable. > > > > > > On Tue, 30 Sept 2025 at 14:22, Stefan Klug <stefan.klug@ideasonboard.com> > > > wrote: > > > > > > > The dw100 allows more features implemented in the dw100 vertex map. > > > > Implement these features for the rkisp1 pipeline. > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- > > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- > > > > 2 files changed, 85 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/src/libcamera/control_ids_draft.yaml > > > > b/src/libcamera/control_ids_draft.yaml > > > > index 03309eeac34f..10ee68db6d8c 100644 > > > > --- a/src/libcamera/control_ids_draft.yaml > > > > +++ b/src/libcamera/control_ids_draft.yaml > > > > @@ -206,7 +206,44 @@ controls: > > > > available only on this camera device are at least this numeric > > > > value. All of the custom test patterns will be static (that > > > > is the > > > > raw image must not vary from frame to frame). > > > > - > > > > + - Dw100ScaleMode: > > > > + type: int32_t > > > > + direction: inout > > > > + description: | > > > > + Scale mode of the dewarper. > > > > + enum: > > > > + - name: Fill > > > > + value: 0 > > > > + description: | > > > > + Fills the given output size with the largest rectangle > > > > possible. > > > > + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are > > > > + ignored. > > > > + - name: Crop > > > > + value: 1 > > > > + description: | > > > > + Crops to the given output size. The scale factor can be > > > > specified > > > > + using Dw100Scale. Aspect ratio is preserved. > > > > + - Dw100Scale: > > > > + type: float > > > > + direction: inout > > > > + description: | > > > > + Scale factor applied to the image when Dw100ScaleMode is set to > > > > Crop. > > > > + This value is clamped, so that all pixels have valid input data. > > > > + Therefore a value of 0 always provides the largest possible field > > > > of > > > > + view. > > > > > > > > > > Couldn't the above 3 be represented by the ScalerCrop control in > > > conjunction with the stream output size? > > The problem here is that ScalerCrop is in integer values. We gave it a > try to use it for digital zoom and you see the rounding artifacts while > zooming. The center of the image jumps around visibly. This surprises me a bit. We perform smooth(ish) zoom with integer precision in single pixel steps without issue. Could it be some other calculation rounding artefact? > > > That could be improved by making ScalerCrop use float rectangles which we > don't have and which would have a larger impact on libcamera. I'm fine with changing this if folks agree it's the right thing. > > > The other issue is that ScalerCrop doesn't carry any rotation > information. So from a theoretical point I'd like to model a mostly > complete 2D transformation matrix. That is (translate(x,y), rotate, > scale(x,y) and shear(x,y)) For the sake of usability I dropped shear and > made scale a single value. > > By applying that transformation matrix after the ScalerCrop, ScalerCrop > is still fully functional but we can have more control "on top". Yes, ScalerCrop will have to be complemented with an additional control for rotation to be specified. However, I think that's a better approach (if workable of course) than adding more controls with similar behavior to existing ones. Regards, Naush > > > > > > > > Also, I would suggest that perhaps these controls belong in a new > > > rksip vendor namespace. > > > > With a dw100 prefix a namespace indeed definitely makes sense. > > > > But if we can model this already on a ScalerCrop (or akin to that if we > > need something extra) ... is there a way we can model these controls > > generically so that it could be used by multiple platforms (or even a > > GPU based dewarper that could run on any platform which I bet would be > > fairly easy to construct once we have the GPU objects in from the > > GPUISP?). > > As noted above, the most generic would be a 2x3 matrix (the upper rows > of the 2D transform > https://graphicmaths.com/pure/matrices/matrix-2d-transformations/ ). > But using a matrix from a user point of view is a nightmare. So I > thought rotation + scale + translate all around the center of the image > makes it pretty easy to use. So I think the controls are already > generic. For upcoming usecases we can easily add shear (x,y) which > defaults to 0 so that it is backwards compatible. To add the second > scale in a backwards compatible way, I'd define a Dw100ScalingRatio. > > So that: > scale_x = Dw100Scale > scale_y = Dw100ScalingRatio * Dw100Scale > > This way we can keep the simple "one scale value" interface while > allowing more complex usecases with the additional ratio. > > All this is quite generic and could already be implemented using a GPU. > > What about naming the controls in a generic way? > My proposal: > TransformFillMode (fits the function better than ScaleMode) > TransformScale > TransformRotation > TranformOffset (maybe this should be named TransformTranslation) > later: > TransformShear > TransformScaleRatio > > Best regards, > Stefan > > > > > I don't think anything the DW100 is 'specific only to that exact > > instance of hardware' ? > > > > -- > > Kieran
diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml index 03309eeac34f..10ee68db6d8c 100644 --- a/src/libcamera/control_ids_draft.yaml +++ b/src/libcamera/control_ids_draft.yaml @@ -206,7 +206,44 @@ controls: available only on this camera device are at least this numeric value. All of the custom test patterns will be static (that is the raw image must not vary from frame to frame). - + - Dw100ScaleMode: + type: int32_t + direction: inout + description: | + Scale mode of the dewarper. + enum: + - name: Fill + value: 0 + description: | + Fills the given output size with the largest rectangle possible. + Aspect ratio is not preserved. Dw100Scale and Dw100Offset are + ignored. + - name: Crop + value: 1 + description: | + Crops to the given output size. The scale factor can be specified + using Dw100Scale. Aspect ratio is preserved. + - Dw100Scale: + type: float + direction: inout + description: | + Scale factor applied to the image when Dw100ScaleMode is set to Crop. + This value is clamped, so that all pixels have valid input data. + Therefore a value of 0 always provides the largest possible field of + view. + - Dw100Rotation: + type: float + direction: inout + description: | + Rotates the image by the given angle in degrees. + - Dw100Offset: + type: Point + direction: inout + description: | + Moves the image by the given values in x and y direction in output + coordinate space. This is clamped, so that all output pixels contain + valid data. The offset is therefore ignored when Dw100ScaleMode is set + to 'Fit' or the Dw100Scale value is too small. - FaceDetectMode: type: int32_t direction: inout diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 8b78c7f213f6..740791ac9c02 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1364,6 +1364,17 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data) scalerMaxCrop_); data->properties_.set(properties::ScalerCropMaximum, scalerMaxCrop_); activeCrop_ = scalerMaxCrop_; + + if (dewarper_->supportsRequests()) { + controls[&controls::draft::Dw100Scale] = ControlInfo(0.2f, 8.0f, 1.0f); + controls[&controls::draft::Dw100Rotation] = ControlInfo(-180.0f, 180.0f, 0.0f); + controls[&controls::draft::Dw100Offset] = ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0)); + controls[&controls::draft::Dw100ScaleMode] = ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill); + } else { + LOG(RkISP1, Warning) + << "dw100 kernel driver has no requests support." + " No dynamic configuration possible."; + } } /* Add the IPA registered controls to list of camera controls. */ @@ -1637,6 +1648,37 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) availableDewarpRequests_.pop(); } + bool update = false; + auto &vertexMap = dewarper_->vertexMap(&data->mainPathStream_); + + const auto &scale = request->controls().get(controls::draft::Dw100Scale); + if (scale) { + vertexMap.setScale(*scale); + update = true; + } + + const auto &rotation = request->controls().get(controls::draft::Dw100Rotation); + if (rotation) { + vertexMap.setRotation(*rotation); + update = true; + } + + const auto &offset = request->controls().get(controls::draft::Dw100Offset); + if (offset) { + vertexMap.setOffset(*offset); + update = true; + } + + const auto &scaleMode = request->controls().get(controls::draft::Dw100ScaleMode); + if (scaleMode) { + vertexMap.setMode(static_cast<Dw100VertexMap::ScaleMode>(*scaleMode)); + update = true; + } + + if (update || info->frame == 0) { + dewarper_->applyVertexMap(&data->mainPathStream_, dewarpRequest); + } + /* Handle scaler crop control. */ const auto &crop = request->controls().get(controls::ScalerCrop); if (crop) { @@ -1700,7 +1742,11 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) } } - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); + auto &meta = request->metadata(); + meta.set(controls::draft::Dw100Scale, vertexMap.effectiveScale()); + meta.set(controls::draft::Dw100Rotation, vertexMap.rotation()); + meta.set(controls::draft::Dw100Offset, vertexMap.effectiveOffset()); + meta.set(controls::ScalerCrop, activeCrop_.value()); } void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request)
The dw100 allows more features implemented in the dw100 vertex map. Implement these features for the rkisp1 pipeline. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/libcamera/control_ids_draft.yaml | 39 ++++++++++++++++++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 48 +++++++++++++++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-)