[PATCH] pipeline: simple: Use proper device for frame start events

[Stanislaw Gruszka]

Hi Hans,

On Tue, Dec 17, 2024 at 02:41:43PM +0100, Hans de Goede wrote:
> On 17-Dec-24 11:52 AM, 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.
> > 
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=241
> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
> 
> I noticed the issue with connecting to the framestart signal of
> the /dev/video# capture node which cannot work myself too and I was
> about to post this patch for this:
> 
> https://github.com/jwrdegoede/libcamera/commit/5abbf43118e80bb4be6b893a6e5e28c65b59744a
> 
> But your solution is obviously better.
> 
> Note that the simple-pipelinehandler already contains a workaroud for this
> in the form of setControls() directly applying the controls rather then
> only pushing them into delayedCtrls_.
> 
> That workaround needs to be disabled in your patch when the frameStart signal
> is used successfully.
> 
> See my attached patch with suggested changes to your patch.

All good points, I'll integrate your changes into v2 and repost.

Thanks
Stanislaw