<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 7 Dec 2021 at 14:22, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
On Tue, Dec 07, 2021 at 08:42:36AM +0000, Naushir Patuck wrote:<br>
> On Tue, 7 Dec 2021 at 00:07, Laurent Pinchart wrote:<br>
> > On Mon, Dec 06, 2021 at 11:39:40PM +0000, Kieran Bingham wrote:<br>
> > > When completing a request, the individual stream buffers contain a<br>
> > > sequence number. This number is generated by the device that ultimately<br>
> > > fills that stream, but it might not be the sensor. Processing through an<br>
> > > ISP could cause the sequence numbers and timestamp data associated with<br>
> > > the completed buffer to be values representative of the ISP processing<br>
> > > rather than the (intended) capture device.<br>
> > ><br>
> > > Provide a new metadata control, still to be fully sketched out which<br>
> > > gives us a defined value to report the Camera sequence number. This<br>
> > > allows pipeline handlers to correctly set the value according to the<br>
> > > device that represents the capture from the sensor.<br>
> > ><br>
> > > This plumbing then allows applications to detect frame drops, which were<br>
> > > otherwise going unnoticed, and as such some basic additions have been<br>
> > > made to cam, qcam, and gstreamer to support this new data.<br>
> > ><br>
> > > Still possible:<br>
> > >  - Adding a validation to lc-compliance to make sure pipelines set the<br>
> > >    SensorSequence on every frame.<br>
> > ><br>
> > >  - Probably expecting some better gstreamer event integration perhaps?<br>
> > ><br>
> > >  - qcam should report more statistics on the processing overall<br>
> > ><br>
> > >  - libcamera Tracepoints could be added as an event to track when<br>
> > >    frames are detected as dropped by the core framework.<br>
> > ><br>
> > > Anything else?<br>
> ><br>
> > A basic design question: when a frame is dropped, shouldn't we report<br>
> > the corresponding request as failed ? That's how the Android camera HAL<br>
> > API operates, and while that by itself isn't a good enough reason to do<br>
> > that same, I think it offers a better way to ensure that controls get<br>
> > synchronized with the buffers that frames are captured to.<br>
> <br>
> For the Raspberry Pi platforms, it is possible for the Unicam driver<br>
> to drop frames without the knowledge of the pipeline handler, if for example<br>
> we do not recycle bayer buffers quick enough.<br>
<br>
So the driver doesn't increment the sequence number in that case ? Could<br>
this be fixed ?<br></blockquote><div><br></div><div>The driver increments sequence numbers based on frame interrupts, so they</div><div>change even if the frame is going to be dropped.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Allowing the application<br>
> to look at sensor timestamps would allow it to detect these drops. Unless<br>
> I am mistaken, this would not be possible by failing the request, as the<br>
> sensor buffer recycling loop is asynchronous to the application request<br>
> loop. Having said that, they could be synchronised....<br>
<br>
This is also related to implementing true per-frame control,<br>
synchronizing the controls from the request with the buffers in which<br>
images are captured :-)<br></blockquote><div><br></div><div>As long as they are ISP based controls we are fine :-)</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> > > Kieran Bingham (8):<br>
> > >   libcamera: controls: Add SensorSequence metadata control<br>
> > >   libcamera: pipeline: Set the Sensor sequence number for all pipelines<br>
> > >   cam: Use SensorTimestamp rather than buffer metadata<br>
> > >   cam: Use Sensor sequence numbers and detect frame drop<br>
> > >   qcam: main_window: Fix include ordering<br>
> > >   qcam: Use Sensor sequence numbers and detect frame drop<br>
> > >   gstreamer: gstlibcamerasrc: Fix include ordering<br>
> > >   gstreamer: Use Sensor sequence numbers and detect frame drop<br>
> > ><br>
> > >  src/cam/camera_session.cpp                    | 24 ++++++++--<br>
> > >  src/cam/camera_session.h                      |  1 +<br>
> > >  src/gstreamer/gstlibcamerasrc.cpp             | 46 +++++++++++++++----<br>
> > >  src/libcamera/control_ids.yaml                |  8 ++++<br>
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  4 +-<br>
> > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 ++<br>
> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 +-<br>
> > >  src/libcamera/pipeline/simple/simple.cpp      | 12 +++--<br>
> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +<br>
> > >  src/qcam/main_window.cpp                      | 28 +++++++++--<br>
> > >  src/qcam/main_window.h                        |  1 +<br>
> > >  11 files changed, 111 insertions(+), 22 deletions(-)<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>