From d9ac3876de12012edc874bc750e2517fe84af88a Mon Sep 17 00:00:00 2001 From: MediaPipe Team Date: Tue, 14 Feb 2023 14:04:32 -0800 Subject: [PATCH] Dos and Donts / Graph outputs + small adjustments for previous section (e.g removing "simply" word) PiperOrigin-RevId: 509632077 --- .../framework_concepts/building_graphs_cpp.md | 78 +++++++++++++++++-- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/docs/framework_concepts/building_graphs_cpp.md b/docs/framework_concepts/building_graphs_cpp.md index 49ee52991..553683f0b 100644 --- a/docs/framework_concepts/building_graphs_cpp.md +++ b/docs/framework_concepts/building_graphs_cpp.md @@ -294,6 +294,8 @@ CalculatorGraphConfig BuildGraph() { Stream b = graph.In(1).SetName("b").Cast() // Bad. Stream d = RunSomething(a, b, graph); // ... + + return graph.GetConfig(); } ``` @@ -304,18 +306,20 @@ In the above code: * Can be error prone overall and hard to maintain in future (e.g. is it a correct index? name? what if some inputs are removed or made optional? etc.). +* `RunSomething` reuse is limited because other graphs may have different + inputs -Instead, simply define your graph inputs at the very beginning of your graph -builder: +Instead, define your graph inputs at the very beginning of your graph builder: ```c++ {.good} -Stream RunSomething(Stream a, Stream b, Stream c, Graph& graph) { +Stream RunSomething(Stream a, Stream b, Stream c, Graph& graph) { // ... } CalculatorGraphConfig BuildGraph() { Graph graph; + // Inputs. Stream a = graph.In(0).SetName("a").Cast(); Stream b = graph.In(1).SetName("b").Cast(); Stream c = graph.In(2).SetName("c").Cast(); @@ -323,11 +327,13 @@ CalculatorGraphConfig BuildGraph() { // 10/100/N lines of code. Stream d = RunSomething(a, b, c, graph); // ... + + return graph.GetConfig(); } ``` -And if you have an input stream or side packet that is not always defined - -simply use `std::optional` and put it at the very beginning as well: +Use `std::optional` if you have an input stream or side packet that is not +always defined and put it at the very beginning: ```c++ {.good} std::optional> a; @@ -341,3 +347,65 @@ where calling `RunSomething1(..., graph)`, ..., `RunSomethingN(..., graph)` is **intended to add new inputs**, so afterwards you can iterate over them and feed only added inputs into the graph. However, in any case, try to make it easy for readers to find out what graph inputs it has or may have. + +### Define graph outputs at the very end + +```c++ {.bad} +void RunSomething(Stream input, Graph& graph) { + // ... + node.Out("OUTPUT_F") + .SetName("output_f").ConnectTo(graph.Out(2)); // Bad. +} + +CalculatorGraphConfig BuildGraph() { + Graph graph; + + // 10/100/N lines of code. + node.Out("OUTPUT_D") + .SetName("output_d").ConnectTo(graph.Out(0)); // Bad. + // 10/100/N lines of code. + node.Out("OUTPUT_E") + .SetName("output_e").ConnectTo(graph.Out(1)); // Bad. + // 10/100/N lines of code. + RunSomething(input, graph); + // ... + + return graph.GetConfig(); +} +``` + +In the above code: + +* It can be hard to guess how many outputs you have in the graph. +* Can be error prone overall and hard to maintain in future (e.g. is it a + correct index? name? what if some outpus are removed or made optional? + etc.). +* `RunSomething` reuse is limited as other graphs may have different outputs + +Instead, define your graph outputs at the very end of your graph builder: + +```c++ {.good} +Stream RunSomething(Stream input, Graph& graph) { + // ... + return node.Out("OUTPUT_F").Cast(); +} + +CalculatorGraphConfig BuildGraph() { + Graph graph; + + // 10/100/N lines of code. + Stream d = node.Out("OUTPUT_D").Cast(); + // 10/100/N lines of code. + Stream e = node.Out("OUTPUT_E").Cast(); + // 10/100/N lines of code. + Stream f = RunSomething(input, graph); + // ... + + // Outputs. + d.SetName("output_d").ConnectTo(graph.Out(0)); + e.SetName("output_e").ConnectTo(graph.Out(1)); + f.SetName("output_f").ConnectTo(graph.Out(2)); + + return graph.GetConfig(); +} +```