[Q] early metadata reporting and the raspberry pi pipelines

[Naushir Patuck]

On Fri, 2 May 2025 at 10:37, Barnabás Pőcze
<barnabas.pocze at ideasonboard.com> wrote:
>
> Hi
>
>
> 2025. 05. 02. 9:09 keltezéssel, Naushir Patuck írta:
> > Hi Barnabás,
> >
> > Thanks for the detailed explanation.  From my initial reading, I don't
> > think this is going to be a big issue for our pipeline handlers at
> > all.
> >
> > On Thu, 1 May 2025 at 18:00, Barnabás Pőcze
> > <barnabas.pocze at ideasonboard.com> wrote:
> >>
> >> Hi David, Naush
> >>
> >> TLDR: Early metadata reporting (before request completion) requires constraints
> >> on how pipeline handlers access the metadata list. The raspberry pi pipelines handlers
> >> clear the metadata list, which would no longer be allowed. When can that happen?
> >> Can raspberry pi pipelines be potentially extended to supply metadata earlier?
> >>
> >> There are plans to provides a mechanism to pipeline handlers by which they can
> >> return metadata items to the application before the request completes (similarly
> >> to `bufferCompleted`).[0][1] This will most likely lead to pipeline handlers being
> >> restricted in how they can access the requests' metadata lists.
> >>
> >> For example, probably most critically, setting a given metadata item would only
> >> be possible once until the request is reused, to ensure that apps receive a metadata
> >> item once and with its final value. Allowing already "published" metadata items to
> >> be modified/erased would require an appreciably more complicated design, and the
> >> likelihood of applications handling it correctly seems slim. So any given metadata
> >> item should only be reported if its final value is available.
> >
> > So will the API require multiple calls to "metadataCompleted()" from
> > pipeline handlers when items of metadata (Controls) have been added to
> > the request?  Or would pipeline handlers call "metadataComplete()"
> > only once when all metadata has been populated?  Either way is fine
> > for us I think.
>
> The current plan is to have the same API as https://patchwork.libcamera.org/patch/22229/
> that is, a single function to "commit" one or more metadata items at a time and notify
> the user about them, as they become available. Pipeline handlers would call the function
> when they have a set of "final" metadata for the request. They can do it in one batch just
> before request completion (in which case there wouldn't be much difference from the current
> status quo), or they can report the metadata items as soon as their value is known.
>
>
> >
> >>
> >> As a consequence, a pipeline handlers will probably be constrained to access the metadata
> >> list as an "append-only log". Currently, however, the raspberry pi pipelines call `clear()`
> >> on the request metadata in `{Vc4,PiSP}CameraData::tryRunPipeline()`[2]:
> >>
> >>           /*
> >>           * Clear the request metadata and fill it with some initial non-IPA
> >>           * related controls. We clear it first because the request metadata
> >>           * may have been populated if we have dropped the previous frame.
> >>           */
> >>           request->metadata().clear();
> >>           fillRequestMetadata(bayerFrame.controls, request);
> >>
> >> As far as I understand, this is mainly to handle the initial N dropped frames,
> >> which are handled in `CameraData::checkRequestCompleted()` (i.e. request gets
> >> metadata but in the end the request is not completed, so clearing is needed),
> >> but I want to get confirmation if that is correct and if there are other cases
> >> that need consideration.
> >
> > This is correct, and I think the call to fillRequestMetdata() can be
> > moved to somewhere in checkRequestCompleted() to avoid having to clear
> > it because of dropped frames.  This change could be done immediately
> > to avoid any problems in the future.
>
> I believe that would still need saving at least the sensor timestamp from the
> `BayerFrame`/`CfeJob` object as that seems to be destroyed in `tryRunPipeline()`.

We could do something like this (completely untested!):

diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
index bfc101e6a831..d2bc2d0afa97 100644
--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
@@ -2318,13 +2318,9 @@ void PiSPCameraData::tryRunPipeline()
        /* See if a new ScalerCrop value needs to be applied. */
        applyScalerCrop(request->controls());

-       /*
-        * Clear the request metadata and fill it with some initial non-IPA
-        * related controls. We clear it first because the request metadata
-        * may have been populated if we have dropped the previous frame.
-        */
-       request->metadata().clear();
-       fillRequestMetadata(job.sensorControls, request);
+       /* Populate metadata only if we know this frame will not be dropped. */
+       if (!dropFrameCount_)
+               fillRequestMetadata(job.sensorControls, request);

        /* Set our state to say the pipeline is active. */

Or even better, get rid of the drop frame logic ;-)

Naush

>
>
> Regards,
> Barnabás Pőcze
>
> >
> > However, I would really like to go one step further and remove the
> > whole notion of dropping frames in the pipeline handlers.  This would
> > allow quite a few simplifications to our code :)  I think we have this
> > on our agenda to discuss in Nice.
> >
> >>
> >> Of course a pipeline handler could still queue metadata in a private `ControlList`
> >> and then set it when the request is completed, in case it does not want to or
> >> cannot provide metadata earlier. This is what is currently implemented in the
> >> raspberry pi pipeline handlers at the moment for testing purposes.
> >
> > This would work as well, but I think can be avoided.
> >
> > Regards,
> >
> > Naush
> >
> >>
> >> So in summary, I would just like to get your thoughts regarding when the raspberry pi
> >> pipelines might need to clear the request metadata, and whether you think the rpi
> >> pipeline handlers could me modified (how easily?) to support early metadata reporting.
> >>
> >>
> >> [0]: https://patchwork.libcamera.org/cover/22111/
> >> [1]: https://patchwork.libcamera.org/cover/22226/
> >> [2]: https://patchwork.libcamera.org/patch/12251/
> >>
> >>
> >> Thanks,
> >> Barnabás Pőcze
>