diff --git a/CHANGELOG.md b/CHANGELOG.md index 978720b66..000523f6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Fix clock selection [#626](https://github.com/membraneframework/membrane_core/pull/626) * Log messages in the default handle_info implementation [#680](https://github.com/membraneframework/membrane_core/pull/680) * Fix typespecs in Membrane.UtilitySupervisor [#681](https://github.com/membraneframework/membrane_core/pull/681) + * Improve callback return types and group actions types [#702](https://github.com/membraneframework/membrane_core/pull/702) ## 1.0.0 * Introduce `:remove_link` action in pipelines and bins. diff --git a/lib/membrane/bin/action.ex b/lib/membrane/bin/action.ex index f813861d5..1e259c64f 100644 --- a/lib/membrane/bin/action.ex +++ b/lib/membrane/bin/action.ex @@ -15,7 +15,8 @@ defmodule Membrane.Bin.Action do By default, component setup ends with the end of `c:Membrane.Bin.handle_setup/2` callback. If `{:setup, :incomplete}` is returned there, setup lasts until `{:setup, :complete}` - is returned from antoher callback. + is returned from antoher callback. You cannot return `{:setup, :incomplete}` from any other + callback than `c:Membrane.Bin.handle_setup/2`. Untils the setup lasts, the component won't enter `:playing` playback. """ diff --git a/lib/membrane/core/pipeline/action_handler.ex b/lib/membrane/core/pipeline/action_handler.ex index ea1196486..92065ed43 100644 --- a/lib/membrane/core/pipeline/action_handler.ex +++ b/lib/membrane/core/pipeline/action_handler.ex @@ -8,11 +8,6 @@ defmodule Membrane.Core.Pipeline.ActionHandler do alias Membrane.Core.Parent.LifecycleController alias Membrane.Core.Pipeline.State - @impl CallbackHandler - def handle_action({action, _args}, :handle_init, _params, _state) when action != :spec do - raise ActionError, action: action, reason: {:invalid_callback, :handle_init} - end - @impl CallbackHandler def handle_action({:spec, args}, _cb, _params, %State{terminating?: true}) do raise Membrane.ParentError, diff --git a/lib/membrane/element/action.ex b/lib/membrane/element/action.ex index 3e4f71b8a..055821142 100644 --- a/lib/membrane/element/action.ex +++ b/lib/membrane/element/action.ex @@ -221,7 +221,7 @@ defmodule Membrane.Element.Action do @typedoc """ This action sets the latency for the element. - This action is not premitted in callback `c:Membrane.Element.Base.handle_init/2`. + This action is permitted only in callback `c:Membrane.Element.Base.handle_init/2`. """ @type latency :: {:latency, latency :: Membrane.Time.non_neg()} @@ -245,27 +245,31 @@ defmodule Membrane.Element.Action do @type terminate :: {:terminate, reason :: :normal | :shutdown | {:shutdown, term} | term} @typedoc """ - Type that defines a single action that may be returned from element callbacks. + Action that can be always returned from each of the callbacks. + """ + @type common_actions :: + notify_parent | start_timer | timer_interval | stop_timer | terminate | split - Depending on element type, callback, current playback and other - circumstances there may be different actions available. + @typedoc """ + Actions that can be returned from callbacks when the element is in `playback: :playing` state. """ - @type t :: - setup - | event - | notify_parent - | split + @type stream_actions :: + event | stream_format | buffer | demand - | redemand | pause_auto_demand | resume_auto_demand - | forward - | start_timer - | timer_interval - | stop_timer - | latency | end_of_stream - | terminate + | redemand + + @typedoc """ + Type that defines a union of actions that may be returned from most of the callbacks. + + Depending on element type, callback, current playback and other + circumstances there may be different actions available. + Check the typespec and documentation of particular callbacks + for details. + """ + @type t :: common_actions | stream_actions | latency | forward | setup end diff --git a/lib/membrane/element/base.ex b/lib/membrane/element/base.ex index b205ca676..de0aab632 100644 --- a/lib/membrane/element/base.ex +++ b/lib/membrane/element/base.ex @@ -33,11 +33,6 @@ defmodule Membrane.Element.Base do alias Membrane.{Element, Event, Pad} alias Membrane.Element.{Action, CallbackContext} - @typedoc """ - Type that defines all valid return values from most callbacks. - """ - @type callback_return :: {[Action.t()], Element.state()} - @doc """ Callback invoked on initialization of element. @@ -48,7 +43,7 @@ defmodule Membrane.Element.Base do By default, it converts the `opts` struct to a map and sets them as the element's state. """ @callback handle_init(context :: CallbackContext.t(), options :: Element.options()) :: - callback_return + {[Action.common_actions() | Action.latency()], Element.state()} @doc """ Callback invoked on element startup, right after `c:handle_init/2`. @@ -59,7 +54,7 @@ defmodule Membrane.Element.Base do @callback handle_setup( context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: {[Action.common_actions() | Action.setup()], Element.state()} @doc """ Callback invoked when bin switches the playback to `:playing`. @@ -71,7 +66,7 @@ defmodule Membrane.Element.Base do @callback handle_playing( context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: {[Action.common_actions() | Action.stream_actions()], Element.state()} @doc """ Callback invoked when element receives a message that is not recognized @@ -84,7 +79,9 @@ defmodule Membrane.Element.Base do message :: any(), context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: + {[Action.common_actions() | Action.stream_actions() | Action.setup()], + Element.state()} @doc """ Callback that is called when new pad has beed added to element. Executed @@ -97,7 +94,9 @@ defmodule Membrane.Element.Base do pad :: Pad.ref(), context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: + {[Action.common_actions() | Action.stream_actions() | Action.setup()], + Element.state()} @doc """ Callback that is called when some pad of the element has beed removed. Executed @@ -110,7 +109,9 @@ defmodule Membrane.Element.Base do pad :: Pad.ref(), context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: + {[Action.common_actions() | Action.stream_actions() | Action.setup()], + Element.state()} @doc """ Callback that is called when event arrives. @@ -124,7 +125,7 @@ defmodule Membrane.Element.Base do event :: Event.t(), context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: {[Action.common_actions() | Action.stream_actions()], Element.state()} @doc """ Callback invoked upon each timer tick. A timer can be started with `Membrane.Element.Action.start_timer` @@ -134,7 +135,9 @@ defmodule Membrane.Element.Base do timer_id :: any, context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: + {[Action.common_actions() | Action.stream_actions() | Action.setup()], + Element.state()} @doc """ Callback invoked when a message from the parent is received. @@ -144,7 +147,9 @@ defmodule Membrane.Element.Base do notification :: Membrane.ParentNotification.t(), context :: CallbackContext.t(), state :: Element.state() - ) :: callback_return + ) :: + {[Action.common_actions() | Action.stream_actions() | Action.setup()], + Element.state()} @doc """ Callback invoked when element is removed by its parent. @@ -155,7 +160,8 @@ defmodule Membrane.Element.Base do context :: CallbackContext.t(), state :: Element.state() ) :: - callback_return() + {[Action.common_actions() | Action.stream_actions() | Action.setup()], + Element.state()} @doc """ A callback for constructing struct. Will be defined by `def_options/1` if used. diff --git a/lib/membrane/element/with_input_pads.ex b/lib/membrane/element/with_input_pads.ex index ba5a08986..a27f99d01 100644 --- a/lib/membrane/element/with_input_pads.ex +++ b/lib/membrane/element/with_input_pads.ex @@ -25,8 +25,12 @@ defmodule Membrane.Element.WithInputPads do stream_format :: Membrane.StreamFormat.t(), context :: CallbackContext.t(), state :: Element.state() - ) :: Membrane.Element.Base.callback_return() - + ) :: + {[ + Membrane.Element.Action.common_actions() + | Membrane.Element.Action.stream_actions() + | Membrane.Element.Action.forward() + ], Element.state()} @doc """ Callback invoked when element receives `Membrane.Event.StartOfStream` event. """ @@ -34,7 +38,12 @@ defmodule Membrane.Element.WithInputPads do pad :: Pad.ref(), context :: CallbackContext.t(), state :: Element.state() - ) :: Membrane.Element.Base.callback_return() + ) :: + {[ + Membrane.Element.Action.common_actions() + | Membrane.Element.Action.stream_actions() + | Membrane.Element.Action.forward() + ], Element.state()} @doc """ Callback invoked when the previous element has finished processing via the pad, @@ -44,7 +53,12 @@ defmodule Membrane.Element.WithInputPads do pad :: Pad.ref(), context :: CallbackContext.t(), state :: Element.state() - ) :: Membrane.Element.Base.callback_return() + ) :: + {[ + Membrane.Element.Action.common_actions() + | Membrane.Element.Action.stream_actions() + | Membrane.Element.Action.forward() + ], Element.state()} @doc """ Callback that is called when buffer should be processed by the Element. @@ -59,7 +73,12 @@ defmodule Membrane.Element.WithInputPads do buffer :: Buffer.t(), context :: CallbackContext.t(), state :: Element.state() - ) :: Membrane.Element.Base.callback_return() + ) :: + {[ + Membrane.Element.Action.common_actions() + | Membrane.Element.Action.stream_actions() + | Membrane.Element.Action.forward() + ], Element.state()} @optional_callbacks handle_buffer: 4, handle_stream_format: 4 diff --git a/lib/membrane/element/with_output_pads.ex b/lib/membrane/element/with_output_pads.ex index be3e5ecf7..55eaf43fc 100644 --- a/lib/membrane/element/with_output_pads.ex +++ b/lib/membrane/element/with_output_pads.ex @@ -37,7 +37,11 @@ defmodule Membrane.Element.WithOutputPads do unit :: Buffer.Metric.unit(), context :: CallbackContext.t(), state :: Element.state() - ) :: Membrane.Element.Base.callback_return() + ) :: + {[ + Membrane.Element.Action.common_actions() + | Membrane.Element.Action.stream_actions() + ], Element.state()} @optional_callbacks handle_demand: 5 diff --git a/lib/membrane/pipeline.ex b/lib/membrane/pipeline.ex index 0031861ee..f78be3d60 100644 --- a/lib/membrane/pipeline.ex +++ b/lib/membrane/pipeline.ex @@ -106,7 +106,7 @@ defmodule Membrane.Pipeline do By default, it converts the `opts` to a map if they're a struct and sets them as the pipeline state. """ @callback handle_init(context :: CallbackContext.t(), options :: pipeline_options) :: - callback_return() + {[Action.common_actions()], state()} @doc """ Callback invoked when pipeline is requested to terminate with `terminate/2`. @@ -114,7 +114,7 @@ defmodule Membrane.Pipeline do By default, it returns `t:Membrane.Pipeline.Action.terminate/0` with reason `:normal`. """ @callback handle_terminate_request(context :: CallbackContext.t(), state) :: - callback_return() + {[Action.common_actions()], state()} @doc """ Callback invoked on pipeline startup, right after `c:handle_init/2`. @@ -126,7 +126,7 @@ defmodule Membrane.Pipeline do context :: CallbackContext.t(), state ) :: - callback_return + {[Action.common_actions()], state()} @doc """ Callback invoked when pipeline switches the playback to `:playing`. @@ -136,7 +136,7 @@ defmodule Membrane.Pipeline do context :: CallbackContext.t(), state ) :: - callback_return + {[Action.common_actions()], state()} @doc """ Callback invoked when a child removes its pad. @@ -151,7 +151,7 @@ defmodule Membrane.Pipeline do pad :: Pad.ref(), context :: CallbackContext.t(), state :: state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked when a notification comes in from a child. @@ -163,7 +163,7 @@ defmodule Membrane.Pipeline do element :: Child.name(), context :: CallbackContext.t(), state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked when pipeline receives a message that is not recognized @@ -177,7 +177,7 @@ defmodule Membrane.Pipeline do context :: CallbackContext.t(), state ) :: - callback_return + {[Action.common_actions()], state()} @doc """ Callback invoked when a child element starts processing stream via given pad. @@ -189,7 +189,7 @@ defmodule Membrane.Pipeline do pad :: Pad.ref(), context :: CallbackContext.t(), state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked when a child element finishes processing stream via given pad. @@ -201,7 +201,7 @@ defmodule Membrane.Pipeline do pad :: Pad.ref(), context :: CallbackContext.t(), state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked when children of `Membrane.ChildrenSpec` are started. @@ -212,7 +212,7 @@ defmodule Membrane.Pipeline do children :: [Child.name()], context :: CallbackContext.t(), state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked upon each timer tick. A timer can be started with `Membrane.Pipeline.Action.start_timer` @@ -222,7 +222,7 @@ defmodule Membrane.Pipeline do timer_id :: any, context :: CallbackContext.t(), state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked when crash of the crash group happens. @@ -234,7 +234,7 @@ defmodule Membrane.Pipeline do group_name :: Child.group(), context :: CallbackContext.t(), state - ) :: callback_return + ) :: {[Action.common_actions()], state()} @doc """ Callback invoked when pipeline is called using a synchronous call. @@ -247,7 +247,7 @@ defmodule Membrane.Pipeline do context :: CallbackContext.t(), state ) :: - callback_return + {[Action.common_actions() | Action.reply()], state()} @optional_callbacks handle_init: 2, handle_setup: 2, diff --git a/lib/membrane/pipeline/action.ex b/lib/membrane/pipeline/action.ex index f41809f6e..9663fdcd7 100644 --- a/lib/membrane/pipeline/action.ex +++ b/lib/membrane/pipeline/action.ex @@ -125,12 +125,9 @@ defmodule Membrane.Pipeline.Action do @type terminate :: {:terminate, reason :: :normal | :shutdown | {:shutdown, term} | term} @typedoc """ - Type describing actions that can be returned from pipeline callbacks. - - Returning actions is a way of pipeline interaction with its children and - other parts of framework. + Type describing the actions that can be returned from any pipeline callback. """ - @type t :: + @type common_actions :: setup | notify_child | spec @@ -139,7 +136,14 @@ defmodule Membrane.Pipeline.Action do | start_timer | timer_interval | stop_timer - | reply | reply_to | terminate + + @typedoc """ + Type describing the union of all actions that can be returned from pipeline callbacks. + + Returning actions is a way of pipeline interaction with its children and + other parts of framework. + """ + @type t :: reply | common_actions end