On 06/09/2022 11:44, Laurent Pinchart wrote:
Hi Tomi,
On Wed, Aug 31, 2022 at 05:21:58PM +0300, Tomi Valkeinen wrote:
On 29/08/2022 20:18, Laurent Pinchart wrote:
On Wed, Aug 10, 2022 at 03:11:02PM +0300, Tomi Valkeinen wrote:
From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
[Note: the code is mostly from Laurent but the patch description is from Tomi]
I'll review the description then :-)
The media_pipeline_start() and media_pipeline_stop() functions use the
media graph walk API to traverse the graph and validate the pipeline.
The graph walk traverses the media graph following links between the
entities.
Also, while the pipeline can't change between the start and stop calls,
the graph is walked again from scratch at stop time, or any time a
driver needs to inspect the pipeline.
With the upcoming multiplexed streams support we will need a bit more
intelligent pipeline construction, as e.g. an entity may be passing
through two independent streams via separate pads, in which case those
Did you mean "as e.g. two independent streams may be passing through a
single entity via separate pads" ?
pads should not be part of the same pipeline.
This patch essentially rewrites the media_pipeline_start/stop so that
a pipeline is defined as a set of pads instead of entities and the media
graph traversal considers the pad interdependencies when choosing which
links to follow.
Currently all the entity's pads are considered as interdependent. This
means that the behavior with all the current drivers stays the same, but
in the future we can define a more fine-grained pipeline construction.
Additionally the media pipeline's pads are cached at
media_pipeline_start() time, and re-used at media_pipeline_stop() which
avoid the need to re-walk the whole graph as the previous implementation
did.
Also, caching pads in the pipeline can serve in the future as the
foundation to provide a better API than the media graph walk to drivers
to iterate over pads and entities in the pipeline.
Note that the old media_pipeline_start/stop used the media graph walk
API. The new version does not use the media graph walk API, but instead
a new implementation.
There are two reason for not changing the graph walk: it proved to be
rather difficult to change the graph walk to have the features
implemented in this patch, and second, this keeps the backward
compatibility of the graph walk as there are users of the graph walk API
The long term plan is that all the existing code would be converted to
use the new cached pipeline, thus allowing us to remove the graph walk.
Could you mark the graph walk API as deprecated in this patch, or in a
subsequent patch in the series ? I think I did in a previous version,
but I may recall incorrectly.
I didn't mark the graph walk API as deprecated in v14 series. We can do
it on top, but I'm not sure if it's a valid thing to say yet. Have we
tried converting any graph walk uses to the new pipeline code? We could
well have code missing that prevents the conversion.
I'd like to avoid new users of the graph walk API. This requires two
patches that you haven't included or squashed in v14 though, namely
20a31d49bd75 media: mc: entity: Add entity iterator for media_pipeline
50659eb74afc media: mc: entity: Add pad iterator for media_pipeline
that you can find at
git://linuxtv.org/pinchartl/media.git streams/v6.0/v11
Should I submit that on top of v14 for inclusion in v6.1 too ?
Both look fine to me with a quick glance, and I'm ok with adding these
on top of v14 (or I can pick them for v15 if there's one).
I don't think we have any code that uses them, have you tested them?
Tomi
