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 ? -- Regards, Laurent Pinchart
