| Message ID | 20260123125903.2469016-2-julien.vuillaumier@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2026. 01. 23. 13:59 keltezéssel, Julien Vuillaumier írta: > The CameraSensor::imageStream() function has a default implementation > in the base class that returns {0, 0} as {pad, stream}, assuming that > the image pad is located on the pad index 0. > This assumption is correct most of the time, but not in some other > cases, for instance when an external ISP entity acts as the sensor. > Such entity would typically have sink pad(s) connected to the actual > sensor and source pad(s) connected to the downstream graph. Associated > pad indexes would likely be different from zero. > > CameraSensorLegacy subclass correctly handles this case in its > functions, using the pad_ variable discovered at init() time to access > the source pad index, instead of using a hardcoded zero value. > Exception is imageStream() that is not overriden in the > CameraSensorLegacy definition so keeps the default implementation of > the parent class, hardcoding the returned source pad index to zero. > > This change declares CameraSensor::imageStream() as a pure virtual to > let the subclasses provide an implementation. Implementation for > CameraSensorLegacy is added, based on pad_ variable usage. > > Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com> > --- As far as I can see, this looks ok. It may be worth to add Fixes: 3feb4df755d5 ("libcamera: camera_sensor: Add support for embedded data") Also, I'm wondering if you could maybe attach (part of) the output of `media-ctl -p` so that the situation is clear (even just as a reply here). Regards, Barnabás Pőcze > include/libcamera/internal/camera_sensor.h | 2 +- > src/libcamera/sensor/camera_sensor.cpp | 5 +---- > src/libcamera/sensor/camera_sensor_legacy.cpp | 6 ++++++ > 3 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index e6b72d22a..58e7df4a4 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -64,7 +64,7 @@ public: > Transform transform = Transform::Identity, > V4L2SubdeviceFormat *sensorFormat = nullptr) = 0; > > - virtual V4L2Subdevice::Stream imageStream() const; > + virtual V4L2Subdevice::Stream imageStream() const = 0; > virtual std::optional<V4L2Subdevice::Stream> embeddedDataStream() const; > virtual V4L2SubdeviceFormat embeddedDataFormat() const; > virtual int setEmbeddedDataEnabled(bool enable); > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > index 05390d1e1..58affd8f5 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -200,6 +200,7 @@ CameraSensor::~CameraSensor() = default; > */ > > /** > + * \fn CameraSensor::imageStream() > * \brief Retrieve the image source stream > * > * Sensors that produce multiple streams do not guarantee that the image stream > @@ -209,10 +210,6 @@ CameraSensor::~CameraSensor() = default; > * > * \return The image source stream > */ > -V4L2Subdevice::Stream CameraSensor::imageStream() const > -{ > - return { 0, 0 }; > -} > > /** > * \brief Retrieve the embedded data source stream > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > index 39c34200b..d2583a615 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -84,6 +84,7 @@ public: > Transform transform = Transform::Identity, > V4L2SubdeviceFormat *sensorFormat = nullptr) override; > > + V4L2Subdevice::Stream imageStream() const override; > const ControlList &properties() const override { return properties_; } > int sensorInfo(IPACameraSensorInfo *info) const override; > Transform computeTransform(Orientation *orientation) const override; > @@ -856,6 +857,11 @@ int CameraSensorLegacy::applyConfiguration(const SensorConfiguration &config, > return 0; > } > > +V4L2Subdevice::Stream CameraSensorLegacy::imageStream() const > +{ > + return { pad_, 0 }; > +} > + > int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const > { > if (!bayerFormat_)
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index e6b72d22a..58e7df4a4 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -64,7 +64,7 @@ public: Transform transform = Transform::Identity, V4L2SubdeviceFormat *sensorFormat = nullptr) = 0; - virtual V4L2Subdevice::Stream imageStream() const; + virtual V4L2Subdevice::Stream imageStream() const = 0; virtual std::optional<V4L2Subdevice::Stream> embeddedDataStream() const; virtual V4L2SubdeviceFormat embeddedDataFormat() const; virtual int setEmbeddedDataEnabled(bool enable); diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp index 05390d1e1..58affd8f5 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -200,6 +200,7 @@ CameraSensor::~CameraSensor() = default; */ /** + * \fn CameraSensor::imageStream() * \brief Retrieve the image source stream * * Sensors that produce multiple streams do not guarantee that the image stream @@ -209,10 +210,6 @@ CameraSensor::~CameraSensor() = default; * * \return The image source stream */ -V4L2Subdevice::Stream CameraSensor::imageStream() const -{ - return { 0, 0 }; -} /** * \brief Retrieve the embedded data source stream diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index 39c34200b..d2583a615 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -84,6 +84,7 @@ public: Transform transform = Transform::Identity, V4L2SubdeviceFormat *sensorFormat = nullptr) override; + V4L2Subdevice::Stream imageStream() const override; const ControlList &properties() const override { return properties_; } int sensorInfo(IPACameraSensorInfo *info) const override; Transform computeTransform(Orientation *orientation) const override; @@ -856,6 +857,11 @@ int CameraSensorLegacy::applyConfiguration(const SensorConfiguration &config, return 0; } +V4L2Subdevice::Stream CameraSensorLegacy::imageStream() const +{ + return { pad_, 0 }; +} + int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const { if (!bayerFormat_)
The CameraSensor::imageStream() function has a default implementation in the base class that returns {0, 0} as {pad, stream}, assuming that the image pad is located on the pad index 0. This assumption is correct most of the time, but not in some other cases, for instance when an external ISP entity acts as the sensor. Such entity would typically have sink pad(s) connected to the actual sensor and source pad(s) connected to the downstream graph. Associated pad indexes would likely be different from zero. CameraSensorLegacy subclass correctly handles this case in its functions, using the pad_ variable discovered at init() time to access the source pad index, instead of using a hardcoded zero value. Exception is imageStream() that is not overriden in the CameraSensorLegacy definition so keeps the default implementation of the parent class, hardcoding the returned source pad index to zero. This change declares CameraSensor::imageStream() as a pure virtual to let the subclasses provide an implementation. Implementation for CameraSensorLegacy is added, based on pad_ variable usage. Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com> --- include/libcamera/internal/camera_sensor.h | 2 +- src/libcamera/sensor/camera_sensor.cpp | 5 +---- src/libcamera/sensor/camera_sensor_legacy.cpp | 6 ++++++ 3 files changed, 8 insertions(+), 5 deletions(-)