| Message ID | 20210805222436.6263-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 05/08/2021 23:24, Laurent Pinchart wrote: > Record the sink and source pads through which an entity is traversed in > the list of entities stored in the camera data. This prepares for > implementing mutually exclusive access to entities between cameras. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 38 ++++++++++++++++++++---- > 1 file changed, 32 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index b29fff9820e5..e0695d052629 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -166,7 +166,22 @@ public: > } > > struct Entity { > + /* The media entity, always valid. */ > MediaEntity *entity; > + /* > + * The local sink pad connected to the upstream entity, null for > + * the camera sensor at the beginning of the pipeline. > + */ > + const MediaPad *sink; > + /* > + * The local source pad connected to the downstream entity, null > + * for the video node at the end of the pipeline. > + */ > + const MediaPad *source; > + /* > + * The link to the downstream entity, null for the video node at > + * the end of the pipeline. > + */ > MediaLink *link; > }; > > @@ -288,16 +303,18 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > * encoders and image converters, and will end in a CSI capture device. > */ > std::unordered_set<MediaEntity *> visited; > - std::queue<MediaEntity *> queue; > + std::queue<std::tuple<MediaEntity *, MediaPad *>> queue; > > /* Remember at each entity where we came from. */ > std::unordered_map<MediaEntity *, Entity> parents; > MediaEntity *entity = nullptr; > > - queue.push(sensor); > + queue.push({ sensor, nullptr }); > > while (!queue.empty()) { > - entity = queue.front(); > + MediaPad *sinkPad; > + > + std::tie(entity, sinkPad) = queue.front(); > queue.pop(); > > /* Found the capture device. */ > @@ -317,8 +334,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > for (MediaLink *link : pad->links()) { > MediaEntity *next = link->sink()->entity(); > if (visited.find(next) == visited.end()) { > - queue.push(next); > - parents.insert({ next, { entity, link } }); > + queue.push({ next, link->sink() }); > + parents.insert({ next, { entity, sinkPad, pad, link } }); > } > } > } > @@ -349,7 +366,16 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > LOG(SimplePipeline, Debug) > << "Found pipeline: " > << utils::join(entities_, " -> ", > - [](const Entity &e) { return e.entity->name(); }); > + [](const Entity &e) { > + std::string s = "["; > + if (e.sink) > + s += std::to_string(e.sink->index()) + "|"; > + s += e.entity->name(); > + if (e.source) > + s += "|" + std::to_string(e.source->index()); > + s += "]"; > + return s; > + }); An example of the changes to string formatting would be nice in the commit message, but not essential. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > int SimpleCameraData::init() >
Hi Kieran, On Mon, Aug 16, 2021 at 03:41:21PM +0100, Kieran Bingham wrote: > On 05/08/2021 23:24, Laurent Pinchart wrote: > > Record the sink and source pads through which an entity is traversed in > > the list of entities stored in the camera data. This prepares for > > implementing mutually exclusive access to entities between cameras. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/pipeline/simple/simple.cpp | 38 ++++++++++++++++++++---- > > 1 file changed, 32 insertions(+), 6 deletions(-) > > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index b29fff9820e5..e0695d052629 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -166,7 +166,22 @@ public: > > } > > > > struct Entity { > > + /* The media entity, always valid. */ > > MediaEntity *entity; > > + /* > > + * The local sink pad connected to the upstream entity, null for > > + * the camera sensor at the beginning of the pipeline. > > + */ > > + const MediaPad *sink; > > + /* > > + * The local source pad connected to the downstream entity, null > > + * for the video node at the end of the pipeline. > > + */ > > + const MediaPad *source; > > + /* > > + * The link to the downstream entity, null for the video node at > > + * the end of the pipeline. > > + */ > > MediaLink *link; > > }; > > > > @@ -288,16 +303,18 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > > * encoders and image converters, and will end in a CSI capture device. > > */ > > std::unordered_set<MediaEntity *> visited; > > - std::queue<MediaEntity *> queue; > > + std::queue<std::tuple<MediaEntity *, MediaPad *>> queue; > > > > /* Remember at each entity where we came from. */ > > std::unordered_map<MediaEntity *, Entity> parents; > > MediaEntity *entity = nullptr; > > > > - queue.push(sensor); > > + queue.push({ sensor, nullptr }); > > > > while (!queue.empty()) { > > - entity = queue.front(); > > + MediaPad *sinkPad; > > + > > + std::tie(entity, sinkPad) = queue.front(); > > queue.pop(); > > > > /* Found the capture device. */ > > @@ -317,8 +334,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > > for (MediaLink *link : pad->links()) { > > MediaEntity *next = link->sink()->entity(); > > if (visited.find(next) == visited.end()) { > > - queue.push(next); > > - parents.insert({ next, { entity, link } }); > > + queue.push({ next, link->sink() }); > > + parents.insert({ next, { entity, sinkPad, pad, link } }); > > } > > } > > } > > @@ -349,7 +366,16 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, > > LOG(SimplePipeline, Debug) > > << "Found pipeline: " > > << utils::join(entities_, " -> ", > > - [](const Entity &e) { return e.entity->name(); }); > > + [](const Entity &e) { > > + std::string s = "["; > > + if (e.sink) > > + s += std::to_string(e.sink->index()) + "|"; > > + s += e.entity->name(); > > + if (e.source) > > + s += "|" + std::to_string(e.source->index()); > > + s += "]"; > > + return s; > > + }); > > An example of the changes to string formatting would be nice in the > commit message, but not essential. Note that this is just a debug message, not the main point of the patch :-) I'll add The debug message that displays detected pipelines now prints pads to describe the pipeline more precisely: [0:00:35.901275750] [260] DEBUG SimplePipeline simple.cpp:404 Found pipeline: [imx290 2-001a|0] -> [0|csis-32e40000.csi|1] -> [0|mxc_isi.0|1] -> [0|mxc_isi.0.capture] > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > } > > > > int SimpleCameraData::init()
On 17/08/2021 01:02, Laurent Pinchart wrote: > Hi Kieran, > > On Mon, Aug 16, 2021 at 03:41:21PM +0100, Kieran Bingham wrote: >> On 05/08/2021 23:24, Laurent Pinchart wrote: >>> Record the sink and source pads through which an entity is traversed in >>> the list of entities stored in the camera data. This prepares for >>> implementing mutually exclusive access to entities between cameras. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> --- >>> src/libcamera/pipeline/simple/simple.cpp | 38 ++++++++++++++++++++---- >>> 1 file changed, 32 insertions(+), 6 deletions(-) >>> >>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >>> index b29fff9820e5..e0695d052629 100644 >>> --- a/src/libcamera/pipeline/simple/simple.cpp >>> +++ b/src/libcamera/pipeline/simple/simple.cpp >>> @@ -166,7 +166,22 @@ public: >>> } >>> >>> struct Entity { >>> + /* The media entity, always valid. */ >>> MediaEntity *entity; >>> + /* >>> + * The local sink pad connected to the upstream entity, null for >>> + * the camera sensor at the beginning of the pipeline. >>> + */ >>> + const MediaPad *sink; >>> + /* >>> + * The local source pad connected to the downstream entity, null >>> + * for the video node at the end of the pipeline. >>> + */ >>> + const MediaPad *source; >>> + /* >>> + * The link to the downstream entity, null for the video node at >>> + * the end of the pipeline. >>> + */ >>> MediaLink *link; >>> }; >>> >>> @@ -288,16 +303,18 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, >>> * encoders and image converters, and will end in a CSI capture device. >>> */ >>> std::unordered_set<MediaEntity *> visited; >>> - std::queue<MediaEntity *> queue; >>> + std::queue<std::tuple<MediaEntity *, MediaPad *>> queue; >>> >>> /* Remember at each entity where we came from. */ >>> std::unordered_map<MediaEntity *, Entity> parents; >>> MediaEntity *entity = nullptr; >>> >>> - queue.push(sensor); >>> + queue.push({ sensor, nullptr }); >>> >>> while (!queue.empty()) { >>> - entity = queue.front(); >>> + MediaPad *sinkPad; >>> + >>> + std::tie(entity, sinkPad) = queue.front(); >>> queue.pop(); >>> >>> /* Found the capture device. */ >>> @@ -317,8 +334,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, >>> for (MediaLink *link : pad->links()) { >>> MediaEntity *next = link->sink()->entity(); >>> if (visited.find(next) == visited.end()) { >>> - queue.push(next); >>> - parents.insert({ next, { entity, link } }); >>> + queue.push({ next, link->sink() }); >>> + parents.insert({ next, { entity, sinkPad, pad, link } }); >>> } >>> } >>> } >>> @@ -349,7 +366,16 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, >>> LOG(SimplePipeline, Debug) >>> << "Found pipeline: " >>> << utils::join(entities_, " -> ", >>> - [](const Entity &e) { return e.entity->name(); }); >>> + [](const Entity &e) { >>> + std::string s = "["; >>> + if (e.sink) >>> + s += std::to_string(e.sink->index()) + "|"; >>> + s += e.entity->name(); >>> + if (e.source) >>> + s += "|" + std::to_string(e.source->index()); >>> + s += "]"; >>> + return s; >>> + }); >> >> An example of the changes to string formatting would be nice in the >> commit message, but not essential. > > Note that this is just a debug message, not the main point of the patch > :-) I'll add Of course, that's why I said not essential > > The debug message that displays detected pipelines now prints pads to > describe the pipeline more precisely: > > [0:00:35.901275750] [260] DEBUG SimplePipeline simple.cpp:404 Found pipeline: [imx290 2-001a|0] -> [0|csis-32e40000.csi|1] -> [0|mxc_isi.0|1] -> [0|mxc_isi.0.capture] > Thanks, I just think it's a good idea to show what the expected change is when something changes something that is user (or developer, in this instance) visible, to show the intention of the update. >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> } >>> >>> int SimpleCameraData::init() >
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index b29fff9820e5..e0695d052629 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -166,7 +166,22 @@ public: } struct Entity { + /* The media entity, always valid. */ MediaEntity *entity; + /* + * The local sink pad connected to the upstream entity, null for + * the camera sensor at the beginning of the pipeline. + */ + const MediaPad *sink; + /* + * The local source pad connected to the downstream entity, null + * for the video node at the end of the pipeline. + */ + const MediaPad *source; + /* + * The link to the downstream entity, null for the video node at + * the end of the pipeline. + */ MediaLink *link; }; @@ -288,16 +303,18 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, * encoders and image converters, and will end in a CSI capture device. */ std::unordered_set<MediaEntity *> visited; - std::queue<MediaEntity *> queue; + std::queue<std::tuple<MediaEntity *, MediaPad *>> queue; /* Remember at each entity where we came from. */ std::unordered_map<MediaEntity *, Entity> parents; MediaEntity *entity = nullptr; - queue.push(sensor); + queue.push({ sensor, nullptr }); while (!queue.empty()) { - entity = queue.front(); + MediaPad *sinkPad; + + std::tie(entity, sinkPad) = queue.front(); queue.pop(); /* Found the capture device. */ @@ -317,8 +334,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, for (MediaLink *link : pad->links()) { MediaEntity *next = link->sink()->entity(); if (visited.find(next) == visited.end()) { - queue.push(next); - parents.insert({ next, { entity, link } }); + queue.push({ next, link->sink() }); + parents.insert({ next, { entity, sinkPad, pad, link } }); } } } @@ -349,7 +366,16 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, LOG(SimplePipeline, Debug) << "Found pipeline: " << utils::join(entities_, " -> ", - [](const Entity &e) { return e.entity->name(); }); + [](const Entity &e) { + std::string s = "["; + if (e.sink) + s += std::to_string(e.sink->index()) + "|"; + s += e.entity->name(); + if (e.source) + s += "|" + std::to_string(e.source->index()); + s += "]"; + return s; + }); } int SimpleCameraData::init()
Record the sink and source pads through which an entity is traversed in the list of entities stored in the camera data. This prepares for implementing mutually exclusive access to entities between cameras. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/pipeline/simple/simple.cpp | 38 ++++++++++++++++++++---- 1 file changed, 32 insertions(+), 6 deletions(-)