| Message ID | 20241218142217.437842-1-stanislaw.gruszka@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi, On 18-Dec-24 3:22 PM, Stanislaw Gruszka wrote: > Currently we use frame start event from video capture device to > apply controls. But the capture device might not generate the events. > Usually CSI-2 receiver is proper device to subscribe for start > frame events. > > Without DelayedConntrols:applyControls() is possible that we can get > call to DelayedControls::get() with frame number that exceed number > of saved entries and get below assertion failure: > > ../src/libcamera/delayed_controls.cpp:227: > libcamera::ControlList libcamera::DelayedControls::get(uint32_t): > Assertion `info.type() != ControlTypeNone' failed > > Assertion failure can happen at the beginning of streaming when > ControlRingBuffer is not yet filled and there are errors on CSI-2. > > To fix, loop over devices in the pipeline (starting from the last one), > find one that emits start frame events and connect applyControls() > to it. Additionally remove direct call to sensor_->setControls() if > the emitter device was found. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=241 > Co-developed-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> which is a bit weird since Stanislaw gave me Co-developed-by credits, but still... Also: Tested-by: Hans de Goede <hdegoede@redhat.com> # Lenovo X1 Yoga IPU6 + ov2740 Regards, Hans > --- > v1 -> v2: > - make eventEmitter_ subdevice part of SimpleCameraData > - add debug log when found event emitter device > - nullify eventEmitter_ on stop > - remove direct sensor_->setControls() > - add delayedCtrls_->reset() on start > > src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++--- > 1 file changed, 35 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 8ac24e6e..a7594c2c 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -277,6 +277,7 @@ public: > std::list<Entity> entities_; > std::unique_ptr<CameraSensor> sensor_; > V4L2VideoDevice *video_; > + V4L2Subdevice *eventEmitter_; > > std::vector<Configuration> configs_; > std::map<PixelFormat, std::vector<const Configuration *>> formats_; > @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) > void SimpleCameraData::setSensorControls(const ControlList &sensorControls) > { > delayedCtrls_->push(sensorControls); > - ControlList ctrls(sensorControls); > - sensor_->setControls(&ctrls); > + /* Directly apply controls now if there is no frameStart signal */ > + if (!eventEmitter_) { > + ControlList ctrls(sensorControls); > + sensor_->setControls(&ctrls); > + } > } > > /* Retrieve all source pads connected to a sink pad through active routes. */ > @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > data->delayedCtrls_ = > std::make_unique<DelayedControls>(data->sensor_->device(), > params); > - data->video_->frameStart.connect(data->delayedCtrls_.get(), > - &DelayedControls::applyControls); > > StreamConfiguration inputCfg; > inputCfg.pixelFormat = pipeConfig->captureFormat; > @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL > > video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady); > > + /* > + * Enable frame start event on last device in the pipeline > + * that provides the events. > + */ > + for (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) { > + V4L2Subdevice *sd = subdev(it->entity); > + if (!sd) > + continue; > + if (sd->setFrameStartEnabled(true) < 0) > + continue; > + > + LOG(SimplePipeline, Debug) > + << "Using " << it->entity->name() << " frameStart signal"; > + > + sd->frameStart.connect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = sd; > + break; > + } > + > + data->delayedCtrls_->reset(); > + > ret = video->streamOn(); > if (ret < 0) { > stop(camera); > @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera) > SimpleCameraData *data = cameraData(camera); > V4L2VideoDevice *video = data->video_; > > + if (data->eventEmitter_) { > + data->eventEmitter_->setFrameStartEnabled(false); > + data->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = NULL; > + } > + > if (data->useConversion_) { > if (data->converter_) > data->converter_->stop(); > -- > 2.43.0 >
Hi Stanislaw, thank you for the fix. Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> writes: > Currently we use frame start event from video capture device to > apply controls. But the capture device might not generate the events. > Usually CSI-2 receiver is proper device to subscribe for start > frame events. > > Without DelayedConntrols:applyControls() is possible that we can get > call to DelayedControls::get() with frame number that exceed number > of saved entries and get below assertion failure: > > ../src/libcamera/delayed_controls.cpp:227: > libcamera::ControlList libcamera::DelayedControls::get(uint32_t): > Assertion `info.type() != ControlTypeNone' failed > > Assertion failure can happen at the beginning of streaming when > ControlRingBuffer is not yet filled and there are errors on CSI-2. > > To fix, loop over devices in the pipeline (starting from the last one), > find one that emits start frame events and connect applyControls() > to it. Additionally remove direct call to sensor_->setControls() if > the emitter device was found. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=241 > Co-developed-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> AFAICT: Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > v1 -> v2: > - make eventEmitter_ subdevice part of SimpleCameraData > - add debug log when found event emitter device > - nullify eventEmitter_ on stop > - remove direct sensor_->setControls() > - add delayedCtrls_->reset() on start > > src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++--- > 1 file changed, 35 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 8ac24e6e..a7594c2c 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -277,6 +277,7 @@ public: > std::list<Entity> entities_; > std::unique_ptr<CameraSensor> sensor_; > V4L2VideoDevice *video_; > + V4L2Subdevice *eventEmitter_; > > std::vector<Configuration> configs_; > std::map<PixelFormat, std::vector<const Configuration *>> formats_; > @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) > void SimpleCameraData::setSensorControls(const ControlList &sensorControls) > { > delayedCtrls_->push(sensorControls); > - ControlList ctrls(sensorControls); > - sensor_->setControls(&ctrls); > + /* Directly apply controls now if there is no frameStart signal */ > + if (!eventEmitter_) { > + ControlList ctrls(sensorControls); > + sensor_->setControls(&ctrls); > + } > } > > /* Retrieve all source pads connected to a sink pad through active routes. */ > @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > data->delayedCtrls_ = > std::make_unique<DelayedControls>(data->sensor_->device(), > params); > - data->video_->frameStart.connect(data->delayedCtrls_.get(), > - &DelayedControls::applyControls); > > StreamConfiguration inputCfg; > inputCfg.pixelFormat = pipeConfig->captureFormat; > @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL > > video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady); > > + /* > + * Enable frame start event on last device in the pipeline > + * that provides the events. > + */ > + for (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) { > + V4L2Subdevice *sd = subdev(it->entity); > + if (!sd) > + continue; > + if (sd->setFrameStartEnabled(true) < 0) > + continue; > + > + LOG(SimplePipeline, Debug) > + << "Using " << it->entity->name() << " frameStart signal"; > + > + sd->frameStart.connect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = sd; > + break; > + } > + > + data->delayedCtrls_->reset(); > + > ret = video->streamOn(); > if (ret < 0) { > stop(camera); > @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera) > SimpleCameraData *data = cameraData(camera); > V4L2VideoDevice *video = data->video_; > > + if (data->eventEmitter_) { > + data->eventEmitter_->setFrameStartEnabled(false); > + data->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = NULL; > + } > + > if (data->useConversion_) { > if (data->converter_) > data->converter_->stop(); > -- > 2.43.0
Hi Stanislaw, Quoting Stanislaw Gruszka (2024-12-18 14:22:17) > Currently we use frame start event from video capture device to > apply controls. But the capture device might not generate the events. > Usually CSI-2 receiver is proper device to subscribe for start > frame events. > > Without DelayedConntrols:applyControls() is possible that we can get > call to DelayedControls::get() with frame number that exceed number > of saved entries and get below assertion failure: > > ../src/libcamera/delayed_controls.cpp:227: > libcamera::ControlList libcamera::DelayedControls::get(uint32_t): > Assertion `info.type() != ControlTypeNone' failed > > Assertion failure can happen at the beginning of streaming when > ControlRingBuffer is not yet filled and there are errors on CSI-2. > > To fix, loop over devices in the pipeline (starting from the last one), > find one that emits start frame events and connect applyControls() > to it. Additionally remove direct call to sensor_->setControls() if > the emitter device was found. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=241 > Co-developed-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > --- > v1 -> v2: > - make eventEmitter_ subdevice part of SimpleCameraData > - add debug log when found event emitter device > - nullify eventEmitter_ on stop > - remove direct sensor_->setControls() > - add delayedCtrls_->reset() on start > > src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++--- > 1 file changed, 35 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 8ac24e6e..a7594c2c 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -277,6 +277,7 @@ public: > std::list<Entity> entities_; > std::unique_ptr<CameraSensor> sensor_; > V4L2VideoDevice *video_; > + V4L2Subdevice *eventEmitter_; > > std::vector<Configuration> configs_; > std::map<PixelFormat, std::vector<const Configuration *>> formats_; > @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) > void SimpleCameraData::setSensorControls(const ControlList &sensorControls) > { > delayedCtrls_->push(sensorControls); > - ControlList ctrls(sensorControls); > - sensor_->setControls(&ctrls); > + /* Directly apply controls now if there is no frameStart signal */ > + if (!eventEmitter_) { > + ControlList ctrls(sensorControls); > + sensor_->setControls(&ctrls); > + } > } > > /* Retrieve all source pads connected to a sink pad through active routes. */ > @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > data->delayedCtrls_ = > std::make_unique<DelayedControls>(data->sensor_->device(), > params); > - data->video_->frameStart.connect(data->delayedCtrls_.get(), > - &DelayedControls::applyControls); > > StreamConfiguration inputCfg; > inputCfg.pixelFormat = pipeConfig->captureFormat; > @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL > > video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady); > > + /* > + * Enable frame start event on last device in the pipeline > + * that provides the events. > + */ > + for (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) { > + V4L2Subdevice *sd = subdev(it->entity); > + if (!sd) > + continue; > + if (sd->setFrameStartEnabled(true) < 0) > + continue; > + > + LOG(SimplePipeline, Debug) > + << "Using " << it->entity->name() << " frameStart signal"; > + > + sd->frameStart.connect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = sd; > + break; > + } > + > + data->delayedCtrls_->reset(); > + > ret = video->streamOn(); > if (ret < 0) { > stop(camera); > @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera) > SimpleCameraData *data = cameraData(camera); > V4L2VideoDevice *video = data->video_; > > + if (data->eventEmitter_) { > + data->eventEmitter_->setFrameStartEnabled(false); There's a segfault here when stopping the pipeline/shutting down. Running qcam and closing the window exhbits the issue very reproducibly: Thread 10 "qcam" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xffffd97aec80 (LWP 36600)] 0x0000fffff7cb8554 in libcamera::V4L2Device::setFrameStartEnabled (this=0x40, enable=false) at ../../src/libcamera/v4l2_device.cpp:463 463 if (frameStartEnabled_ == enable) Missing rpms, try: dnf --enablerepo='*debug*' install libtiff-debuginfo-4.7.0-3.fc42.aarch64 qt6-qtbase-debuginfo-6.8.1-11.fc42.aarch64 qt6-qtbase-gui-debuginfo-6.8.1-11.fc42.aarch64 libstdc++-debuginfo-15.0.1-0.4.fc42.aarch64 libgcc-debuginfo-15.0.1-0.4.fc42.aarch64 glibc-debuginfo-2.40.9000-35.fc42.aarch64 libjpeg-turbo-debuginfo-3.1.0-2.fc42.aarch64 gnutls-debuginfo-3.8.8-2.fc42.aarch64 systemd-libs-debuginfo-257.2-17.fc42.aarch64 libyaml-debuginfo-0.2.5-16.fc42.aarch64 elfutils-libs-debuginfo-0.192-8.fc42.aarch64 libunwind-debuginfo-1.8.0-5.fc42.aarch64 libwebp-debuginfo-1.5.0-2.fc42.aarch64 libzstd-debuginfo-1.5.6-3.fc42.aarch64 liblerc-debuginfo-4.0.0-8.fc42.aarch64 jbigkit-libs-debuginfo-2.1-31.fc42.aarch64 zlib-ng-compat-debuginfo-2.2.3-2.fc42.aarch64 libicu-debuginfo-76.1-4.fc42.aarch64 glib2-debuginfo-2.83.2-6.fc42.aarch64 double-conversion-debuginfo-3.3.0-5.fc42.aarch64 libb2-debuginfo-0.98.1-13.fc42.aarch64 pcre2-utf16-debuginfo-10.44-1.fc42.2.aarch64 openssl-libs-debuginfo-3.2.2-13.fc42.aarch64 libglvnd-egl-debuginfo-1.7.0-7.fc42.aarch64 fontconfig-debuginfo-2.16.0-2.fc42.aarch64 libX11-debuginfo-1.8.10-3.fc42.aarch64 libxkbcommon-debuginfo-1.7.0-6.fc42.aarch64 libglvnd-glx-debuginfo-1.7.0-7.fc42.aarch64 libglvnd-opengl-debuginfo-1.7.0-7.fc42.aarch64 libpng-debuginfo-1.6.44-2.fc42.aarch64 harfbuzz-debuginfo-10.2.0-2.fc42.aarch64 freetype-debuginfo-2.13.3-2.fc42.aarch64 p11-kit-debuginfo-0.25.5-5.fc42.aarch64 libidn2-debuginfo-2.3.7-3.fc42.aarch64 libunistring-debuginfo-1.1-9.fc42.aarch64 libtasn1-debuginfo-4.19.0-11.fc42.aarch64 nettle-debuginfo-3.10-6.fc42.aarch64 gmp-debuginfo-6.3.0-2.fc41.aarch64 libcap-debuginfo-2.73-2.fc42.aarch64 elfutils-libelf-debuginfo-0.192-8.fc42.aarch64 xz-libs-debuginfo-5.6.3-3.fc42.aarch64 bzip2-libs-debuginfo-1.0.8-20.fc42.aarch64 pcre2-debuginfo-10.44-1.fc42.2.aarch64 libgomp-debuginfo-15.0.1-0.4.fc42.aarch64 libglvnd-debuginfo-1.7.0-7.fc42.aarch64 libxml2-debuginfo-2.12.9-2.fc42.aarch64 libxcb-debuginfo-1.17.0-5.fc42.aarch64 dbus-libs-debuginfo-1.16.0-3.fc42.aarch64 libXext-debuginfo-1.3.6-3.fc42.aarch64 graphite2-debuginfo-1.3.14-18.fc42.aarch64 libbrotli-debuginfo-1.1.0-6.fc42.aarch64 libffi-debuginfo-3.4.6-5.fc42.aarch64 libXau-debuginfo-1.0.12-2.fc42.aarch64 qt6-qtwayland-debuginfo-6.8.1-3.fc42.aarch64 libwayland-client-debuginfo-1.23.0-3.fc42.aarch64 libwayland-cursor-debuginfo-1.23.0-3.fc42.aarch64 gtk3-debuginfo-3.24.43-3.fc42.aarch64 pango-debuginfo-1.56.1-1.fc42.aarch64 gdk-pixbuf2-debuginfo-2.42.12-10.fc42.aarch64 cairo-debuginfo-1.18.2-3.fc42.aarch64 fribidi-debuginfo-1.0.16-2.fc42.aarch64 cairo-gobject-debuginfo-1.18.2-3.fc42.aarch64 atk-debuginfo-2.55.0.1-2.fc42.aarch64 libepoxy-debuginfo-1.5.10-9.fc42.aarch64 libXi-debuginfo-1.8.2-2.fc42.aarch64 at-spi2-atk-debuginfo-2.55.0.1-2.fc42.aarch64 libcloudproviders-debuginfo-0.3.5-6.fc42.aarch64 libtracker-sparql-debuginfo-3.7.3-5.fc42.aarch64 libXfixes-debuginfo-6.0.1-5.fc42.aarch64 libwayland-egl-debuginfo-1.23.0-3.fc42.aarch64 libXcursor-debuginfo-1.2.3-2.fc42.aarch64 libXdamage-debuginfo-1.1.6-5.fc42.aarch64 libXcomposite-debuginfo-0.4.6-5.fc42.aarch64 libXrandr-debuginfo-1.5.4-5.fc42.aarch64 libXinerama-debuginfo-1.1.5-8.fc42.aarch64 libthai-debuginfo-0.1.29-10.fc42.aarch64 libmount-debuginfo-2.40.4-2.fc42.aarch64 libselinux-debuginfo-3.8-0.rc3.1.fc42.3.aarch64 libXrender-debuginfo-0.9.12-2.fc42.aarch64 pixman-debuginfo-0.44.2-2.fc42.aarch64 at-spi2-core-debuginfo-2.55.0.1-2.fc42.aarch64 json-glib-debuginfo-1.10.6-2.fc42.aarch64 sqlite-libs-debuginfo-3.47.2-2.fc42.aarch64 libdatrie-debuginfo-0.2.13-11.fc42.aarch64 libblkid-debuginfo-2.40.4-2.fc42.aarch64 gvfs-client-debuginfo-1.56.1-3.fc42.aarch64 libcanberra-gtk3-debuginfo-0.30-37.fc42.aarch64 libcanberra-debuginfo-0.30-37.fc42.aarch64 libvorbis-debuginfo-1.3.7-12.fc42.aarch64 libtdb-debuginfo-1.4.12-5.fc42.aarch64 libtool-ltdl-debuginfo-2.5.4-4.fc42.aarch64 libogg-debuginfo-1.3.5-11.fc42.aarch64 PackageKit-gtk3-module-debuginfo-1.2.8-9.fc42.aarch64 qt6-qtsvg-debuginfo-6.8.1-3.fc42.aarch64 qt6-qtpdf-debuginfo-6.8.1-4.fc42.aarch64 krb5-libs-debuginfo-1.21.3-4.fc42.aarch64 libproxy-debuginfo-0.5.8-2.fc42.aarch64 libcom_err-debuginfo-1.47.2-3.fc42.aarch64 keyutils-libs-debuginfo-1.6.3-5.fc42.aarch64 libcurl-minimal-debuginfo-8.11.1-3.fc42.aarch64 duktape-debuginfo-2.7.0-9.fc42.aarch64 libnghttp2-debuginfo-1.64.0-3.fc42.aarch64 (gdb) bt #0 0x0000fffff7cb8554 in libcamera::V4L2Device::setFrameStartEnabled (this=0x40, enable=false) at ../../src/libcamera/v4l2_device.cpp:463 #1 0x0000fffff7da1d24 in libcamera::SimplePipelineHandler::stopDevice (this=0xffffbc00cc40, camera=0xffffbc002bc0) at ../../src/libcamera/pipeline/simple/simple.cpp:1531 #2 0x0000fffff7cab650 in libcamera::PipelineHandler::stop (this=0xffffbc00cc40, camera=0xffffbc002bc0) at ../../src/libcamera/pipeline_handler.cpp:364 #3 0x0000fffff7c41d7c in libcamera::BoundMethodMember<libcamera::PipelineHandler, void, libcamera::Camera*>::invoke (this=0x700170, args#0=0xffffbc002bc0) at ../../include/libcamera/base/bound_method.h:191 #4 0x0000fffff7c42124 in libcamera::BoundMethodArgs<void, libcamera::Camera*>::invokePack<0ul, void> (this=0x700170, pack=0x617ff0) at ../../include/libcamera/base/bound_method.h:115 #5 0x0000fffff7c41d00 in libcamera::BoundMethodArgs<void, libcamera::Camera*>::invokePack (this=0x700170, pack=0x617ff0) at ../../include/libcamera/base/bound_method.h:124 #6 0x0000fffff7718154 in libcamera::InvokeMessage::invoke (this=0x78dc00) at ../../src/libcamera/base/message.cpp:153 #7 0x0000fffff7700014 in libcamera::Object::message (this=0xffffbc00cc40, msg=0x78dc00) at ../../src/libcamera/base/object.cpp:211 #8 0x0000fffff77197dc in libcamera::Thread::dispatchMessages (this=0x6e9090, type=libcamera::Message::None) at ../../src/libcamera/base/thread.cpp:649 #9 0x0000fffff7709078 in libcamera::EventDispatcherPoll::processEvents (this=0xffffbc0162b0) at ../../src/libcamera/base/event_dispatcher_poll.cpp:146 #10 0x0000fffff7718a84 in libcamera::Thread::exec (this=0x6e9090) at ../../src/libcamera/base/thread.cpp:311 #11 0x0000fffff7c42a58 in libcamera::CameraManager::Private::run (this=0x6e9080) at ../../src/libcamera/camera_manager.cpp:89 #12 0x0000fffff7718a24 in libcamera::Thread::startThread (this=0x6e9090) at ../../src/libcamera/base/thread.cpp:289 #13 0x0000fffff771d58c in std::__invoke_impl<void, void (libcamera::Thread::*)(), libcamera::Thread*> (__f=@0x6aec80: (void (libcamera::Thread::*)(libcamera::Thread * const)) 0xfffff7718924 <libcamera::Thread::startThread()>, __t=@0x6aec78: 0x6e9090) at /usr/include/c++/15/bits/invoke.h:76 #14 0x0000fffff771d4d8 in std::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*> (__fn=@0x6aec80: (void (libcamera::Thread::*)(libcamera::Thread * const)) 0xfffff7718924 <libcamera::Thread::startThread()>) at /usr/include/c++/15/bits/invoke.h:98 #15 0x0000fffff771d448 in std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> >::_M_invoke<0ul, 1ul> (this=0x6aec78) at /usr/include/c++/15/bits/std_thread.h:303 #16 0x0000fffff771d3f8 in std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> >::operator() (this=0x6aec78) at /usr/include/c++/15/bits/std_thread.h:310 #17 0x0000fffff771d3d8 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> > >::_M_run (this=0x6aec70) at /usr/include/c++/15/bits/std_thread.h:255 #18 0x0000fffff5bb2980 in execute_native_thread_routine () at /lib64/libstdc++.so.6 #19 0x0000fffff594d364 in start_thread () at /lib64/libc.so.6 #20 0x0000fffff59b868c in thread_start () at /lib64/libc.so.6 So I think storing data->eventEmitter_ has not been done safely. -- Kieran > + data->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = NULL; > + } > + > if (data->useConversion_) { > if (data->converter_) > data->converter_->stop(); > -- > 2.43.0 >
Hi Stanislaw, Thank you for the patch, and sorry this got out of my radar. On Wed, Dec 18, 2024 at 03:22:17PM +0100, Stanislaw Gruszka wrote: > Currently we use frame start event from video capture device to > apply controls. But the capture device might not generate the events. > Usually CSI-2 receiver is proper device to subscribe for start > frame events. > > Without DelayedConntrols:applyControls() is possible that we can get > call to DelayedControls::get() with frame number that exceed number > of saved entries and get below assertion failure: > > ../src/libcamera/delayed_controls.cpp:227: > libcamera::ControlList libcamera::DelayedControls::get(uint32_t): > Assertion `info.type() != ControlTypeNone' failed > > Assertion failure can happen at the beginning of streaming when > ControlRingBuffer is not yet filled and there are errors on CSI-2. > > To fix, loop over devices in the pipeline (starting from the last one), > find one that emits start frame events and connect applyControls() > to it. Additionally remove direct call to sensor_->setControls() if > the emitter device was found. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=241 > Co-developed-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > --- > v1 -> v2: > - make eventEmitter_ subdevice part of SimpleCameraData > - add debug log when found event emitter device > - nullify eventEmitter_ on stop > - remove direct sensor_->setControls() > - add delayedCtrls_->reset() on start > > src/libcamera/pipeline/simple/simple.cpp | 39 +++++++++++++++++++++--- > 1 file changed, 35 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 8ac24e6e..a7594c2c 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -277,6 +277,7 @@ public: > std::list<Entity> entities_; > std::unique_ptr<CameraSensor> sensor_; > V4L2VideoDevice *video_; > + V4L2Subdevice *eventEmitter_; > > std::vector<Configuration> configs_; > std::map<PixelFormat, std::vector<const Configuration *>> formats_; > @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) > void SimpleCameraData::setSensorControls(const ControlList &sensorControls) > { > delayedCtrls_->push(sensorControls); > - ControlList ctrls(sensorControls); > - sensor_->setControls(&ctrls); > + /* Directly apply controls now if there is no frameStart signal */ > + if (!eventEmitter_) { > + ControlList ctrls(sensorControls); > + sensor_->setControls(&ctrls); > + } > } > > /* Retrieve all source pads connected to a sink pad through active routes. */ > @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > data->delayedCtrls_ = > std::make_unique<DelayedControls>(data->sensor_->device(), > params); > - data->video_->frameStart.connect(data->delayedCtrls_.get(), > - &DelayedControls::applyControls); > > StreamConfiguration inputCfg; > inputCfg.pixelFormat = pipeConfig->captureFormat; > @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL > > video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady); > > + /* > + * Enable frame start event on last device in the pipeline > + * that provides the events. > + */ > + for (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) { > + V4L2Subdevice *sd = subdev(it->entity); > + if (!sd) > + continue; > + if (sd->setFrameStartEnabled(true) < 0) > + continue; > + > + LOG(SimplePipeline, Debug) > + << "Using " << it->entity->name() << " frameStart signal"; > + > + sd->frameStart.connect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = sd; > + break; > + } Could we do this at init time instead ? V4L2 doesn't offer an API to list what events are supported by a subdev (that should probably be added to the kernel, but that's a separate problem), so we need to subscribe to an event in order to know if it's supported. This could be implemented as a V4L2Subdevice::supportsEvent() helper function that would subscribe (and unsubscribe is the subscription is successful). > + > + data->delayedCtrls_->reset(); > + > ret = video->streamOn(); > if (ret < 0) { > stop(camera); > @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera) > SimpleCameraData *data = cameraData(camera); > V4L2VideoDevice *video = data->video_; > > + if (data->eventEmitter_) { > + data->eventEmitter_->setFrameStartEnabled(false); > + data->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(), > + &DelayedControls::applyControls); > + data->eventEmitter_ = NULL; > + } > + > if (data->useConversion_) { > if (data->converter_) > data->converter_->stop();
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 8ac24e6e..a7594c2c 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -277,6 +277,7 @@ public: std::list<Entity> entities_; std::unique_ptr<CameraSensor> sensor_; V4L2VideoDevice *video_; + V4L2Subdevice *eventEmitter_; std::vector<Configuration> configs_; std::map<PixelFormat, std::vector<const Configuration *>> formats_; @@ -911,8 +912,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId) void SimpleCameraData::setSensorControls(const ControlList &sensorControls) { delayedCtrls_->push(sensorControls); - ControlList ctrls(sensorControls); - sensor_->setControls(&ctrls); + /* Directly apply controls now if there is no frameStart signal */ + if (!eventEmitter_) { + ControlList ctrls(sensorControls); + sensor_->setControls(&ctrls); + } } /* Retrieve all source pads connected to a sink pad through active routes. */ @@ -1299,8 +1303,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) data->delayedCtrls_ = std::make_unique<DelayedControls>(data->sensor_->device(), params); - data->video_->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); StreamConfiguration inputCfg; inputCfg.pixelFormat = pipeConfig->captureFormat; @@ -1368,6 +1370,28 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady); + /* + * Enable frame start event on last device in the pipeline + * that provides the events. + */ + for (auto it = data->entities_.rbegin(); it != data->entities_.rend(); ++it) { + V4L2Subdevice *sd = subdev(it->entity); + if (!sd) + continue; + if (sd->setFrameStartEnabled(true) < 0) + continue; + + LOG(SimplePipeline, Debug) + << "Using " << it->entity->name() << " frameStart signal"; + + sd->frameStart.connect(data->delayedCtrls_.get(), + &DelayedControls::applyControls); + data->eventEmitter_ = sd; + break; + } + + data->delayedCtrls_->reset(); + ret = video->streamOn(); if (ret < 0) { stop(camera); @@ -1400,6 +1424,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera) SimpleCameraData *data = cameraData(camera); V4L2VideoDevice *video = data->video_; + if (data->eventEmitter_) { + data->eventEmitter_->setFrameStartEnabled(false); + data->eventEmitter_->frameStart.disconnect(data->delayedCtrls_.get(), + &DelayedControls::applyControls); + data->eventEmitter_ = NULL; + } + if (data->useConversion_) { if (data->converter_) data->converter_->stop();