[libcamera-devel] [RFC PATCH 0/8] Request metadata: SensorSequence

[Laurent Pinchart]

Hi Naush,

On Tue, Dec 07, 2021 at 03:15:23PM +0000, Naushir Patuck wrote:
> On Tue, 7 Dec 2021 at 14:22, Laurent Pinchart wrote:
> > On Tue, Dec 07, 2021 at 08:42:36AM +0000, Naushir Patuck wrote:
> > > On Tue, 7 Dec 2021 at 00:07, Laurent Pinchart wrote:
> > > > On Mon, Dec 06, 2021 at 11:39:40PM +0000, Kieran Bingham wrote:
> > > > > When completing a request, the individual stream buffers contain a
> > > > > sequence number. This number is generated by the device that ultimately
> > > > > fills that stream, but it might not be the sensor. Processing through an
> > > > > ISP could cause the sequence numbers and timestamp data associated with
> > > > > the completed buffer to be values representative of the ISP processing
> > > > > rather than the (intended) capture device.
> > > > >
> > > > > Provide a new metadata control, still to be fully sketched out which
> > > > > gives us a defined value to report the Camera sequence number. This
> > > > > allows pipeline handlers to correctly set the value according to the
> > > > > device that represents the capture from the sensor.
> > > > >
> > > > > This plumbing then allows applications to detect frame drops, which were
> > > > > otherwise going unnoticed, and as such some basic additions have been
> > > > > made to cam, qcam, and gstreamer to support this new data.
> > > > >
> > > > > Still possible:
> > > > >  - Adding a validation to lc-compliance to make sure pipelines set the
> > > > >    SensorSequence on every frame.
> > > > >
> > > > >  - Probably expecting some better gstreamer event integration perhaps?
> > > > >
> > > > >  - qcam should report more statistics on the processing overall
> > > > >
> > > > >  - libcamera Tracepoints could be added as an event to track when
> > > > >    frames are detected as dropped by the core framework.
> > > > >
> > > > > Anything else?
> > > >
> > > > A basic design question: when a frame is dropped, shouldn't we report
> > > > the corresponding request as failed ? That's how the Android camera HAL
> > > > API operates, and while that by itself isn't a good enough reason to do
> > > > that same, I think it offers a better way to ensure that controls get
> > > > synchronized with the buffers that frames are captured to.
> > >
> > > For the Raspberry Pi platforms, it is possible for the Unicam driver
> > > to drop frames without the knowledge of the pipeline handler, if for example
> > > we do not recycle bayer buffers quick enough.
> >
> > So the driver doesn't increment the sequence number in that case ? Could
> > this be fixed ?
> 
> The driver increments sequence numbers based on frame interrupts, so they
> change even if the frame is going to be dropped.

Does this mean that the pipeline handler could then detect that
condition, and react appropriately ?

> > > Allowing the application
> > > to look at sensor timestamps would allow it to detect these drops. Unless
> > > I am mistaken, this would not be possible by failing the request, as the
> > > sensor buffer recycling loop is asynchronous to the application request
> > > loop. Having said that, they could be synchronised....
> >
> > This is also related to implementing true per-frame control,
> > synchronizing the controls from the request with the buffers in which
> > images are captured :-)
> 
> As long as they are ISP based controls we are fine :-)

There's also manual exposure and analogue gain I'm afraid :-)

> > > > > Kieran Bingham (8):
> > > > >   libcamera: controls: Add SensorSequence metadata control
> > > > >   libcamera: pipeline: Set the Sensor sequence number for all pipelines
> > > > >   cam: Use SensorTimestamp rather than buffer metadata
> > > > >   cam: Use Sensor sequence numbers and detect frame drop
> > > > >   qcam: main_window: Fix include ordering
> > > > >   qcam: Use Sensor sequence numbers and detect frame drop
> > > > >   gstreamer: gstlibcamerasrc: Fix include ordering
> > > > >   gstreamer: Use Sensor sequence numbers and detect frame drop
> > > > >
> > > > >  src/cam/camera_session.cpp                    | 24 ++++++++--
> > > > >  src/cam/camera_session.h                      |  1 +
> > > > >  src/gstreamer/gstlibcamerasrc.cpp             | 46 +++++++++++++++----
> > > > >  src/libcamera/control_ids.yaml                |  8 ++++
> > > > >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 +-
> > > > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 ++
> > > > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 +-
> > > > >  src/libcamera/pipeline/simple/simple.cpp      | 12 +++--
> > > > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +
> > > > >  src/qcam/main_window.cpp                      | 28 +++++++++--
> > > > >  src/qcam/main_window.h                        |  1 +
> > > > >  11 files changed, 111 insertions(+), 22 deletions(-)

-- 
Regards,

Laurent Pinchart