Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect diamonds #909

Merged
merged 35 commits into from
Dec 23, 2024
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7263c9a
Write separate module for diamonds detection WiP
FelonEkonom Nov 20, 2024
f4bce2d
Implement parallel diamond detection WiP
FelonEkonom Nov 20, 2024
e9e8c77
Small refactor
FelonEkonom Nov 20, 2024
cabbcfb
Fix bug
FelonEkonom Nov 20, 2024
a58b363
Fix typo
FelonEkonom Nov 21, 2024
5a76b10
Comment out triggering diamond detection in parents
FelonEkonom Nov 21, 2024
0be84b9
Trigger diamon detection between elements WiP
FelonEkonom Nov 21, 2024
58c5384
Trigger diamon detection between elements WiP
FelonEkonom Nov 22, 2024
7b5fdad
Fix typos
FelonEkonom Nov 22, 2024
646898c
wip
FelonEkonom Nov 22, 2024
30b757e
Log diamond wip
FelonEkonom Nov 25, 2024
39030a7
Add test wip
FelonEkonom Nov 26, 2024
a3c6bdd
Add more test cases wip
FelonEkonom Nov 26, 2024
9855e8a
Refactor diamond detection code
FelonEkonom Nov 26, 2024
f5d6941
Remove unused import
FelonEkonom Nov 26, 2024
acbe87d
Improve diamond log message, fix credo
FelonEkonom Nov 27, 2024
f02d57e
Remove leftover files
FelonEkonom Nov 27, 2024
2327b24
Merge remote-tracking branch 'origin/master' into pull-diamonds
FelonEkonom Nov 27, 2024
35cf655
Update changelog
FelonEkonom Nov 27, 2024
c8b586a
Remove leftovers
FelonEkonom Nov 27, 2024
97d15cf
Fix Connector test
FelonEkonom Nov 27, 2024
6314803
Fix typo in the comment
FelonEkonom Nov 27, 2024
1f94101
Refactor State due to CR
FelonEkonom Dec 4, 2024
090488a
Refactor diamond detection messages
FelonEkonom Dec 4, 2024
1d003ba
stop creating serialized component path every time
FelonEkonom Dec 4, 2024
501aeee
Write some comments describing how the diamond detection mechanism works
FelonEkonom Dec 5, 2024
07fcfd1
Move the comments within the file
FelonEkonom Dec 5, 2024
cc9039d
Fix typos in the algorithm description
FelonEkonom Dec 6, 2024
d364df9
Implement CR suggestions WiP
FelonEkonom Dec 16, 2024
c158ee8
Implement CR suggestions WiP
FelonEkonom Dec 16, 2024
8df71f8
Further refactor
FelonEkonom Dec 16, 2024
0bff060
Algorithm description refactor
FelonEkonom Dec 16, 2024
2dff094
Implement suggestions from CR
FelonEkonom Dec 17, 2024
0db0f3f
Refactor algortithm description
FelonEkonom Dec 23, 2024
b70fa7b
Merge remote-tracking branch 'origin/master' into pull-diamonds
FelonEkonom Dec 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix typos in the algorithm description
FelonEkonom committed Dec 6, 2024
commit cc9039d0b55471ec62f7926f4d638a69b3e10cf6
88 changes: 43 additions & 45 deletions lib/membrane/core/element/diamond_detection_controller.ex
Original file line number Diff line number Diff line change
@@ -5,72 +5,71 @@ defmodule Membrane.Core.Element.DiamondDetectionController do

# Definitions:

# diamond - directed graph, that has at least two distinct elements (sink and source) and
# diamond - directed graph that has at least two distinct elements (sink and source) and
# has two vertex-disjoint paths from the source to the sink.

# This algotithm takes the directed graph made by all Elements within single Pipeline and
# finds some diamond-subgraphs having all the edges (links) working in the :pull mode,
# it means in :manual flow control or in :auto flow controll if the effective flow control
# This algorithm takes the directed graph made by all elements within a single pipeline and
# finds some diamond-subgraphs where all the edges (links) work in the :pull mode,
# it means in :manual flow control or in :auto flow control if the effective flow control
# is set to :pull.

# These diamonds can be dangerous when used with pull flow control, e.g. let's consider
# a pipeline, that contains:
# * MP4 demuxer that has two output pads
# * MKV muxer that is linked to both of them
# and let's assume, that MP4 container that is consumed by MP4 demuxer is unbalanced.
# If MKV muxer has pads working in pull mode, then demand on one pad will be satisified,
# but on the another won't, because the MP4 container is unbalanced. Then, if MP4 demuxer
# a pipeline that contains:
# * MP4 demuxer that has two output pads
# * MKV muxer that is linked to both of them
# and let's assume that the MP4 container that is consumed by the MP4 demuxer is unbalanced.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# and let's assume that the MP4 container that is consumed by the MP4 demuxer is unbalanced.
# and let's assume that the MP4 file that is consumed by the MP4 demuxer is unbalanced
# (audio and video streams are not interleaved, for example audio comes first and then video)

