| Message ID | 20251002080952.3077149-1-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2025-10-02 09:09:22) > The PixelArraySize property is incorrectly deduced from > V4L2_SEL_TGT_CROP_BOUNDS instead of V4L2_SEL_TGT_NATIVE_SIZE. This is > sometimes the same value, but for newer sensor drivers > V4L2_SEL_TGT_NATIVE_SIZE is often larger than crop bounds. Most sensor > drivers already support V4L2_SEL_TGT_NATIVE_SIZE and for the ones that > don't, the fallback to the largest supported size is acceptable. I don't see where the fallback happens if V4L2_SEL_TGT_NATIVE_SIZE isn't available on the sensor. Is that target always guaranteed now to be provided ? Or do we mandate it now ? > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > > Hi all, > > On the imx335 we had an issue that we got the following reports: > PixelArrayActiveAreas: [ (36, 50)/2624x1944 ] > PixelArraySize: (2624,1944) > > As you can see, the PixelArraySize doesn't hold the active area which > must be the case according to https://www.libcamera.org/api-html/namespacelibcamera_1_1properties.html#acec5675f79b6c456aca72c7532a263a4 > > The native_size reported by the driver is [ (0, 0)/2696x2044 ] I think we should be reporting this indeed to be correct. > So I think this is the right thing to do :-) > Could you verify that this doesn't negatively impact the ScalerCropMaximum handling ? > Best regards, > Stefan > > > --- > src/libcamera/sensor/camera_sensor_legacy.cpp | 2 +- > src/libcamera/sensor/camera_sensor_raw.cpp | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > index a84f084ceeeb..c0adc393950f 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -399,7 +399,7 @@ int CameraSensorLegacy::validateSensorDriver() > * test platforms have been updated. > */ > Rectangle rect; > - int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect); > + int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_NATIVE_SIZE, &rect); > if (ret) { > /* > * Default the pixel array size to the largest size supported > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > index 759cccafe4a9..a975028a5e70 100644 > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > @@ -420,10 +420,10 @@ std::optional<int> CameraSensorRaw::init() > */ > > Rectangle rect; > - ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_CROP_BOUNDS, > + ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_NATIVE_SIZE, Both of these remove the crop bounds. > &rect); > if (ret) { > - LOG(CameraSensor, Error) << "No pixel array crop bounds"; > + LOG(CameraSensor, Error) << "No pixel array native size"; > return { ret }; > } > > -- > 2.48.1 >
Quoting Kieran Bingham (2025-10-02 10:36:42) > Quoting Stefan Klug (2025-10-02 09:09:22) > > The PixelArraySize property is incorrectly deduced from > > V4L2_SEL_TGT_CROP_BOUNDS instead of V4L2_SEL_TGT_NATIVE_SIZE. This is > > sometimes the same value, but for newer sensor drivers > > V4L2_SEL_TGT_NATIVE_SIZE is often larger than crop bounds. Most sensor > > drivers already support V4L2_SEL_TGT_NATIVE_SIZE and for the ones that > > don't, the fallback to the largest supported size is acceptable. > > I don't see where the fallback happens if V4L2_SEL_TGT_NATIVE_SIZE isn't > available on the sensor. Is that target always guaranteed now to be > provided ? Or do we mandate it now ? > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > > > Hi all, > > > > On the imx335 we had an issue that we got the following reports: > > PixelArrayActiveAreas: [ (36, 50)/2624x1944 ] > > PixelArraySize: (2624,1944) > > > > As you can see, the PixelArraySize doesn't hold the active area which > > must be the case according to https://www.libcamera.org/api-html/namespacelibcamera_1_1properties.html#acec5675f79b6c456aca72c7532a263a4 > > > > The native_size reported by the driver is [ (0, 0)/2696x2044 ] > > I think we should be reporting this indeed to be correct. > > > So I think this is the right thing to do :-) > > > > Could you verify that this doesn't negatively impact the > ScalerCropMaximum handling ? > > > Best regards, > > Stefan > > > > > > --- > > src/libcamera/sensor/camera_sensor_legacy.cpp | 2 +- > > src/libcamera/sensor/camera_sensor_raw.cpp | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > > index a84f084ceeeb..c0adc393950f 100644 > > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > > @@ -399,7 +399,7 @@ int CameraSensorLegacy::validateSensorDriver() > > * test platforms have been updated. > > */ > > Rectangle rect; > > - int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect); > > + int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_NATIVE_SIZE, &rect); > > if (ret) { > > /* > > * Default the pixel array size to the largest size supported > > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > > index 759cccafe4a9..a975028a5e70 100644 > > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > > @@ -420,10 +420,10 @@ std::optional<int> CameraSensorRaw::init() > > */ > > > > Rectangle rect; > > - ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_CROP_BOUNDS, > > + ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_NATIVE_SIZE, > > Both of these remove the crop bounds. This was supposed to say: Both of these remove the crop bounds. Do we need that one for anything else now ? Should we report each rectangle in the sensor properties ? > > > &rect); > > if (ret) { > > - LOG(CameraSensor, Error) << "No pixel array crop bounds"; > > + LOG(CameraSensor, Error) << "No pixel array native size"; > > return { ret }; > > } > > > > -- > > 2.48.1 > >
diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index a84f084ceeeb..c0adc393950f 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -399,7 +399,7 @@ int CameraSensorLegacy::validateSensorDriver() * test platforms have been updated. */ Rectangle rect; - int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect); + int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_NATIVE_SIZE, &rect); if (ret) { /* * Default the pixel array size to the largest size supported diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp index 759cccafe4a9..a975028a5e70 100644 --- a/src/libcamera/sensor/camera_sensor_raw.cpp +++ b/src/libcamera/sensor/camera_sensor_raw.cpp @@ -420,10 +420,10 @@ std::optional<int> CameraSensorRaw::init() */ Rectangle rect; - ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_CROP_BOUNDS, + ret = subdev_->getSelection(streams_.image.sink, V4L2_SEL_TGT_NATIVE_SIZE, &rect); if (ret) { - LOG(CameraSensor, Error) << "No pixel array crop bounds"; + LOG(CameraSensor, Error) << "No pixel array native size"; return { ret }; }
The PixelArraySize property is incorrectly deduced from V4L2_SEL_TGT_CROP_BOUNDS instead of V4L2_SEL_TGT_NATIVE_SIZE. This is sometimes the same value, but for newer sensor drivers V4L2_SEL_TGT_NATIVE_SIZE is often larger than crop bounds. Most sensor drivers already support V4L2_SEL_TGT_NATIVE_SIZE and for the ones that don't, the fallback to the largest supported size is acceptable. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- Hi all, On the imx335 we had an issue that we got the following reports: PixelArrayActiveAreas: [ (36, 50)/2624x1944 ] PixelArraySize: (2624,1944) As you can see, the PixelArraySize doesn't hold the active area which must be the case according to https://www.libcamera.org/api-html/namespacelibcamera_1_1properties.html#acec5675f79b6c456aca72c7532a263a4 The native_size reported by the driver is [ (0, 0)/2696x2044 ] So I think this is the right thing to do :-) Best regards, Stefan --- src/libcamera/sensor/camera_sensor_legacy.cpp | 2 +- src/libcamera/sensor/camera_sensor_raw.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)