[libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple: Don't disable links carrying other streams
Jacopo Mondi
jacopo at jmondi.org
Wed Aug 3 14:04:34 CEST 2022
Hi Laurent
forgot to leave a tag here
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
On Tue, Aug 02, 2022 at 09:51:10AM +0200, Jacopo Mondi via libcamera-devel wrote:
> Hi Laurent
>
> On Mon, Aug 01, 2022 at 11:59:33PM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > On Mon, Aug 01, 2022 at 06:25:23PM +0200, Jacopo Mondi wrote:
> > > On Mon, Aug 01, 2022 at 03:05:42AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > > From: Phi-Bang Nguyen <pnguyen at baylibre.com>
> > > >
> > > > If a subdev supports the internal routing API, pads unrelated to the
> > > > pipeline for a given camera sensor may carry streams for other cameras.
> > > > The link setup logic is updated to take this into account, by avoiding
> > > > disabling links to unrelated pads.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > ---
> > > > src/libcamera/pipeline/simple/simple.cpp | 28 +++++++++++++++++++++---
> > > > 1 file changed, 25 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > > index 2a8811183907..c80e462bc449 100644
> > > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > > @@ -221,6 +221,11 @@ public:
> > > > struct Entity {
> > > > /* The media entity, always valid. */
> > > > MediaEntity *entity;
> > > > + /*
> > > > + * Whether or not the entity is a subdev that supports the
> > > > + * routing API.
> > > > + */
> > > > + bool supportsRouting;
> > > > /*
> > > > * The local sink pad connected to the upstream entity, null for
> > > > * the camera sensor at the beginning of the pipeline.
> > > > @@ -404,9 +409,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > > > */
> > > >
> > > > std::vector<const MediaPad *> pads;
> > > > + bool supportsRouting = false;
> > > >
> > > > - if (sinkPad)
> > > > + if (sinkPad) {
> > > > pads = routedSourcePads(sinkPad);
> > > > + if (!pads.empty())
> > > > + supportsRouting = true;
> > > > + }
> > > >
> > > > if (pads.empty()) {
> > > > for (const MediaPad *pad : entity->pads()) {
> > > > @@ -421,7 +430,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > > > MediaEntity *next = link->sink()->entity();
> > > > if (visited.find(next) == visited.end()) {
> > > > queue.push({ next, link->sink() });
> > > > - parents.insert({ next, { entity, sinkPad, pad, link } });
> > > > +
> > > > + Entity e{ entity, supportsRouting, sinkPad, pad, link };
> > > > + parents.insert({ next, e });
> > > > }
> > > > }
> > > > }
> > > > @@ -435,7 +446,7 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > > > * to the sensor. Store all the entities in the pipeline, from the
> > > > * camera sensor to the video node, in entities_.
> > > > */
> > > > - entities_.push_front({ entity, sinkPad, nullptr, nullptr });
> > > > + entities_.push_front({ entity, false, sinkPad, nullptr, nullptr });
> > > >
> > > > for (auto it = parents.find(entity); it != parents.end();
> > > > it = parents.find(entity)) {
> > > > @@ -617,6 +628,17 @@ int SimpleCameraData::setupLinks()
> > >
> > > I would also update the comment at function begin that reports
> > >
> > > /*
> > > * Configure all links along the pipeline. Some entities may not allow
> > > * multiple sink links to be enabled together, even on different sink
> > > * pads. We must thus start by disabling all sink links (but the one we
> > > * want to enable) before enabling the pipeline link.
> > > */
> > >
> > > To mention that this doesn't apply to routed subdevs
> >
> > I had a look but it would really duplicate the comment below. As it
> > describes the internal implementation and not a public API, and the two
> > comments are 9 lines of code apart, would you mind if I left it as-is ?
> >
>
> fine
>
> > > > }
> > > >
> > > > for (MediaPad *pad : e.entity->pads()) {
> > > > + /*
> > > > + * If the entity supports the V4L2 internal routing API,
> > > > + * assume that it may carry multiple independent streams
> > > > + * concurrently, and only disable links on the sink and
> > > > + * source pads used by the pipeline.
> > > > + */
> > > > + if (e.supportsRouting) {
> > > > + if (pad != e.sink && pad != e.source)
> > >
> > > Or
> > > if (e.supportsRouting &&
> > > (pad != e.sink && pad != e.source))
> >
> > I'll drop the inner parentheses too.
>
> Better thanks
>
> >
> > > > + continue;
> > > > + }
> > > > +
> > > > for (MediaLink *link : pad->links()) {
> > > > if (link == sinkLink)
> > > > continue;
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
More information about the libcamera-devel
mailing list