# If the MKV muxer has pads working in pull mode, then demand on one pad will be satisfied,
# but on the other won't, because the MP4 container is unbalanced. Then, if MP4 demuxer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# but on the other won't, because the MP4 container is unbalanced. Then, if MP4 demuxer
# but on the other won't, because the source MP4 file is unbalanced. Then, if the MP4 demuxer

# has pads in auto flow control and its effective flow control is set to :pull, it won't
# demand on the input, because one of the pads output with :auto flow control doesn't
# have positive demand, so the whole pipeline will get stuck and won't process more data.

# The algorithms is made of two phases: (1) triggering and (2) proper searching.
# The algorithm is made of two phases: (1) triggering and (2) proper searching.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why proper searching? :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is supposed to be a translation of "właściwe szukanie"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, 'proper' is more like 'correct'. The best term I know would be 'actual search', though I'd just go for 'search', since the whole process is called 'detection', not 'search', so there's no other 'search' unless I'm missing something ;)


# (1) Triggering

# Let's notice, that:
# * new diamond can be created only after linking new spec
# * if the new spec caused some new diamond to occur, this diamond will contain some of
# the links spawned in this spec
# Let's notice that:
# * a new diamond can be created only after linking a new spec
# * if the new spec caused some new diamond to occur, this diamond will contain some of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# * if the new spec caused some new diamond to occur, this diamond will contain some of
# * if the new spec created a new diamond, this diamond will contain some of

# the links spawned in this spec
# If the diamond contains a link, it must also contain an element whose output pad
# is part of this link.

# If the diamond contains a link, it must also contain an element which output pad takes
# is the part of this link.

# After spec status is set to :done, parent component that returned the spec will trigger
# all elements, which output pads has been linked in this spec. Reference of the trigger
# After the spec status is set to :done, the parent component that returned the spec will trigger
# all elements whose output pads have been linked in this spec. The reference of the trigger
# is always set to the spec reference.

# If the element is triggered with a specific reference for the first time, it does two
# things:
# * the element forwards the trigger with the same reference via all input pads working
# in the pull mode
# * if the element has at least two output pads working in the pull mode, it postpones
# a proper searching that will be spawned from itself. Time between postponing and the
# proper searching is one second. If in this time an element will be triggered once
# again with a different reference, it won't cause another postponing a proper
# searching, this means that at the time there is at most one proper searching
# postponed in the single element
# * the element forwards the trigger with the same reference via all input pads working
# in the pull mode
# * if the element has at least two output pads working in the pull mode, it postpones
# the proper searching that will be spawned from itself. The time between postponing and the
# proper searching is one second. If during this time an element is triggered once
# again with a different reference, it won't cause another postponement of the proper
# searching, this means that at the time there is at most one proper searching
# postponed in the single element

# (2) Proper searching:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like 'proper' is still there, can we make it just 'searching'?


# Proper searching is started only in the elements that have at lest two output pads
# working in the pull mode. When an elements starts proper searching, it assings
# Proper searching is started only in elements that have at least two output pads
# working in the pull mode. When an element starts proper searching, it assigns
# a new reference to it, different from the reference of the related trigger.

# When a proper searching enters the element (no matter if it is the element that has
# hust started the proper searching, or maybe it was forwarded to it via link):
# * if the element sees the proper searching reference for the first time, then:
# - it forwards proper searching via all output pads working in the pull mode
# - when the proper searching is forwarded, it remembers the path in grapth through
# the elments that is has already passed
# * if the element has already seen the reference of proper searching, but there is
# a repeated element on the path, that proper searching traversed to this element,
# the element does nothing
# * if the element has already seen the reference of proper searching and the traversed
# path doesn't contain any repeated elements, it means that the current traversed path
# and the path that the proper searching traversed when it entered the element
# previous time together make a diamond. Then, the element logs the found diamond
# and doesn't forward proper searching fruther
# When proper searching enters the element (no matter if it is the element that has
# just started the proper searching, or maybe it was forwarded to it via a link):
# * if the element sees the proper searching reference for the first time, then:
# - it forwards proper searching via all output pads working in the pull mode
# - when proper searching is forwarded, it remembers the path in the graph through
# the elements that it has already passed
# * if the element has already seen the reference of proper searching, but there is
# a repeated element on the path that proper searching traversed to this element,
# the element does nothing
# * if the element has already seen the reference of proper searching and the traversed
# path doesn't contain any repeated elements, it means that the current traversed path
# and the path that the proper searching traversed when it entered the element
# the previous time together make a diamond. Then, the element logs the found diamond
# and doesn't forward proper searching further.

alias __MODULE__.{DiamondLogger, PathInGraph}
alias __MODULE__.PathInGraph.Vertex
@@ -96,7 +95,6 @@ defmodule Membrane.Core.Element.DiamondDetectionController do
optional(:pad_ref) => Pad.ref()
}


@spec handle_diamond_detection_message(diamond_detection_message(), State.t()) :: State.t()
def handle_diamond_detection_message(%{type: type} = message, state) do
case type do